SDk/Sampler concepts leaked in API

[cijothomas]

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/src/trace/tracer.rs#L277
SamplingResult is the type returned by Samplers, and it is a pure SDK concept.
If this was unintentional, we should remove it before 1.0 stable release.

Similar: #1042

[cijothomas]

I think the only "Sampling" related concepts that should be part of Tracing API are:

1. spanContext TraceFlags for SAMPLED.
2. span.IsRecording

Based on the comment (https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/src/trace/tracer.rs#L174-L176), I think this is done to accommodate tracing-opentelemetry crate. Related discussion around that crate : #1032

[cijothomas]

@hdost Moving back to Tracing API stable, as the fix for this (if we chose to), would be in the API itself!

[cijothomas]

@open-telemetry/rust-approvers
The natural fix for this would be to simply remove this from the API. But that'd probably (or surely?) break tracing-opentelemetry, unless tracing-opentelemetry can depend on the SDK (it does seem to depend on SDK today...)... But even then, there is no such support in OTel spec to support externally sampled spans..

Given the popularity of tracing crate for both logging and distributed tracing in Rust ecosystem, I think we should keep this, but make this a well-documented decision by OTel Rust maintainers/approvers. At minimum, we need to document this as a conscious decision (and not an accidental leak/bug), and document we commit to keep support tracing-opentelemetry by specially treating the crate.

Please share your thoughts on how best to proceed.

My proposal:

1. Document this in the same place Document spec deviations with rationale #1297 prefers (that is undecided as of now)
2. Call this out in this repo's main readme file itself. I can work on some wordings once I get an okay for this direction.

[lalitb]

• 1 on keeping it. We can also document it as doc comments for SamplingResult and SamplingDecision.

[djc]

@jtescher any thoughts on changing tracing-opentelemetry to accomodate this?

[cijothomas]

From SIG meeting:
Yes, we want to support tracing specially
Hide from docs, so people won't accidentally see/use it.

[jtescher]

@jtescher any thoughts on changing tracing-opentelemetry to accomodate this?

We could look at updating tracing-opentelemetry, the main reason it needs these types currently is so that it can do sampling ahead of time as it creates spans on end and needs to consistently apply the same sampling to sub spans. It could potentially avoid this via a mechanism to specify the parent trace id at span start and disallowing assigning the parent after that point.

More context for the current implementation is here https://github.com/tokio-rs/tracing-opentelemetry/blob/v0.1.x/src/tracer.rs#L14-L27