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 OTEL_TRACE_SAMPLER. #1136

Merged
merged 5 commits into from
Oct 28, 2020
Merged

Conversation

carlosalberto
Copy link
Contributor

Adds OTEL_TRACE_SAMPLER, required for properly resolving #1105 .

This is a super minimalistic, but hopefully correct approach. Following @jmacd's comment about the possibility of adding sampling for metrics in the future, I added TRACE in it.

@carlosalberto carlosalberto added area:sdk Related to the SDK area:sampling Related to trace sampling spec:trace Related to the specification/trace directory priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs labels Oct 23, 2020
@carlosalberto carlosalberto requested review from a team as code owners October 23, 2020 13:05
carlosalberto and others added 2 commits October 23, 2020 16:34
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@carlosalberto
Copy link
Contributor Author

@open-telemetry/specs-approvers Please review this ;)

@andrewhsu andrewhsu removed priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs labels Oct 27, 2020
@andrewhsu
Copy link
Member

looks like this PR is blocking PR #1117

@carlosalberto
Copy link
Contributor Author

@open-telemetry/java-instrumentation-approvers @open-telemetry/python-approvers as you work on auto-instrumentation components, would you mind reviewing this and verify it's all good?

@carlosalberto
Copy link
Contributor Author

Merging this is as 1) has enough reviews, 2) we need this merged before proceeding with #1105 and 3) It's minimalistic enough. Let's follow up any missing detail if/as needed.

@carlosalberto carlosalberto merged commit c9eeaae into open-telemetry:master Oct 28, 2020
@carlosalberto carlosalberto deleted the ot_sampler branch October 28, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants