-
Notifications
You must be signed in to change notification settings - Fork 904
Description
Is your feature request related to a problem? Please describe.
The spec defines CompositeSampler
and ComposableSampler
as experimental samplers. Java has an implementation matching the behavior, though not naming of the spec (as it was made to support OTEP), as well as an older sampling implementation that is superceded.
https://opentelemetry.io/docs/specs/otel/trace/sdk/#compositesampler
https://github.com/open-telemetry/opentelemetry-java-contrib/tree/main/consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56
Describe the solution you'd like
Having an implementation matching the spec naming in the SDK as defined for experimentation.
Describe alternatives you've considered
Clean up the naming in contrib and remove the legacy sampler from there.
Additional context
I am helping with implement composite samplers and have PRs for Python and JS. I would like to have similar for Java and happy to send a PR for it - @trask suggested on Slack I should open an issue to confirm it's ok first.
Implementation-wise, I think it can be either in .internal
package of the SDK while the spec is experimental, or in incubator.
Alternatively, we can just stick to contrib but it would be nice to clean it up then to make the changes from OTEP to spec, and remove the legacy implementation (there is actually an env var provider only for one of the legacy implementations that I would like to not include in a downstream build). Ideally this could happen without worrying about breaking changes - if there would need to be deprecation work, I would suggest it's simpler to start from scratch in the SDK.
Let me know any thoughts.
Tip: React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1
or me too
, to help us triage it. Learn more here.