Risk of update_name

[rex-remind101]

I'm hoping to leverage update_name for GraphQL so that after the GraphQL query is parsed I can update the root span name of the trace to the OperationName of the GraphQL query we're tracing over. However, update_name comes with a caveat documented here

opentelemetry-rust/opentelemetry/src/trace/span.rs

Line 156 in 561426e

fn update_name<T>(&mut self, new_name: T)

However, reading this it's unclear if this is introducing a "performance reasons" issue or a correctness issue. If this affects the correctness of spans that seems like a non-starter and should be called out. If the results are correct but we sacrifice some performance that may be ok. It's also unclear how much performance is sacrificed - phrasing this in terms of big O would be helpful for evaluation.

Another option would be to not rely on the root span name as the primary operation name in DataDog, however, this seems broken as soon as I switched to install_batch, which I would assume is a much larger performance concern than using update_name. That issue is documented here open-telemetry/opentelemetry-rust-contrib#7 .

[TommyCpp]

We should probably update the doc to be in line with the current version of specs. But the caveat here is when spans are created usually a sampler will decide whether or not to keep these spans. It can leverage all information available at that time including the span name. If you change the span name after it will not change the decision whether to sample it or not.

If you don't rely on the span name for sampling then it should be fine. Otherwise, it could be confusing.

[rex-remind101]

Thank you, we have sampling on the side of DataDog but that's upstream. I'm assuming this Sampler is an actual structure one would need to instantiate for opentelemetry-rust?

[rex-remind101]

Looks like you mean this

opentelemetry-rust/opentelemetry/src/sdk/trace/sampler.rs

Line 92 in 9f3abed

pub enum Sampler {

[TommyCpp]

Sorry for the late reply. I was referring to ShouldSample trait. The Sampler enum is a set of buildin sampler