Opentelemetry-dotnet: Comparing OpenTelemetry Tracing API with .NET Activity API

[cijothomas]

I was planning to do a comparison post between using Activity API and Span API after Step1 (https://github.com/open-telemetry/opentelemetry-dotnet/pull/660) is merged, but posting this along with the PR, to help drive more clarity.

Background read:
https://github.com/dotnet/designs/pull/98
https://medium.com/opentelemetry/opentelemetry-net-sdk-progress-3a63dcdc6cb0
https://github.com/open-telemetry/opentelemetry-dotnet/pull/660

Notes:
This list is not complete. (Will add more issues, and resolutions as they happen)
This list is composed from responses spread in several repos/PRs/issues, and thanks for @tarekgh @noahfalk @SergeyKanzhelev @reyang @MikeGoldsmith for feedbacks and clarifications.

1. SpanCreation vs ActivityCreation

using (var span = Tracer.StartActiveSpan("spanname"))
{
  span.AddAttribute("http.host","hostname");
  ...
}

using (var activity = ActivitySource.StartActivity("activityname"))
{
  activity?.AddTag("http.host","hostname");
  ...
}

1. NoOp Span, vs Null Activity.
   OT defines a NoOpSpan, to be returned if no OT SDK installed.
   In .NET, instead of a NoOpActivity, null is returned to user if no listener. This means, user is expected to do null check on every action on activity as shown below (using c# ? operator for convenience)

   activity?.AddTag("http.host","hostname");
   vs
   span.AddAttribute("http.host","hostname");

The approach which Activity is taking aligns with the .NET general practice. In addition it provides much better performance.
We believe this is a good addition versus returning a No-op activity.
(It's quite easy to unit test if user misses the null check)

1. SpanCreation restriction.
   OT Spans can only be created from Tracer. There is no new Span("spanname").
   Though Activities are encouraged to be created from ActivitySource, nothing is preventing user from doing new Activity("myactivity");

This shouldn't be a blocker. We'll have docs and examples showing sample code which strongly encourages the right usage pattern.

1. String only attribute value for Activity vs long, string, bool for Span
   span.AddAttribute("http.responsecode",200);
activity?.AddTag("http.responsecode","200");

This is a known gap and .NET team is actively working with us on closing it, given we can add this at any time before .NET 5 GA release (Nov), we decided to not block on this and release a preview ASAP. This would help us integrate with OpenTelemetry soon.

1. Processing Pipeline.
   My initial (Step1) PR only allowed a single processing pipeline, But this is just to keep my PR small. The processing pipeline will match the current capabilities offered. (Sampler, Processor, Exporter, Resource etc)

.SetProcessorPipeline(builder => builder.AddProcessor((proc) => new MyCustomProcessor(proc))
                .SetExporter(new MyCustomExporter())));

1. ActivityContext is tied to W3CTraceContext format only?
   ActivityContext maps to OT SpanContext. There should be no difference here as both are conforming to the format of W3CTraceContext.
   Propagation is a separate concern, and its TBD at the moment. If OT allows multiple propagation formats, we'll also allow it.
   To be more clear here: neither Activity nor Span has any specialized knowledge about how to extract/propagate correlation context. Its achieved via TraceContextFormat currently. Once we have more formats, we can add more like OtherContextFormat. This is not embedded into Activity or Span

2. Missing fields in Activity vs OTSpan.
   In the current version, Activity lacks certain fields which are part of OT Span. The most notable being Status.
   The following is achievable in OT Span.
   span.Status = Status.Unavailable;

Similar to # 4 from above, this is known gap. Adding new fields (eg:Status) to Activity would be fairly simple. We need to address it before .NET 5 (Nov release), but this should not block us now.

1. Why use Activity and ActivitySource to replace Span and Tracer? Why not have them as additional instrumentation options, along with OT span.
   We want to avoid two competing APIs to achieve the same thing. This can confuse customers in the long term.
   Its possible to do a lightweight wrapper/adapter on top Activity APIs. This was discussed, but likely not needed.

The general direction we are heading is to have .NET platform provide the instrumentation API for Traces, Metrics and Logs.
We are trying to tackle Traces first with Activity API.
For logs, possibly with Microsoft.Extensions.Logging, and for metrics EventCounter or similar.

The end goal is to have .NET platform provide the API, matching the OpenTelemetry specification. As this post pointed out, we certainly have gaps between OT API and .NET API today, but this is work-in-progress towards the the end goal.

[tarekgh]

Thanks @cijothomas, this is very useful. Here are some comments mapping to your bullet numbers:

2- We can add an extra comment that the nullable pattern (using '?') also will avoid calling the Activity method if the object value is null. That can avoid calculating the method parameters which in most cases can save some memory allocations too.
3- it'll be good to mention creating an Activity object using ActivitySource is the preferred way if there is no other reason require creating the Activity object without starting it.
4- & 7- mentioning given we can add this at any time before .NET 5 GA release (Nov), it will be good to clarify that that depends on if it is hard requirements that need to be part of that release.

I would rephrase the sentence The end goal is to have .NET platform provide the API, matching the OpenTelemetry specification. to something like The goal is to have .NET enable the OT scenarios and capabilities by providing APIs to achieve that.

I would like to see the terminology difference section to remove the confusion when looking at the OT spec and .NET APIs. some examples of that are, Tags are the equivalent to OT Attributes, Activity.Recorded is not equivalent to Span.IsRecording...etc.
Last, maybe worth mentioning Activity providing some more capability (e.g. SetCustomProperty) which enables some flexibility to stick more stuff to the Activity.

[CodeBlanch]

I've chatted with @cijothomas about this, but for discussion's sake, I'll mirror here:

One thing I would really like to have in the .NET design is a "TerminalSpan" solution. That's the whole idea of non-capturing spans/activities for internal stuff we need to do (like calling Zipkin to transmit data). See #620. Today we have an infinite loop of span/activity generation unless we URL sniff and break it. .NET has ActivityKind, it would be nice to have that on there. Or someway to handle it. If we move away from Span in favor of Activity, we are really at the mercy of the .NET guys to give us everything we need to build something useful. Updating OT is a lot easier than the .NET runtime :)

Sorry if this is the wrong spot for this. There's so many spots where discussion is going on!

[reyang]

@CodeBlanch in OpenTelemetry Python we achieved the goal by having a flag in the context: https://github.com/open-telemetry/opentelemetry-python/blob/82359d5bfc6fe34f3012a44b3d8580f083706c9c/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py#L78

[CodeBlanch]

@reyang Thanks. Cijo sent me the same a few days ago. I think that is (more or less) what @lmolkova's proposal is doing. Totally fine with that approach. If we move away from Span in favor of Activity though, do we also move away from SpanContext/DistributedContext? I see in the .NET design there is ActivityContext & ActivityTraceFlags that seems close to what we need. Maybe we need to add a flag on ActivityTraceFlags to express the intent more clearly?

[tarekgh]

Maybe we need to add a flag on ActivityTraceFlags to express the intent more clearly?

I am wondering if this will be proposed to OT specs too? or we expect every language implementation will do its own thing?

[cijothomas]

Maybe we need to add a flag on ActivityTraceFlags to express the intent more clearly?

I am wondering if this will be proposed to OT specs too? or we expect every language implementation will do its own thing?

This issue would exist in every language, and must be defined in spec as well in my opinion.
Also - I'd like to move the discussion about Terminal Span to this PR https://github.com/open-telemetry/opentelemetry-dotnet/pull/620, so its easier to find these discussions later.

[SergeyKanzhelev]

Great summary. Worth to put it as a google doc to simplify editing and comments

[cijothomas]

Great summary. Worth to put it as a google doc to simplify editing and comments

Sure. I'll start a more detailed doc and share. I already realize editing is tricky to this issue!

[tarekgh]

https://github.com/dotnet/runtime/issues/31372

[cijothomas]

Note to self:
Span.IsRecording maps to activity.IsAllDataRequested

[cijothomas]

One more: (https://github.com/open-telemetry/opentelemetry-dotnet/pull/683#discussion_r427284489)

For OT sampling algorithms, one of the input parameter is the SpanID of the Span to be created. This is not achievable in new Activity API. We can create a Activity.GenerateSpanID, but there is no option to create an activity using this generated Span ID in the new model of creating activity (which is ActivitySource.StartActivity())
ActivityCreationOptions has everything other than the "future" SpanID of the Activity to be created.

[tarekgh]

@cijothomas trace id is interesting too as the span id.

[reyang]

Update on Span.Status:

• Notes taken from the OpenTelemetry specification SIG meeting
• Notes taken from the follow up meeting

[cijothomas]

one more:
SpanContext has isRemote, ActivityContext doesn't have isRemote.
IsRemote is not part of W3C spec.
(There was a note in .NET pr that isRemote is likely going to be removed from OT spec, need to find the status on that)

[tarekgh]

@reyang could you please track discussing removing IsRemote from the SpanContext spec in the SIG meeting?

[reyang]

I don't think SpanContext.IsRemote should be removed and I cannot recall any discussion in the OT spec. @cijothomas do you have more context on this?

I've added a topic in the 06/2/2020 specification SIG meeting to clarify/confirm it. @tarekgh

[tarekgh]

I don't think SpanContext.IsRemote should be removed and I cannot recall any discussion in the OT spec.

IIRC, this was feedback from @lmolkova but I don't have much context. @noahfalk @SergeyKanzhelev do you recall more details about that?
But, in general, I am wondering how this property is useful if marking the context as remote? I mean how this going to be used in the OT scenarios.

[MikeGoldsmith]

The IsRemote flag is set only on a span context that is built during propagation extraction to identify the context has a parent that was created outside the current process.

As far as I'm aware, there is no current discussion about removing it.

[reyang]

I've clarified in today's OpenTelemetry specification SIG - IsRemote is useful and will NOT be removed from SpanContext.

OpenTelemetry specification SIG meeting notes (06/2/2020).

[cijothomas]

Thanks @reyang for getting this clarified! @tarekgh Could we consider making this part of ActivityContext/Activity ?

[tarekgh]

Thanks @MikeGoldsmith and @reyang for confirming. I'll track this one.

[cijothomas]

Posted a doc with this:
https://docs.google.com/document/d/1iFHnvaTohbmHtGQWR1eijHkhYrForq8xhmpS0gp06gE/edit

[cijothomas]

Closing in favor of https://github.com/open-telemetry/opentelemetry-dotnet/issues/947