From 5da616bbc1a25c3dfcbeef057ed259e8c06ee24f Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Fri, 27 Dec 2024 16:12:38 +0100 Subject: [PATCH 1/9] Do not prefix URL when URL is already absolute Management URLs were prefixed twice when absolute: http://localhost:9000http://localhost:9000/q/health Which was defeating the logic removing the host when collecting suppressed URIs. Fixes #36510 --- .../vertx/http/deployment/devmode/ConfiguredPathInfo.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/devmode/ConfiguredPathInfo.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/devmode/ConfiguredPathInfo.java index a2cceab2f5cee..bb93a19b566b2 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/devmode/ConfiguredPathInfo.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/devmode/ConfiguredPathInfo.java @@ -33,12 +33,12 @@ public String getEndpointPath(HttpRootPathBuildItem httpRoot) { public String getEndpointPath(NonApplicationRootPathBuildItem nonAppRoot, ManagementInterfaceBuildTimeConfig mibt, LaunchModeBuildItem mode) { + if (absolutePath) { + return endpointPath; + } if (management && mibt.enabled) { var prefix = NonApplicationRootPathBuildItem.getManagementUrlPrefix(mode); return prefix + endpointPath; - } - if (absolutePath) { - return endpointPath; } else { return TemplateHtmlBuilder.adjustRoot(nonAppRoot.getNormalizedHttpRootPath(), endpointPath); } From c377d4abb02ff0b27dcbf905689c9d49612b77aa Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Fri, 10 Jan 2025 15:16:52 +0100 Subject: [PATCH 2/9] Identify non application routes with a name The name is used in the metrics if defined and thus we can pass a fully qualifier path for the route. --- .../NonApplicationRootPathBuildItem.java | 9 +++++- .../vertx/http/deployment/RouteBuildItem.java | 14 +++++++++ .../vertx/http/runtime/BasicRoute.java | 31 +++++++++++++++++-- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/NonApplicationRootPathBuildItem.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/NonApplicationRootPathBuildItem.java index 0dc4b5e1f280c..8f0b9f91369a4 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/NonApplicationRootPathBuildItem.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/NonApplicationRootPathBuildItem.java @@ -270,6 +270,7 @@ public static class Builder extends RouteBuildItem.Builder { private final NonApplicationRootPathBuildItem buildItem; private RouteBuildItem.RouteType routeType = RouteBuildItem.RouteType.FRAMEWORK_ROUTE; private RouteBuildItem.RouteType routerType = RouteBuildItem.RouteType.FRAMEWORK_ROUTE; + private String name; private String path; Builder(NonApplicationRootPathBuildItem buildItem) { @@ -330,7 +331,13 @@ public Builder orderedRoute(String route, Integer order, Consumer routeFu this.path = route; this.routerType = RouteBuildItem.RouteType.ABSOLUTE_ROUTE; } - super.orderedRoute(this.path, order, routeFunction); + + // we normalize the route name to remove trailing *, this is to be consistent with the path + // see RouteImpl#setPath() + String routeName = route.charAt(route.length() - 1) == '*' ? route.substring(0, route.length() - 1) : route; + + // we pass a route name for proper identification in the metrics + super.orderedRoute(routeName, this.path, order, routeFunction); return this; } diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/RouteBuildItem.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/RouteBuildItem.java index 7156e0db32b43..c4cf342638e4a 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/RouteBuildItem.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/RouteBuildItem.java @@ -176,6 +176,20 @@ public Builder orderedRoute(String route, Integer order, Consumer routeCu return this; } + /** + * @param name The name of the route. It is used to identify the route in the metrics. + * @param route A normalized path used to define a basic route + * (e.g. use HttpRootPathBuildItem to construct/resolve the path value). This path this is also + * used on the "Not Found" page in dev mode. + * @param order Priority ordering of the route + * @param routeCustomizer Route customizer. + */ + public Builder orderedRoute(String name, String route, Integer order, Consumer routeCustomizer) { + this.routeFunction = new BasicRoute(name, route, order, routeCustomizer); + this.notFoundPagePath = this.routePath = route; + return this; + } + public Builder handler(Handler handler) { this.handler = handler; return this; diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/BasicRoute.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/BasicRoute.java index 064145e894d45..bd5ec80d091a8 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/BasicRoute.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/BasicRoute.java @@ -8,6 +8,8 @@ public class BasicRoute implements Function { + private String name; + private String path; private Integer order; @@ -15,15 +17,29 @@ public class BasicRoute implements Function { private Consumer customizer; public BasicRoute(String path) { - this(path, null); + this(null, path); } public BasicRoute(String path, Integer order) { + this(null, path, order); + } + + public BasicRoute(String path, Integer order, Consumer customizer) { + this(null, path, order, customizer); + } + + public BasicRoute(String name, String path) { + this(name, path, null); + } + + public BasicRoute(String name, String path, Integer order) { + this.name = name; this.path = path; this.order = order; } - public BasicRoute(String path, Integer order, Consumer customizer) { + public BasicRoute(String name, String path, Integer order, Consumer customizer) { + this.name = name; this.path = path; this.order = order; this.customizer = customizer; @@ -32,6 +48,14 @@ public BasicRoute(String path, Integer order, Consumer customizer) { public BasicRoute() { } + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + public String getPath() { return path; } @@ -60,6 +84,9 @@ public BasicRoute setCustomizer(Consumer customizer) { @Override public Route apply(Router router) { Route route = router.route(path); + if (name != null) { + route.setName(name); + } if (order != null) { route.order(order); } From 9f806c0b7f25aa79f8a70fdce67d66e30d7a90bf Mon Sep 17 00:00:00 2001 From: brunobat Date: Wed, 15 Jan 2025 11:25:13 +0000 Subject: [PATCH 3/9] Test for non app uris --- extensions/opentelemetry/deployment/pom.xml | 5 + .../OpenTelemetrySuppressNonAppUriTest.java | 103 ++++++++++++++++++ .../common/exporter/TestSpanExporter.java | 19 +++- 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java diff --git a/extensions/opentelemetry/deployment/pom.xml b/extensions/opentelemetry/deployment/pom.xml index f0b9e0ef6d2a4..306f138c26fcd 100644 --- a/extensions/opentelemetry/deployment/pom.xml +++ b/extensions/opentelemetry/deployment/pom.xml @@ -145,6 +145,11 @@ quarkus-security-deployment test + + io.quarkus + quarkus-vertx-http-dev-ui-tests + test + io.opentelemetry opentelemetry-sdk-testing diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java new file mode 100644 index 0000000000000..6a6e314aa3f48 --- /dev/null +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java @@ -0,0 +1,103 @@ +package io.quarkus.opentelemetry.deployment; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; + +import jakarta.inject.Inject; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.opentelemetry.sdk.trace.data.SpanData; +import io.quarkus.opentelemetry.deployment.common.exporter.TestSpanExporter; +import io.quarkus.opentelemetry.deployment.common.exporter.TestSpanExporterProvider; +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class OpenTelemetrySuppressNonAppUriTest { + + @RegisterExtension + final static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .withApplicationRoot((jar) -> jar + .addClasses(HelloResource.class, TestSpanExporter.class, TestSpanExporterProvider.class) + .addAsResource(new StringAsset(TestSpanExporterProvider.class.getCanonicalName()), + "META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider") + .add(new StringAsset( + """ + quarkus.otel.traces.exporter=test-span-exporter + quarkus.otel.metrics.exporter=none + quarkus.otel.bsp.export.timeout=1s + quarkus.otel.bsp.schedule.delay=50 + """), + "application.properties")); + + @Test + void test() { + + // Must not be traced + RestAssured.given() + .get("/q/health/") + .then() + .statusCode(200); + RestAssured.given() + .get("/q/dev-ui") + .then() + .statusCode(200); + RestAssured.given() + .get("/q/dev-ui/icon/font-awesome.js") + .then() + .statusCode(200); + // Valid trace + RestAssured.given() + .get("/hello") + .then() + .statusCode(200); + // Get span names + List spans = Arrays.asList( + RestAssured.given() + .get("/hello/spans") + .then() + .statusCode(200) + .extract().body() + .asString() + .split(";")); + + assertThat(spans.size()) + .withFailMessage("Expected only one span but found: " + spans) + .isEqualTo(1); + + assertThat(spans).contains("GET /hello"); + } + + @Path("/hello") + public static class HelloResource { + + @Inject + TestSpanExporter spanExporter; + + @GET + public String greetings() { + return "Hello test"; + } + + /** + * Gets a string with the received spans names concatenated by ; + * + * @return + */ + @GET + @Path("/spans") + public String greetingsInsertAtLeast() { + String spanNames = spanExporter.getFinishedSpanItemsAtLeast(1).stream() + .map(SpanData::getName) + .reduce((s1, s2) -> s1 + ";" + s2).orElse(""); + System.out.println(spanNames); + return spanNames; + } + } +} diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java index 573816c3ed9d0..c55856ea52c1e 100644 --- a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java @@ -4,6 +4,7 @@ import static java.util.stream.Collectors.toList; import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collection; import java.util.List; @@ -50,11 +51,27 @@ public List getFinishedSpanItems(int spanCount) { return finishedSpanItems.stream().collect(toList()); } + /** + * Careful when retrieving the list of finished spans. There is a chance when the response is already sent to the + * client and Vert.x still writing the end of the spans. This means that a response is available to assert from the + * test side but not all spans may be available yet. For this reason, this method requires the number of expected + * spans. + */ + public List getFinishedSpanItemsAtLeast(int spanCount) { + assertSpanAtLeast(spanCount); + return finishedSpanItems; + } + public void assertSpanCount(int spanCount) { - await().atMost(30, SECONDS).untilAsserted( + await().atMost(5, SECONDS).untilAsserted( () -> assertEquals(spanCount, finishedSpanItems.size(), "Spans: " + finishedSpanItems.toString())); } + public void assertSpanAtLeast(int spanCount) { + await().atMost(5, SECONDS).untilAsserted( + () -> assertTrue(spanCount < finishedSpanItems.size(), "Spans: " + finishedSpanItems.toString())); + } + public void reset() { finishedSpanItems.clear(); } From ac983ac86ee9c801079a5f9f5f3e1701b2fadaab Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 11:04:57 +0100 Subject: [PATCH 4/9] Avoid duplicates in FrameworkEndpointsBuildItem --- .../http/deployment/spi/FrameworkEndpointsBuildItem.java | 8 ++++---- .../quarkus/vertx/http/deployment/VertxHttpProcessor.java | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/extensions/vertx-http/deployment-spi/src/main/java/io/quarkus/vertx/http/deployment/spi/FrameworkEndpointsBuildItem.java b/extensions/vertx-http/deployment-spi/src/main/java/io/quarkus/vertx/http/deployment/spi/FrameworkEndpointsBuildItem.java index 90c140b6d6aad..d13509e196b4a 100644 --- a/extensions/vertx-http/deployment-spi/src/main/java/io/quarkus/vertx/http/deployment/spi/FrameworkEndpointsBuildItem.java +++ b/extensions/vertx-http/deployment-spi/src/main/java/io/quarkus/vertx/http/deployment/spi/FrameworkEndpointsBuildItem.java @@ -1,17 +1,17 @@ package io.quarkus.vertx.http.deployment.spi; -import java.util.List; +import java.util.Set; import io.quarkus.builder.item.SimpleBuildItem; public final class FrameworkEndpointsBuildItem extends SimpleBuildItem { - private final List endpoints; + private final Set endpoints; - public FrameworkEndpointsBuildItem(final List endpoints) { + public FrameworkEndpointsBuildItem(final Set endpoints) { this.endpoints = endpoints; } - public List getEndpoints() { + public Set getEndpoints() { return endpoints; } } diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java index 0b6349b91db9a..fc70a7c4eafd3 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/VertxHttpProcessor.java @@ -8,8 +8,10 @@ import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.concurrent.SubmissionPublisher; import java.util.logging.Level; import java.util.stream.Collectors; @@ -131,7 +133,7 @@ NonApplicationRootPathBuildItem frameworkRoot(HttpBuildTimeConfig httpBuildTimeC FrameworkEndpointsBuildItem frameworkEndpoints(NonApplicationRootPathBuildItem nonApplicationRootPath, ManagementInterfaceBuildTimeConfig managementInterfaceBuildTimeConfig, LaunchModeBuildItem launchModeBuildItem, List routes) { - List frameworkEndpoints = new ArrayList<>(); + Set frameworkEndpoints = new HashSet<>(); for (RouteBuildItem route : routes) { if (FRAMEWORK_ROUTE.equals(route.getRouteType())) { if (route.getConfiguredPathInfo() != null) { From 7c3e93a35e5de16aa995831be52f67bf677d62e8 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 11:11:24 +0100 Subject: [PATCH 5/9] Simplify OpenTelemetrySuppressNonAppUriTest a bit --- .../deployment/OpenTelemetrySuppressNonAppUriTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java index 6a6e314aa3f48..8c685a11b9568 100644 --- a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriTest.java @@ -45,7 +45,7 @@ void test() { .then() .statusCode(200); RestAssured.given() - .get("/q/dev-ui") + .get("/q/dev-ui/") .then() .statusCode(200); RestAssured.given() @@ -67,11 +67,7 @@ void test() { .asString() .split(";")); - assertThat(spans.size()) - .withFailMessage("Expected only one span but found: " + spans) - .isEqualTo(1); - - assertThat(spans).contains("GET /hello"); + assertThat(spans).containsExactly("GET /hello"); } @Path("/hello") From 9cd2b132cec1637d0dae7cbe60fe74f90ff5d07b Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 13:44:33 +0100 Subject: [PATCH 6/9] Use an HashSet contract for DropTargetsSamplerTest It's a detail but it's the contract we want and matching will be faster. --- .../AutoConfiguredOpenTelemetrySdkBuilderCustomizer.java | 5 +++-- .../opentelemetry/runtime/tracing/DropTargetsSampler.java | 5 +++-- .../runtime/tracing/DropTargetsSamplerTest.java | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/AutoConfiguredOpenTelemetrySdkBuilderCustomizer.java b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/AutoConfiguredOpenTelemetrySdkBuilderCustomizer.java index cbfc7d6e2d79b..3706eb9fd5c73 100644 --- a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/AutoConfiguredOpenTelemetrySdkBuilderCustomizer.java +++ b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/AutoConfiguredOpenTelemetrySdkBuilderCustomizer.java @@ -5,8 +5,9 @@ import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Predicate; @@ -171,7 +172,7 @@ public Sampler apply(Sampler existingSampler, ConfigProperties configProperties) .orElse(existingSampler); //collect default filtering targets (Needed for all samplers) - List dropTargets = new ArrayList<>(); + Set dropTargets = new HashSet<>(); if (oTelRuntimeConfig.traces().suppressNonApplicationUris()) {//default is true dropTargets.addAll(TracerRecorder.dropNonApplicationUriTargets); } diff --git a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java index f42b26fbaa9e8..e327e0e5efba0 100644 --- a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java +++ b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java @@ -4,6 +4,7 @@ import static io.opentelemetry.semconv.UrlAttributes.URL_QUERY; import java.util.List; +import java.util.Set; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanKind; @@ -14,10 +15,10 @@ public class DropTargetsSampler implements Sampler { private final Sampler sampler; - private final List dropTargets; + private final Set dropTargets; public DropTargetsSampler(final Sampler sampler, - final List dropTargets) { + final Set dropTargets) { this.sampler = sampler; this.dropTargets = dropTargets; } diff --git a/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java b/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java index c2aae5f318f7a..7997cde9b18ad 100644 --- a/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java +++ b/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java @@ -1,10 +1,10 @@ package io.quarkus.opentelemetry.runtime.tracing; import static io.opentelemetry.semconv.UrlAttributes.URL_PATH; -import static io.quarkus.opentelemetry.runtime.OpenTelemetryUtil.*; import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import org.junit.jupiter.api.Test; @@ -21,7 +21,7 @@ class DropTargetsSamplerTest { @Test void testDropTargets() { CountingSampler countingSampler = new CountingSampler(); - var sut = new DropTargetsSampler(countingSampler, List.of("/q/swagger-ui", "/q/swagger-ui*")); + var sut = new DropTargetsSampler(countingSampler, Set.of("/q/swagger-ui", "/q/swagger-ui*")); assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/other")); assertEquals(1, countingSampler.count.get()); From 081c7b174d4de50fcf439fd928ea6159bddd6998 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 14:03:50 +0100 Subject: [PATCH 7/9] Let TestSpanExporter when we have at least X elements included The current condition is weird because we end up saying we want at least 1, and it only returns when there are 2. --- .../deployment/common/exporter/TestSpanExporter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java index c55856ea52c1e..7c0fbf5a3fbb5 100644 --- a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/common/exporter/TestSpanExporter.java @@ -69,7 +69,7 @@ public void assertSpanCount(int spanCount) { public void assertSpanAtLeast(int spanCount) { await().atMost(5, SECONDS).untilAsserted( - () -> assertTrue(spanCount < finishedSpanItems.size(), "Spans: " + finishedSpanItems.toString())); + () -> assertTrue(spanCount <= finishedSpanItems.size(), "Spans: " + finishedSpanItems.toString())); } public void reset() { From 7615750541c405523f7de9de89f9e703124da10c Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 14:12:52 +0100 Subject: [PATCH 8/9] Fix wildcard dropping in DropTargetsSampler While /q/dev-ui/* would exclude: - /q/dev-ui/ - /q/dev-ui/whatever it wouldn't exclude /q/dev-ui/whatever/whenever/, which was problematic. See additional tests. Note that I think this should be improved with a proper matcher that handles all our matching rules efficiently but for now, it will have to do. --- .../runtime/tracing/DropTargetsSampler.java | 26 +++++++++++------- .../tracing/DropTargetsSamplerTest.java | 27 +++++++++++++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java index e327e0e5efba0..9e4a718dbcda8 100644 --- a/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java +++ b/extensions/opentelemetry/runtime/src/main/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSampler.java @@ -3,8 +3,10 @@ import static io.opentelemetry.semconv.UrlAttributes.URL_PATH; import static io.opentelemetry.semconv.UrlAttributes.URL_QUERY; +import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanKind; @@ -15,12 +17,18 @@ public class DropTargetsSampler implements Sampler { private final Sampler sampler; - private final Set dropTargets; + private final Set dropTargetsExact; + private final Set dropTargetsWildcard; public DropTargetsSampler(final Sampler sampler, final Set dropTargets) { this.sampler = sampler; - this.dropTargets = dropTargets; + this.dropTargetsExact = dropTargets.stream().filter(s -> !s.endsWith("*")) + .collect(Collectors.toCollection(HashSet::new)); + this.dropTargetsWildcard = dropTargets.stream() + .filter(s -> s.endsWith("*")) + .map(s -> s.substring(0, s.length() - 1)) + .collect(Collectors.toCollection(HashSet::new)); } @Override @@ -49,26 +57,24 @@ private boolean shouldDrop(String target) { if ((target == null) || target.isEmpty()) { return false; } - if (safeContains(target)) { // check exact match + if (containsExactly(target)) { // check exact match return true; } if (target.charAt(target.length() - 1) == '/') { // check if the path without the ending slash matched - if (safeContains(target.substring(0, target.length() - 1))) { + if (containsExactly(target.substring(0, target.length() - 1))) { return true; } } - int lastSlashIndex = target.lastIndexOf('/'); - if (lastSlashIndex != -1) { - if (safeContains(target.substring(0, lastSlashIndex) + "*") - || safeContains(target.substring(0, lastSlashIndex) + "/*")) { // check if a wildcard matches + for (String dropTargetsWildcard : dropTargetsWildcard) { + if (target.startsWith(dropTargetsWildcard)) { return true; } } return false; } - private boolean safeContains(String target) { - return dropTargets.contains(target); + private boolean containsExactly(String target) { + return dropTargetsExact.contains(target); } @Override diff --git a/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java b/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java index 7997cde9b18ad..db3896ed7ce2b 100644 --- a/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java +++ b/extensions/opentelemetry/runtime/src/test/java/io/quarkus/opentelemetry/runtime/tracing/DropTargetsSamplerTest.java @@ -39,6 +39,33 @@ void testDropTargets() { assertEquals(2, countingSampler.count.get()); } + @Test + void testDropTargetsWildcards() { + CountingSampler countingSampler = new CountingSampler(); + var sut = new DropTargetsSampler(countingSampler, Set.of("/q/dev-ui", "/q/dev-ui/*")); + + assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/other")); + assertEquals(1, countingSampler.count.get()); + + assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/q/dev-ui-test")); + assertEquals(2, countingSampler.count.get()); + + assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui")); + assertEquals(2, countingSampler.count.get()); + + assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/")); + assertEquals(2, countingSampler.count.get()); + + assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/whatever")); + assertEquals(2, countingSampler.count.get()); + + assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/whatever/wherever/whenever")); + assertEquals(2, countingSampler.count.get()); + + assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/q/test")); + assertEquals(3, countingSampler.count.get()); + } + private static SamplingResult getShouldSample(DropTargetsSampler sut, String target) { return sut.shouldSample(null, null, null, SpanKind.SERVER, Attributes.of(URL_PATH, target), null); From d76ade7630818bbda6fbbc82fd8260960d8968b1 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 16 Jan 2025 14:22:39 +0100 Subject: [PATCH 9/9] Add a SuppressNonAppUri test when management interface is enabled --- ...pressNonAppUriManagementInterfaceTest.java | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriManagementInterfaceTest.java diff --git a/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriManagementInterfaceTest.java b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriManagementInterfaceTest.java new file mode 100644 index 0000000000000..f4d2edfeeeded --- /dev/null +++ b/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/OpenTelemetrySuppressNonAppUriManagementInterfaceTest.java @@ -0,0 +1,101 @@ +package io.quarkus.opentelemetry.deployment; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; + +import jakarta.inject.Inject; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.opentelemetry.sdk.trace.data.SpanData; +import io.quarkus.opentelemetry.deployment.common.exporter.TestSpanExporter; +import io.quarkus.opentelemetry.deployment.common.exporter.TestSpanExporterProvider; +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class OpenTelemetrySuppressNonAppUriManagementInterfaceTest { + + @RegisterExtension + final static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .withApplicationRoot((jar) -> jar + .addClasses(HelloResource.class, TestSpanExporter.class, TestSpanExporterProvider.class) + .addAsResource(new StringAsset(TestSpanExporterProvider.class.getCanonicalName()), + "META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider") + .add(new StringAsset( + """ + quarkus.otel.traces.exporter=test-span-exporter + quarkus.otel.metrics.exporter=none + quarkus.otel.bsp.export.timeout=1s + quarkus.otel.bsp.schedule.delay=50 + quarkus.management.enabled=true + quarkus.management.port=9001 + """), + "application.properties")); + + @Test + void test() { + + // Must not be traced + RestAssured.given() + .get("http://localhost:9001/q/health/") + .then() + .statusCode(200); + RestAssured.given() + .get("/q/dev-ui/") + .then() + .statusCode(200); + RestAssured.given() + .get("/q/dev-ui/icon/font-awesome.js") + .then() + .statusCode(200); + // Valid trace + RestAssured.given() + .get("/hello") + .then() + .statusCode(200); + // Get span names + List spans = Arrays.asList( + RestAssured.given() + .get("/hello/spans") + .then() + .statusCode(200) + .extract().body() + .asString() + .split(";")); + + assertThat(spans).containsExactly("GET /hello"); + } + + @Path("/hello") + public static class HelloResource { + + @Inject + TestSpanExporter spanExporter; + + @GET + public String greetings() { + return "Hello test"; + } + + /** + * Gets a string with the received spans names concatenated by ; + * + * @return + */ + @GET + @Path("/spans") + public String greetingsInsertAtLeast() { + String spanNames = spanExporter.getFinishedSpanItemsAtLeast(1).stream() + .map(SpanData::getName) + .reduce((s1, s2) -> s1 + ";" + s2).orElse(""); + System.out.println(spanNames); + return spanNames; + } + } +}