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

Span vs Context #201

Closed
mwear opened this issue Feb 29, 2020 · 4 comments
Closed

Span vs Context #201

mwear opened this issue Feb 29, 2020 · 4 comments
Labels
question Further information is requested

Comments

@mwear
Copy link
Member

mwear commented Feb 29, 2020

Span vs Context

The context propagation changes from OTEP 66 have lead to a few wrinkles in our APIs that need be addressed. Tracer#start_span returns a span. Often times this is what you want, but sometimes you really want that span in a Context object. On the other hand, sometimes you have a Context object and you want the current span from it.

What should Tracer#start_span return?

We could return a context instead a span, but I'm not sure that would improve things. It's worth considering though. Go has take the approach of returning both a context and span from start_span. Multiple return values would be possible, but not idiomatic in Ruby, and sadly would cost an additional array allocation. I suppose we can still consider that as an option.

Current issues

I have a span, but I want it in a context

An option is:

Tracer#in_span(span) { ctx = Context.current }

Is this good enough or should we add more convienence methods around this?

I have a context, but I want a span

Currently you have to do this:

Context.current[OpenTelemetry::Trace::Propagation::ContextKeys.current_span_key]

Proposal ContextUtils

While the above options work I think we can improve this situation. The OpenTelemetry::Context object is meant to be a general purpose context that could possibly be extracted out from OpenTelemetry and standalone as its own package. As such, adding a convenience methods to the Context directly should be avoided, but we can introduce a OpenTelemetry::ContextUtils class. Here's how I would see that working.

# read the current span from a given context 
ContextUtils.span_from(context)

# read the span context from a given context 
ContextUtils.span_context_from(context)

# return a new context with a given span and parent context
ContextUtils.set_span(span, parent: Context.current)

# return a new context with a given span context and parent context
ContextUtils.set_span(span, parent: Context.current)

While something like open-telemetry/oteps#68 could be a better long term solution, this will improve some difficulties that we're running into with the current API.

@mwear mwear added the question Further information is requested label Feb 29, 2020
@mwear mwear mentioned this issue Feb 29, 2020
@fbogsany
Copy link
Contributor

fbogsany commented Mar 4, 2020

I think this sort of thing should be the responsibility of objects in the Trace API rather than sticking them in a separate utils module. Having seen example uses in #202, the code doesn't seem any cleaner to me.

@mwear
Copy link
Member Author

mwear commented Mar 9, 2020

@Oberon00
Copy link
Member

Oberon00 commented Sep 4, 2020

If you are interested in this still, please take a look at open-telemetry/opentelemetry-specification#875 "Spans must have parent Contexts instead of parent Span(Context)s."

@fbogsany
Copy link
Contributor

I think this issue can be closed @mwear.

@mwear mwear closed this as completed Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants