Tracer#in_span behavior

[mwear]

Behavior of Tracer#in_span

Tracer#in_span is a non-spec'd convenience method that we've added to the Ruby OpenTelemetry implementation. It's widely used in our instrumentation and is pretty straightforward when used with an implicit context. However, when using an explicit context the behavior and expectations become a little more murky. Consider the following examples:

With implicit context

If the current (implicit) context is:

{ 
    span: #<OpenTelemetry::SDK::Span @name: "span a">,
    baggage: { "k1": "foo" }
}

then

tracer.in_span("child") do |span|
    # parent: span a
    # baggage: { "k1": "foo" }
end

Details

in_span does the following when using an implicit context:

• Creates a new span, using the parent span from the implicit context
• Creates a child context derived from the implicit context, with the newly created span as current
• Sets the newly created context as current
• Executes block
• Restores original context

With explicit context

If we have the same current (implicit) context:

{ 
    span: #<OpenTelemetry::SDK::Span @name: "span a">,
    baggage: { "k1": "foo" }
}

and an explicit context, named ctx

{ 
    span: #<OpenTelemetry::SDK::Span @name: "span b">,
    baggage: { "k1": "bar" }
}

then

tracer.in_span("child", with_parent: ctx) do |span|
    # parent: span b
    # baggage: { "k1": "foo" }
end

Details

in_span does the following when using an explicit context:

• Creates a new span, using the parent span from the explicit context
• Creates a child context derived from the implicit context, with the newly created span as current
• Sets the newly created context as current
• Executes block
• Restores original context

In this example, we derive a new context from the implicit context, but use the parent span from the explicit context. Any other entries in the explicit context, such as baggage, are not part of the newly created context. The resulting context is somewhat of a hybrid between the implicit and explicit contexts.

Alternative

There are some questions as to what the actual behavior should be here, but it can be surprising that only the span is used from the explicit context and that baggage (and anything else) stored in the explicit context are not available in the block. It might make sense to derive the new context from the explicit context, rather than the implicit context. For example:

tracer.in_span("child", with_parent: ctx) do |span|
    # parent: span b
    # baggage: { "k1": "bar" }
end

There are currently bugs in some of our instrumentation packages due to this exact issue. See

• Sinatra
• Sidekiq

In both the baggage from the extracted context is not available within the in_span block.

What should we do?

Since Tracer#in_span is not officially spec'd, we need to agree on what the behavior should before our 1.0 release and commit to supporting the behavior. We have a handful of options:

• Leave it as is
• Adopt the alternative behavior discussed above
• Remove the with_parent option as the explicit context case is controversial one
• Remove in_span completely
• Something else?

I just wanted to open this up for discussion and list out some possible ways forward. Feel free to chime in with other suggestions.

See also

This has been discussed in the following PRs / Issues:

• Baggage not propagated for consumers who use ruby_kafka instrumentation #549
• fix: set context in in_span #551

[fbogsany]

• Remove the with_parent option as the explicit context case is controversial one

This is an interesting option - it leaves in_span available for common implicit context use-cases in application code while requiring instrumentation at service boundaries to be explicit about context propagation. We would need to provide clear documentation of each use-case.

• Remove in_span completely

Alternatively, we could move it to another convenience gem that does not have the same versioning and stability constraints as the API.

[ahayworth]

Remove in_span completely
Alternatively, we could move it to another convenience gem that does not have the same versioning and stability constraints as the API.

If this is the route we take, we should do it before 1.0. 😄 I think my main concern there is that it is a very nice thing for users to use, and I hate the idea of making it even harder for the average developer to get started. Conceptually, I think in_span just works well for people in the common case.

I'm not sure what the right approach is regarding splitting it out - but as long as it doesn't increase the burden on end-users to get started, then I think splitting it out could be beneficial (for example, it could allow us to closely track the "Convenience API" if and when it ever happens).

[fbogsany]

If this is the route we take, we should do it before 1.0.

I opened a tracking issue for this and added it to the RC milestone - we definitely need to resolve this before 1.0.

[ahayworth]

As I mentioned elsewhere, I didn't know about this behavior until today! Suffice it to say it surprised me. 😄

---

Regarding the options, my thoughts are:

Leave it as is

🚫 The current behavior is pretty confusing, and it's not even clear that it's confusing until you find out the hard way. I don't think we should leave it as-is.

Adopt the alternative behavior discussed above

😐 I'm neutral on this. I think that if we keep the with_parent option, then this behavior would make sense to me. However, I would almost want to call it with_context at that point.

Remove the with_parent option as the explicit context case is controversial one

✅ I think this is my preferred option. If we take this approach, then I think we must document to users what their alternatives are; since there are some people (me!) that are definitely mucking around with the with_parent option and would need to change their approach. I think it would suffice to tell users something like "This method creates a child span of the currently-active span, and inherits the current global context. If you need to explicitly set the parent span or use a manually managed context object, then you need to use start_span and control the span lifecycle yourself - or ensure that the currently-active span and current global context reflect the values you desire." (Or something that's a little more clear, perhaps).

Remove in_span completely

🚫 Perhaps it could be extracted to a gem, maybe. But I think we should support it somehow, somewhere - it's just so elegant and ruby-ish to me. I really, truly like it. 😆

---

As an aside - I know the "Convenience API" is an idea percolating about the community. I think we should take care to balance useful things in our library against an updated API that may not exist for a long time (if ever!). That is to say: we should find a way to support useful things like this and let that contribute back to the "Convenience API" discussion as it unfolds, so that our end-users have a better time of things. After all, the more traction that OpenTelemetry gets, the better the whole ecosystem gets for everyone! 😄

(I'd be willing to help out significantly on the convenience API front - that overlaps a lot with what I'm doing at $DAYJOB).

[fbogsany]

Follow-up from the SIG meeting...

We discussed this extensively on today's call. The consensus was that we should leave this in the API but remove the explicit parent span option (since that's the contentious bit, and we can add it back later without breaking API compatibility). This bit of the Span Creation spec permits this addition:

In languages with implicit Context propagation, Span creation MUST NOT set the newly created Span as the active Span in the current Context by default, but this functionality MAY be offered additionally as a separate operation.