Merge pull request #26356 from radcortez/fix-26216

Fix Span names for 404 and when Micrometer is available and disabled
quarkusio · Jun 29, 2022 · ec774fb · ec774fb
2 parents 1b2aaac + 9ab4f19
commit ec774fb
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 9 deletions.
diff --git a/...deployment/src/main/java/io/quarkus/opentelemetry/deployment/tracing/TracerProcessor.java b/...deployment/src/main/java/io/quarkus/opentelemetry/deployment/tracing/TracerProcessor.java
@@ -11,6 +11,7 @@
 import java.util.Set;
 import java.util.function.BooleanSupplier;
 
+import org.eclipse.microprofile.config.ConfigProvider;
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.DotName;
@@ -60,7 +61,8 @@ static class MetricsExtensionAvailable implements BooleanSupplier {
 
         @Override
         public boolean getAsBoolean() {
-            return IS_MICROMETER_EXTENSION_AVAILABLE;
+            return IS_MICROMETER_EXTENSION_AVAILABLE && ConfigProvider.getConfig()
+                    .getOptionalValue("quarkus.micrometer.binder.http-server.enabled", Boolean.class).orElse(true);
         }
     }
 
@@ -178,7 +180,7 @@ VertxOptionsConsumerBuildItem vertxTracingOptions(TracerRecorder recorder) {
     @BuildStep(onlyIf = TracerEnabled.class, onlyIfNot = MetricsExtensionAvailable.class)
     @Record(ExecutionTime.STATIC_INIT)
     VertxOptionsConsumerBuildItem vertxTracingMetricsOptions(TracerRecorder recorder) {
-        return new VertxOptionsConsumerBuildItem(recorder.getVertxTracingMetricsOptions(), LIBRARY_AFTER);
+        return new VertxOptionsConsumerBuildItem(recorder.getVertxTracingMetricsOptions(), LIBRARY_AFTER + 1);
     }
 
     @BuildStep(onlyIf = TracerEnabled.class)

diff --git a/...ntelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/TracerRouter.java b/...ntelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/TracerRouter.java
@@ -22,5 +22,14 @@ public void register(@Observes StartupEvent ev) {
                     .end();
             rc.response().end("Hello Tracer!");
         });
+
+        router.get("/hello/:name").handler(rc -> {
+            String name = rc.pathParam("name");
+            if (name.equals("Naruto")) {
+                rc.response().end("hello " + name);
+            } else {
+                rc.response().setStatusCode(404).end();
+            }
+        });
     }
 }
diff --git a/.../deployment/src/test/java/io/quarkus/opentelemetry/deployment/VertxOpenTelemetryTest.java b/.../deployment/src/test/java/io/quarkus/opentelemetry/deployment/VertxOpenTelemetryTest.java
@@ -4,11 +4,18 @@
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_CLIENT_IP;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_FLAVOR;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_HOST;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_METHOD;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_ROUTE;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_SCHEME;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_STATUS_CODE;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_TARGET;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_USER_AGENT;
+import static io.restassured.RestAssured.given;
+import static io.vertx.core.http.HttpMethod.GET;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static java.net.HttpURLConnection.HTTP_OK;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.stringContainsInOrder;
@@ -67,7 +74,7 @@ void trace() throws NoSuchFieldException, IllegalAccessException, InvocationTarg
 
         assertEquals("io.quarkus.vertx.opentelemetry", spans.get(0).getName());
         assertEquals("hello!", spans.get(0).getAttributes().get(stringKey("test.message")));
-        assertEquals(200, spans.get(1).getAttributes().get(HTTP_STATUS_CODE));
+        assertEquals(HTTP_OK, spans.get(1).getAttributes().get(HTTP_STATUS_CODE));
         assertEquals("1.1", spans.get(1).getAttributes().get(HTTP_FLAVOR));
         assertEquals("/tracer", spans.get(1).getAttributes().get(HTTP_TARGET));
         assertEquals("http", spans.get(1).getAttributes().get(HTTP_SCHEME));
@@ -91,12 +98,54 @@ void spanNameWithoutQueryString() {
         assertEquals("io.quarkus.vertx.opentelemetry", spans.get(0).getName());
         assertEquals("hello!", spans.get(0).getAttributes().get(stringKey("test.message")));
         assertEquals("/tracer", spans.get(1).getName());
-        assertEquals(200, spans.get(1).getAttributes().get(HTTP_STATUS_CODE));
+        assertEquals(HTTP_OK, spans.get(1).getAttributes().get(HTTP_STATUS_CODE));
         assertEquals("1.1", spans.get(1).getAttributes().get(HTTP_FLAVOR));
         assertEquals("/tracer?id=1", spans.get(1).getAttributes().get(HTTP_TARGET));
         assertEquals("http", spans.get(1).getAttributes().get(HTTP_SCHEME));
         assertEquals("localhost:8081", spans.get(1).getAttributes().get(HTTP_HOST));
         assertEquals("127.0.0.1", spans.get(1).getAttributes().get(HTTP_CLIENT_IP));
         assertNotNull(spans.get(1).getAttributes().get(HTTP_USER_AGENT));
     }
+
+    @Test
+    void spanPath() {
+        given()
+                .get("/hello/{name}", "Naruto")
+                .then()
+                .statusCode(HTTP_OK)
+                .body(equalTo("hello Naruto"));
+
+        List<SpanData> spans = spanExporter.getFinishedSpanItems(1);
+
+        assertEquals("/hello/:name", spans.get(0).getName());
+        assertEquals(HTTP_OK, spans.get(0).getAttributes().get(HTTP_STATUS_CODE));
+        assertEquals(GET.toString(), spans.get(0).getAttributes().get(HTTP_METHOD));
+        assertEquals("/hello/:name", spans.get(0).getAttributes().get(HTTP_ROUTE));
+    }
+
+    @Test
+    void notFound() {
+        RestAssured.when().get("/notFound").then().statusCode(404);
+
+        List<SpanData> spans = spanExporter.getFinishedSpanItems(1);
+
+        assertEquals("/*", spans.get(0).getName());
+        assertEquals("/*", spans.get(0).getAttributes().get(HTTP_ROUTE));
+        assertEquals(HTTP_NOT_FOUND, spans.get(0).getAttributes().get(HTTP_STATUS_CODE));
+    }
+
+    @Test
+    void notFoundPath() {
+        given()
+                .get("/hello/{name}", "Goku")
+                .then()
+                .statusCode(HTTP_NOT_FOUND);
+
+        List<SpanData> spans = spanExporter.getFinishedSpanItems(1);
+
+        assertEquals("/hello/:name", spans.get(0).getName());
+        assertEquals(HTTP_NOT_FOUND, spans.get(0).getAttributes().get(HTTP_STATUS_CODE));
+        assertEquals(GET.toString(), spans.get(0).getAttributes().get(HTTP_METHOD));
+        assertEquals("/hello/:name", spans.get(0).getAttributes().get(HTTP_ROUTE));
+    }
 }
diff --git a/...main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/HttpInstrumenterVertxTracer.java b/...main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/HttpInstrumenterVertxTracer.java
@@ -1,12 +1,14 @@
 package io.quarkus.opentelemetry.runtime.tracing.vertx;
 
+import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource.FILTER;
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HTTP_CLIENT_IP;
 import static io.quarkus.opentelemetry.runtime.OpenTelemetryConfig.INSTRUMENTATION_NAME;
 
 import java.util.List;
 import java.util.function.BiConsumer;
 
+import io.netty.handler.codec.http.HttpResponseStatus;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Scope;
@@ -17,7 +19,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter;
-import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteGetter;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteBiGetter;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter;
@@ -90,7 +92,7 @@ public <R> void sendResponse(
             final TagExtractor<R> tagExtractor) {
 
         HttpRouteHolder.updateHttpRoute(spanOperation.getSpanContext(), FILTER, RouteGetter.ROUTE_GETTER,
-                ((HttpRequestSpan) spanOperation.getRequest()));
+                ((HttpRequestSpan) spanOperation.getRequest()), (HttpResponse) response);
         InstrumenterVertxTracer.super.sendResponse(context, response, spanOperation, failure, tagExtractor);
     }
 
@@ -131,11 +133,12 @@ private static Instrumenter<HttpRequest, HttpResponse> getClientInstrumenter(fin
                 .newClientInstrumenter(new HttpRequestTextMapSetter());
     }
 
-    private static class RouteGetter implements HttpRouteGetter<HttpRequestSpan> {
+    private static class RouteGetter implements HttpRouteBiGetter<HttpRequestSpan, HttpResponse> {
         static final RouteGetter ROUTE_GETTER = new RouteGetter();
 
         @Override
-        public String get(final io.opentelemetry.context.Context context, final HttpRequestSpan requestSpan) {
+        public String get(final io.opentelemetry.context.Context context, final HttpRequestSpan requestSpan,
+                final HttpResponse response) {
             // RESTEasy
             String route = requestSpan.getContext().getLocal("UrlPathTemplate");
             if (route == null) {
@@ -147,6 +150,10 @@ public String get(final io.opentelemetry.context.Context context, final HttpRequ
                 return route;
             }
 
+            if (HttpResponseStatus.NOT_FOUND.code() == response.statusCode()) {
+                return "/*";
+            }
+
             return null;
         }
     }

diff --git a/integration-tests/opentelemetry-vertx/pom.xml b/integration-tests/opentelemetry-vertx/pom.xml
@@ -26,6 +26,10 @@
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-agroal</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-micrometer</artifactId>
+        </dependency>
 
         <!-- Needed for InMemorySpanExporter to verify captured traces -->
         <dependency>
@@ -128,6 +132,19 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-micrometer-deployment</artifactId>
+            <version>${project.version}</version>
+            <type>pom</type>
+            <scope>test</scope>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>
+                    <artifactId>*</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/...n-tests/opentelemetry-vertx/src/main/java/io/quarkus/it/opentelemetry/vertx/SpanData.java b/...n-tests/opentelemetry-vertx/src/main/java/io/quarkus/it/opentelemetry/vertx/SpanData.java
@@ -22,7 +22,8 @@
         "io.opentelemetry.sdk.resources.Resource",
         "io.opentelemetry.sdk.resources.AutoValue_Resource",
         "io.opentelemetry.api.common.Attributes",
-        "io.quarkus.opentelemetry.runtime.tracing.DelayedAttributes"
+        "io.quarkus.opentelemetry.runtime.tracing.DelayedAttributes",
+        "io.opentelemetry.sdk.common.AutoValue_InstrumentationScopeInfo"
 })
 public class SpanData {
 }
diff --git a/integration-tests/opentelemetry-vertx/src/main/resources/application.properties b/integration-tests/opentelemetry-vertx/src/main/resources/application.properties
@@ -0,0 +1 @@
+quarkus.micrometer.binder.http-server.enabled=false
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		quarkus.micrometer.binder.http-server.enabled=false