eclipse / microprofile-telemetry

microprofile telemetry
Apache License 2.0
19 stars 18 forks source link

Clarify the behaviour of Span and Baggage beans when the current span or baggage changes #90

Closed Azquelt closed 1 year ago

Azquelt commented 1 year ago

Description

The spec says:

An implementation of MicroProfile Telemetry Tracing must provide the following CDI beans for supporting contextual instance injection:

  • ...
  • io.opentelemetry.api.trace.Span
  • io.opentelemetry.api.baggage.Baggage

Calling the OpenTelemetry API directly must work in the same way and yield the same results:

  • io.opentelemetry.api.trace.Span.current()
  • io.opentelemetry.api.baggage.Baggage.current()

Does this mean that when a Span is injected, the implementation should call Span.current() to get the instance, or that calling setAttribute on an injected Span instance should be equivalent to calling Span.current().setAttribute?

The difference is important if the current span changes. A contrived example:

@Inject
Span injectedSpan;

public void test() {
    // Get the current span context
    String oldSpanId = injectedSpan.getSpanContext().getSpanId();

    // Start a new span...
    Span newSpan = tracer.spanBuilder("my span").startSpan();
    // ...and make it current
    try (Scope s = newSpan.makeCurrent()) {
        String newSpanId = injectedSpan.getSpanContext().getSpanId();
        // is newSpanId the id for the original span, or for the newly created span?
    }
    ...
}

A more practical example might be if an ApplicationScoped bean had a Span injected. Would the injected span have the current span from whenever that bean was first instantiated, or would it always reflect whatever was the current span?

I would push for saying that an injected Span should always reflect the current span. That seems more useful, even if it would be harder to implement.

When we decide on the expected behaviour, we should also add a TCK test.

fmhwong commented 1 year ago

In the spec, we have listed 2 examples of getting the current span. My understand is the injected span should be always the current span.

fmhwong commented 1 year ago

BTW, in the example above.

String newSpanId = injectedSpan.getSpanContext();

should be

String newSpanId = injectedSpan.getSpanContext().getSpanId();
Azquelt commented 1 year ago

Thanks, I've updated the example.