Merge pull request #11 from Yleisradio/feature/async-wrap

async implementation for wrap-apm-transaction

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"

src/clojure_elastic_apm/ring.clj

@@ -1,6 +1,6 @@
 (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]]))
 
 (defn match-uri [pattern uri]
   (let [pattern-segs (clojure.string/split pattern #"/")
@@ -20,28 +20,55 @@
           (clojure.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 [{:keys [status] :as response}]
+                           (when status
+                             (.setResult tx (str "HTTP " status)))
+                           (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 [{:keys [status] :as response}]
+                             (when status
+                               (.setResult tx (str "HTTP " status)))
+                             (respond response)) raise)))
+          (handler respond raise)))))))

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

test/clojure_elastic_apm/ring_test.clj

@@ -40,6 +40,46 @@
       (is (= "GET /foo/bar" (get-in tx-details [:transaction :name])))
       (is (nil? (:parent tx-details))))))
 
+(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."}
+        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-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"