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

[Prototype] Move SpanContext into Span #1191

Closed

Conversation

carlosalberto
Copy link
Contributor

Description

Prototype for moving SpanContext into Span (open-telemetry/opentelemetry-specification#1022)

For simplicity purposes, only updated:

  • API
  • SDK
  • The Jaeger exporter

I'd like to get the Maintainers feedback - from my side here are my points:

  • This prototype required changes that have been only very recently merged to the Specification (such as Define Propagation-only Span to simplify active Span logic in Context. opentelemetry-specification#994). Although we have prototypes for the aforementioned changes, we actually won't know how good it all these recent changes will play along with this specific change, and there's the general possibility that this may become a can of warms.
  • The changes, from instrumentation code perspective, do not really represent a change, as mostly this has to be updated in the API/SDK/Propagators (no instrumentation, based on my digging, would need to be greatly updated).
  • Now Link will either reference Span (as shown in the prototype) or replicate the Trace/Span/ ids and flag/state. Not a pretty change, but it will work.
  • The change itself is neither a big issue nor a big improvement for Python overall. It won't break much existing user's code, but it doesn't bring anything useful to the table IMHO.

cc @lzchen @codeboten in case I may have overseen either good or bad impacts of this change ;)

@codeboten
Copy link
Contributor

Looks like the spec change was closed, are you ok with closing this PR @carlosalberto?

@carlosalberto
Copy link
Contributor Author

Closing this as the related PR in the Specification as closed as well.

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants