Merge pull request quarkusio#26898 from radcortez/fix-26776

Use http.target to drop non application spans
gastaldi · Aug 23, 2022 · 2f5a0bf · 2f5a0bf
2 parents 5677f43 + 9572d44
commit 2f5a0bf
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 19 deletions.
diff --git a/...telemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/HelloResource.java b/...telemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/HelloResource.java
@@ -11,6 +11,7 @@ public class HelloResource {
 
     @GET
     public String get() {
+        // The span name is not updated with the route yet.
         return ((ReadableSpan) Span.current()).getName();
     }
 }
diff --git a/...eployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryDevModeTest.java b/...eployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetryDevModeTest.java
@@ -23,7 +23,7 @@ void testDevMode() {
         //and the hot replacement stuff is not messing things up
         RestAssured.when().get("/hello").then()
                 .statusCode(200)
-                .body(is("/hello"));
+                .body(is("HTTP GET"));
 
         RestAssured.when().get("/tracer").then()
                 .statusCode(200)

diff --git a/...try/runtime/tracing/DropNamesSampler.java → ...y/runtime/tracing/DropTargetsSampler.java b/...try/runtime/tracing/DropNamesSampler.java → ...y/runtime/tracing/DropTargetsSampler.java
@@ -8,25 +8,26 @@
 import io.opentelemetry.sdk.trace.data.LinkData;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 
-public class DropNamesSampler implements Sampler {
+public class DropTargetsSampler implements Sampler {
     private final Sampler sampler;
-    private final List<String> dropNames;
+    private final List<String> dropTargets;
 
-    public DropNamesSampler(Sampler sampler, List<String> dropNames) {
+    public DropTargetsSampler(Sampler sampler, List<String> dropTargets) {
         this.sampler = sampler;
-        this.dropNames = dropNames;
+        this.dropTargets = dropTargets;
     }
 
     @Override
     public SamplingResult shouldSample(Context parentContext, String traceId, String name, SpanKind spanKind,
             Attributes attributes, List<LinkData> parentLinks) {
 
         if (spanKind.equals(SpanKind.SERVER)) {
-            for (String dropName : dropNames) {
-                if (name.startsWith(dropName)) {
-                    return SamplingResult.drop();
-                }
+            String target = attributes.get(SemanticAttributes.HTTP_TARGET);
+            // TODO - radcortez - Match /* endpoints
+            if (target != null && dropTargets.contains(target)) {
+                return SamplingResult.drop();
             }
         }
 

diff --git a/...emetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerRecorder.java b/...emetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerRecorder.java
@@ -139,14 +139,14 @@ public void setupSampler(
             List<String> dropStaticResources) {
 
         LateBoundSampler lateBoundSampler = CDI.current().select(LateBoundSampler.class, Any.Literal.INSTANCE).get();
-        List<String> dropNames = new ArrayList<>();
+        List<String> dropTargets = new ArrayList<>();
         if (config.suppressNonApplicationUris) {
-            dropNames.addAll(dropNonApplicationUris);
+            dropTargets.addAll(dropNonApplicationUris);
         }
         if (!config.includeStaticResources) {
-            dropNames.addAll(dropStaticResources);
+            dropTargets.addAll(dropStaticResources);
         }
-        Sampler samplerBean = TracerUtil.mapSampler(config.sampler, dropNames);
+        Sampler samplerBean = TracerUtil.mapSampler(config.sampler, dropTargets);
         lateBoundSampler.setSamplerDelegate(samplerBean);
     }
 }
diff --git a/...ntelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerUtil.java b/...ntelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/TracerUtil.java
@@ -38,15 +38,15 @@ private static Sampler getBaseSampler(String samplerName, Optional<Double> ratio
     }
 
     public static Sampler mapSampler(TracerRuntimeConfig.SamplerConfig samplerConfig,
-            List<String> dropNames) {
+            List<String> dropTargets) {
         Sampler sampler = CDI.current()
                 .select(Sampler.class, Any.Literal.INSTANCE)
                 .stream()
                 .filter(o -> !(o instanceof LateBoundSampler))
                 .findFirst().orElseGet(() -> getBaseSampler(samplerConfig.samplerName, samplerConfig.ratio));
 
-        if (!dropNames.isEmpty()) {
-            sampler = new DropNamesSampler(sampler, dropNames);
+        if (!dropTargets.isEmpty()) {
+            sampler = new DropTargetsSampler(sampler, dropTargets);
         }
 
         if (samplerConfig.parentBased) {

diff --git a/...main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/HttpInstrumenterVertxTracer.java b/...main/java/io/quarkus/opentelemetry/runtime/tracing/vertx/HttpInstrumenterVertxTracer.java
@@ -190,7 +190,7 @@ public String target(final HttpRequest request) {
 
         @Override
         public String route(final HttpRequest request) {
-            return request.uri().length() > 1 ? request.uri() : null;
+            return null;
         }
 
         @Override

diff --git a/...tx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java b/...tx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java
@@ -1,5 +1,6 @@
 package io.quarkus.vertx.http.deployment;
 
+import static io.quarkus.runtime.TemplateHtmlBuilder.adjustRoot;
 import static io.quarkus.vertx.http.deployment.RouteBuildItem.RouteType.FRAMEWORK_ROUTE;
 
 import java.io.IOException;
@@ -51,6 +52,7 @@
 import io.quarkus.vertx.http.deployment.devmode.HttpRemoteDevClientProvider;
 import io.quarkus.vertx.http.deployment.devmode.NotFoundPageDisplayableEndpointBuildItem;
 import io.quarkus.vertx.http.deployment.spi.FrameworkEndpointsBuildItem;
+import io.quarkus.vertx.http.runtime.BasicRoute;
 import io.quarkus.vertx.http.runtime.CurrentRequestProducer;
 import io.quarkus.vertx.http.runtime.CurrentVertxRequest;
 import io.quarkus.vertx.http.runtime.HttpBuildTimeConfig;
@@ -93,8 +95,19 @@ FrameworkEndpointsBuildItem frameworkEndpoints(NonApplicationRootPathBuildItem n
             List<RouteBuildItem> routes) {
         List<String> frameworkEndpoints = new ArrayList<>();
         for (RouteBuildItem route : routes) {
-            if (FRAMEWORK_ROUTE.equals(route.getRouteType()) && route.getConfiguredPathInfo() != null) {
-                frameworkEndpoints.add(route.getConfiguredPathInfo().getEndpointPath(nonApplicationRootPath));
+            if (FRAMEWORK_ROUTE.equals(route.getRouteType())) {
+                if (route.getConfiguredPathInfo() != null) {
+                    frameworkEndpoints.add(route.getConfiguredPathInfo().getEndpointPath(nonApplicationRootPath));
+                    continue;
+                }
+                if (route.getRouteFunction() != null && route.getRouteFunction() instanceof BasicRoute) {
+                    BasicRoute basicRoute = (BasicRoute) route.getRouteFunction();
+                    if (basicRoute.getPath() != null) {
+                        // Calling TemplateHtmlBuilder does not see very correct here, but it is the underlying API for ConfiguredPathInfo
+                        frameworkEndpoints
+                                .add(adjustRoot(nonApplicationRootPath.getNonApplicationRootPath(), basicRoute.getPath()));
+                    }
+                }
             }
         }
         return new FrameworkEndpointsBuildItem(frameworkEndpoints);