Impossible to set Activity.IdFormat when using ActivitySource

[pakrym]

Description

Because the ActivitySource doesn't expose the IdFormat in any way it's impossible to set the value before the new activity is started. The format is inferred from the global setting or the parent if it exists but if there is no parent and the global setting is hierarchical it's impossible to start a W3C root activity.

Regression?

Regression from the previous pattern, it was possible to set Activity.IdFormat using Activity.SetIdFormat before calling DiagnosticSource.StartActivity

cc @noahfalk @tarekgh

The proposal is added by @tarekgh

Proposal

In 6.0 we already added an overload for ActivitySource.StartActivity. The proposal here is just adding the idFormat as optional parameter to that overload.

namespace System.Diagnostics
{
    public sealed class ActivitySource : IDisposable
    {
	      public Activity? StartActivity(
	          ActivityKind kind, 
	          ActivityContext parentContext = default, 
	          IEnumerable<KeyValuePair<string, object?>>? tags = null, 
	          IEnumerable<ActivityLink>? links = null, 
+	          ActivityIdFormat idFormat = ActivityIdFormat.Unknown,
	          DateTimeOffset startTime = default,
              [CallerMemberName] string name = "");
    }
}

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

[pakrym]

cc @cijothomas @reyang

[noahfalk]

Thanks @pakrym! Not pushing back but curious to get your thoughts, at what point do you envision the library should own the choice of id format? For example a distributed system has a request come in to a particular process that has a hierarchial id, framework creates an Activity when the message is received, optional additional Activities in the user's code, some more Activities in the library code, and then eventually the id gets serialized on the wire of an outbound request. At some point a change in id format would need to happen. My intuition would be to do it as late as possible, ideally as the last Activity before the format hits the wire. In that model we would look more at overriding the HttpClient behavior rather than eagerly doing format conversions as soon as the library creates its first Activity.

In terms of workarounds overall these are the only ones I could think of:

1. set the global default id format - libraries probably don't want that responsibility
2. use the Activity constructor with DiagnosticListener as I assume you are already doing
3. use ActivitySource most places, but funnel everything through a path that creates at least one Activity with the old constructor, allowing the format to be coerced if needed before it gets to the wire
4. Customize or override HttpClient's DiagnosticHandler to take more control of the id that gets written on the wire.

As for the best forward looking solution, I'd be tempted to change the default id format from Hierarchical -> W3C for .NET Framework apps too. Apps wouldn't get the new behavior unless they updated to a new major version of DiagnosticSource.dll (for example v6) which implies these are apps in at least semi-active development as opposed to stable apps that are just trying to pick up security fixes. Hopefully our experience changing the default for the netcoreapp TFM would give us some warning signs if this is more dangerous than I expect. Thoughts?

[pakrym]

Not pushing back but curious to get your thoughts, at what point do you envision the library should own the choice of id format?

To be honest, I don't think I care about Azure SDK having a choice of IdFormat it seems more like an issue of a bad default and not being able to override it.

My intuition would be to do it as late as possible, ideally as the last Activity before the format hits the wire. In that model we would look more at overriding the HttpClient behavior rather than eagerly doing format conversions as soon as the library creates its first Activity.

In practice, you need Ids earlier than that. For example to add links to the activity.

As for the best forward looking solution, I'd be tempted to change the default id format from Hierarchical -> W3C for .NET Framework apps too

I think changing the default for all ActivitySource consumers to W3C would be the best solution. Can we patch it in 5.0.1?

[tarekgh]

I think changing the default for all ActivitySource consumers to W3C would be the best solution. Can we patch it in 5.0.1?

This will be a breaking change which we try to avoid in the servicing releases.