DataDog / dd-trace-dotnet

.NET Client Library for Datadog APM
https://docs.datadoghq.com/tracing/
Apache License 2.0
434 stars 135 forks source link

Ability to set top level resource name in 2.x library #2695

Open kmcc049 opened 2 years ago

kmcc049 commented 2 years ago

We run an asp.net mvc 6.x application that uses Hot Chocolate 11 by default in the service view this means we get 99% of all requests showing up under a resource called "POST /graphql", this is not very useful.

In the 1.x version of datadog we were using I was able to within a span do the following Tracer.Instance?.ActiveScope.Span.ResourceName = "graphql operationname" and it would propagate to the top level resource. This made the resource view in datadog actually useful. To be honest I never really considered this too much but I had already started a manual span at this point, but setting this value would change the name in the resources view, split each GQL operation into a different entry in the default view of the service and solved my problem nicely.

In the 2.x versions of datadog it seems that this no longer works. I tried setting the resource name inside my manually created span but this does not alter the default service view. I also tried renaming it before I created my manual span, but this also did not seem to work. It seems that before I reach a point where I know the gql operation name there are already two active, nested spans, one for asp.net and one for asp.net mvc and I cannot rename the resource of the root one. I can't access(without reflection) a spans parent to crawl backwards up the tree to set the resource name on the the top level span.

I think the ISpan interface should expose a nullable Parent ISpan if this was accessible it would be trivial to traverse to the root and set the resource name.

The only alternative I can see is reflecting to the root of the ISpan.

andrewlock commented 2 years ago

Hi @kmcc049, thanks for your message - there's a few things to unpack here.

We run an asp.net mvc 6.x application that uses Hot Chocolate 11 by default in the service view this means we get 99% of all requests showing up under a resource called "POST /graphql", this is not very useful.

Unfortunately we don't yet have integration support for HotChocolate. I can understand your frustration with the POST /graphql names, as you say, it's not very useful. With our existing GraphQL integration (graphql-dotnet) you can select the graphql.execute operation name, which will then break out by the resource:

image

When we have a HotChcolate integration, you will be able to do the same thing, without any changes required on your side. I realise that doesn't help you now, just wanted to show a roadmap.

In the 2.x versions of datadog it seems that this no longer works... It seems that before I reach a point where I know the gql operation name there are already two active, nested spans, one for asp.net and one for asp.net mvc

Yes, this was an option in version 1.x, but we made it the default in 2.x as it also adds additional capabilities to the aspnetcore integration - additional tags, resolves inconsistencies, improves resource names etc. We don't recommend disabling it, but if you need to you can set DD_TRACE_ROUTE_TEMPLATE_RESOURCE_NAMES_ENABLED=0 which will revert to the 1.x behaviour. Hopefully this provides a workaround for your immediate problem, even if it is sub-optimal.

I think the ISpan interface should expose a nullable Parent ISpan if this was accessible it would be trivial to traverse to the root and set the resource name.

This has been something we've been actively considering for some time now (this is a similar request to #1147). Unfortunately, there are many complications with that. To give one example: in partial-flush scenarios, the parent span may no longer actually be in memory. It's an edge case, but there are other issues with it too, so we've been exploring other options.

I don't have any other information to share at this point, other than to say it's something we're actively looking into, in particular how to set tags and the resource name of the local root span.

Hope that makes sense!

alexmarmon commented 1 year ago

@andrewlock I see that HotChocolate support was added in v2.15.0 but it appears the operation name still isn't coming through. I wasn't able to find any docs on the integration, should this be working or is it still on the roadmap? I am on version 2.20 and everything else seems to be working.

Screenshot 2023-02-07 at 10 26 28 AM Screenshot 2023-02-07 at 10 30 02 AM Screenshot 2023-02-07 at 10 29 42 AM
martindisch commented 1 year ago

We're also using Hot Chocolate (v13) and were happy to see that auto instrumentation is already working. However, we think the current behavior could be improved.

  1. The graphql.execute spans are using a different service name with the -graphql suffix. That can be a valid choice, but we'd prefer to have them attached to the same service that also handles "normal" HTTP API requests (aspnet_core.request) and be able to easily switch between the two operation types on the same service. We learned that we can override the name with TracerSettings.SetServiceNameMappings, but that alone doesn't help us because of the second issue that was already discussed.
  2. The GraphQL spans have the /graphql/{*slug} HTTP span as a parent, which we agree is not useful. Therefore, even if we can have them show up in the service we want using the service name mappings, they're not service entry spans and therefore don't show up as a selectable operation type in the APM dropdown.

Is there a way to discard specific aspnet_core.request spans without losing everything below them?

pierotibou commented 1 year ago

The graphql.execute spans are using a different service name with the -graphql suffix.

Hello, we've announced at DASH a new way to represent services in Datadog, removing the -graphql suffix for instance. You will find more information about the feature here, but the idea is to have a unique service name and add a peer.service tag to still populate the service map. To enable this feature, please follow the documentation and let us know of any feedback.

martindisch commented 1 year ago

Cool, thanks for the heads-up.

williamdenton commented 1 year ago

I also struggled with not being able to set the root span, so i cheated and made some HttpMiddleware, stashed the desired operation name in the HttpContext Items from within a HotChocolate Diagnostic Observer, then set the span name in the middleware after invoke(next) (ie on the way out of the pipeline). At this point there are no child spans and the current span is the root span.