-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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 Dynatrace sampler to OpenTelemetry Tracing Provider #50202
Merged
istio-testing
merged 7 commits into
istio:master
from
dynatrace-oss-contrib:dynatrace-sampler
Apr 2, 2024
Merged
Add Dynatrace sampler to OpenTelemetry Tracing Provider #50202
istio-testing
merged 7 commits into
istio:master
from
dynatrace-oss-contrib:dynatrace-sampler
Apr 2, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Skipping CI for Draft Pull Request. |
istio-testing
added
do-not-merge/work-in-progress
Block merging of a PR because it isn't ready yet.
size/L
Denotes a PR that changes 100-499 lines, ignoring generated files.
labels
Apr 2, 2024
/test all |
/test all |
istio-testing
removed
the
do-not-merge/work-in-progress
Block merging of a PR because it isn't ready yet.
label
Apr 2, 2024
Hi @howardjohn @zirain this is the implementation of samplers we have been talking. It would be great if we can get this in time for the 1.22 release. Thank you! |
howardjohn
approved these changes
Apr 2, 2024
zirain
reviewed
Apr 3, 2024
jmcshane
pushed a commit
to superorbital/istio
that referenced
this pull request
Apr 3, 2024
* Add Dynatrace sampler to OpenTelemetry Tracing Provider * Add release notes * Re-use http headers from exporter * Improve comments * Refactor http header creation * precommit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please provide a description of this PR:
Closes #50001
This is a follow up on the recent API change to allow configuring custom Samplers to the OpenTelemetry Tracing Provider in Istio. istio/api#3134
This PR then uses this new API config and allow configuring the first custom sampler, the Dynatrace sampler. A bit of context: Today, there's multiple ways to configure sampling in Istio, which works/have the following precedence:
This PR adds another option, changing the order above to:
Which means:
If
OpenTelemetryTracingProvider
has a custom sampler configured (what this PR adds):telemetry.RandomSamplingPercentage
to100
as all spans should arrive on the sampler, so it can perform its algorithm. This is hardcoded and the user cannot change (doesn't make sense to). Without all spans a sampler can't make consistent sampling decisions.Else, continue with the same precedence as today. Essentially nothing changes when users don't use a Sampler:
cc @howardjohn @zirain
I plan to add a new documentation page to explain how the ways sampling can be configured when using the OpenTelemetry tracer in Istio.