-
Notifications
You must be signed in to change notification settings - Fork 309
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 trace search sampling configuration #407
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
||
it('should sample a span with the operation specific rate', () => { | ||
eventSampler.sample(span, { | ||
'web.request': 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I like this object option for configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing I'm not sure is how this will work for GraphQL resolvers since there can be hundreds of graphql.resolve
events for a single request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let that be future Roch's problem.
The issue here is because the tag might not be only on the root span right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the operation name, and will apply at any resolver level. But then, if you want the resolver events, I would also expect that you want it for every resolver, so it's probably expected. The only problem would be high cost at that point given the number of spans.
* v0.8.0 * prefix span context properties with an underscore (#397) * add log propagator and trace identifiers for log correlation (#396) * fix baggage items at the span level instead of trace level (#398) * add middleware and stack trace support for express (#399) * add dns integration (#405) * add net integration (#406) * add trace search sampling configuration (#407) * add more metadata to net.connect and update operation name (#409) * add automatic log correlation of tracer identifiers for winston (#408) * add automatic log correlation of trace identifiers for bunyan (#410) * add noop span context with the correct API (#413) * add automatic log correlation of tracer identifiers for pino (#414) * add automatic log correlation of tracer identifiers for pino * add missing plugins to the build * Add protocol as a configuration option for the dd-trace-agent URL (#416) * Add protocol as a configuration option for the dd-trace agent URL. This is useful for scenarios where a SSL-terminating load balancer sits in front of the datadog-agent * Fix lint errors * Switching from protocol override to url override per CR * Fixing missing comma from previous commit * Fix tests * overrideUrl -> url * add support for latest module versions in all plugins (#417) * fix using log correlation with a custom logger (#419) * disable net and dns plugins (#421) * fix http server response handlers not running in the request scope (#422) * fix log injection with a null active span and update injection keys (#423) * Revert "add trace search sampling configuration (#407)" (#425) This reverts commit 4dac394. * add documentation about enabling log correlation (#427) * fix references to the old trace IDs (#426) * fix noop span context trace IDs to match the backend expectation (#429)
This PR adds trace search sampling configuration on web frameworks, and also as a tag that can be set on any span.