opentelemetry-specification icon indicating copy to clipboard operation
opentelemetry-specification copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

Links with invalid SpanContext are recorded by default.

Open carlosalberto opened this issue 11 months ago • 1 comments

As Links with invalid SpanContext may contain attributes describing the operation (specially useful and occurrent for messaging systems), we still want to collect them, also allowing the user to drop them via a SDK configuration option. Fixes #2176 (in the issue extensive discussion about took place and we came, back in the day, with an initial agreement).

Now, I'm curious about:

  • We currently have a Implementations MAY ignore links with an invalid SpanContext, and I'm changing it to Implementations SHOULD record links with an invalid SpanContext, which doesn't sound like a breaking change, but I'd like to hear (practical) opinions.
  • In similar PRs we also suggest names for the new configuration options - maybe go with WithRecordInvalidLinks() / recordInvalidLinks() perhaps?

carlosalberto avatar Mar 07 '24 14:03 carlosalberto

Thanks @carlosalberto for getting this going.

We currently have a "Implementations MAY ignore links with an invalid SpanContext", and I'm changing it to "Implementations SHOULD record links with an invalid SpanContext", which doesn't sound like a breaking change, but I'd like to hear (practical) opinions.

I think this (practically) depends on what implementations are currently doing. If all implementations are currently recording links to invalid span contexts, then this change wouldn't be a breaking change. If there is an implementation that drops links to invalid span contexts without good reason, then this change might be considered breaking for that implementation.

I'd suggest:

  1. Identify if there are any implementations dropping links to invalid span contexts.
  2. Change those implementations to record links to invalid span contexts.
  3. Merge this specification change.

pyohannes avatar Mar 07 '24 14:03 pyohannes

@jmacd @trask Applied feedback. Please review.

cc @bogdandrutu

carlosalberto avatar Mar 14 '24 14:03 carlosalberto

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Mar 22 '24 03:03 github-actions[bot]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Apr 10 '24 03:04 github-actions[bot]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Apr 26 '24 03:04 github-actions[bot]