From dbe1c6175e6d886c2fb1c61bc2aef4d3fbd00443 Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Wed, 11 Oct 2023 07:37:11 +0000 Subject: [PATCH 1/8] async implementation for wrap --- src/clojure_elastic_apm/core.clj | 4 +- src/clojure_elastic_apm/ring.clj | 86 +++++++++++++++++--------- test/clojure_elastic_apm/ring_test.clj | 20 ++++++ 3 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/clojure_elastic_apm/core.clj b/src/clojure_elastic_apm/core.clj index 241c09a..c43280e 100644 --- a/src/clojure_elastic_apm/core.clj +++ b/src/clojure_elastic_apm/core.clj @@ -142,7 +142,9 @@ (finally (end span)))))) -(defmacro with-apm-transaction [binding & body] +(defmacro with-apm-transaction + {:clj-kondo/lint-as 'clojure.core/let} + [binding & body] `(apm-transaction* (^{:once true} fn* [~(first binding)] ~@body) ~(second binding))) diff --git a/src/clojure_elastic_apm/ring.clj b/src/clojure_elastic_apm/ring.clj index 9429605..d41165f 100644 --- a/src/clojure_elastic_apm/ring.clj +++ b/src/clojure_elastic_apm/ring.clj @@ -1,15 +1,16 @@ (ns clojure-elastic-apm.ring - (:require [clojure-elastic-apm.core :refer [with-apm-transaction - type-request]])) + (:require + [clojure-elastic-apm.core :refer [type-request with-apm-transaction]] + [clojure.string :as string])) (defn match-uri [pattern uri] - (let [pattern-segs (clojure.string/split pattern #"/") - uri-segs (clojure.string/split uri #"/") + (let [pattern-segs (string/split pattern #"/") + uri-segs (string/split uri #"/") matcher (fn [p u] (cond (= p "*") u - (= p "_") "_" - (= p u) u + (p "_") "_" + (p u) u :else false))] (if (> (count pattern-segs) (count uri-segs)) @@ -17,31 +18,58 @@ (let [matches (map matcher pattern-segs uri-segs) matched? (reduce #(and %1 %2) matches)] (if matched? - (clojure.string/join "/" matches) + (string/join "/" matches) matched?))))) +(defn match-patterns [patterns uri] + (->> patterns + (map #(match-uri % uri)) + (drop-while false?) + (first))) + (defn wrap-apm-transaction ([handler] - (fn [{:keys [request-method uri headers] :as request}] - (let [tx-name (str (.toUpperCase (name request-method)) " " uri) - traceparent (get-in headers ["traceparent"])] - (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] - (let [{:keys [status] :as response} (handler (assoc request :clojure-elastic-apm/transaction tx))] - (when status - (.setResult tx (str "HTTP " status))) - response))))) + (fn + ([{:keys [request-method uri headers] :as request}] + (let [tx-name (str (.toUpperCase (name request-method)) " " uri) + traceparent (get-in headers ["traceparent"])] + (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] + (let [{:keys [status] :as response} (handler (assoc request :clojure-elastic-apm/transaction tx))] + (when status + (.setResult tx (str "HTTP " status))) + response)))) + ([{:keys [request-method uri headers] :as request} respond raise] + (let [tx-name (str (.toUpperCase (name request-method)) " " uri) + traceparent (get-in headers ["traceparent"])] + (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] + (let [req (assoc request :clojure-elastic-apm/transaction tx)] + (handler req (fn [response] + (when (:status response) + (.setResult tx (str "HTTP " (:status response)))) + (respond response)) raise))))))) ([handler patterns] - (fn [{:keys [request-method uri headers] :as request}] - (let [matched (->> patterns - (map #(match-uri % uri)) - (drop-while false?) - (first)) - tx-name (str (.toUpperCase (name request-method)) " " matched) - traceparent (get-in headers ["traceparent"])] - (if matched - (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] - (let [{:keys [status] :as response} (handler (assoc request :clojure-elastic-apm/transaction tx))] - (when status - (.setResult tx (str "HTTP " status))) - response)) - (handler request)))))) + (fn + ([{:keys [request-method uri headers] :as request}] + (let [matched (match-patterns patterns uri) + tx-name (str (.toUpperCase (name request-method)) " " matched) + traceparent (get-in headers ["traceparent"])] + (if matched + (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] + (let [{:keys [status] :as response} (handler (assoc request :clojure-elastic-apm/transaction tx))] + (when status + (.setResult tx (str "HTTP " status))) + response)) + (handler request)))) + ([{:keys [request-method uri headers] :as request} respond raise] + + (let [matched (match-patterns patterns uri) + tx-name (str (.toUpperCase (name request-method)) " " matched) + traceparent (get-in headers ["traceparent"])] + (if matched + (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] + (let [req (assoc request :clojure-elastic-apm/transaction tx)] + (handler req (fn [response] + (when (:status response) + (.setResult tx (str "HTTP " (:status response)))) + (respond response)) raise))) + (handler respond raise))))))) diff --git a/test/clojure_elastic_apm/ring_test.clj b/test/clojure_elastic_apm/ring_test.clj index 7d6f0a3..7118287 100644 --- a/test/clojure_elastic_apm/ring_test.clj +++ b/test/clojure_elastic_apm/ring_test.clj @@ -40,6 +40,26 @@ (is (= "GET /foo/bar" (get-in tx-details [:transaction :name]))) (is (nil? (:parent tx-details)))))) +(deftest wrap-apm-transaction-async-test + (let [transaction-id (atom nil) + request {:request-method :get, :uri "/foo/bar"} + response {:status 200 :body "Ok."} + respond (fn [r] + (is (= (:status r) 200)) + r) + handler (fn [request respond _] + (is (not= "" (.getId (apm/current-apm-transaction))) "transaction should've been activated for the duration of the request") + (is (= (.getId (:clojure-elastic-apm/transaction request)) (.getId (apm/current-apm-transaction))) "transaction should've been activated for the duration of the request") + (reset! transaction-id (.getId (:clojure-elastic-apm/transaction request))) + (respond response)) + wrapped-handler (apm-ring/wrap-apm-transaction handler)] + (is (= (wrapped-handler request respond nil) response)) + (let [tx-details (es-find-first-document (str "(processor.event:transaction%20AND%20transaction.id:" @transaction-id ")"))] + (is (= apm/type-request (get-in tx-details [:transaction :type]))) + (is (= "HTTP 200" (get-in tx-details [:transaction :result]))) + (is (= "GET /foo/bar" (get-in tx-details [:transaction :name]))) + (is (nil? (:parent tx-details)))))) + (deftest wrap-apm-remote-transaction-test (let [transaction-id (atom nil) parent-id "3574dfeefa12d57e" From 13ad3cc6806d4cc382d99c7a0ddfd85f513b87ef Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Wed, 11 Oct 2023 12:43:26 +0000 Subject: [PATCH 2/8] reverted accidental changes --- src/clojure_elastic_apm/ring.clj | 13 ++++++------- test/clojure_elastic_apm/ring_test.clj | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/clojure_elastic_apm/ring.clj b/src/clojure_elastic_apm/ring.clj index d41165f..88c0a49 100644 --- a/src/clojure_elastic_apm/ring.clj +++ b/src/clojure_elastic_apm/ring.clj @@ -1,16 +1,15 @@ (ns clojure-elastic-apm.ring (:require - [clojure-elastic-apm.core :refer [type-request with-apm-transaction]] - [clojure.string :as string])) + [clojure-elastic-apm.core :refer [type-request with-apm-transaction]])) (defn match-uri [pattern uri] - (let [pattern-segs (string/split pattern #"/") - uri-segs (string/split uri #"/") + (let [pattern-segs (clojure.string/split pattern #"/") + uri-segs (clojure.string/split uri #"/") matcher (fn [p u] (cond (= p "*") u - (p "_") "_" - (p u) u + (= p "_") "_" + (= p u) u :else false))] (if (> (count pattern-segs) (count uri-segs)) @@ -18,7 +17,7 @@ (let [matches (map matcher pattern-segs uri-segs) matched? (reduce #(and %1 %2) matches)] (if matched? - (string/join "/" matches) + (clojure.string/join "/" matches) matched?))))) (defn match-patterns [patterns uri] diff --git a/test/clojure_elastic_apm/ring_test.clj b/test/clojure_elastic_apm/ring_test.clj index 7118287..54982ef 100644 --- a/test/clojure_elastic_apm/ring_test.clj +++ b/test/clojure_elastic_apm/ring_test.clj @@ -52,7 +52,7 @@ (is (= (.getId (:clojure-elastic-apm/transaction request)) (.getId (apm/current-apm-transaction))) "transaction should've been activated for the duration of the request") (reset! transaction-id (.getId (:clojure-elastic-apm/transaction request))) (respond response)) - wrapped-handler (apm-ring/wrap-apm-transaction handler)] + wrapped-handler (apm-ring/wrap-apm-transaction handler ["/*/*"])] (is (= (wrapped-handler request respond nil) response)) (let [tx-details (es-find-first-document (str "(processor.event:transaction%20AND%20transaction.id:" @transaction-id ")"))] (is (= apm/type-request (get-in tx-details [:transaction :type]))) From 95377b940875c0d64ff6ff0de67b750905241a3b Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Thu, 12 Oct 2023 05:47:37 +0000 Subject: [PATCH 3/8] added async test for wrap-apm-transaction when pattern is not provided --- test/clojure_elastic_apm/ring_test.clj | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/test/clojure_elastic_apm/ring_test.clj b/test/clojure_elastic_apm/ring_test.clj index 54982ef..16358bc 100644 --- a/test/clojure_elastic_apm/ring_test.clj +++ b/test/clojure_elastic_apm/ring_test.clj @@ -40,7 +40,7 @@ (is (= "GET /foo/bar" (get-in tx-details [:transaction :name]))) (is (nil? (:parent tx-details)))))) -(deftest wrap-apm-transaction-async-test +(deftest wrap-apm-transaction-with-pattern-async-test (let [transaction-id (atom nil) request {:request-method :get, :uri "/foo/bar"} response {:status 200 :body "Ok."} @@ -60,6 +60,26 @@ (is (= "GET /foo/bar" (get-in tx-details [:transaction :name]))) (is (nil? (:parent tx-details)))))) +(deftest wrap-apm-transaction-async-test + (let [transaction-id (atom nil) + request {:request-method :get, :uri "/foo/bar"} + response {:status 200 :body "Ok."} + respond (fn [r] + (is (= (:status r) 200)) + r) + handler (fn [request respond _] + (is (not= "" (.getId (apm/current-apm-transaction))) "transaction should've been activated for the duration of the request") + (is (= (.getId (:clojure-elastic-apm/transaction request)) (.getId (apm/current-apm-transaction))) "transaction should've been activated for the duration of the request") + (reset! transaction-id (.getId (:clojure-elastic-apm/transaction request))) + (respond response)) + wrapped-handler (apm-ring/wrap-apm-transaction handler)] + (is (= (wrapped-handler request respond nil) response)) + (let [tx-details (es-find-first-document (str "(processor.event:transaction%20AND%20transaction.id:" @transaction-id ")"))] + (is (= apm/type-request (get-in tx-details [:transaction :type]))) + (is (= "HTTP 200" (get-in tx-details [:transaction :result]))) + (is (= "GET /foo/bar" (get-in tx-details [:transaction :name]))) + (is (nil? (:parent tx-details)))))) + (deftest wrap-apm-remote-transaction-test (let [transaction-id (atom nil) parent-id "3574dfeefa12d57e" From c52e5e868cd8fdf390e9eda6e7835a7a2a31aa42 Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Thu, 12 Oct 2023 05:48:46 +0000 Subject: [PATCH 4/8] removed clj-kondo rule from with-apm-transaction macro --- src/clojure_elastic_apm/core.clj | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/clojure_elastic_apm/core.clj b/src/clojure_elastic_apm/core.clj index c43280e..241c09a 100644 --- a/src/clojure_elastic_apm/core.clj +++ b/src/clojure_elastic_apm/core.clj @@ -142,9 +142,7 @@ (finally (end span)))))) -(defmacro with-apm-transaction - {:clj-kondo/lint-as 'clojure.core/let} - [binding & body] +(defmacro with-apm-transaction [binding & body] `(apm-transaction* (^{:once true} fn* [~(first binding)] ~@body) ~(second binding))) From dd6581a8d223e04fc6067eae59139007642c8955 Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Thu, 12 Oct 2023 05:50:55 +0000 Subject: [PATCH 5/8] destructure response status --- src/clojure_elastic_apm/ring.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clojure_elastic_apm/ring.clj b/src/clojure_elastic_apm/ring.clj index 88c0a49..08063b8 100644 --- a/src/clojure_elastic_apm/ring.clj +++ b/src/clojure_elastic_apm/ring.clj @@ -67,8 +67,8 @@ (if matched (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] (let [req (assoc request :clojure-elastic-apm/transaction tx)] - (handler req (fn [response] - (when (:status response) - (.setResult tx (str "HTTP " (:status response)))) + (handler req (fn [{:keys [status] :as response}] + (when status + (.setResult tx (str "HTTP " status))) (respond response)) raise))) (handler respond raise))))))) From 3e9033fa7ce2e1724f8b37f06da074b3ddc26fd0 Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Thu, 12 Oct 2023 05:55:38 +0000 Subject: [PATCH 6/8] fixed expectation in core-test/with-apm-exit-span-defaults-test --- test/clojure_elastic_apm/core_test.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/clojure_elastic_apm/core_test.clj b/test/clojure_elastic_apm/core_test.clj index f9b3e44..6273c0c 100644 --- a/test/clojure_elastic_apm/core_test.clj +++ b/test/clojure_elastic_apm/core_test.clj @@ -98,7 +98,7 @@ (let [span-details (es-find-first-document (str "(processor.event:span%20AND%20span.id:" @span-id ")"))] (is (= "TestExitSpan" (get-in span-details [:span :name]))) (is (= "ext" (get-in span-details [:span :type]))) - (is (= "undefined" (get-in span-details [:span :subtype]))) + (is (= "undefined subtype" (get-in span-details [:span :subtype]))) (is (nil? (get-in span-details [:span :action])))))) (deftest with-apm-span-no-activation-test From 8c3b8a683278dd6d7a354cf86735640b8096213a Mon Sep 17 00:00:00 2001 From: Lasse Linkola Date: Thu, 12 Oct 2023 08:54:23 +0000 Subject: [PATCH 7/8] refactor: remaining response destructuring --- src/clojure_elastic_apm/ring.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clojure_elastic_apm/ring.clj b/src/clojure_elastic_apm/ring.clj index 08063b8..ab43d74 100644 --- a/src/clojure_elastic_apm/ring.clj +++ b/src/clojure_elastic_apm/ring.clj @@ -42,9 +42,9 @@ traceparent (get-in headers ["traceparent"])] (with-apm-transaction [tx {:name tx-name :type type-request :traceparent traceparent}] (let [req (assoc request :clojure-elastic-apm/transaction tx)] - (handler req (fn [response] - (when (:status response) - (.setResult tx (str "HTTP " (:status response)))) + (handler req (fn [{:keys [status] :as response}] + (when status + (.setResult tx (str "HTTP " status))) (respond response)) raise))))))) ([handler patterns] (fn From 03625649b667a0ff395fd2e213470e780964f6c2 Mon Sep 17 00:00:00 2001 From: Ari Paasonen Date: Thu, 12 Oct 2023 14:49:25 +0300 Subject: [PATCH 8/8] Bump the version from 0.8.0 to 0.9.0 --- project.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project.clj b/project.clj index 7df5832..c8944e1 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject clojure-elastic-apm "0.8.0" +(defproject clojure-elastic-apm "0.9.0" :description "Clojure wrapper for Elastic APM Java Agent" :url "https://github.com/Yleisradio/clojure-elastic-apm" :license {:name "Eclipse Public License"