Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Use whole spancontext for link #57

Closed
dyladan opened this issue May 3, 2021 · 4 comments · Fixed by #58
Closed

Use whole spancontext for link #57

dyladan opened this issue May 3, 2021 · 4 comments · Fixed by #58
Assignees

Comments

@dyladan
Copy link
Member

dyladan commented May 3, 2021

From the spec:

A Link is structurally defined by the following properties:

  • SpanContext of the Span to link to.
  • Zero or more Attributes further describing the link.

Seems pretty clear to me that the link should expose the whole SpanContext including the trace state.

Originally posted by @dyladan in #36 (comment)

Currently our Link is an object that shares some properties with SpanContext. Should we (1) add the missing properties to Link or should we (2) make it type Link = { spanContext: SpanContext, attributes: Attributes }?

To me, the spec seems to imply 2 is the more "correct" choice, but 1 is the less breaking change.

@dyladan dyladan self-assigned this May 3, 2021
@blumamir
Copy link
Member

blumamir commented May 3, 2021

I vote for (2).
Seems like the right time to do things more correctly with the cost of minor breaking change.
Also, Links are not in wide use currently so it won't break much.

@dyladan
Copy link
Member Author

dyladan commented May 3, 2021

I think they're used by at least the GCP exporter so they might be used more than you think.

@dyladan
Copy link
Member Author

dyladan commented May 3, 2021

I would also vote for 2

@blumamir
Copy link
Member

blumamir commented May 3, 2021

I think they're used by at least the GCP exporter so they might be used more than you think.

I also use them in aws-sdk instrumentation for implementing the messaging systems semantic convention specification for batch receiving.
But it should be a very easy fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants