Public setter for System.Diagnostics.Activity.Current

[lmolkova]

Motivation

Activity represents a context for distributed operation tracing.
It is created when an operation (e.g. Http request) starts and flows with async calls within a process.

Activity.Current is AsyncLocal static variable that any logger/tracing system can use to stamp logs and telemetry with 'correlation ids'.

Activity.Current could be set via calling Activity.Start or Activity.Stop which are enough as long as managed asyncronous flow is guaranteed which is not always the case.

Typical example is ASP.NET application hosed with IIS where hop from managed to native thread chould happen at any step in the request execution pipeline. ASP.NET propagates HttpContext through such hops.

With current API, it is not possible to assign Current Activity 'back' to what it was before the hop.

Proposed API

Add public setter for Activity.Current.

Usage

Such method could be called from the certain points in the request execution pipeline:

public void RestoreActivityIfNeeded(HttpContext context)
{
    if (Activity.Current == null)
    {
        var storedActivity = (Activity)context.Items["AspNetActivityKey"];
        if (storedActivity != null)
        {
            Activity.Current = storedActivity; //!!!
        }
    }
}

Details

• stopped/not started Activities should not be allowed as it makes Current stack invalid. E.g.

    Activity.Current = stopped;
    Activity.Current.Stop();  // Noop, never changes Current to Parent

    Activity.Current = notStarted;
    notStarted.Stop() // Noop, never changes Current to Parent

• arguably, Activity.Current = null is valid. E.g. fork a background task with no context without stopping all stack

Activity a = new Activity("incoming request").Start();

Task.Run(() => {
    Activity.Current = null;
    // start long-running task
});

// continune execution in the same context

Pull Request

Proposed changes: dotnet/corefx#29208

cc @vancem

[vancem]

I agree that that scenario is useful (you have an activity and something outside your control (typically native code), transfers it to another thread, that in turn calls managed code, and you wish to 'stamp' the activity on this thread (this happens automatically then tasks/threadpool items create sub-tasks, but won't happen automatically if the thread hop was done by native code).

It is a straightforward change to the API (adding a setter to something that has a getter), so I am happy with the design.

[vancem]

As a API design thing, I toyed with the idea of making a method 'SetCurrent' rather than having a property setter. This is an attempt to emphasize that this setter is UNUSUAL SCENARIOS only.

Ultimately, however I decided this whole API is reasonably rare, and I think the current design (that does the 'obvious' thing, is OK as well.

[lmolkova]

@karelz can we put it into 2.2 scope? Or did I miss this train?

[karelz]

At this moment there is not much difference between 2.2 and Future. We didn't review/triage against 2.2 yet in the repo - see milestone description.