Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add path to span name of graphq.resolve spans created by graphql instrumentation #1791

Closed
satazor opened this issue Nov 10, 2023 · 1 comment · Fixed by #1796
Closed

Add path to span name of graphq.resolve spans created by graphql instrumentation #1791

satazor opened this issue Nov 10, 2023 · 1 comment · Fixed by #1796

Comments

@satazor
Copy link
Contributor

satazor commented Nov 10, 2023

Is your feature request related to a problem? Please describe

Currently, the graphql instrumentation packages adds spans for all resolvers where span name equals to graphql.resolve. Here's how it looks in datadog for example:

Screenshot 2023-11-10 at 13 29 46

However, to have improved DX, the span name could contain the actual path.

Describe the solution you'd like to see

Something like graphl.resolve - <path> as the span name would make this much better:

  • Should this be the default behavior?
  • Should this be under an option?
  • Should the instrumentation offer a hook for every span added, so we could change it, including changing span name or adding attributes?

Describe alternatives you've considered

The most obvious workound is to add a processor that changes the span name:

if (span.name === 'graphql.resolve' && span.attributes['graphql.field.path']) {
  span.updateName(`${span.name} ${span.attributes['graphql.field.path']}`);
}
@diogotorres97
Copy link
Contributor

I did a PR to update the span name to have more information. If you can check, if this #1796 resolves the issue 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants