From d4419995682532a83727b1cfda7a85b14148590e Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Sat, 26 Oct 2024 17:59:48 +0530 Subject: [PATCH 1/9] refactor: prometheus attributes format to otel attributes --- .../opentelemetry/OpenTelemetryMetrics.scala | 32 +++++++++---- .../OpenTelemetryMetricsTest.scala | 46 +++++++++++-------- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index 9389c4c121..dfec33b98a 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -80,15 +80,28 @@ object OpenTelemetryMetrics { def requestActive[F[_]](meter: Meter, labels: MetricLabels): Metric[F, LongUpDownCounter] = Metric[F, LongUpDownCounter]( meter - .upDownCounterBuilder("request_active") + .upDownCounterBuilder("http.server.active_requests") .setDescription("Active HTTP requests") .setUnit("1") .build(), onRequest = (req, counter, m) => { m.unit { EndpointMetric() - .onEndpointRequest { ep => m.eval(counter.add(1, asOpenTelemetryAttributes(labels, ep, req))) } - .onResponseBody { (ep, _) => m.eval(counter.add(-1, asOpenTelemetryAttributes(labels, ep, req))) } + .onEndpointRequest { ep => + m.eval { + val attrs = asOpenTelemetryAttributes(labels, ep, req) + println(s"[DEBUG] Incrementing active requests with attributes: $attrs") + + counter.add(1, attrs) + } + } + .onResponseBody { (ep, _) => + m.eval { + val attrs = asOpenTelemetryAttributes(labels, ep, req) + println(s"[DEBUG] Decrementing active requests with attributes: $attrs") + counter.add(-1, attrs) + } + } .onException { (ep, _) => m.eval(counter.add(-1, asOpenTelemetryAttributes(labels, ep, req))) } } } @@ -97,7 +110,7 @@ object OpenTelemetryMetrics { def requestTotal[F[_]](meter: Meter, labels: MetricLabels): Metric[F, LongCounter] = Metric[F, LongCounter]( meter - .counterBuilder("request_total") + .counterBuilder("http.server.request.total") .setDescription("Total HTTP requests") .setUnit("1") .build(), @@ -106,8 +119,11 @@ object OpenTelemetryMetrics { EndpointMetric() .onResponseBody { (ep, res) => m.eval { - val otLabels = + val otLabels: Attributes = merge(asOpenTelemetryAttributes(labels, ep, req), asOpenTelemetryAttributes(labels, Right(res), None)) + + println(s"[DEBUG] Incrementing total requests with labels: $otLabels") + counter.add(1, otLabels) } } @@ -125,14 +141,14 @@ object OpenTelemetryMetrics { def requestDuration[F[_]](meter: Meter, labels: MetricLabels): Metric[F, DoubleHistogram] = Metric[F, DoubleHistogram]( meter - .histogramBuilder("request_duration") + .histogramBuilder("http.server.request.duration") .setDescription("Duration of HTTP requests") - .setUnit("ms") + .setUnit("s") .build(), onRequest = (req, recorder, m) => m.eval { val requestStart = Instant.now() - def duration = Duration.between(requestStart, Instant.now()).toMillis.toDouble + def duration = Duration.between(requestStart, Instant.now()).toMillis.toDouble / 1000.0 EndpointMetric() .onResponseHeaders { (ep, res) => m.eval { diff --git a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala index fcc096a963..4581b09594 100644 --- a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala +++ b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala @@ -25,7 +25,7 @@ import scala.concurrent.Future class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { - "default metrics" should "collect requests active" in { + "default metrics" should "collect http.server.active_requests" in { // given val reader = InMemoryMetricReader.create() val provider = SdkMeterProvider.builder().registerMetricReader(reader).build() @@ -51,17 +51,27 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { // then val point = longSumData(reader).head - point.getAttributes shouldBe Attributes.of(AttributeKey.stringKey("method"), "GET", AttributeKey.stringKey("path"), "/person") + point.getAttributes shouldBe Attributes.of( + AttributeKey.stringKey("http.request.method"), + "GET", + AttributeKey.stringKey("path"), + "/person" + ) point.getValue shouldBe 1 ScalaFutures.whenReady(response, Timeout(Span(3, Seconds))) { _ => val point = longSumData(reader).head - point.getAttributes shouldBe Attributes.of(AttributeKey.stringKey("method"), "GET", AttributeKey.stringKey("path"), "/person") + point.getAttributes shouldBe Attributes.of( + AttributeKey.stringKey("http.request.method"), + "GET", + AttributeKey.stringKey("path"), + "/person" + ) point.getValue shouldBe 0 } } - "default metrics" should "collect requests total" in { + "default metrics" should "collect http.server.request.total" in { // given val reader = InMemoryMetricReader.create() val provider = SdkMeterProvider.builder().registerMetricReader(reader).build() @@ -87,29 +97,29 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { .count { case dp if dp.getAttributes == Attributes.of( - AttributeKey.stringKey("method"), + AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.stringKey("status"), - "2xx" + AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long + 200L ) && dp.getValue == 2 => true case dp if dp.getAttributes == Attributes.of( - AttributeKey.stringKey("method"), + AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.stringKey("status"), - "4xx" + AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long + 400L ) && dp.getValue == 2 => true case _ => false } shouldBe 2 } - "default metrics" should "collect requests duration" in { + "default metrics" should "collect http.server.request.duration" in { // given val reader = InMemoryMetricReader.create() val provider = SdkMeterProvider.builder().registerMetricReader(reader).build() @@ -140,14 +150,12 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { val point = reader.collectAllMetrics().asScala.head.getHistogramData.getPoints.asScala point.map(_.getAttributes) should contain( Attributes.of( - AttributeKey.stringKey("method"), + AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.stringKey("status"), - "2xx", - AttributeKey.stringKey("phase"), - "body" + AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long + 200L ) ) } @@ -197,12 +205,12 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { // then val point = longSumData(reader).head point.getAttributes shouldBe Attributes.of( - AttributeKey.stringKey("method"), + AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.stringKey("status"), - "5xx" + AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long + 500L ) point.getValue shouldBe 1 } From 657e1096511c081d2c41a0619027b99641aeeb2f Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Wed, 30 Oct 2024 17:47:52 +0530 Subject: [PATCH 2/9] Update OpenTelemetryMetrics.scala --- .../server/metrics/opentelemetry/OpenTelemetryMetrics.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index dfec33b98a..2e20fd48e1 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -138,6 +138,7 @@ object OpenTelemetryMetrics { } ) + def requestDuration[F[_]](meter: Meter, labels: MetricLabels): Metric[F, DoubleHistogram] = Metric[F, DoubleHistogram]( meter From a6c7d6c78d4c297c448a70abfdaa2d6fdba6c7e2 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Wed, 30 Oct 2024 22:50:43 +0530 Subject: [PATCH 3/9] refactor!: prometheus and OTEL defaul Metric formats --- .../opentelemetry/OpenTelemetryMetrics.scala | 38 ++++++------------- .../OpenTelemetryMetricsTest.scala | 18 +++++---- .../sttp/tapir/server/metrics/Metric.scala | 19 ++++++++++ 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index 2e20fd48e1..d52368fd2e 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -7,7 +7,7 @@ import OpenTelemetryMetrics._ import io.opentelemetry.api.OpenTelemetry import sttp.tapir.model.ServerRequest import sttp.tapir.server.interceptor.metrics.MetricsRequestInterceptor -import sttp.tapir.server.metrics.{EndpointMetric, Metric, MetricLabels} +import sttp.tapir.server.metrics.{EndpointMetric, Metric, MetricLabels, OTELMetricLabels} import sttp.tapir.server.model.ServerResponse import java.time.{Duration, Instant} @@ -15,15 +15,15 @@ import java.time.{Duration, Instant} case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]]) { /** Registers a `request_active{path, method}` up-down-counter (assuming default labels). */ - def addRequestsActive(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsActive(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestActive(meter, labels)) /** Registers a `request_total{path, method, status}` counter (assuming default labels). */ - def addRequestsTotal(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsTotal(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestTotal(meter, labels)) /** Registers a `request_duration_seconds{path, method, status, phase}` histogram (assuming default labels). */ - def addRequestsDuration(labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsDuration(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestDuration(meter, labels)) /** Registers a custom metric. */ @@ -50,7 +50,7 @@ object OpenTelemetryMetrics { * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ def default[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] = - default(defaultMeter(otel), MetricLabels.Default) + default(defaultMeter(otel), OTELMetricLabels.Default) /** Registers default metrics (see other variants) using custom labels. */ def default[F[_]](otel: OpenTelemetry, labels: MetricLabels): OpenTelemetryMetrics[F] = default(defaultMeter(otel), labels) @@ -64,10 +64,10 @@ object OpenTelemetryMetrics { * Status is by default the status code class (1xx, 2xx, etc.), and phase can be either `headers` or `body` - request duration is * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ - def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, MetricLabels.Default) + def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, OTELMetricLabels.Default) /** Registers default metrics (see other variants) using custom labels. */ - def default[F[_]](meter: Meter, labels: MetricLabels = MetricLabels.Default): OpenTelemetryMetrics[F] = + def default[F[_]](meter: Meter, labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = OpenTelemetryMetrics( meter, List[Metric[F, _]]( @@ -87,21 +87,8 @@ object OpenTelemetryMetrics { onRequest = (req, counter, m) => { m.unit { EndpointMetric() - .onEndpointRequest { ep => - m.eval { - val attrs = asOpenTelemetryAttributes(labels, ep, req) - println(s"[DEBUG] Incrementing active requests with attributes: $attrs") - - counter.add(1, attrs) - } - } - .onResponseBody { (ep, _) => - m.eval { - val attrs = asOpenTelemetryAttributes(labels, ep, req) - println(s"[DEBUG] Decrementing active requests with attributes: $attrs") - counter.add(-1, attrs) - } - } + .onEndpointRequest { ep => m.eval(counter.add(1, asOpenTelemetryAttributes(labels, ep, req))) } + .onResponseBody { (ep, _) => m.eval(counter.add(-1, asOpenTelemetryAttributes(labels, ep, req))) } .onException { (ep, _) => m.eval(counter.add(-1, asOpenTelemetryAttributes(labels, ep, req))) } } } @@ -119,11 +106,9 @@ object OpenTelemetryMetrics { EndpointMetric() .onResponseBody { (ep, res) => m.eval { - val otLabels: Attributes = + val otLabels = merge(asOpenTelemetryAttributes(labels, ep, req), asOpenTelemetryAttributes(labels, Right(res), None)) - println(s"[DEBUG] Incrementing total requests with labels: $otLabels") - counter.add(1, otLabels) } } @@ -138,7 +123,6 @@ object OpenTelemetryMetrics { } ) - def requestDuration[F[_]](meter: Meter, labels: MetricLabels): Metric[F, DoubleHistogram] = Metric[F, DoubleHistogram]( meter @@ -149,7 +133,7 @@ object OpenTelemetryMetrics { onRequest = (req, recorder, m) => m.eval { val requestStart = Instant.now() - def duration = Duration.between(requestStart, Instant.now()).toMillis.toDouble / 1000.0 + def duration = Duration.between(requestStart, Instant.now()).toMillis.toDouble EndpointMetric() .onResponseHeaders { (ep, res) => m.eval { diff --git a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala index 4581b09594..093cd8d85f 100644 --- a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala +++ b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala @@ -101,8 +101,8 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long - 200L + AttributeKey.stringKey("http.response.status_code"), + "200" ) && dp.getValue == 2 => true case dp @@ -111,8 +111,8 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long - 400L + AttributeKey.stringKey("http.response.status_code"), + "400" ) && dp.getValue == 2 => true case _ => false @@ -154,8 +154,10 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long - 200L + AttributeKey.stringKey("http.response.status_code"), + "200", + AttributeKey.stringKey("phase"), + "body" ) ) } @@ -209,8 +211,8 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", - AttributeKey.longKey("http.response.status_code").asInstanceOf[AttributeKey[Long]], // Explicitly cast to Long - 500L + AttributeKey.stringKey("http.response.status_code"), + "500" ) point.getValue shouldBe 1 } diff --git a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala index 8c90e9fb3b..05bb9762ed 100644 --- a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala +++ b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala @@ -64,3 +64,22 @@ object MetricLabels { ) ) } + +object OTELMetricLabels { + + /** Labels request by path and http.request.method, response by http.response.status_code */ + lazy val Default: MetricLabels = MetricLabels( + forRequest = List( + "http.request.method" -> { case (_, req) => req.method.method }, + "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } + ), + forResponse = List( + "http.response.status_code" -> { + // OpenTelemetry-compliant + case Right(r) => r.code.code.toString + // Default to 500 for exceptions + case Left(_) => "500" + } + ) + ) +} From ce718700816742ff1b450d5e9c7c2ccffe0693b7 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Sun, 3 Nov 2024 09:49:28 +0530 Subject: [PATCH 4/9] feat!: add review comments --- .../opentelemetry/OpenTelemetryMetrics.scala | 14 +++++++------- .../scala/sttp/tapir/server/metrics/Metric.scala | 14 ++++++++------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index d52368fd2e..566944d308 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -7,7 +7,7 @@ import OpenTelemetryMetrics._ import io.opentelemetry.api.OpenTelemetry import sttp.tapir.model.ServerRequest import sttp.tapir.server.interceptor.metrics.MetricsRequestInterceptor -import sttp.tapir.server.metrics.{EndpointMetric, Metric, MetricLabels, OTELMetricLabels} +import sttp.tapir.server.metrics.{EndpointMetric, Metric, MetricLabels} import sttp.tapir.server.model.ServerResponse import java.time.{Duration, Instant} @@ -15,15 +15,15 @@ import java.time.{Duration, Instant} case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]]) { /** Registers a `request_active{path, method}` up-down-counter (assuming default labels). */ - def addRequestsActive(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsActive(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestActive(meter, labels)) /** Registers a `request_total{path, method, status}` counter (assuming default labels). */ - def addRequestsTotal(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsTotal(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestTotal(meter, labels)) /** Registers a `request_duration_seconds{path, method, status, phase}` histogram (assuming default labels). */ - def addRequestsDuration(labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = + def addRequestsDuration(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestDuration(meter, labels)) /** Registers a custom metric. */ @@ -50,7 +50,7 @@ object OpenTelemetryMetrics { * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ def default[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] = - default(defaultMeter(otel), OTELMetricLabels.Default) + default(defaultMeter(otel), MetricLabels.OpenTelemetryAttributes) /** Registers default metrics (see other variants) using custom labels. */ def default[F[_]](otel: OpenTelemetry, labels: MetricLabels): OpenTelemetryMetrics[F] = default(defaultMeter(otel), labels) @@ -64,10 +64,10 @@ object OpenTelemetryMetrics { * Status is by default the status code class (1xx, 2xx, etc.), and phase can be either `headers` or `body` - request duration is * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ - def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, OTELMetricLabels.Default) + def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, MetricLabels.OpenTelemetryAttributes) /** Registers default metrics (see other variants) using custom labels. */ - def default[F[_]](meter: Meter, labels: MetricLabels = OTELMetricLabels.Default): OpenTelemetryMetrics[F] = + def default[F[_]](meter: Meter, labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = OpenTelemetryMetrics( meter, List[Metric[F, _]]( diff --git a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala index 05bb9762ed..5b3ba343d6 100644 --- a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala +++ b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala @@ -63,19 +63,21 @@ object MetricLabels { } ) ) -} - -object OTELMetricLabels { - /** Labels request by path and http.request.method, response by http.response.status_code */ - lazy val Default: MetricLabels = MetricLabels( + /** Default labels for OpenTelemetry-compliant metrics, as recommended here: + * https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#http-server + * + * - `http.request.method` - HTTP request method (e.g., GET, POST). + * - `path` - The request path or route template. + * - `http.response.status_code` - HTTP response status code (200, 404, etc.). + */ + lazy val OpenTelemetryAttributes: MetricLabels = MetricLabels( forRequest = List( "http.request.method" -> { case (_, req) => req.method.method }, "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } ), forResponse = List( "http.response.status_code" -> { - // OpenTelemetry-compliant case Right(r) => r.code.code.toString // Default to 500 for exceptions case Left(_) => "500" From 1d8c612551863406cbed24e7db90ad52d5863e3a Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Fri, 8 Nov 2024 11:04:34 +0530 Subject: [PATCH 5/9] feat: add req additional attributes --- .../OpenTelemetryMetricsTest.scala | 32 +++++++++++++++---- .../sttp/tapir/server/metrics/Metric.scala | 6 ++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala index 093cd8d85f..1f0225c82a 100644 --- a/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala +++ b/metrics/opentelemetry-metrics/src/test/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetricsTest.scala @@ -55,7 +55,9 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), - "/person" + "/person", + AttributeKey.stringKey("url.scheme"), + "http" ) point.getValue shouldBe 1 @@ -65,7 +67,9 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { AttributeKey.stringKey("http.request.method"), "GET", AttributeKey.stringKey("path"), - "/person" + "/person", + AttributeKey.stringKey("url.scheme"), + "http" ) point.getValue shouldBe 0 } @@ -101,8 +105,12 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", + AttributeKey.stringKey("url.scheme"), + "http", AttributeKey.stringKey("http.response.status_code"), - "200" + "200", + AttributeKey.stringKey("error.type"), + "" ) && dp.getValue == 2 => true case dp @@ -111,8 +119,12 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", + AttributeKey.stringKey("url.scheme"), + "http", AttributeKey.stringKey("http.response.status_code"), - "400" + "400", + AttributeKey.stringKey("error.type"), + "" ) && dp.getValue == 2 => true case _ => false @@ -157,7 +169,11 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { AttributeKey.stringKey("http.response.status_code"), "200", AttributeKey.stringKey("phase"), - "body" + "body", + AttributeKey.stringKey("url.scheme"), + "http", + AttributeKey.stringKey("error.type"), + "" ) ) } @@ -211,8 +227,12 @@ class OpenTelemetryMetricsTest extends AnyFlatSpec with Matchers { "GET", AttributeKey.stringKey("path"), "/person", + AttributeKey.stringKey("url.scheme"), + "http", AttributeKey.stringKey("http.response.status_code"), - "500" + "500", + AttributeKey.stringKey("error.type"), + "" ) point.getValue shouldBe 1 } diff --git a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala index 5b3ba343d6..c229f6bf24 100644 --- a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala +++ b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala @@ -74,6 +74,7 @@ object MetricLabels { lazy val OpenTelemetryAttributes: MetricLabels = MetricLabels( forRequest = List( "http.request.method" -> { case (_, req) => req.method.method }, + "url.scheme" -> { case (_, req) => req.uri.scheme.getOrElse("unknown") }, "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } ), forResponse = List( @@ -81,6 +82,11 @@ object MetricLabels { case Right(r) => r.code.code.toString // Default to 500 for exceptions case Left(_) => "500" + }, + "error.type" -> { + case Left(ex) => ex.getClass.getSimpleName + case Right(_) => "" + } ) ) From c8c19709641388eca40529785df9fc65f9373800 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Fri, 8 Nov 2024 13:45:10 +0530 Subject: [PATCH 6/9] feat: move OTEL metrics related code to OTEL Module --- .../opentelemetry/OpenTelemetryMetrics.scala | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index 566944d308..b46d6a421d 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -15,15 +15,15 @@ import java.time.{Duration, Instant} case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]]) { /** Registers a `request_active{path, method}` up-down-counter (assuming default labels). */ - def addRequestsActive(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = + def addRequestsActive(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestActive(meter, labels)) /** Registers a `request_total{path, method, status}` counter (assuming default labels). */ - def addRequestsTotal(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = + def addRequestsTotal(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestTotal(meter, labels)) /** Registers a `request_duration_seconds{path, method, status, phase}` histogram (assuming default labels). */ - def addRequestsDuration(labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = + def addRequestsDuration(labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] = copy(metrics = metrics :+ requestDuration(meter, labels)) /** Registers a custom metric. */ @@ -36,6 +36,33 @@ case class OpenTelemetryMetrics[F[_]](meter: Meter, metrics: List[Metric[F, _]]) object OpenTelemetryMetrics { + /** Default labels for OpenTelemetry-compliant metrics, as recommended here: + * https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#http-server + * + * - `http.request.method` - HTTP request method (e.g., GET, POST). + * - `path` - The request path or route template. + * - `http.response.status_code` - HTTP response status code (200, 404, etc.). + */ + lazy val OpenTelemetryAttributes: MetricLabels = MetricLabels( + forRequest = List( + "http.request.method" -> { case (_, req) => req.method.method }, + "url.scheme" -> { case (_, req) => req.uri.scheme.getOrElse("unknown") }, + "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } + ), + forResponse = List( + "http.response.status_code" -> { + case Right(r) => r.code.code.toString + // Default to 500 for exceptions + case Left(_) => "500" + }, + "error.type" -> { + case Left(ex) => ex.getClass.getSimpleName + case Right(_) => "" + + } + ) + ) + def apply[F[_]](meter: Meter): OpenTelemetryMetrics[F] = apply(meter, Nil) def apply[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] = apply(defaultMeter(otel), Nil) def apply[F[_]](otel: OpenTelemetry, metrics: List[Metric[F, _]]): OpenTelemetryMetrics[F] = apply(defaultMeter(otel), metrics) @@ -50,7 +77,7 @@ object OpenTelemetryMetrics { * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ def default[F[_]](otel: OpenTelemetry): OpenTelemetryMetrics[F] = - default(defaultMeter(otel), MetricLabels.OpenTelemetryAttributes) + default(defaultMeter(otel), OpenTelemetryAttributes) /** Registers default metrics (see other variants) using custom labels. */ def default[F[_]](otel: OpenTelemetry, labels: MetricLabels): OpenTelemetryMetrics[F] = default(defaultMeter(otel), labels) @@ -64,10 +91,10 @@ object OpenTelemetryMetrics { * Status is by default the status code class (1xx, 2xx, etc.), and phase can be either `headers` or `body` - request duration is * measured separately up to the point where the headers are determined, and then once again when the whole response body is complete. */ - def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, MetricLabels.OpenTelemetryAttributes) + def default[F[_]](meter: Meter): OpenTelemetryMetrics[F] = default(meter, OpenTelemetryAttributes) /** Registers default metrics (see other variants) using custom labels. */ - def default[F[_]](meter: Meter, labels: MetricLabels = MetricLabels.OpenTelemetryAttributes): OpenTelemetryMetrics[F] = + def default[F[_]](meter: Meter, labels: MetricLabels = OpenTelemetryAttributes): OpenTelemetryMetrics[F] = OpenTelemetryMetrics( meter, List[Metric[F, _]]( From ba7b18fa02f7807e6c4fa294961b3975463a3198 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Fri, 8 Nov 2024 13:48:08 +0530 Subject: [PATCH 7/9] cleanup --- .../sttp/tapir/server/metrics/Metric.scala | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala index c229f6bf24..8c90e9fb3b 100644 --- a/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala +++ b/server/core/src/main/scala/sttp/tapir/server/metrics/Metric.scala @@ -63,31 +63,4 @@ object MetricLabels { } ) ) - - /** Default labels for OpenTelemetry-compliant metrics, as recommended here: - * https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#http-server - * - * - `http.request.method` - HTTP request method (e.g., GET, POST). - * - `path` - The request path or route template. - * - `http.response.status_code` - HTTP response status code (200, 404, etc.). - */ - lazy val OpenTelemetryAttributes: MetricLabels = MetricLabels( - forRequest = List( - "http.request.method" -> { case (_, req) => req.method.method }, - "url.scheme" -> { case (_, req) => req.uri.scheme.getOrElse("unknown") }, - "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } - ), - forResponse = List( - "http.response.status_code" -> { - case Right(r) => r.code.code.toString - // Default to 500 for exceptions - case Left(_) => "500" - }, - "error.type" -> { - case Left(ex) => ex.getClass.getSimpleName - case Right(_) => "" - - } - ) - ) } From 673396085dbf7ce2c40050359eb81df5c123dcd7 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Sat, 16 Nov 2024 20:55:15 +0530 Subject: [PATCH 8/9] Try to return an Option[String] with review suggestions --- .../opentelemetry/OpenTelemetryMetrics.scala | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index b46d6a421d..eaf159c161 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -49,15 +49,16 @@ object OpenTelemetryMetrics { "url.scheme" -> { case (_, req) => req.uri.scheme.getOrElse("unknown") }, "path" -> { case (ep, _) => ep.showPathTemplate(showQueryParam = None) } ), - forResponse = List( - "http.response.status_code" -> { - case Right(r) => r.code.code.toString - // Default to 500 for exceptions - case Left(_) => "500" - }, - "error.type" -> { - case Left(ex) => ex.getClass.getSimpleName - case Right(_) => "" + forResponse = List( + "http.response.status_code" -> { + case Right(r) => Some(r.code.code.toString) + case Left(_) => Some("500") + }, + "error.type" -> { + case Left(ex) => Some(ex.getClass.getName) + case Right(_) => None + } + ).collect { case (k, Some(v)) => k -> v } } ) From c654d5a1a2f6d42ec677a03eb637146767a00773 Mon Sep 17 00:00:00 2001 From: Vamshi Maskuri <117595548+varshith257@users.noreply.github.com> Date: Sat, 16 Nov 2024 21:08:01 +0530 Subject: [PATCH 9/9] Fix compilation error --- .../server/metrics/opentelemetry/OpenTelemetryMetrics.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala index eaf159c161..4f1d497e9e 100644 --- a/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala +++ b/metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala @@ -60,8 +60,6 @@ object OpenTelemetryMetrics { } ).collect { case (k, Some(v)) => k -> v } - } - ) ) def apply[F[_]](meter: Meter): OpenTelemetryMetrics[F] = apply(meter, Nil)