-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
opentelemetrytracer: Dynatrace sampler: Enable adaptative sampling #32848
opentelemetrytracer: Dynatrace sampler: Enable adaptative sampling #32848
Conversation
… API (#21) * Dynatrace sampler to fetch configuration from an API Signed-off-by: Thomas Ebner <96168670+samohte@users.noreply.github.com> Co-authored-by: Joao Grassi <5938087+joaopgrassi@users.noreply.github.com>
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
…l-config Signed-off-by: thomas.ebner <thomas.ebner@dynatrace.com>
…l-config Signed-off-by: thomas.ebner <thomas.ebner@dynatrace.com>
…l-config Signed-off-by: thomas.ebner <thomas.ebner@dynatrace.com>
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.
Thanks for you contribution. And some comments are added. :)
source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc
Outdated
Show resolved
Hide resolved
source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc
Outdated
Show resolved
Hide resolved
/lgtm api |
/wait |
Signed-off-by: thomas.ebner <thomas.ebner@dynatrace.com>
Thanks for your valuable feedback. I addressed your comments in 535d99b |
source/extensions/tracers/opentelemetry/samplers/dynatrace/sampler_config_provider.cc
Show resolved
Hide resolved
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.
Thanks for you in timely update. Now, LGTM overall except the last comment about the thread safe. And please resolve the conflict. Thanks.
…l-config Signed-off-by: thomas.ebner <thomas.ebner@dynatrace.com>
I have resolved the conflict and answered your comment. Pls. let me know what you think about. |
/retest |
Commit Message: Dynatrace sampler to fetch configuration from an API
Additional Description: In #32598 a custom Dynatrace sampler was added to Envoy. This sampler should now be extended to fetch its configuration via an HTTP call from a Dyantrace cluster.
Testing: Unit, Integration, Manual
Docs Changes: N/A
Release Notes:Dynatrace sampler to fetch configuration from an API
Platform Specific Features: N/A
[Optional Runtime guard:] N/A
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]