From eac6ff7dfbe1564291e6b205585511088be0e72b Mon Sep 17 00:00:00 2001 From: StevenPJ Date: Fri, 14 Oct 2022 11:18:48 +0100 Subject: [PATCH] Set http.route attribute as path template (#149) * Set http.route attribute as path template We have seen issues with the current spans exported from the http library in observability backends, due to too granular span names when the routes contain path variables. As per the opentelemetry specification, this PR extracts the path template from the UriRouteMatch object, and sets it as the `http.route` attribute. https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/http/#http-server-semantic-conventions * Update tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/server/MicronautHttpServerAttributesGetter.java Co-authored-by: Sergio del Amo Co-authored-by: Nemanja Mikic --- tracing-opentelemetry-http/build.gradle | 1 + .../MicronautHttpServerAttributesGetter.java | 13 ++++++ .../http/OpenTelemetryHttpSpec.groovy | 43 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/tracing-opentelemetry-http/build.gradle b/tracing-opentelemetry-http/build.gradle index 0f64b3831..cfc7af864 100644 --- a/tracing-opentelemetry-http/build.gradle +++ b/tracing-opentelemetry-http/build.gradle @@ -8,6 +8,7 @@ dependencies { api platform (libs.boms.opentelemetry.instrumentation.alpha) api mn.micronaut.runtime api mn.micronaut.http.client + api mn.micronaut.router api projects.tracingAnnotation api projects.tracingOpentelemetry api libs.opentelemetry.api diff --git a/tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/server/MicronautHttpServerAttributesGetter.java b/tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/server/MicronautHttpServerAttributesGetter.java index 2f83025d5..7a035ea4e 100644 --- a/tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/server/MicronautHttpServerAttributesGetter.java +++ b/tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/server/MicronautHttpServerAttributesGetter.java @@ -21,10 +21,14 @@ import io.micronaut.http.HttpAttributes; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; +import io.micronaut.http.uri.UriMatchTemplate; +import io.micronaut.web.router.UriRoute; +import io.micronaut.web.router.UriRouteMatch; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.util.List; +import java.util.Optional; @Internal enum MicronautHttpServerAttributesGetter implements HttpServerAttributesGetter, HttpResponse> { @@ -79,6 +83,15 @@ public String target(HttpRequest request) { @Override @Nullable public String route(HttpRequest request) { + Optional routeInfo = request.getAttribute(HttpAttributes.ROUTE_INFO) + .filter(UriRouteMatch.class::isInstance) + .map(ri -> (UriRouteMatch) ri) + .map(UriRouteMatch::getRoute) + .map(UriRoute::getUriMatchTemplate) + .map(UriMatchTemplate::toPathString); + if (routeInfo.isPresent()) { + return routeInfo.get().toString(); + } String route = request.getAttribute(HttpAttributes.URI_TEMPLATE).map(Object::toString) .orElse(request.getUri().toString()); return request.getMethodName() + " - " + route; diff --git a/tracing-opentelemetry-http/src/test/groovy/io/micronaut/tracing/instrument/http/OpenTelemetryHttpSpec.groovy b/tracing-opentelemetry-http/src/test/groovy/io/micronaut/tracing/instrument/http/OpenTelemetryHttpSpec.groovy index 8ce71a4df..ddaac3a4a 100644 --- a/tracing-opentelemetry-http/src/test/groovy/io/micronaut/tracing/instrument/http/OpenTelemetryHttpSpec.groovy +++ b/tracing-opentelemetry-http/src/test/groovy/io/micronaut/tracing/instrument/http/OpenTelemetryHttpSpec.groovy @@ -3,6 +3,7 @@ package io.micronaut.tracing.instrument.http import groovy.util.logging.Slf4j import io.micronaut.context.ApplicationContext import io.micronaut.core.annotation.Introspected +import io.micronaut.core.annotation.Nullable import io.micronaut.http.HttpRequest import io.micronaut.http.HttpResponse import io.micronaut.http.annotation.* @@ -251,6 +252,36 @@ class OpenTelemetryHttpSpec extends Specification { exporter.reset() } + void 'route match template is added as route attribute'() { + def testExporter = embeddedServer.applicationContext.getBean(InMemorySpanExporter) + def warehouseClient = embeddedServer.applicationContext.getBean(WarehouseClient) + + expect: + + warehouseClient.order(UUID.randomUUID()) + conditions.eventually { + testExporter.finishedSpanItems.attributes.stream().anyMatch(x -> x.get(AttributeKey.stringKey("http.route")) == "/client/order/{orderId}") + } + + cleanup: + testExporter.reset() + } + + void 'query variables are not included in route template attribute'() { + def testExporter = embeddedServer.applicationContext.getBean(InMemorySpanExporter) + def warehouseClient = embeddedServer.applicationContext.getBean(WarehouseClient) + + expect: + + warehouseClient.order(UUID.randomUUID(), UUID.randomUUID()) + conditions.eventually { + testExporter.finishedSpanItems.attributes.stream().anyMatch(x -> x.get(AttributeKey.stringKey("http.route")) == "/client/order/{orderId}") + } + + cleanup: + testExporter.reset() + } + @Introspected static class SomeBody { } @@ -429,6 +460,12 @@ class OpenTelemetryHttpSpec extends Specification { } + @Get("/order/{orderId}") + void order(@PathVariable("orderId") UUID orderId, @Nullable @QueryValue("customerId") UUID customerId) { + + } + + } @Client("/client") @@ -442,6 +479,12 @@ class OpenTelemetryHttpSpec extends Specification { @NewSpan void order(@SpanTag("warehouse.order") Map json); + @Get("/order/{orderId}") + void order(UUID orderId); + + @Get("/order/{orderId}?customerId={customerId}") + void order(UUID orderId, UUID customerId); + } @Client(id = "correctspanname")