Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenTracing layer support for Baggage #2137

Closed
carlosalberto opened this issue Nov 18, 2021 · 3 comments · Fixed by #2194
Closed

OpenTracing layer support for Baggage #2137

carlosalberto opened this issue Nov 18, 2021 · 3 comments · Fixed by #2194
Assignees
Labels
area:miscellaneous For issues that don't match any other area label question Question for discussion spec:miscellaneous For issues that don't match any other spec label

Comments

@carlosalberto
Copy link
Contributor

In OpenTracing, SpanContext and Baggage are part of the same object, whereas in OpenTelemetry they are different objects. This would be straightforward and trivial to support if the OpenTracing Shim couldn't be used along the OpenTelemetry API in the same code base -- which, however, has been deemed important to support gradual adoption in large codebases.

This currently is supported in the OpenTracing Shim by associating any OpenTelemetry Span with a given SpanContext + Baggage combo at all times, in order to see changes reflected in different threads at all times. A typical use case is

// Main thread, OTel API
io.opentelemetry.trace.Span span1 = ...; // Create OpenTelemetry Span alone
propagateAsCurrentSpanToThreads(span1);

// A thread consuming the the propagated span1 through the OT Shim
openTracingTracer.activeSpan().setBaggageItem("hello", "foo");

// Another thread consuming the propagated span1 through the OT Shim too
openTracingTracer.activeSpan().getBaggageItem("hello"); // returns foo

This is supported in Python easily by decorating a pair of values in the OpenTelemetry Span itself, whereas in Java (and similar languages) it requires a global map with a global read-write lock, which may still impact the performance greatly.

Alternative 1

Completely discard support for Baggage, providing the related functionality as no-op, given:

  • Its complicated handling (for some languages) when the OT Shim is used along the OTel API.
  • Its potential lack of common usage in OpenTracing
  • Additionally: its heavy memory footprint (as SpanContext baggage values are copied from the the parent Span).

Alternative 2

Provide a best-hope design, in a relatively simplistic fashion, in order to handle common cases (such as an OpenTracing Span being created, have its baggage set, and have its baggage propagated).

  • Have SpanContext + Baggage be part of the Span Shim object (hence, no need for global maps/locks), and propagate this object itself (along its current Baggage). This would allow:
    • OTel API usage: simply consume the propagated Baggage.
    • OT Shim usage: have different threads update the Baggage value as needed.
  • If the current Context has no Span Shim object, all the OT Shim Baggage operations are no-op.

This WOULD NOT satisfy the expected OT Handling, but it would offer limited support though.

cc @yurishkuro @tedsuo

@carlosalberto carlosalberto added question Question for discussion spec:miscellaneous For issues that don't match any other spec label area:miscellaneous For issues that don't match any other area label labels Nov 18, 2021
@yurishkuro
Copy link
Member

if the OpenTracing Shim couldn't be used along the OpenTelemetry API

Honestly, at this point I would settle for using OT Shim alone. Co-existence with OTel instrumentation is nice to have, but we don't seem to have a lot of demand for it.

@carlosalberto
Copy link
Contributor Author

@yurishkuro I prototyped a simple approach where the SpanContext + Baggage are stored/locked in the Span Shim object itself, which greatly simplifies things (and allows to have full compatibility with OT).

One thing to notice is that in OpenTracing children Span inherit their parent's Baggage, potentially duplicating data - whereas in the Shim, as Baggage is immutable, no data duplication happens (unless the children mutate the Baggage, of course). A small nice win.

Remember that interoperability with the OTel API won't be greatly supported (we won't break but Baggage may be lost along operations). If this seems right, I will update the Spec as expected. Let me know.

@yurishkuro
Copy link
Member

sgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:miscellaneous For issues that don't match any other area label question Question for discussion spec:miscellaneous For issues that don't match any other spec label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants