Skip to content

Commit

Permalink
Merge pull request #12 from Yleisradio/fix/async-middleware
Browse files Browse the repository at this point in the history 
Fix transaction handling in async middleware
  • Loading branch information
@AriPaaWun
AriPaaWun authored Oct 26, 2023
2 parents 77e9598 + b5ce294 commit d01d549
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 48 deletions.
2 changes: 1 addition & 1 deletion project.clj
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
(defproject clojure-elastic-apm "0.9.0"
(defproject clojure-elastic-apm "0.10.0"
:description "Clojure wrapper for Elastic APM Java Agent"
:url "https://github.com/Yleisradio/clojure-elastic-apm"
:license {:name "Eclipse Public License"
Expand Down
54 changes: 23 additions & 31 deletions src/clojure_elastic_apm/ring.clj
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(ns clojure-elastic-apm.ring
(:require
[clojure-elastic-apm.core :refer [type-request with-apm-transaction]]))
[clojure-elastic-apm.core :as apm :refer [type-request with-apm-transaction]]))

(defn match-uri [pattern uri]
(let [pattern-segs (clojure.string/split pattern #"/")
Expand All @@ -21,31 +21,16 @@
matched?)))))

(defn match-patterns [patterns uri]
(->> patterns
(map #(match-uri % uri))
(drop-while false?)
(first)))
(if patterns
(->> patterns
(map #(match-uri % uri))
(drop-while false?)
(first))
uri))

(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))))
([{: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 [{:keys [status] :as response}]
(when status
(.setResult tx (str "HTTP " status)))
(respond response)) raise)))))))
(wrap-apm-transaction handler nil))
([handler patterns]
(fn
([{:keys [request-method uri headers] :as request}]
Expand All @@ -60,15 +45,22 @@
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 [{:keys [status] :as response}]
(when status
(.setResult tx (str "HTTP " status)))
(respond response)) raise)))
(handler respond raise)))))))
(let [tx (apm/start-transaction {:name tx-name :type type-request :traceparent traceparent})
req (assoc request :clojure-elastic-apm/transaction tx)]
(with-open [_ (apm/activate tx)]
(handler req
(fn [{:keys [status] :as response}]
(when status
(.setResult tx (str "HTTP " status)))
(apm/end tx)
(respond response))
(fn [err]
(when (instance? Exception err)
(apm/capture-exception tx err))
(apm/end tx)
(raise err)))))
(handler request respond raise)))))))
56 changes: 40 additions & 16 deletions test/clojure_elastic_apm/ring_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,45 +41,69 @@
(is (nil? (:parent tx-details))))))

(deftest wrap-apm-transaction-with-pattern-async-test
(let [transaction-id (atom nil)
(let [transaction-id (promise)
request {:request-method :get, :uri "/foo/bar"}
response {:status 200 :body "Ok."}
response-promise (promise)
respond (fn [r]
(is (= (:status r) 200))
r)
(deliver response-promise 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))
(future
(with-open [_ (apm/activate (:clojure-elastic-apm/transaction request))]
(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")
(deliver 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))
(wrapped-handler request respond nil)
(is (= response (deref response-promise 1000 ::timeout)))
(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-transaction-async-test
(let [transaction-id (atom nil)
(let [transaction-id (promise)
response-promise (promise)
request {:request-method :get, :uri "/foo/bar"}
response {:status 200 :body "Ok."}
respond (fn [r]
(is (= (:status r) 200))
r)
(deliver response-promise 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))
(future
(with-open [_ (apm/activate (:clojure-elastic-apm/transaction request))]
(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")
(deliver 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))
(wrapped-handler request respond nil)
(is (= response (deref response-promise 1000 ::timeout)))
(is (not= "" @transaction-id))
(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-transaction-async-exception-test
(let [transaction-id (promise)
request {:request-method :get, :uri "/foo/bar"}
raise (fn [_])
handler (fn [request _ raise]
(future
(with-open [_ (apm/activate (:clojure-elastic-apm/transaction request))]
(deliver transaction-id (.getId (:clojure-elastic-apm/transaction request)))
(raise (ex-info "Error in handler" {})))))
wrapped-handler (apm-ring/wrap-apm-transaction handler)]
(wrapped-handler request nil raise)
(is (not= "" @transaction-id))
(let [tx-details (es-find-first-document (str "(processor.event:error%20AND%20transaction.id:" @transaction-id ")"))]
(is (= apm/type-request (get-in tx-details [:transaction :type])))
(is (= "clojure.lang.ExceptionInfo" (get-in tx-details [:error :exception 0 :type])))
(is (= "Error in handler" (get-in tx-details [:error :exception 0 :message]))))))

(deftest wrap-apm-remote-transaction-test
(let [transaction-id (atom nil)
parent-id "3574dfeefa12d57e"
Expand Down

0 comments on commit d01d549

Please sign in to comment.