search

DataDog / dd-trace-java

Datadog APM client for Java
https://docs.datadoghq.com/tracing/languages/java
Apache License 2.0
570 stars 283 forks source link

resource name not properly set on Quarkus #2258

Open pcasaes opened 3 years ago

pcasaes commented 3 years ago

Recently we upgrade from 0.61.0 to 0.70.0. and netty.request operations started tracing with an incomplete resource name under Quarkus.

Doing some investigation we found that if we don't set quarkus.http.root-path then the resource names report just fine but if it is set then all resources will report as being just what's in the root-path.

Example quarkus.http.root-path=/api

All netty.request will report with resource /api but method reports ok: GET /api POST /api DELETE /api

pcasaes commented 3 years ago

Did some more testing. Looks like that started happening on 0.69.0. On 0.68.0 it's working fine.

pcasaes commented 3 years ago

Looks like the issue is in this line

https://github.com/DataDog/dd-trace-java/blob/6e4d65ff54b79fd5af196f4846b9f3670661cf39/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java8/datadog/trace/instrumentation/vertx_3_4/server/VertxRouterDecorator.java#L52

According to the Vertx docs Route#getPath returns the path prefix (if any) for this route https://vertx.io/docs/apidocs/io/vertx/ext/web/Route.html#getPath--

I suppose you could use

routingContext.request().path()
tylerbenson commented 3 years ago

I just submitted a PR that validates the Vertx resource name is being set properly, even with path parameters #2284. Given your initial description mentioned Quarkus, I don't think that the Vertx instrumentation is related.

Do you think you could provide a sample app to demonstrate the problem?

tylerbenson commented 3 years ago

Upon further review of the description above, I think I misunderstood what was going on. I didn't fully understand how Quarkus and Vertx were related.

When writing the test I mentioned, I demonstrated that your suggestion to use routingContext.request().path() would be problematic because it returns the original URL, not the route with the path variable names. I think more research on how vertx handles nested routing will need to be done. (My guess is that we take the first level route name, and don't take nesting into account.)

pcasaes commented 3 years ago

I demonstrated that your suggestion to use routingContext.request().path() would be problematic because it returns the original URL

That makes sense.

If the root path quarkus.http.root-path property is not set then routingContext.currentRoute().getPath() will return null and the resource for the span will be set elsewhere.

pcasaes commented 3 years ago

Going back to having the property quarkus.http.root-path set, if I disable vertx instrumentation -Ddd.integration.vertx-3.4.enabled=false then it works!

It looks like if the current route's path is null or if Vertx instrumentation is disabled it falls back to the JaxRsAnnotationsDecorator which updates the root span's resource from what it can derive from the jaxrs request.

Quarkus uses Vertx for serving http requests and users can stick to doing everything with Vertx but many choose to use Resteasy in order to program in an imperative style.

Side note: we've had to add this configuration -Ddd.trace.executors=org.jboss.threads.EnhancedQueueExecutor in order to get tracing context propagation to work in Quarkus. Vertx will respond to calls but forward the request to worker threads that integrate with Resteasy. The executor for these worker threads is org.jboss.threads.EnhancedQueueExecutor

asyle83 commented 1 year ago

Going back to having the property quarkus.http.root-path set, if I disable vertx instrumentation -Ddd.integration.vertx-3.4.enabled=false then it works!

It looks like if the current route's path is null or if Vertx instrumentation is disabled it falls back to the JaxRsAnnotationsDecorator which updates the root span's resource from what it can derive from the jaxrs request.

Quarkus uses Vertx for serving http requests and users can stick to doing everything with Vertx but many choose to use Resteasy in order to program in an imperative style.

Side note: we've had to add this configuration -Ddd.trace.executors=org.jboss.threads.EnhancedQueueExecutor in order to get tracing context propagation to work in Quarkus. Vertx will respond to calls but forward the request to worker threads that integrate with Resteasy. The executor for these worker threads is org.jboss.threads.EnhancedQueueExecutor

@pcasaes Your suggestions were very useful: I tried to disable vertx integration (for vertx 4 the property is -Ddd.trace.integration.vertx.enabled) and add -Ddd.trace.executors=org.jboss.threads.EnhancedQueueExecutor as you described. Now the resource path is written and the context is propagated (only one trace is created) but now I can't see hibernate spans anymore. Without context propagation, db spans were automatically created in one of the 2 traces (exactly in the "executor thread" trace). Do you know how can get them back now? Thanks a lot