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 support for custom tags to gqlgen contrib #2597

Closed
samsullivan opened this issue Mar 6, 2024 · 6 comments · Fixed by #2598
Closed

Add support for custom tags to gqlgen contrib #2597

samsullivan opened this issue Mar 6, 2024 · 6 comments · Fixed by #2598
Labels
apm:ecosystem contrib/* related feature requests or bugs needs-triage New issues that have not yet been triaged

Comments

@samsullivan
Copy link
Contributor

Most contrib tracer packages expose a WithCustomTags option: https://github.com/search?q=repo%3ADataDog%2Fdd-trace-go+WithCustomTag+path%3Acontrib&type=code

One exception is the gqlgen package: https://github.com/DataDog/dd-trace-go/blob/63d7047cfd4d8e3d078dc5460fb385e544c2eb1d/contrib/99designs/gqlgen/option.go

This seems super easy to add, so I'm happy to open a PR; however, I saw that you want an open issue first for changes other than bug fixes.

@github-actions github-actions bot added apm:ecosystem contrib/* related feature requests or bugs needs-triage New issues that have not yet been triaged labels Mar 6, 2024
samsullivan added a commit to samsullivan/dd-trace-go that referenced this issue Mar 6, 2024
The gqlgen middleware was missing the ability to customize tags on the created spans.
Implement a WithCustomTag option that follows the pattern of other similar packages.

Fixes DataDog#2597
@felixge
Copy link
Member

felixge commented Mar 10, 2024

Thanks for raising this issue and PR 🙇 . I see no reason against adding this feature. I'll ping the team to get you a review/merge.

@felixge felixge assigned felixge and unassigned felixge Mar 10, 2024
@samsullivan
Copy link
Contributor Author

Sweet, thanks @felixge; it'll be nice to remove the custom GQL spans from my app, so they can be tagged correctly for our retention policies 👍

I'm assuming this would need to wait for a new minor version, not a patch release, seeing how it's added functionality? Or is it small enough for the next patch release after merge?

@felixge
Copy link
Member

felixge commented Mar 11, 2024

Yes, this would need a minor release. We only do patch releases for critical fixes. However, we do one minor release per month, so your code would be released relatively soon after merging.

@samsullivan
Copy link
Contributor Author

Thanks!

@dianashevchenko
Copy link
Contributor

dianashevchenko commented Mar 11, 2024

Hi there @samsullivan, I confirm that there's no reason not to add it :) . I will review the PR right away 🙇

@samsullivan
Copy link
Contributor Author

Thanks again, both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:ecosystem contrib/* related feature requests or bugs needs-triage New issues that have not yet been triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants