API: Span - should it provide HasEnded property

[SergeyKanzhelev]

I can imagine scenarios where async operation may want to implement different behavior depending on whether the Span that started it up ended or nor. For instance, instead of adding Event to Span, report a separate Span. Or simply log information.

I hit this scenario in the following case:

Redis library in .NET allows to associate "session" with the current span. And there is a way to get all the redis operations happened in the "session". I need an indication that "session" has ended and span.HasEnded is a great way to check it.

Basically, any logic that requires to cache Span objects and then do some logic when Span in this cache ended would need this method.

API vs SDK: I need this on data collection module. So it will be great to have this method a part of API.

[austinlparker]

I'm generally in favor of this property being in the API, but would be interested in hearing more opinions either way. I know we've implemented a property like this on some of our tracers (javascript, iirc?)

[carlosalberto]

I remember this being discussed in OpenTracing, and one of the mentioned issues was how easy would it be for vendors to expose this detail. Hey @yurishkuro happen to remember anything about this?

[yurishkuro]

Some people have been asking, including in OpenTelemetry iirc, that implementations are not required to hold onto an allocated & writable Span, because some of them might want to stream out recorded events as soon as they are captured (e.g. to support long-running spans and don't lose them if the process crashes, they want StartSpan method to be able to report the event immediately). Essentially a Span in this case becomes equivalent to immutable, pass-by-value SpanContext. If isFinished() is required by the spec it prevents such implementations because it fundamentally implies there is a mutable state somewhere.

In practice, it seems that in frameworks where it's useful to read back isFinished(), it can be implemented within the framework instrumentation itself next to keeping the reference to the Span.

[SergeyKanzhelev]

in #63 I also mention whether the second call to End needs to be processed at all. Perhaps streaming tracers will be exempt from this requirement.

For the mutable state - API defines an instance of a class that will be kept around. It probably will be trivial to keep the state in it, would it? Or such implementations will not store span in context at all?

[yurishkuro]

A streaming implementation doesn't need to have any mutable object. Instead, every span API calls results in an immediate write of some events to the buffer, which is async flushed. All you need to know for these writes is the immutable span context as identity.

[jmacd]

I've implemented such an asynchronous implementation w.o mutable state as a proof of concept, here:
open-telemetry/opentelemetry-go#4

I've been meaning to write a position statement on this matter in general: Instrumentation libraries should not support reading their own state. I still plan to write this, but the rationale is:

1. The diagnostics data plane and the application's data plane should remain independent. It should be possible to provide a No-Op implementation of the instrumentation library without breaking the application, so the application may not rely on instrumentation state
2. Synchronization problems and deadlocks arise quickly
3. Performance cost of additional state management & synchronization are harmful
4. Allowing applications to read diagnostic state prevent the kind of efficient asynchronous implementations described in this thread.
5. With an asynchronous implementation, the business logic of turning events into span data can be performed in another process => this makes it possible to implement active span management out of process once, rather than once per SDK (as the OpenCensus does for tracking active spans).

[SergeyKanzhelev]

It seems that the decisions is not to expose it for now. Closing. Let's re-open if this will become an issue

[Oberon00]

Related: open-telemetry/opentelemetry-python#148 (comment)

[SergeyKanzhelev]

@Oberon00 do you suggest to re-open this issue? Can you please elaborate on specific use case when it is needed

[Oberon00]

No need to reopen, the Python issue will probably be solved by fixing the Span.end() there to be spec-conformant and thus callable multiple times.