-
Notifications
You must be signed in to change notification settings - Fork 893
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
Remove restriction that sampler description is immutable #4137
Conversation
Signed-off-by: Yuri Shkuro <[email protected]>
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.
While this can be seen as a breaking change for the callers (previously it was allowing caching - now it is not) I personally see it more like a bugfix.
In Go SDK we never had indicated that callers may cache the results and I do not found any code that would rely on that. Go docs for Sampler: https://pkg.go.dev/go.opentelemetry.io/otel/sdk/trace#Sampler
Can you add a changelog entry? 😉
Signed-off-by: Yuri Shkuro <[email protected]>
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 see this as a bugfix. The arguments here are convincing.
I think we should assume that there are no implementations caching this since this seems reasonable, but leave this open for a period of time which allows people to come forward and claim otherwise. Say 2-3 weeks.
…try#4137) Fixes open-telemetry#2095 ## Changes State that sampler description can change over time. Provide examples. --------- Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Robert Pająk <[email protected]>
Fixes #2095
Changes
State that sampler description can change over time. Provide examples.