Fix how Vert.x routes are identified in metrics and OpenTelemetry#45300
Conversation
There was a problem hiding this comment.
Based on the package, is this code path for devmode only ? I was reproducing #36510 in production mode.
There was a problem hiding this comment.
Nope, it's used when building the application itself in this particular case (and yes, it probably shouldn't!).
I'm not a big fan of how things are handled here and I might have a look at improving how these paths are handled at some point (what is extremely problematic is that we resolve the path at build time while they can only be resolved at runtime).
I'm interested in having someone test it because I actually used the debugger and didn't set up a Prometheus. my debugging session seemed in line with what people observed though.
|
🎊 PR Preview e75b0fd has been successfully built and deployed to https://quarkus-pr-main-45300-preview.surge.sh/version/main/guides/
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@brunobat if you could validate it indeed fixes the whole issue when you're back from PTO, that would be awesome! |
|
Sure, will do on Thu. |
brunobat
left a comment
There was a problem hiding this comment.
Unfortunately the fix doesn't work.
I see traces for the dev UI but the application spans and the /metrics one are not collected.
|
Could you clarify what you see and shouldn't see or the opposite? What I was trying to fix here are the If you have other calls to clarify, let me know and I'll have a look on Monday. |
|
Using the otel QuickStart as an example, After the change I don't see traces for the app's We probably can have a quick chat about this. I feel we need a reproducer test for this. |
cescoffier
left a comment
There was a problem hiding this comment.
Just read @brunobat comment - need further investigation.
|
FWIW, I agree the issue is broader than this particular fix but this fix actually fixes one of the issues. Let's say you configure the OpenTelemetry quickstart with: You end up with
As you can see, all the URLs are prefixed with The thing is that if you have a look the generated spans, you will quickly understand why things are not filtered: As you can see, requests to Thus I suspect these URIs can't be properly dropped. |
|
Me and @gsmet meet and we agree there are 3 issues at least:
|
|
We agreed on me digging further and come up with a good idea of what's going on. For me, we should include what's in this PR + a fix for the span not having the correct route path. This would be backportable. And then take a step back and fix the reason why we are having absolute URL with host:port at build time as that's not something we want (especially since the port is set at runtime). But this is for later as I'm pretty sure whatever we do to fix that won't be backportable. |
|
I pushed two additional commits to give an idea of where the problem is. Unfortunately, it's more complex than that as we have several routes mounted at I will discuss this with @cescoffier . |
a450c7d to
ce1d85f
Compare
|
In the end, I think I found a smart way to fix it. Vert.x has the notion of route name that is specifically used for metrics. So when building the route for the non application routes, I push a name with the full path. The name has precedence over the path when publishing the metrics. I'll discuss all this with @cescoffier on Monday morning but I think we are on the right path. |
|
@brunobat we had a discussion with @cescoffier this morning and we agreed that this is the way to go for now (we will need to take a step back and maybe redesign part of what we do but this won't be backportable and we need a fix). This patch is composed of 2 commits:
I would like to wrap this up relatively soon as I think it would be great to have it merged soon, and backported for 3.15.4 LTS. |
This comment has been minimized.
This comment has been minimized.
It's a detail but it's the contract we want and matching will be faster.
The current condition is weird because we end up saying we want at least 1, and it only returns when there are 2.
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.
82fb61b to
d76ade7
Compare
Status for workflow
|
Status for workflow
|
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | Native Tests - Misc4 | Build |
Failures | Logs | Raw logs | 🔍 |
Full information is available in the Build summary check run.
You can consult the Develocity build scans.
Failures
⚙️ Native Tests - Misc4 #
- Failing: integration-tests/gradle
📦 integration-tests/gradle
✖ io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImage - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImage(BasicJavaNativeBuildIT.java:24)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImageWithCustomName - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImageWithCustomName(BasicJavaNativeBuildIT.java:53)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImageWithCustomNameWithoutSuffix - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.BasicJavaNativeBuildIT.shouldBuildNativeImageWithCustomNameWithoutSuffix(BasicJavaNativeBuildIT.java:84)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.CustomNativeTestSourceSetIT.runNativeTests - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.CustomNativeTestSourceSetIT.runNativeTests(CustomNativeTestSourceSetIT.java:17)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.nativeTestShouldRunIntegrationTest - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.nativeTestShouldRunIntegrationTest(NativeIntegrationTestIT.java:24)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.runNativeTestsWithOutputName - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.runNativeTestsWithOutputName(NativeIntegrationTestIT.java:35)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
✖ io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.runNativeTestsWithoutRunnerSuffix - History - More details - Source on GitHub
Details
java.lang.AssertionError: Gradle build failed with exit code 137
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:140)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:57)
at app//io.quarkus.gradle.QuarkusGradleWrapperTestBase.runGradleWrapper(QuarkusGradleWrapperTestBase.java:52)
at app//io.quarkus.gradle.nativeimage.QuarkusNativeGradleITBase.runGradleWrapper(QuarkusNativeGradleITBase.java:36)
at app//io.quarkus.gradle.nativeimage.NativeIntegrationTestIT.runNativeTestsWithoutRunnerSuffix(NativeIntegrationTestIT.java:45)
at java.base@17.0.13/java.lang.reflect.Method.invoke(Method.java:569)
at java.base@17.0.13/java.util.ArrayList.forEach(ArrayList.java:1511)
Flaky tests - Develocity
⚙️ Maven Tests - JDK 17
📦 integration-tests/devmode
✖ io.quarkus.test.devui.DevUIGrpcSmokeTest.testTestService - History
Too many recursions, message not returned for id [1272823025]-java.lang.RuntimeException
Details
java.lang.RuntimeException: Too many recursions, message not returned for id [1272823025]
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:164)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
at io.quarkus.devui.tests.DevUIJsonRPCTest.objectResultFromJsonRPC(DevUIJsonRPCTest.java:167)
⚙️ Gradle Tests - JDK 17 Windows
📦 integration-tests/gradle
✖ io.quarkus.gradle.devmode.MavenExclusionInExtensionDependencyDevModeTest.main - History
Condition with Lambda expression in io.quarkus.test.devmode.util.DevModeClient was not fulfilled within 1 minutes 30 seconds.-org.awaitility.core.ConditionTimeoutException
Details
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.test.devmode.util.DevModeClient was not fulfilled within 1 minutes 30 seconds.
at app//org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
at app//io.quarkus.test.devmode.util.DevModeClient.getHttpResponse(DevModeClient.java:164)
at app//io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpResponse(QuarkusDevGradleTestBase.java:164)
|
Woooot, we can now merge this! |
The paths were changed in quarkusio/quarkus#45300
The paths were changed in quarkusio/quarkus#45300 See quarkusio/quarkus#45754 for details
The paths were changed in quarkusio/quarkus#45300 See quarkusio/quarkus#45754 for details
The paths were changed in quarkusio/quarkus#45300 See quarkusio/quarkus#45754 for details
The paths were changed in quarkusio/quarkus#45300 See quarkusio/quarkus#45754 for details


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