Fix spring-webflux-5 http.route #8112
Conversation
...etry/javaagent/instrumentation/spring/webflux/v5_0/server/HandlerAdapterInstrumentation.java
Show resolved
Hide resolved
heyams
changed the title
...etry/javaagent/instrumentation/spring/webflux/v5_0/server/RouterFunctionInstrumentation.java
Outdated
Show resolved
Hide resolved
...va/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/server/RouteOnSuccess.java
Outdated
Show resolved
Hide resolved
| @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) | ||
| public static void methodExit( | ||
| @Advice.This RouterFunction<?> thiz, | ||
| @Advice.Return(readOnly = false) Mono<HandlerFunction<?>> result, | ||
| @Advice.Thrown Throwable throwable) { | ||
| @Advice.Thrown Throwable throwable, | ||
| @Advice.Local("otelContext") Context context, |
There was a problem hiding this comment.
context is not used anywhere within this method; the span started in the OnMethodEnter advice does not seem to be ended. Is that correct? Is this span needed?
There was a problem hiding this comment.
context is not used here. i can remove it.
There was a problem hiding this comment.
span is needed for initializing the ServletContextPath. I have updated the code to end advice.
There was a problem hiding this comment.
You shouldn't need to create a span to use ServletContextPath. The ServletContextPath is supposed to be initialized somewhere in the application server layer, in a different instrumentation; here you're just reading whatever value was set before.
There was a problem hiding this comment.
@mateuszrzeszutek after some further investigation, http.route is correct when otel.instrumentation.common.experimental.controller-telemetry.enabled is false with my change and http.route is incorrect when controllerTelemetryEnabled is true.
| return; | ||
| } | ||
|
|
||
| context = instrumenter().start(parentContext, request); |
There was a problem hiding this comment.
it's not clear to me why creating a new span?
trask
force-pushed
the
heya/fix-spring-5-webflux-route
branch
from
cb2c04c to
7a99d7b
Compare
relate to #5239
http.route is populated correctly:
getAttributes={http.flavor="1.1", http.method="GET", http.route="/api/accounts/{id:[A-Z|0-9|\-]+}", http.scheme="http", http.status_code=200, http.user_agent="Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36 Edg/111.0.1661.41", net.host.name="localhost", net.host.port=8080}