If SpanData is immutable, setting Tracer's Resource on it before exporting will require an allocation

[SergeyKanzhelev]

if SpanData is immutable, Tracer will need to create a new SpanData with the Tracer's Resource set before passing it to the exporter. Which requires a new allocation. Creating issue:

[jmacd]

I read through the comments in issue PR69 and couldn't exactly see what the recommended action is here. Why should SpanData be considered immutable before passing it to an exporter? I've read through the Java repository and couldn't find an implementation of Span.setAttribute, which I'm assuming has to modify the active span's data, too. Could you provide more context?

[SergeyKanzhelev]

SpanData (not Span) only has getters in Java API intentionally. The idea is that you can get a SpanData and use in various delivery pipeines without the need of cloning it to avoid unwanted modification by unrelated thread. For instance, send it to two separate exporters and do not worry that one of them will add a few backend-specific attributes to it that would leak into another exporter.

@bogdandrutu pointed that in OpenTelemetry now there is one more abstraction - proto.Span (io.opentelemetry.proto.trace.v1.Span) - generated class that is what Span and SpanData will be converted to before sending to the exporter.

This changes things comparing to the OpenCensus. And at this point I'm not even sure why we need getters on SpanData at all. Class with constructors and maybe a single method to check if resource was explicitly set on it to override it by Tracer would suffice.

I will keep an issue open with this caveat. If SpanData is not used by exporters - it probably should not have any getters.

[rochdev]

@bogdandrutu pointed that in OpenTelemetry now there is one more abstraction - proto.Span (io.opentelemetry.proto.trace.v1.Span) - generated class that is what Span and SpanData will be converted to before sending to the exporter.

What if the exporter doesn't use Protobuf? And why even have SpanData and proto.Span in general? Wouldn't it be more flexible and efficient to just pass the span?

[rochdev]

In JavaScript it's not even really possible to make something completely immutable, and to make it somewhat immutable is really heavyweight. Same for going from/to Protobuf.

[SergeyKanzhelev]

There are a few design principles we were following:

• Span must not be used for information exchange. Thus it's write-only.
• Span life cycle must allow to create traces (and default SDK) where in-process timestamps of Spans are guaranteed to be consistent. Thus forbidding creating Span with externally supplied timestamps
• There are still scenarios where spans are created from something outside the direct API calls. Spans might be provided by some adapters out of band as a "completed" objects. SpanData can be used for these scenarios.

So Span is designed for instrumentation points. SpanData will only be used for out-of-band reporting.

[jmacd]

Where does this requirement ("Span life cycle must allow to create traces (and default SDK) where in-process timestamps of Spans are guaranteed to be consistent. ") come from?

It's incompatible with OpenTracing, to begin with. It assumes that every exporter wants this SpanData format, which I view as a good default but will not address every need we have. I would suggest that if you want guaranteed consistent timestamps (and let's be honest, there's no such thing in the real world), then you should allow the user to supply their timestamp and the system to provide one as well.

[rochdev]

Thus forbidding creating Span with externally supplied timestamps

In some cases you want to rebuild an entire trace from metadata generated from a different process. For example this is how apollo-tracing works. In this case you need to be able to control everything, including timing.

So Span is designed for instrumentation points. SpanData will only be used for out-of-band reporting.

Makes sense, but I'm still not sure I understand what is the role of proto.Span in that model.

It assumes that every exporter wants this SpanData format, which I view as a good default but will not address every need we have.

I agree with this. In my opinion, every package should have a replaceable input/internal/output. While this is difficult to achieve, it will ensure it's possible to mix and match OTel packages with vendor-specific packages when needed.

[jmacd]

To agree with @rochdev, the caller should be able to override all Span properties including span/event timestamps as well as span resources. The argument is that we must provide a way for the caller to set timestamps and resources because these APIs will be called via integrations from other tracing/logging implementations. See this proposal #71 to remove SpanData from the API.

[SergeyKanzhelev]

SpanData was removed. Closing