Merge pull request quarkusio#44523 from jcuero/main

Ensure that the name generated in opentelemetry matches the endpoint being called

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/ClientRequestContextImpl.java

@@ -44,7 +44,6 @@
 import io.smallrye.common.vertx.VertxContext;
 import io.smallrye.stork.api.ServiceInstance;
 import io.vertx.core.Context;
-import io.vertx.core.Vertx;
 
 public class ClientRequestContextImpl implements ResteasyReactiveClientRequestContext {
 
@@ -63,16 +62,10 @@ public ClientRequestContextImpl(RestClientRequestContext restClientRequestContex
         this.headersMap = new ClientRequestHeadersMap(); //restClientRequestContext.requestHeaders.getHeaders()
         this.providers = new ProvidersImpl(restClientRequestContext);
 
-        // TODO This needs to be challenged:
         // Always create a duplicated context because each REST Client invocation must have its own context
         // A separate context allows integrations like OTel to create a separate Span for each invocation (expected)
-        Context ctxt = Vertx.currentContext();
-        if (ctxt != null && VertxContext.isDuplicatedContext(ctxt)) {
-            this.context = ctxt;
-        } else {
-            Context current = client.vertx.getOrCreateContext();
-            this.context = VertxContext.createNewDuplicatedContext(current);
-        }
+        Context current = client.vertx.getOrCreateContext();
+        this.context = VertxContext.createNewDuplicatedContext(current);
         restClientRequestContext.properties.put(VERTX_CONTEXT_PROPERTY, context);
     }
 

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/HandlerChain.java

@@ -47,9 +47,27 @@ public HandlerChain(boolean captureStacktrace, int maxChunkSize, boolean followR
         this.clientErrorHandler = new ClientErrorHandler(loggingScope);
     }
 
+    private HandlerChain(ClientRestHandler clientCaptureCurrentContextRestHandler,
+            ClientRestHandler clientSwitchToRequestContextRestHandler, ClientRestHandler clientSendHandler,
+            ClientRestHandler clientSetResponseEntityRestHandler, ClientRestHandler clientResponseCompleteRestHandler,
+            ClientRestHandler clientErrorHandler) {
+        this.clientCaptureCurrentContextRestHandler = clientCaptureCurrentContextRestHandler;
+        this.clientSwitchToRequestContextRestHandler = clientSwitchToRequestContextRestHandler;
+        this.clientSendHandler = clientSendHandler;
+        this.clientSetResponseEntityRestHandler = clientSetResponseEntityRestHandler;
+        this.clientResponseCompleteRestHandler = clientResponseCompleteRestHandler;
+        this.clientErrorHandler = clientErrorHandler;
+    }
+
+    private HandlerChain newInstance() {
+        return new HandlerChain(clientCaptureCurrentContextRestHandler, clientSwitchToRequestContextRestHandler,
+                clientSendHandler, clientSetResponseEntityRestHandler, clientResponseCompleteRestHandler, clientErrorHandler);
+    }
+
     HandlerChain setPreClientSendHandler(ClientRestHandler preClientSendHandler) {
-        this.preClientSendHandler = preClientSendHandler;
-        return this;
+        HandlerChain newHandlerChain = newInstance();
+        newHandlerChain.preClientSendHandler = preClientSendHandler;
+        return newHandlerChain;
     }
 
     ClientRestHandler[] createHandlerChain(ConfigurationImpl configuration) {

independent-projects/resteasy-reactive/client/runtime/src/test/java/org/jboss/resteasy/reactive/client/impl/HandlerChainTest.java

@@ -1,6 +1,7 @@
 package org.jboss.resteasy.reactive.client.impl;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.Collections;
@@ -20,11 +21,12 @@ public class HandlerChainTest {
     @Test
     public void preSendHandlerIsAlwaysFirst() throws Exception {
 
-        var chain = new HandlerChain(false, 8096, true, LoggingScope.NONE, Collections.emptyMap(), new DefaultClientLogger());
+        var initialChain = new HandlerChain(false, 8096, true, LoggingScope.NONE, Collections.emptyMap(),
+                new DefaultClientLogger());
 
         ClientRestHandler preHandler = ctx -> {
         };
-        chain.setPreClientSendHandler(preHandler);
+        HandlerChain chain = initialChain.setPreClientSendHandler(preHandler);
 
         var config = new ConfigurationImpl(RuntimeType.CLIENT);
         ClientRequestFilter testReqFilter = ctx -> {
@@ -41,6 +43,9 @@ public void preSendHandlerIsAlwaysFirst() throws Exception {
 
         // Ensure pre-send is the very first
         assertEquals(handlers[0], preHandler);
+
+        // Ensure a chain is created when a pre-send handler is set
+        assertNotSame(initialChain, chain);
     }
 
 }

integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveResource.java

@@ -73,6 +73,17 @@ public Uni<String> helloMultipleUsingCombine() {
                 .combinedWith((s, s2) -> s + " and " + s2);
     }
 
+    @GET
+    @Path("/multiple-combine-different-paths")
+    public Uni<String> helloMultipleUsingCombineDifferentPaths() {
+        return Uni.combine().all().unis(
+                client.helloGetUniDelay(),
+                client.helloGet("Naruto"),
+                client.helloGet("Goku"),
+                client.helloGetUniExecutor())
+                .with((s, s2, s3, s4) -> s + " and " + s2 + " and " + s3 + " and " + s4);
+    }
+
     @POST
     public Uni<String> helloPost(String body) {
         Span span = tracer.spanBuilder("helloPost").startSpan();

integration-tests/opentelemetry-reactive/src/main/java/io/quarkus/it/opentelemetry/reactive/ReactiveRestClient.java

@@ -17,4 +17,12 @@ interface ReactiveRestClient {
 
     @POST
     Uni<String> helloPost(String body);
+
+    @GET
+    @Path("/hello-get-uni-executor")
+    Uni<String> helloGetUniExecutor();
+
+    @GET
+    @Path("/hello-get-uni-delay")
+    Uni<String> helloGetUniDelay();
 }

integration-tests/opentelemetry-reactive/src/test/java/io/quarkus/it/opentelemetry/reactive/OpenTelemetryReactiveTest.java

@@ -253,6 +253,92 @@ void multipleUsingCombine() {
         assertEquals("helloGet", gokuInternal.get("name"));
     }
 
+    @Test
+    void multipleUsingCombineDifferentPaths() {
+        given()
+                .when()
+                .get("/reactive/multiple-combine-different-paths")
+                .then()
+                .statusCode(200)
+                .body(equalTo("helloGetUniDelay and Hello Naruto and Hello Goku and helloGetUniExecutor"));
+
+        await().atMost(5, SECONDS).until(() -> getSpans().size() == 13);
+
+        List<Map<String, Object>> spans = getSpans();
+        assertEquals(13, spans.size());
+        assertEquals(1, spans.stream().map(map -> map.get("traceId")).collect(toSet()).size());
+
+        // First span is the call getting into the server. It does not have a parent span.
+        Map<String, Object> parent = getSpanByKindAndParentId(spans, SERVER, "0000000000000000");
+
+        // We should get 2 client spans originated by the server
+        List<Map<String, Object>> clientSpans = getSpansByKindAndParentId(spans, CLIENT, parent.get("spanId"));
+        assertEquals(4, clientSpans.size());
+
+        // Each client calls the server and programmatically create a span, so each have a server and an internal span
+
+        // helloGetUniDelay Span
+        Optional<Map<String, Object>> helloGetUniDelaySpan = clientSpans.stream()
+                .filter(map -> ((String) ((Map<?, ?>) map.get("attributes")).get(URL_FULL.getKey()))
+                        .contains("/hello-get-uni-delay"))
+                .findFirst();
+        assertTrue(helloGetUniDelaySpan.isPresent());
+        Map<String, Object> helloGetUniDelay = helloGetUniDelaySpan.get();
+        assertEquals("GET /reactive/hello-get-uni-delay", helloGetUniDelay.get("name"));
+
+        Map<String, Object> helloGetUniDelayServer = getSpanByKindAndParentId(spans, SERVER, helloGetUniDelay.get("spanId"));
+        assertEquals("/reactive/hello-get-uni-delay",
+                ((Map<?, ?>) helloGetUniDelayServer.get("attributes")).get(URL_PATH.getKey()));
+        Map<String, Object> helloGetUniDelayInternal = getSpanByKindAndParentId(spans, INTERNAL,
+                helloGetUniDelayServer.get("spanId"));
+        assertEquals("helloGetUniDelay", helloGetUniDelayInternal.get("name"));
+
+        // Naruto Span
+        Optional<Map<String, Object>> narutoSpan = clientSpans.stream()
+                .filter(map -> ((String) ((Map<?, ?>) map.get("attributes")).get(URL_FULL.getKey())).contains("Naruto"))
+                .findFirst();
+        assertTrue(narutoSpan.isPresent());
+        Map<String, Object> naruto = narutoSpan.get();
+        assertEquals("GET /reactive", naruto.get("name"));
+
+        Map<String, Object> narutoServer = getSpanByKindAndParentId(spans, SERVER, naruto.get("spanId"));
+        assertEquals("/reactive", ((Map<?, ?>) narutoServer.get("attributes")).get(URL_PATH.getKey()));
+        assertEquals("name=Naruto", ((Map<?, ?>) narutoServer.get("attributes")).get(URL_QUERY.getKey()));
+        Map<String, Object> narutoInternal = getSpanByKindAndParentId(spans, INTERNAL, narutoServer.get("spanId"));
+        assertEquals("helloGet", narutoInternal.get("name"));
+
+        // Goku Span
+        Optional<Map<String, Object>> gokuSpan = clientSpans.stream()
+                .filter(map -> ((String) ((Map<?, ?>) map.get("attributes")).get(URL_FULL.getKey())).contains("Goku"))
+                .findFirst();
+        assertTrue(gokuSpan.isPresent());
+        Map<String, Object> goku = gokuSpan.get();
+        assertEquals("GET /reactive", goku.get("name"));
+
+        Map<String, Object> gokuServer = getSpanByKindAndParentId(spans, SERVER, goku.get("spanId"));
+        assertEquals("/reactive", ((Map<?, ?>) gokuServer.get("attributes")).get(URL_PATH.getKey()));
+        assertEquals("name=Goku", ((Map<?, ?>) gokuServer.get("attributes")).get(URL_QUERY.getKey()));
+        Map<String, Object> gokuInternal = getSpanByKindAndParentId(spans, INTERNAL, gokuServer.get("spanId"));
+        assertEquals("helloGet", gokuInternal.get("name"));
+
+        // helloGetUniDelay Span
+        Optional<Map<String, Object>> helloGetUniExecutorSpan = clientSpans.stream()
+                .filter(map -> ((String) ((Map<?, ?>) map.get("attributes")).get(URL_FULL.getKey()))
+                        .contains("/hello-get-uni-executor"))
+                .findFirst();
+        assertTrue(helloGetUniExecutorSpan.isPresent());
+        Map<String, Object> helloGetUniExecutor = helloGetUniExecutorSpan.get();
+        assertEquals("GET /reactive/hello-get-uni-executor", helloGetUniExecutor.get("name"));
+
+        Map<String, Object> helloGetUniExecutorServer = getSpanByKindAndParentId(spans, SERVER,
+                helloGetUniExecutor.get("spanId"));
+        assertEquals("/reactive/hello-get-uni-executor",
+                ((Map<?, ?>) helloGetUniExecutorServer.get("attributes")).get(URL_PATH.getKey()));
+        Map<String, Object> helloGetUniExecutorInternal = getSpanByKindAndParentId(spans, INTERNAL,
+                helloGetUniExecutorServer.get("spanId"));
+        assertEquals("helloGetUniExecutor", helloGetUniExecutorInternal.get("name"));
+    }
+
     @Test
     public void securedInvalidCredential() {
         given().auth().preemptive().basic("scott", "reader2").when().get("/foo/secured/item/something")