Provide default activity name value from [CallerMemberName] in ActivitySource.StartActivity

[kzu]

The improvements up to part 2 have made things substantially more usable for the Activity API. This is a small proposed improvement to introduce a sensible default activity name.

Updated proposal based on feedback

ActivitySource.StartActivity

We currently require an activity name to be provided as the only required argument in ActivitySource.StartActivity overloads. For scenarios where the caller wants to match the activity with the calling method name, providing that as a default value would make calling code less repetitive.

API Proposal

    public Activity? StartActivity([CallerMemberName] string name = "", ActivityKind kind = ActivityKind.Internal)
            => StartActivity(name, kind, default, null, null, null, default);

Additionally, for cases where users want to get the caller member name alongside other parameters, add the following overload too:

API Proposal

public System.Diagnostics.Activity StartActivity (System.Diagnostics.ActivityKind kind, 
                                                  System.Diagnostics.ActivityContext parentContext = default,
                                                  System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string,object>>? tags = default, 
                                                  System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityLink> links = default, 
                                                  DateTimeOffset startTime = default,
                                                  [CallerMemberName] string name = "");

[paulomorgado]

Why not just this one?

public Activity? StartActivity(ActivityKind kind = ActivityKind.Internal, [CallerMemberName] string name = "")
        => StartActivity(name, kind, default, null, null, null, default);

Not that I'm a fan on optional arguments...

I would prefer this:

public Activity? StartActivity(ActivityKind kind, [CallerMemberName] string name = "")
        => StartActivity(name, kind, default, null, null, null, default);
public Activity? StartActivity([CallerMemberName] string name = "")
        => StartActivity(name, ActivityKind.Internal, default, null, null, null, default);

[ghost]

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

[tommcdon]

@noahfalk @tarekgh

[noahfalk]

when the only thing the caller wants to customize is the ActivityKind, a new overload would also be useful

How often do you forsee this occuring? The only situation that came to mind is when creating a client or producer Activity around a new outbound communication protocol. That seemed fairly niche to me.

[tarekgh]

I agree with @noahfalk that I am not seeing this is main stream scenario. Also, it is easy to write one line extension method to do so.

    public static class ActivitySourceExtensions
    {
        public static Activity? StartActivity(this ActivitySource source, ActivityKind kind, [CallerMemberName] string name = "") => source.StartActivity(name, kind);
    }

[kzu]

I don't think the easy-ness of writing an extension method should be of any concern WRT to thinking about potential additions as built-in API/overloads. (otherwise you could only justify the one method with the most parameters in every API, since it's easy to create the extension methods that call into that).

My main scenario is being able to track the flow of calls across components in a loosely coupled system where things go through pub/sub/command bus intermediaries (think CQRS/ES style). This makes it hard to reason about the logic flow in the app.

By simply starting activities at those boundaries (with just using var _ source.StartActivity()) means you can later on plot the flows. I don't think it's a corner case to want the activity name to default to the current method name. Neither that you'd want to just say source.StartActivity(ActivityKind.Client) for your outgoing calls.

[tarekgh]

I don't think the easy-ness of writing an extension method should be of any concern WRT to thinking about potential additions as built-in API/overloads. (otherwise you could only justify the one method with the most parameters in every API, since it's easy to create the extension methods that call into that).

We add overloads to the core when thought it is going to be used in main scenarios or it is hard to achieve the scenario without this overload. Otherwise we'll end up with tons of rarely used APIs in the core. This is why I suggested the extension method. So, the discussion here is about how popular you scenario is. I am not objecting the request but trying to understand the importance of your scenario. I can see how useful your scenario is but I cannot judge how this is not a corner case especially no-one requested such scenario before. @noahfalk do you have thoughts around this scenario?

[noahfalk]

@noahfalk do you have thoughts around this scenario?

Using the method name as the activity name seemed like a reasonable default that might get broad usage. Specifying ActivityKind.Client as the sole argument seemed likely to be a corner case, and if so I'd suggest not adding an overload for that part.

[tarekgh]

Considering we are currently have the overloads:

public System.Diagnostics.Activity? StartActivity (string name, System.Diagnostics.ActivityKind kind = System.Diagnostics.ActivityKind.Internal);

public System.Diagnostics.Activity StartActivity (string name, 
                                                  System.Diagnostics.ActivityKind kind, 
                                                  System.Diagnostics.ActivityContext parentContext, 
                                                  System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string,object>>? tags = default, 
                                                  System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityLink> links = default, 
                                                  DateTimeOffset startTime = default);

I am suggesting to change the first one to

public System.Diagnostics.Activity? StartActivity ([CallerMemberName] string name = "", System.Diagnostics.ActivityKind kind = System.Diagnostics.ActivityKind.Internal);

and then add the overload:

public System.Diagnostics.Activity StartActivity (System.Diagnostics.ActivityKind kind, 
                                                  System.Diagnostics.ActivityContext parentContext = default, 
                                                  System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string,object>>? tags = default, 
                                                  System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityLink> links = default, 
                                                  DateTimeOffset startTime = default,
                                                  [CallerMemberName] string name = "");

The reason for that, if anyone want to provide more parameters and still need to get the caller member names, this overload can do it.

what you think about that?

[kzu]

That sounds great!

[tarekgh]

@kzu could you please update the proposal on the top to reflect the recent?

[kzu]

BTW, your first proposal is exactly what my first proposal was too 😉

[tarekgh]

@noahfalk I marked this ready for design review.

[noahfalk]

That looks fine to me 👍

[terrajobst]

Video

• Looks good as proposed

namespace System.Diagnostics
{
    public static partial class ActivitySource
    {
        // Existing API, just mark first parameter optional and add [CallerMemberName]
        public Activity? StartActivity ([CallerMemberName] string name = "",
                                        ActivityKind kind = ActivityKind.Internal);

        public Activity StartActivity (ActivityKind kind,
                                       ActivityContext? parentContext = default,
                                       IEnumerable<KeyValuePair<string,object>>? tags = default,
                                       IEnumerable<ActivityLink>? links = default,
                                       DateTimeOffset startTime = default,
                                       [CallerMemberName] string name = "");
    }
}

[kzu]

The video time is more at https://youtu.be/viYdlWGUiro?t=4757 :)