Simplify setting W3C Trace Context in SignalR browser clients

[iinuwa]

Is there an existing issue for this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

I am trying to add W3C Trace Context to our client's SignalR calls over WebSockets.

Our client generates a root trace ID when the page loads, and we are able to use interceptors on our REST calls to add the trace context to HTTP requests very easily, and ASP.NET Core automatically handles mapping the trace context to System.Diagnostics.Activity.Current. This allows us to do distributed tracing and track requests initiated by a single user's page load across all of our services.

I would like the same thing for SignalR: automatic sending (for clients) and parsing (for servers/hubs) of W3C trace context, as well as handling Activity.Current. This may only be applicable to the browser-based WebSocket clients, since you should be able to set arbitrary HTTP headers, including traceparent and tracestate, using other clients/transports, and I assume those would still be parsed by ASP.NET Core.

I found a workaround to do this in the browser, but it passes the trace context using a query string parameter in the connection URL, which I am not sure is supported or not. It also requires you to manually reset the current for each hub method call, which is easy to miss.

Describe the solution you'd like

I would like it if there was a way to specify a trace context in the client libraries and send it to the hub somehow, and for the HubContext to automatically parse the context and set Activity.Current accordingly. (This would require some trickery for JS/browser clients since browsers don't allow extra headers in WebSocket connections.)

For example:

/**
  * Add W3C trace context to the negotiation and connection requests
  * @param {string?} traceparent - Parent trace context string.
  *   If not specified, a random context will be created.
  */
function withTraceContext(traceparent: string?) {

}

const connection = new HubConnectionBuilder()
  .withUrl("/myHub")
  .withTraceContext()
  .build()

In my example, the hub methods' parent activity is a "StartSession" activity created on connection, but I would probably be fine if the parent was set to the client's trace context directly.

Another option is to have another property in HubContext or a magic key in HubContext.Items :magic_wand: (e.g. Context.Items["traceparent"]) that is automatically parsed by the hub. That way, the user could decide how they want to get the trace context to the hub on the initial connection and parse it in OnConnectedAsync. From there, the HubContext could automatically pick up the trace context from that HubContext property or Item.

Additional context

I have found a way to add this, but it feels a little hacky:

In the client:

// client.js
const traceparent = TraceContext.getRootContext().toString() // 00-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx-01
const connection = new HubConnectionBuilder()
  .withUrl(`${baseUrl}/myhub?traceparent=${traceparent}`)
  .build();
connection.on(/* ... */)
connection.start()

On the server:

//MyHub.cs
public class MyHub
{
// ...
    public Task OnConnectedAsync()
    {
        ActivityContext activityCtx = default;
        var httpCtx = Context.GetHttpContext();
        if (httpCtx.Request.Query.TryGetValue("traceparent", out StringValues tpParam) && tpParam.SingleOrDefault() is string traceParent)
        {
            try
            {
                string traceState = (httpCtx.Request.Query.TryGetValue("tracestate", out StringValues tsParam)) ? tsParam.SingleOrDefault() : null;
                if (ActivityContext.TryParse(traceParent, traceState, out activityCtx)) { }
            }
            catch { }
        }
        Activity activity = MyDiagnostics.ActivitySource.StartActivity("MyHub.StartSession", ActivityKind.Client, activityCtx);
        Context.Items["tracecontext"] = activity;
        return Task.CompletedTask;
    }

    public async Task DoSomething()
    {
        // Add this line to the beginning of every hub method.
        using var activity = StartActivity($"{nameof(MyHub)}.{nameof(DoSomething)});
    }

    private Activity GetCurrentConnectionActivity()
    {
        return Context.Items["tracecontext"] as Activity;
    }

    private Activity StartActivity(string operation)
    {
        return MyDiagnostics.ActivitySource.StartActivity(operation, ActivityKind.Client, GetCurrentConnectionActivity().Context);
    }
}

// MyDiagnostics.cs

public static class MyDiagnostics
{
    public static readonly ActivitySource ActivitySource = StartActivitySource();

    private static ActivitySource StartActivitySource()
    {
        var source = new ActivitySource(
            "MyActivitySource",
            FileVersionInfo.GetVersionInfo(Assembly.GetAssembly(typeof(MyDiagnostics))!.Location).FileVersion
        );

        ActivitySource.AddActivityListener(new ActivityListener
        {
            ShouldListenTo = s => true,
            SampleUsingParentId = (ref ActivityCreationOptions<string> activityOptions) => ActivitySamplingResult.AllData,
            Sample = (ref ActivityCreationOptions<ActivityContext> activityOptions) => ActivitySamplingResult.AllData,
        });

        return source;
    }
}

[rafikiassumani-msft]

Triage: related issue: https://github.com/dotnet/aspnetcore/issues/29846

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[davidfowl]

cc @BrennanConroy min bar is that we should document this.