Allow setting the Activity properties after Activity creation and before starting it when using ActivitySource

[macrogreg]

The description and the Proposal edited by @tarekgh. The original description is moved down in the comment #42784 (comment)

ActivitySource provides the method StartActivity which will try to create and start the Activity object. There is a scenario that needs to add some more properties to the Activity object after it is created and before it gets started.
The reason is the caller wants to avoid creating the objects need to set to the Activity before ensuring the Activity object is created. It is important to have these properties set before starting the Activity so the listeners of the Start event will get the full set of the data.

Proposal

namespace System.Diagnostics
{
    public sealed class ActivitySource : IDisposable
    {
        // Alternative proposed name is CreateUnstartedActivity
        public Activity? CreateActivity(
                             string name,
                             ActivityKind kind, 
                             ActivityContext parentContext,
                             IEnumerable<KeyValuePair<string, object?>>? tags = null, 
                             IEnumerable<ActivityLink>? links = null);

        public Activity? CreateActivity(
                             string name, 
                             ActivityKind kind,
                             string parentContext,
                             IEnumerable<KeyValuePair<string, object?>>? tags = null, 
                             IEnumerable<ActivityLink>? links = null);
    }
}

Example

    using (Activity activity = activitySource.CreateActivity("SomeName", ActivityKind.Server, context))
    {
        activity?.AddBaggage("db.type", "mongo");
        activity?.Start();

        // Do any necessary work      
    }

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

[macrogreg]

Some tags to raise awareness for folks involved into relevant work streams:
@cijothomas @noahfalk

Thanks! :)

[tarekgh]

I don't think it is intended to support all possible scenario in ActivitySource in first version. Your scenario can be considered in the future releases. You still have the option to use old way of creating the Activity and using DiagnosticSource. This still work.

For tags allocation, you can still avoid the allocation before creating the activity by setting ActivityCreationOptions.SamplingTags in the listener callbacks. This will still allocate at that time but for sure it will be determined the Activity is going to be allocated anyway.

It is too late to change any APIs in the 5.0 release.

[macrogreg]

You still have the option to use old way of creating the Activity and using DiagnosticSource. This still work.

From the perspective of a telemetry producer - yes. But a producer does not know whether this scenario is important for any actual consumer. And if a producer who followed the existing guidance wants to transition to the "new way" while enabling all previous scenarios, they cannot. I completely realize the current spot in the release cycle, but if this item was to be tracked for prioritization in the next cycles, then, for the record, I think that the workaround you mention does not apply. :)

For tags allocation, you can still avoid the allocation before creating the activity by setting ActivityCreationOptions.SamplingTags in the listener callbacks.

Thanks for pointing it out. I had missed the existence of that property. However, having had a look at it, I do not fully understand its use. Is there a doc or an example? What I mean above is that when invoking ActivitySource.StartActivity(..) you need to pass an IEnumerable object on the heap. So the allocation will occur when creating that object while the sampling decision is taken within the scope of the ActivitySource.StartActivity(..) method. So regardless of whether or not you decided to allocate the activity, you will have to allocate the tags collection object.

Please let me know if you guys consider these Activity-related submissions as a valuable input for forward-looking planning / for the next cycle. I do not want to create a distraction if it is not productive. :)

Thanks!

[tarekgh]

if a producer who followed the existing guidance wants to transition to the "new way" while enabling all previous scenarios, they cannot.

Agree. What i was trying to say is, this is not supported scenario yet with ActivitySource. I was pointing if someone blocked for now can consider use the old way.

For ActivityCreationOptions.SamplingTags, the idea is when you are calling ActivitySource.StartActivity you'll not pass any tags at that time but in the listener you'll have a chance to add the tags during then. I am not sure if this will fully satisfy your scenario. you may look at our test

runtime/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs

Line 605 in e2e43f4

activityOptions.SamplingTags.Add("tag1", "value1");

which show how it is used.

If the allocation is very critical to your scenario, you may consider having IEnumerable thread local object (e.g. List<>) which you can use for all subsequent calls on same thread. The allocation would be once per thread instead to be ones per ActivityStart call.

[macrogreg]

Agree. What i was trying to say is, this is not supported scenario yet with ActivitySource. I was pointing if someone blocked for now can consider use the old way.

Gotcha, thanks!

For ActivityCreationOptions.SamplingTags, the idea is when you are calling ActivitySource.StartActivity you'll not pass any tags at that time but in the listener you'll have a chance to add the tags during then.

Ah, understood. This is a different scenario. I was talking about saving the allocation that is only required only to pass parameters to StartActivity(..)

If the allocation is very critical to your scenario, you may consider having IEnumerable thread local object (e.g. List<>) which you can use for all subsequent calls on same thread.

That might work, but the complexity of implementing such cache and the perf overhead required for cleaning it before each use make this feel concerning. Of course, .NET5 is done. I would love a performance-focused overload similar to what I mentioned above to be considered for the next release.

If the team decides that such an overload is beneficial, I'd be happy to contribute when the time comes. :)

Thanks!

[cijothomas]

StartActivity(..) overloads that have parameters like string tag1Key, string tag1Value, ... for the first few tags.

This sounds like a good improvement. In most cases the number of initial tags/attributes is very low (or none). More tags are added after actually creating the activity and knowing the sampling decision.

(if activity != null && activity.IsAllDataRequested == true) 
{
  add tags.
}

[noahfalk]

Thanks @macrogreg!

System.Diagnostics.ActivitySource.StartActivity(..) is missing an overload that takes Baggage items

StartActivity() was intended to take the minimum amount of data necessary for sampling. Adding more types of data trades off performance for more flexibility in sampling and richer data available on the start callback. Another direction is to create an API that returns an unstarted Activity allowing Start() to be decoupled from sampling. That option is trading off more complicated API usage for richer data available on the start callback. I'm not opposed to this in principle, but they first hurdle to clear would be to understand why do we need specific data attached to a Start callback if that same data will be available later in the Stop callback?

However, this will require that an enumerable is allocated before StartActivity(..) is called

We added the overload which took tags because OpenTelemetry defined that it must be possible to give tags to the sampling algorithm. My hope is that in practice it is a rare case that the sampling algorithm needs to consider tags because doing so implies you are paying perf costs to generate them and analyze them even for sampled-out activities. The higher performing route would be to determine in advance that the tags aren't relevant for sampling and provide them only after an Activity has been sampled in using Activity.SetTag(). If there is a need for higher performance scenarios where certain tags are available to the sampler or available in the Start() callback I'm not opposed to optimizations you suggested, but I'd like to be able to tie to some specific customer or use-case that the work is improving.