-
Notifications
You must be signed in to change notification settings - Fork 375
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 sampling configuration by DD_TRACE_SAMPLING_RULES #2968
Conversation
# @return [String,nil] | ||
# @public_api | ||
option :rules do |o| | ||
o.default { ENV.fetch(Configuration::Ext::Sampling::ENV_RULES, nil) } |
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.
Isn't ENV.fetch(Configuration::Ext::Sampling::ENV_RULES, nil)
the same as ENV[Configuration::Ext::Sampling::ENV_RULES]
?
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.
Yeah, we just write ENV.fetch(blah, nil)
more than ENV#[]
in this file.
I feel like this will mostly go away when we refactor 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.
I'll leave this one like this, because it will be simple to refactor when this code gets simplified in the configuration refactoring.
sample_rate: sample_rate, | ||
} | ||
|
||
Core::BackportFrom24.hash_compact!(kwargs) |
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 don't like how this BackportFrom
comes in right in the middle of the method, can we start using refinements again? 🙏
Adds support for configuring the
RuleSampler
using the environment variableDD_TRACE_SAMPLING_RULES
.This is already supported by most tracers of other languages.
This environment variable is a JSON array of objects, with each object being of the format:
The supporting feature,
RuleSampler
, already exists; this PR only adds an additional configuration avenue.