Convert metrics from HandlerAspect to Middleware #2320 (#2334)

feature: - convert metrics from HandlerAspect to Middleware - refactor tests following the feature
zio · Aug 12, 2023 · fa76fe1 · fa76fe1
1 parent 05158cd
commit fa76fe1
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 79 deletions.
diff --git a/zio-http/src/main/scala/zio/http/HandlerAspect.scala b/zio-http/src/main/scala/zio/http/HandlerAspect.scala
@@ -581,76 +581,6 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand
   ): HandlerAspect.InterceptPatchZIO[Env, S] =
     new HandlerAspect.InterceptPatchZIO[Env, S](fromRequest)
 
-  /**
-   * Creates middleware that will track metrics.
-   *
-   * @param pathLabelMapper
-   *   A mapping function to map incoming paths to patterns, such as /users/1 to
-   *   /users/:id.
-   * @param totalRequestsName
-   *   Total HTTP requests metric name.
-   * @param requestDurationName
-   *   HTTP request duration metric name.
-   * @param requestDurationBoundaries
-   *   Boundaries for the HTTP request duration metric.
-   * @param extraLabels
-   *   A set of extra labels all metrics will be tagged with.
-   * @note
-   *   When using Prometheus as your metrics backend, make sure to provide a
-   *   `pathLabelMapper` in order to avoid
-   *   [[https://prometheus.io/docs/practices/naming/#labels high cardinality labels]].
-   */
-  def metrics(
-    pathLabelMapper: PartialFunction[Request, String] = Map.empty,
-    concurrentRequestsName: String = "http_concurrent_requests_total",
-    totalRequestsName: String = "http_requests_total",
-    requestDurationName: String = "http_request_duration_seconds",
-    requestDurationBoundaries: MetricKeyType.Histogram.Boundaries = defaultBoundaries,
-    extraLabels: Set[MetricLabel] = Set.empty,
-  ): HandlerAspect[Any, Unit] = {
-    val requestsTotal: Metric.Counter[RuntimeFlags] = Metric.counterInt(totalRequestsName)
-    val concurrentRequests: Metric.Gauge[Double]    = Metric.gauge(concurrentRequestsName)
-    val requestDuration: Metric.Histogram[Double]   = Metric.histogram(requestDurationName, requestDurationBoundaries)
-    val nanosToSeconds: Double                      = 1e9d
-
-    def labelsForRequest(req: Request): Set[MetricLabel] =
-      Set(
-        MetricLabel("method", req.method.toString),
-        MetricLabel("path", pathLabelMapper.lift(req).getOrElse(req.path.toString())),
-      ) ++ extraLabels
-
-    def labelsForResponse(res: Response): Set[MetricLabel] =
-      Set(
-        MetricLabel("status", res.status.code.toString),
-      )
-
-    def report(
-      start: Long,
-      requestLabels: Set[MetricLabel],
-      labels: Set[MetricLabel],
-    )(implicit trace: Trace): ZIO[Any, Nothing, Unit] =
-      for {
-        _   <- requestsTotal.tagged(labels).increment
-        _   <- concurrentRequests.tagged(requestLabels).decrement
-        end <- Clock.nanoTime
-        took = end - start
-        _ <- requestDuration.tagged(labels).update(took / nanosToSeconds)
-      } yield ()
-
-    HandlerAspect.interceptHandlerStateful(Handler.fromFunctionZIO[Request] { req =>
-      val requestLabels = labelsForRequest(req)
-
-      for {
-        start <- Clock.nanoTime
-        _     <- concurrentRequests.tagged(requestLabels).increment
-      } yield ((start, requestLabels), (req, ()))
-    })(Handler.fromFunctionZIO[((Long, Set[MetricLabel]), Response)] { case ((start, requestLabels), response) =>
-      val allLabels = requestLabels ++ labelsForResponse(response)
-
-      report(start, requestLabels, allLabels).as(response)
-    })
-  }
-
   /**
    * Creates a middleware that produces a Patch for the Response
    */

diff --git a/zio-http/src/main/scala/zio/http/Middleware.scala b/zio-http/src/main/scala/zio/http/Middleware.scala
@@ -16,6 +16,7 @@
 package zio.http
 
 import zio._
+import zio.metrics._
 import zio.stacktracer.TracingImplicits.disableAutoTrace
 
 trait Middleware[-UpperEnv] { self =>
@@ -172,4 +173,74 @@ object Middleware extends HandlerAspects {
           handler.timeoutFail(Response(status = Status.RequestTimeout))(duration)
         }
     }
+
+  /**
+   * Creates middleware that will track metrics.
+   *
+   * @param totalRequestsName
+   *   Total HTTP requests metric name.
+   * @param requestDurationName
+   *   HTTP request duration metric name.
+   * @param requestDurationBoundaries
+   *   Boundaries for the HTTP request duration metric.
+   * @param extraLabels
+   *   A set of extra labels all metrics will be tagged with.
+   */
+  def metrics(
+    concurrentRequestsName: String = "http_concurrent_requests_total",
+    totalRequestsName: String = "http_requests_total",
+    requestDurationName: String = "http_request_duration_seconds",
+    requestDurationBoundaries: MetricKeyType.Histogram.Boundaries = defaultBoundaries,
+    extraLabels: Set[MetricLabel] = Set.empty,
+  )(implicit trace: Trace): Middleware[Any] = {
+    val requestsTotal: Metric.Counter[RuntimeFlags] = Metric.counterInt(totalRequestsName)
+    val concurrentRequests: Metric.Gauge[Double]    = Metric.gauge(concurrentRequestsName)
+    val requestDuration: Metric.Histogram[Double]   = Metric.histogram(requestDurationName, requestDurationBoundaries)
+    val nanosToSeconds: Double                      = 1e9d
+
+    def labelsForRequest(routePattern: RoutePattern[_]): Set[MetricLabel] =
+      Set(
+        MetricLabel("method", routePattern.method.render),
+        MetricLabel("path", routePattern.pathCodec.render),
+      ) ++ extraLabels
+
+    def labelsForResponse(res: Response): Set[MetricLabel] =
+      Set(
+        MetricLabel("status", res.status.code.toString),
+      )
+
+    def report(
+      start: Long,
+      requestLabels: Set[MetricLabel],
+      labels: Set[MetricLabel],
+    )(implicit trace: Trace): ZIO[Any, Nothing, Unit] =
+      for {
+        _   <- requestsTotal.tagged(labels).increment
+        _   <- concurrentRequests.tagged(requestLabels).decrement
+        end <- Clock.nanoTime
+        took = end - start
+        _ <- requestDuration.tagged(labels).update(took / nanosToSeconds)
+      } yield ()
+
+    def aspect(routePattern: RoutePattern[_])(implicit trace: Trace): HandlerAspect[Any, Unit] =
+      HandlerAspect.interceptHandlerStateful(Handler.fromFunctionZIO[Request] { req =>
+        val requestLabels = labelsForRequest(routePattern)
+
+        for {
+          start <- Clock.nanoTime
+          _     <- concurrentRequests.tagged(requestLabels).increment
+        } yield ((start, requestLabels), (req, ()))
+      })(Handler.fromFunctionZIO[((Long, Set[MetricLabel]), Response)] { case ((start, requestLabels), response) =>
+        val allLabels = requestLabels ++ labelsForResponse(response)
+
+        report(start, requestLabels, allLabels).as(response)
+      })
+
+    new Middleware[Any] {
+      def apply[Env1, Err](routes: Routes[Env1, Err]): Routes[Env1, Err] =
+        Routes.fromIterable(
+          routes.routes.map(route => route.transform[Env1](_ @@ aspect(route.routePattern))),
+        )
+    }
+  }
 }
diff --git a/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala b/zio-http/src/test/scala/zio/http/internal/middlewares/MetricsSpec.scala
@@ -61,17 +61,13 @@ object MetricsSpec extends ZIOHttpSpec with HttpAppTestExtensions {
         )
       },
       test("http_requests_total with path label mapper") {
-        val app = Handler.ok.toHttpApp @@ metrics(
-          pathLabelMapper = {
-            case req if req.path.startsWith(Path("/user/")) =>
-              "/user/:id"
-          },
+        val app = (Method.GET / "user" / int("id") -> Handler.ok).toHttpApp @@ metrics(
           extraLabels = Set(MetricLabel("test", "http_requests_total with path label mapper")),
         )
 
         val total   =
           Metric.counterInt("http_requests_total").tagged("test", "http_requests_total with path label mapper")
-        val totalOk = total.tagged("path", "/user/:id").tagged("method", "GET").tagged("status", "200")
+        val totalOk = total.tagged("path", "/user/{id}").tagged("method", "GET").tagged("status", "200")
 
         for {
           _            <- app.runZIO(Request.get(url = URL(Root / "user" / "1")))
@@ -91,7 +87,9 @@ object MetricsSpec extends ZIOHttpSpec with HttpAppTestExtensions {
           .tagged("status", "200")
 
         val app: HttpApp[Any] =
-          Handler.ok.toHttpApp @@ metrics(extraLabels = Set(MetricLabel("test", "http_request_duration_seconds")))
+          (Method.GET / "ok" -> Handler.ok).toHttpApp @@ metrics(extraLabels =
+            Set(MetricLabel("test", "http_request_duration_seconds")),
+          )
 
         for {
           _        <- app.runZIO(Request.get(url = URL(Root / "ok")))
@@ -102,8 +100,8 @@ object MetricsSpec extends ZIOHttpSpec with HttpAppTestExtensions {
         val gauge = Metric
           .gauge("http_concurrent_requests_total")
           .tagged("test", "http_concurrent_requests_total")
-          .tagged("path", "/slow")
-          .tagged("method", "GET")
+          .tagged("path", "/...")
+          .tagged("method", "*")
 
         for {
           promise <- Promise.make[Nothing, Unit]