dd-trace-java icon indicating copy to clipboard operation
dd-trace-java copied to clipboard

resource name not properly set on Quarkus

Open pcasaes opened this issue 4 years ago • 8 comments

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 avatar Jan 07 '21 16:01 pcasaes

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

pcasaes avatar Jan 08 '21 21:01 pcasaes

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()

pcasaes avatar Jan 09 '21 02:01 pcasaes

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 avatar Jan 12 '21 18:01 tylerbenson

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.)

tylerbenson avatar Jan 12 '21 20:01 tylerbenson

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 avatar Jan 13 '21 03:01 pcasaes

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 avatar Jan 13 '21 04:01 pcasaes

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

asyle83 avatar Nov 13 '22 21:11 asyle83