Support ring metrics for asynchronous requests

[lsnape]

The Aleph web server, built on top of Netty, gives the option of returning a manifold deferred object in addition to the standard ring response map. This allows the request to be carried out asynchronously.

The current instrument middleware assumes a synchronous implementation. It would be great if async was supported as well.

It wouldn't be much work to get this working. Something along the lines of:

(defn wrap-metrics
  [handler]
  (fn [req]
    (start-metrics)
    (let [resp (handler req)]
      (if (instance? manifold.deferred.Deferred resp)
        (on-realized resp stop-metrics)
        (stop-metrics))
      resp)))

Of course, it means introducing manifold as a dependency. It's a small library though, and only introduces a couple of transitive dependencies.

Happy to submit a PR for this.

[michaelklishin]

This would tie the library to manifold.deferred.Deferred, whatever that is. I'm in favour of introducing a new sub-project.

[lsnape]

That's fair enough. It's a shame though as most of the implementation is the same as that of metrics-clojure-ring. Would refactoring metrics-clojure-ring so that metrics-clojure-aleph can pull it in and use the start/stop-metrics functions be a possibility?

[mping]

Ring has support for async middleware through asynchronous handlers: https://github.com/ring-clojure/ring/wiki/Concepts#handlers. Can't this be supported through that? It's a matter of supplying a 3-arity middleware fn, I guess.

[mping]

Here's my attempt in case someone wants to quickly hack it:

Go to metrics.ring.instrument:

(defn- time-request [thunk metrics request]
    (let [{:keys [active-requests requests responses
                  schemes statuses times request-methods]} metrics]
      (inc! active-requests)
      (try
        (let [request-method (:request-method request)
              request-scheme (:scheme request)]
          (mark! requests)
          (mark-in! request-methods request-method)
          (mark-in! schemes request-scheme)
          (let [resp (time! (times request-method (times :other))
                            (thunk))
                ^{:tag "int"} status-code (or (:status resp) 404)]
            (mark! responses)
            (mark-in! statuses (int (/ status-code 100)))
            resp))
        (finally (dec! active-requests)))))

(defn handle-request 
  ([handler metrics request]
    (time-request #(handler request) metrics request))
  ([handler metrics request respond raise]
    (try 
      (time-request #(handler request respond raise) metrics request)
      (catch Exception e (raise e)))))

Ensure instrument fns handle multiple arity

(defn instrument
  ([handler]
   (instrument handler default-registry))
  ([handler ^MetricRegistry reg]
   (fn 
     ([request]
      (handle-request handler
                      (ring-metrics reg {:prefix []})
                      request))
     ([request respond raise]
      (handle-request handler
                      (ring-metrics reg {:prefix []})
                      request respond raise)))))

(defn instrument-by
  "Instrument a ring handler using the metrics returned by the `metrics-for`
   function. `metrics-by` should be a function which takes an atom, a registry
   and a request and return a collection of metrics objects that pertain to
   some type of request.

   For example, `metrics-by-uri` maintains separate metrics for each endpoint

   This middleware should be added as late as possible (nearest to the outside of
   the \"chain\") for maximum effect.
  "
  ([handler ^MetricRegistry metrics-prefix]
   (instrument-by handler default-registry metrics-prefix))
  ([handler ^MetricRegistry reg metrics-prefix]
   (let [metrics-db (atom {})]
    (fn 
     ([request]
      (handle-request handler
                      (metrics-for metrics-db (metrics-prefix request) reg)
                      request))
     ([request respond raise]
      (handle-request handler
                      (metrics-for metrics-db (metrics-prefix request) reg)
                      request respond raise))))))