Merge pull request quarkusio#39534 from luneo7/rest-client-otel

Use URL path template when tracing REST clients where possible
aloubyansky · Mar 21, 2024 · 3da6a27 · 3da6a27
2 parents a949acb + d7b1ca5
commit 3da6a27
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 13 deletions.
diff --git a/...ava/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java b/...ava/io/quarkus/opentelemetry/deployment/OpenTelemetryTextMapPropagatorCustomizerTest.java
@@ -56,7 +56,7 @@ void testPropagatorCustomizer_NoPropagation() {
 
         List<SpanData> 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/...java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java b/...java/io/quarkus/opentelemetry/deployment/instrumentation/RestClientOpenTelemetryTest.java
@@ -71,7 +71,7 @@ void client() {
         List<SpanData> 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/...us/opentelemetry/runtime/tracing/intrumentation/restclient/OpenTelemetryClientFilter.java b/...us/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/...arkus/opentelemetry/runtime/tracing/intrumentation/vertx/HttpInstrumenterVertxTracer.java b/...arkus/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<HttpRequest, HttpResponse> {
     private final Instrumenter<HttpRequest, HttpResponse> serverInstrumenter;
@@ -102,6 +106,31 @@ public <R> void sendResponse(
         InstrumenterVertxTracer.super.sendResponse(context, response, spanOperation, failure, tagExtractor);
     }
 
+    @Override
+    public <R> OpenTelemetryVertxTracer.SpanOperation sendRequest(Context context,
+            SpanKind kind,
+            TracingPolicy policy,
+            R request,
+            String operation,
+            BiConsumer<String, String> headers,
+            TagExtractor<R> 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<String, String> headers) {

diff --git a/...src/main/java/org/jboss/resteasy/reactive/client/handlers/ClientObservabilityHandler.java b/...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/...e/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveClientTest.java b/...e/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<String, Object> 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<String, Object> 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/...rc/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryWithSpanAtStartupTest.java b/...rc/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<String, Object> server = getSpanByKindAndParentId(spans, CLIENT, client.get("spanId"));
-        assertEquals("GET", server.get("name"));
+        assertEquals("GET /stub", server.get("name"));
     }
 
     @Startup

diff --git a/...tion-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java b/...tion-tests/opentelemetry/src/test/java/io/quarkus/it/opentelemetry/OpenTelemetryTest.java
@@ -140,7 +140,7 @@ void testEmptyClientPath() {
         Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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"));