runtime: Allow creation of a root Activity when Activity.Current is not null
Regarding Span Creation the OpenTelemetry specification states that the API must accept:
The parent Context or an indication that the new Span should be a root Span.
The current ActivitySource.StartActivity
API does not allow for this if Activity.Current != null
. The following code creates a new activity as a child of Activity.Current
.
var rootSpan = activitySource.StartActivity(
"RootSpan",
ActivityKind.Internal,
parentContext: default)
One solution may be to set Activity.Current = null
when StartActivity
receives parentContext = default
, but then it may be debatable whether Activity.Current
should then be set to:
- the newly created root activity, or
- be restored to the previous value of Activity.Current prior to returning from
StartActivity
.
The OpenTelemetry specification also states:
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.
This implies that StartActivity
should not affect the value of Activity.Current
, but since it already does today it may be reasonable to set Activity.Current
to the newly created root activity.
Another option could be to introduce a new API StartRootActivity
that starts a new activity but does not affect Activity.Current
. It would be up to the user to manage Activity.Current
.
This need was originally outlined in https://github.com/open-telemetry/opentelemetry-dotnet/issues/984.
About this issue
- Original URL
- State: open
- Created 2 years ago
- Reactions: 3
- Comments: 20 (17 by maintainers)
I think we might want to consider doing something here. Sure there are workarounds, but I don’t think the API makes it obvious that they are needed. Users have to fail, realize the failure, and then figure out what to do about it. This is a somewhat common scenario/question we see on the OpenTelemetry .NET slack.
API Proposal
If we add this API it might be useful to supply a parameter which would automatically add the current parent context (if it exists) as a link.
Thanks to everyone for taking a close review this. Apologies, somehow I missed @tarekgh initial suggestion, but alas it sounds like it’s a no go.
Yes, agreed. I think something like
StartRootActivity
could work well to clearly indicate the intent to create a root activity. MaybeStartRootActivity
could take abool makeCurrent
parameter indicating to setActivity.Current
to the new root activity.I’m going to submit a doc request so that the current behavior is at least documented. I imagine that
CreateActivity(String, ActivityKind)
andStartActivity(String, ActivityKind)
useActivity.Current
as well?If you wanted to create a new root using the APIs that currently exist, you can do this:
@NinoFloris based on your latest fiddle (https://dotnetfiddle.net/1gkh9Q) it appears you discovered the same result. Writing 4 lines of code doesn’t look as elegant as one line of course, so it still seems perfectly legitimate to explore having a more concise API as @alanwest was originally asking about.
This isn’t correct unfortunately, neither of these options create new root Activities.
@tarekgh - I don’t know if we’ve done a good job defining what the behavior on that input should be, but the current behavior on this input does weird things. The easiest option for us to do would be to define that as bad input (it is an invalid parent ActivityContext after all), or if we define the behavior some other way then we’ll need to change the implementation to match. In the sample callback we pass it as the ParentContext and OpenTelemetry treats any non-default ParentContext to mean that the Activity has a parent and isn’t a root (incidentally classifying it as a parented Activity is also what caused OT to not to sample it in which is why @NinoFloris saw that null result). In Start() we use _parentSpanId == null to infer that the Activity doesn’t have a parent, but then we fallback to making Activity.Current be the parent . So instead of a new root Activity we get a new parented Activity where the sampling callback and Activity.Parent disagree about what the identity of the parent is. This path also opens the door to have an Activity whose TraceId doesn’t match its parent’s id which we’ve attempted to never allow.
This one doesn’t work either I’m afraid. The code treats default(ActivityContext) as the sentinel value indicating that it should parent the new Activity to Activity.Current.
CC @noahfalk @cijothomas @reyang