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

Clarify default sampler #284

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Clarify default sampler #284

wants to merge 9 commits into from

Conversation

seemk
Copy link
Contributor

@seemk seemk commented Dec 6, 2023

OpenTelemetry defaults to parentbased_always_on, but this is not consistent across our distributions:

  • parentbased_always_on is used by Node.js, .NET, Python
  • always_on is used by Java, Go

If we are advertising as a no sampling solution, we should default to always_on, but still allow for customers to change it.
Using parentbased_always_on can cause data loss if customers have previously set up cloud tracing, which injects a traceparent with 00 for traceflags.

@seemk seemk requested review from a team as code owners December 6, 2023 13:22
@seemk
Copy link
Contributor Author

seemk commented Dec 6, 2023

@akubik-splunk

@pellared
Copy link
Contributor

pellared commented Dec 6, 2023

Wouldn't changing the default be a breaking change for a distribution?

I see it as a breaking change from semantic point of view. If we change the default in Node.js, .NET, Python distributions then I would advise to bump the major version.

@akubik-splunk
Copy link

In which distros this is marked as stable?
If this is marked as stable we must consider releasing this as braking change and bump the major release
As option we can consider aligning the Node.js and making it braking change only there
This is another reason we need compatibility matrix

@pellared
Copy link
Contributor

pellared commented Dec 6, 2023

In which distros this is marked as stable?

For sure in .NET it is stable.

In Python it is part of its SDK which is stable.

@akubik-splunk
Copy link

In which distros this is marked as stable?

For sure in .NET it is stable.

In Python it is part of its SDK which is stable.

So it seems to me that we should prep for the major bump and if possible bundle it up with other changes e.g. HTTP semantics :) for .NET and for Python as well. If there are other major changes that we would like to do let's use this as opportunity to do so. We would target such changes for the Jan-Feb next year though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants