From b78e585418f756b1cace5361de09c59f5a6bdc16 Mon Sep 17 00:00:00 2001 From: luneo7 Date: Mon, 18 Mar 2024 16:04:05 -0500 Subject: [PATCH] Use URL path template when tracing REST clients where possible --- ...emetryTextMapPropagatorCustomizerTest.java | 2 +- .../RestClientOpenTelemetryTest.java | 6 ++-- .../restclient/OpenTelemetryClientFilter.java | 7 +++++ .../vertx/HttpInstrumenterVertxTracer.java | 29 +++++++++++++++++++ .../handlers/ClientObservabilityHandler.java | 1 + .../OpenTelemetryReactiveClientTest.java | 4 +-- .../OpenTelemetryWithSpanAtStartupTest.java | 2 +- .../it/opentelemetry/OpenTelemetryTest.java | 12 ++++---- 8 files changed, 50 insertions(+), 13 deletions(-) diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java index caf4b0ccaad5e6..a230d019f7c411 100644 --- a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java @@ -56,7 +56,7 @@ void testPropagatorCustomizer_NoPropagation() { List spans = spanExporter.getFinishedSpanItems(2); SpanData clientSpan = getSpanByKindAndParentId(spans, SpanKind.CLIENT, "0000000000000000"); - assertEquals("GET", clientSpan.getName()); + assertEquals("GET /hello", clientSpan.getName()); // There is a parent id, therefore propagation is working. SpanData serverSpan = getSpanByKindAndParentId(spans, SpanKind.SERVER, clientSpan.getSpanId()); diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java index 4cff5b2f144b48..23730f03604081 100644 --- a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java @@ -71,7 +71,7 @@ void client() { List spans = spanExporter.getFinishedSpanItems(2); SpanData client = getSpanByKindAndParentId(spans, CLIENT, "0000000000000000"); - assertEquals("GET", client.getName()); + assertEquals("GET /hello", client.getName()); assertSemanticAttribute(client, (long) HTTP_OK, HTTP_STATUS_CODE); assertSemanticAttribute(client, HttpMethod.GET, HTTP_METHOD); assertSemanticAttribute(client, uri.toString() + "hello", HTTP_URL); @@ -96,7 +96,7 @@ void spanNameWithoutQueryString() { SpanData client = getSpanByKindAndParentId(spans, CLIENT, "0000000000000000"); assertEquals(CLIENT, client.getKind()); - assertEquals("GET", client.getName()); + assertEquals("GET /hello", client.getName()); assertSemanticAttribute(client, (long) HTTP_OK, HTTP_STATUS_CODE); assertSemanticAttribute(client, HttpMethod.GET, HTTP_METHOD); assertSemanticAttribute(client, uri.toString() + "hello?query=1", HTTP_URL); @@ -132,7 +132,7 @@ void path() { SpanData client = getSpanByKindAndParentId(spans, CLIENT, "0000000000000000"); assertEquals(CLIENT, client.getKind()); - assertEquals("GET", client.getName()); + assertEquals("GET /hello/{path}", client.getName()); assertSemanticAttribute(client, (long) HTTP_OK, HTTP_STATUS_CODE); assertSemanticAttribute(client, HttpMethod.GET, HTTP_METHOD); assertSemanticAttribute(client, uri.toString() + "hello/another", HTTP_URL); diff --git a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/restclient/OpenTelemetryClientFilter.java b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/restclient/OpenTelemetryClientFilter.java index d94ae7032e5c2c..c46e2ab4d2adef 100644 --- a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/restclient/OpenTelemetryClientFilter.java +++ b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/restclient/OpenTelemetryClientFilter.java @@ -17,6 +17,7 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.TextMapSetter; @@ -41,6 +42,7 @@ public class OpenTelemetryClientFilter implements ClientRequestFilter, ClientRes public static final String REST_CLIENT_OTEL_SPAN_CLIENT_CONTEXT = "otel.span.client.context"; public static final String REST_CLIENT_OTEL_SPAN_CLIENT_PARENT_CONTEXT = "otel.span.client.parentContext"; public static final String REST_CLIENT_OTEL_SPAN_CLIENT_SCOPE = "otel.span.client.scope"; + private static final String URL_PATH_TEMPLATE_KEY = "UrlPathTemplate"; /** * Property stored in the Client Request context to retrieve the captured Vert.x context. @@ -118,6 +120,11 @@ public void filter(final ClientRequestContext request, final ClientResponseConte Context spanContext = (Context) request.getProperty(REST_CLIENT_OTEL_SPAN_CLIENT_CONTEXT); try { + String pathTemplate = (String) request.getProperty(URL_PATH_TEMPLATE_KEY); + if (pathTemplate != null && !pathTemplate.isEmpty()) { + Span.fromContext(spanContext) + .updateName(request.getMethod() + " " + pathTemplate); + } instrumenter.end(spanContext, request, response, null); } finally { scope.close(); diff --git a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/vertx/HttpInstrumenterVertxTracer.java b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/vertx/HttpInstrumenterVertxTracer.java index cdf4b86522bb88..32ef18de5b1e30 100644 --- a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/vertx/HttpInstrumenterVertxTracer.java +++ b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/intrumentation/vertx/HttpInstrumenterVertxTracer.java @@ -14,6 +14,7 @@ import io.netty.handler.codec.http.HttpResponseStatus; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapSetter; @@ -30,6 +31,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor; import io.opentelemetry.semconv.SemanticAttributes; import io.quarkus.opentelemetry.runtime.config.runtime.SemconvStabilityType; +import io.smallrye.common.vertx.VertxContext; import io.vertx.core.Context; import io.vertx.core.MultiMap; import io.vertx.core.http.HttpHeaders; @@ -42,7 +44,9 @@ import io.vertx.core.net.SocketAddress; import io.vertx.core.spi.observability.HttpRequest; import io.vertx.core.spi.observability.HttpResponse; +import io.vertx.core.spi.tracing.SpanKind; import io.vertx.core.spi.tracing.TagExtractor; +import io.vertx.core.tracing.TracingPolicy; public class HttpInstrumenterVertxTracer implements InstrumenterVertxTracer { private final Instrumenter serverInstrumenter; @@ -102,6 +106,31 @@ public void sendResponse( InstrumenterVertxTracer.super.sendResponse(context, response, spanOperation, failure, tagExtractor); } + @Override + public OpenTelemetryVertxTracer.SpanOperation sendRequest(Context context, + SpanKind kind, + TracingPolicy policy, + R request, + String operation, + BiConsumer headers, + TagExtractor tagExtractor) { + OpenTelemetryVertxTracer.SpanOperation spanOperation = InstrumenterVertxTracer.super.sendRequest(context, kind, policy, + request, + operation, headers, tagExtractor); + if (spanOperation != null) { + Context runningCtx = spanOperation.getContext(); + if (VertxContext.isDuplicatedContext(runningCtx)) { + String pathTemplate = runningCtx.getLocal("ClientUrlPathTemplate"); + if (pathTemplate != null && !pathTemplate.isEmpty()) { + Span.fromContext(spanOperation.getSpanContext()) + .updateName(((HttpRequest) spanOperation.getRequest()).method().name() + " " + pathTemplate); + } + } + } + + return spanOperation; + } + @Override public HttpRequest writableHeaders( final HttpRequest request, final BiConsumer headers) { diff --git a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientObservabilityHandler.java b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientObservabilityHandler.java index 522297601f0bea..5dde9b31a20cf1 100644 --- a/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientObservabilityHandler.java +++ b/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientObservabilityHandler.java @@ -18,5 +18,6 @@ public ClientObservabilityHandler(String templatePath) { @Override public void handle(RestClientRequestContext requestContext) throws Exception { requestContext.getClientFilterProperties().put("UrlPathTemplate", templatePath); + requestContext.getOrCreateClientRequestContext().getContext().putLocal("ClientUrlPathTemplate", templatePath); } } diff --git a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java index d552573c744086..a2c01974d6715b 100644 --- a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java +++ b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java @@ -57,7 +57,7 @@ void get() { // First span is the client call. It does not have a parent span. Map client = getSpanByKindAndParentId(spans, CLIENT, "0000000000000000"); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); - assertEquals("GET", client.get("name")); + assertEquals("GET /reactive", client.get("name")); assertEquals(HTTP_OK, ((Map) client.get("attributes")).get(HTTP_STATUS_CODE.getKey())); assertEquals(HttpMethod.GET.name(), ((Map) client.get("attributes")).get(HTTP_METHOD.getKey())); @@ -92,7 +92,7 @@ void post() { // First span is the client call. It does not have a parent span. Map client = getSpanByKindAndParentId(spans, CLIENT, "0000000000000000"); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); - assertEquals("POST", client.get("name")); + assertEquals("POST /reactive", client.get("name")); assertEquals(HTTP_OK, ((Map) client.get("attributes")).get(HTTP_STATUS_CODE.getKey())); assertEquals(HttpMethod.POST.name(), ((Map) client.get("attributes")).get(HTTP_METHOD.getKey())); diff --git a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryWithSpanAtStartupTest.java b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryWithSpanAtStartupTest.java index 0a1763f5bcf47d..b6fa9159bc6f8b 100644 --- a/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryWithSpanAtStartupTest.java +++ b/integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryWithSpanAtStartupTest.java @@ -47,7 +47,7 @@ void testGeneratedSpansUsingRestClientReactive() { // We should get one client span, from the internal method. Map server = getSpanByKindAndParentId(spans, CLIENT, client.get("spanId")); - assertEquals("GET", server.get("name")); + assertEquals("GET /stub", server.get("name")); } @Startup diff --git a/integration-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java b/integration-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java index b0a1f43fa33cad..a8df12fd2304b8 100644 --- a/integration-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java +++ b/integration-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java @@ -140,7 +140,7 @@ void testEmptyClientPath() { Map client = getSpanByKindAndParentId(spans, CLIENT, server.get("spanId")); assertEquals(CLIENT.toString(), client.get("kind")); verifyResource(client); - assertEquals("GET", client.get("name")); + assertEquals("GET /", client.get("name")); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); assertTrue((Boolean) client.get("ended")); assertTrue((Boolean) client.get("parent_valid")); @@ -206,7 +206,7 @@ void testSlashClientPath() { Map client = getSpanByKindAndParentId(spans, CLIENT, server.get("spanId")); assertEquals(CLIENT.toString(), client.get("kind")); - assertEquals("GET", client.get("name")); + assertEquals("GET /", client.get("name")); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); assertTrue((Boolean) client.get("ended")); assertTrue((Boolean) client.get("parent_valid")); @@ -261,7 +261,7 @@ void testBaggagePath() { Map client = getSpanByKindAndParentId(spans, CLIENT, server.get("spanId")); assertEquals(CLIENT.toString(), client.get("kind")); - assertEquals("GET", client.get("name")); + assertEquals("GET /from-baggage", client.get("name")); assertEquals("http://localhost:8081/from-baggage", client.get("attr_http.url")); assertEquals("200", client.get("attr_http.status_code")); assertEquals(client.get("parentSpanId"), server.get("spanId")); @@ -467,7 +467,7 @@ void testClientTracing() { assertNotNull(server.get("attr_user_agent.original")); Map client = getSpanByKindAndParentId(spans, CLIENT, server.get("spanId")); - assertEquals("GET", client.get("name")); + assertEquals("GET /client/pong/{message}", client.get("name")); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); assertTrue((Boolean) client.get("ended")); assertTrue((Boolean) client.get("parent_valid")); @@ -530,7 +530,7 @@ void testAsyncClientTracing() { assertNotNull(server.get("attr_user_agent.original")); Map client = getSpanByKindAndParentId(spans, CLIENT, server.get("spanId")); - assertEquals("GET", client.get("name")); + assertEquals("GET /client/pong/{message}", client.get("name")); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); assertTrue((Boolean) client.get("ended")); assertTrue((Boolean) client.get("parent_valid")); @@ -602,7 +602,7 @@ void testClientTracingWithInterceptor() { assertEquals("one", fromInterceptor.get("attr_message")); Map client = getSpanByKindAndParentId(spans, CLIENT, fromInterceptor.get("spanId")); - assertEquals("GET", client.get("name")); + assertEquals("GET /client/pong/{message}", client.get("name")); assertEquals(SpanKind.CLIENT.toString(), client.get("kind")); assertTrue((Boolean) client.get("ended")); assertTrue((Boolean) client.get("parent_valid"));