Opentelemetry-java: API: Tags vs. Baggage

[SergeyKanzhelev]

from https://docs.google.com/document/d/1WhmNrl7ZGBTYAyo23OjTW7IXj2G4AtsTcwv3Zieg2tk/edit#

OT: Tied to the Span, and part of SpanContext (which is immutable).

OC: Exist independently, can be propagated without a Span.
Possible approaches:
Have Baggage/Tags decoupled from the Span/SpanContext and have it stand independently (NOTE: this seems to be the path we will take).
Advantage: Simplicity, decoupling.
Disadvantage: Would need to provide a backwards layer for OT-compliant Tracers using the current approach.
Have Baggage/Tags moved to SpanContext.

[SergeyKanzhelev]

questions:

1. Common name
2. Do we have "Get" on baggage
3. Is Baggage on spans or separate API?

[bogdandrutu]

• Everyone agrees having baggage/tags separate from the SpanContext.

[bogdandrutu]

Decision: Tags will support get by key, (maybe an iterator).

[bogdandrutu]

Sergey: Proposed the default propagation to be local in the process.

[bogdandrutu]

Decision: Everything limited (key, value), see OpenCensus as a starting point.

[bogdandrutu]

Decision: Rename to baggage for the moment, may be revisited.

[SergeyKanzhelev]

So we can keep the Tags name if we will make local TTL as a default

[tedsuo]

@pavolloffay @carlosalberto @yurishkuro there has been a lot of push back against calling this Baggage, and we are considering sticking with Tags as the name for this. By making the default propagation context local, this should not be too bad.

[yurishkuro]

there has been a lot of push back ... baggage ...

Can we make the push back public and with arguments?

Local-only tags do not overlap with baggage functionality, so I don't think it addresses the decision here.

Tentative name for baggage in w3c is correlation context, I'd be fine to go with this name as an abstraction, eg in Go it could be

ctx = corrctx.Set(ctx, key, value)

[tedsuo]

Let's make sure there's alignment:

• Tags is a good name for metrics-related labeling
• But Baggage has more use cases than Tags.

[yurishkuro]

My proposal was to keep Tags as an API layer for managing metrics-related labeling, but the inter-process context propagation mechanism should have more general name, preferably "correlation context" to be in sync with https://github.com/w3c/correlation-context

Here are a few of examples of correlation context that don't make sense to be called tags:

• tenancy of the request at Uber is encoded as a json blob with many different attributes
• pivot tracing
• fault injection instructions for lineage-driven chaos engineering

[tedsuo]

Conclusion

API: We want only one package, which includes getters, and supports both use cases (tagging metrics and user baggage).

Naming: This will stay Tags for now because we don't have time to pick a name before Monday. But the name is still under discussion, Tags is only a placeholder for now.

[yurishkuro]

Corresponding discussion in Specification repo: https://github.com/open-telemetry/opentelemetry-specification/issues/9

[yurishkuro]

It seems we agreed more than once that "tags" is very misleading name. I suggested "correlation context" earlier https://github.com/open-telemetry/opentelemetry-java/issues/11#issuecomment-484604871, even though I have not seen any stated arguments against "baggage" (except for some thumbs-down, which I don't consider an "argument").

[bogdandrutu]

Correlation context sounds like a long name, I would like to see a proposal that includes the name of:

• Package - current tags
• The map of elements - current TagMap
• The element - current Tag

It will be good to have all of these, for "correlation context" I have a hard time finding these names.

[tedsuo]

Discussion 6/4

• we want a name that is clear
• we don't want "metaphorical names" like "mailman" or "baggage"
• Tags is too metrics-specific, and a confusing re-mapping of an OpenTracing term.
• The View object is where observability comes in. It is a metrics-specfic application of this more general propagation mechanism. So this package should not have a metrics-specfic name.
• This is related to in-process context propagation, so they should have "twin" names.

So, we're choosing DistributedContext as the name for this. We recommend changing the w3c proposal from CorrelationContext to DistributedContext for clarity.

• Package - dctx
• Map of elements - DistributedContext
• The elements - Attribute

Because we need to get a first draft of Java (and the spec) out the door, we'd prefer to not litigate this issue any further, at least not as a blocker for the first draft.

[bogdandrutu]

What about ObservabilityContext, TelemetryContext?

[yurishkuro]

@bogdandrutu I think the main idea is that DistributedContext (correlation context) is a general mechanism, not specific to telemetry. The needs of telemetry can be limited to views that read data from general context.

[tedsuo]

Update:

• changed the name of Attribute to Entry, as their signature is different from Span Attributes.
• clarified package name.

New name:

• Package - dctx or distributedcontext, depending on the language norms.
• Map of elements - DistributedContext
• The elements - Entry

[jmacd]

My objections to the name "DistributedContext" are:

(1) It looks like it does not contain a SpanContext. It's not a complete distributed context without a span context included. If we include the SpanContext, or establish a 1:1 correspondence between a structured tag value and a span context, then it would be at least accurate.

For example, if we adopted structured values (https://github.com/open-telemetry/opentelemetry-specification/issues/76), then a SpanContext could be included in a DistrubutedContext as:

"span.context": {
  "@type": "opentelemetry.org/v1/span_context",
  "span_id": "................",
  "trace_id": "................"
}

It feels likely add to confusion if something called DistributedContext isn't actually the complete distributed context. If we make it a container for a map of keys to entries _plus_ a SpanContext, or establish the equivalence shown above, then it's a sensible name, to me.

1. My second objection is just that it's a terribly long name. TagMap is far more descriptive of the actual function performed by this class, as it stands.

[bogdandrutu]

We do rename this for the moment, but we want to make it clear that this is an initial proposal and we are open to discuss a better name in the specs repo and specs SIG meetings.

We are doing the renaming to close the initial API proposal milestone.