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

SpanContext and Link spec/proto are not clear #1667

Open
dyladan opened this issue May 3, 2021 · 3 comments
Open

SpanContext and Link spec/proto are not clear #1667

dyladan opened this issue May 3, 2021 · 3 comments
Assignees
Labels
area:span-relationships Related to span relationships spec:protocol Related to the specification/protocol directory spec:trace Related to the specification/trace directory

Comments

@dyladan
Copy link
Member

dyladan commented May 3, 2021

From SpanContext spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spancontext:

SpanContext contains traceId, spanId, TraceFlags, and TraceState.

From Link specification https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#specifying-links:

A Link contains a SpanContext and zero or more Attributes.

From proto definition of Link https://github.com/open-telemetry/opentelemetry-proto/blob/a17f202fdae65e828ac29fb663b5bc5b64b13290/opentelemetry/proto/trace/v1/trace.proto#L197:

Link contains trace_id, span_id, and trace_state, but does not contain TraceFlags.

Thus, if a user wants to create a Link they must first create or get a SpanContext which has a valid TraceFlags. TraceFlags are not included in the Link proto so it might be confusing to the user to have to specify flags for the link which have no effect.

I would suggest that the Link spec is modified to specifically state that only the fields included in the proto are required for a link.

This issue was originally brought to my attention by follow up from an issue @carlosalberto filed on the JS API suggesting we use SpanContext in our Link type instead of the LinkContext type we had created specifically for this purpose.

@dyladan dyladan added the spec:trace Related to the specification/trace directory label May 3, 2021
@Oberon00 Oberon00 added area:api Cross language API specification issue spec:protocol Related to the specification/protocol directory and removed area:api Cross language API specification issue labels May 3, 2021
@Oberon00
Copy link
Member

Oberon00 commented May 3, 2021

Or maybe the trace flags should be included in the protocol? It might be interesting to know whether the parent had the sampled flag set or not.

@dyladan
Copy link
Member Author

dyladan commented May 3, 2021

As I see it, the options are:

  1. include traceflags in the protocol
  2. specify that links only contain traceid, spanid, and tracestate (and attributes of course)
  3. close this issue and leave SIG maintainers to resolve the discrepancy by either creating a new type or just using spancontext and dropping the unnecessary field in the otlp exporter

@reyang
Copy link
Member

reyang commented May 4, 2021

traceflags might be useful for certain backends to optimize indexing.

Discussed during the spec SIG meeting, we haven't seen a strong case that traceflags are needed in the protocol.

One thinking is that it might be easier/consistent to have the OTLP covering the entire SpanContext (which flows across RPC boundaries), which makes the spec simpler, and also makes it easier to add new stuff in the future in case W3C TraceContext has introduced new optional things.

@reyang reyang added the area:span-relationships Related to span relationships label May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:span-relationships Related to span relationships spec:protocol Related to the specification/protocol directory spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

4 participants