open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.22k stars 764 forks source link

How to properly propagate logging baggage to subsequent service invocations and Task.Run async invocations? #5660

Closed akakarikos closed 5 months ago

akakarikos commented 5 months ago

What is the question?

I have a scenario of a service invocation chain like this: Service A -> Service B. Also inside the Service B there is a new task started using Task.Run.

What I have done so far, is create a .NET middleware that is being used by Service A, where I am creating the logging baggage info and attaching it to the current activity like this:

Activity.Current.SetBaggage();
Activity.Current.SetTag();

All is good until inside the Service B there is an async operation started using Task.Run. It seems that the context/baggage is being lost. Also the Activity.Current is null inside the Task.Run method handler.

Is the practice above valid or should I follow any other way? Also, how could I propagate the Activity Baggage to the async Task.Run scope?

Thank you in advance and I really appreciate your feedback.

Additional context

No response

CodeBlanch commented 5 months ago

Don't use the baggage APIs on Activity. Those aren't compatible with the OTel spec.

What you want to do is in the middleware of Service A set Baggage using the OTel API:

Baggage.SetBaggage("myBaggageKey", "myBaggageValue");

Assuming you haven't changed the default propagator the SDK is using, that should be sent to Service B automatically.

Inside Service B you should be able to access it:

string? value = Baggage.GetBaggage("myBaggageKey");
akakarikos commented 5 months ago

Thank you @CodeBlanch, I will change it as you suggest.

What about passing the Baggage inside an async invocation running on a new thread using Task.Run inside Service B? Will Baggage be propagated or do I have to handle it in a different way?

CodeBlanch commented 5 months ago

What about passing the Baggage inside an async invocation running on a new thread using Task.Run inside Service B? Will Baggage be propagated or do I have to handle it in a different way?

It should work automatically. When you start a task runtime will flow the execution context automatically which is where baggage is stored. If you wanted it NOT to flow, you would need to do something special like use ExecutionContext.SuppressFlow.

akakarikos commented 5 months ago

Unfortunately, the propagation of the baggage to Service B didn't work for me. Just to let you know that I'm using Dapr for the service invocation and I haven't changed the default propagator.

This is how I'm initializing the OTel:

loggingBuilder
        .SetResourceBuilder(
            ResourceBuilder.CreateEmpty()
                .AddService(
                    serviceName,
                    serviceNamespace,
                    Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "unknown")
                .AddTelemetrySdk()
        );
        loggingBuilder.IncludeScopes = true;
        loggingBuilder.ParseStateValues = true;
        loggingBuilder.IncludeFormattedMessage = true;

        loggingBuilder.AddOtlpExporter((opt, opt2) =>
        {
            opt.Protocol = OtlpExportProtocol.Grpc;
            opt.Endpoint = new Uri(otlpCollectorEndpoint);
        });

Also in the middleware I'm doing the following:

public async Task InvokeAsync(HttpContext httpContext)
        {
            var baggage = new List<KeyValuePair<string, string>>()
            {
                    new KeyValuePair<string, string>(TraceAttributes.THREAD_ID, Thread.CurrentThread.ManagedThreadId.ToString()),
            };

            Baggage.SetBaggage(baggage );

            await _next(httpContext);
        }

Is there anything I have to change so I get this working?

CodeBlanch commented 5 months ago

I just poked around the Dapr/dotnet-sdk repo. It doesn't appear to be using the OpenTelemetry .NET SDK.

Two options for you to try 😄

In Service A and Service B add this code in your startup:

appBuilder.Services.AddOpenTelemetry()
    .WithTracing(tracing => tracing
        .AddAspNetCoreInstrumentation()
        .AddHttpClientInstrumentation()); 

That will add AspNetCore and HttpClient instrumentation for incoming and outgoing http calls. That should handle the propagation of baggage for you.

The other option is: https://learn.microsoft.com/dotnet/api/system.diagnostics.distributedcontextpropagator.current?view=net-8.0#system-diagnostics-distributedcontextpropagator-current

The built-in propagation in runtime uses that. You could register your own propagator to inject baggage in Service A and then extract it in Service B.

akakarikos commented 5 months ago

Many thanks for the answer. I'm sorry I missed that part in the code. I'm initializing the tracing as you mentioned and the logging as well. So I have this:

services.AddOpenTelemetry().WithTracing(tracingBuilder) (with AspNetCore and HttpClient instrumentations enabled) services.AddOpenTelemetry(loggingBuilder)

Still, the logs don't contain the Baggage.

I don't see why this is not working with the default propagator. I will try with a custom propagator and see if that works.

CodeBlanch commented 5 months ago

Interesting. Is there anyway you can set up a couple slim projects which represent Service A and Service B I can use to try and debug?

Still, the logs don't contain the Baggage.

Can you clarify this for me. We don't automatically add baggage to logs. If you set a breakpoint in Service B and inspect Baggage.Current does it have your value? If yes I can help you getting that onto your logs if you want.

akakarikos commented 5 months ago

Thank you @CodeBlanch. So, after your guidance, I've reached a point where I successfully propagated the Baggage from Service A to Service B and attached it to the logs using an Activity Listener as shown below:

var listener = new ActivityListener
{
    ShouldListenTo = _ => true,
    ActivityStopped = activity =>
    {
        foreach (var (key, value) in Baggage.Current)
        {
            activity.SetTag(key, value);
        }
    }
};

ActivitySource.AddActivityListener(listener);

I hope that's the correct way of doing so. If not, please let me know.

Now, I have another issue, propagating the Baggage to a Service C invoked from Service B, through the Dapr pub/sub building block (Service B publishes a message to Service C which listens for it). Dapr is wrapping the messages to cloud events as depicted here. I'm wondering, in order to propagate the Baggage, If I need to use the Propagators API to inject and extract it. Is this proper way of doing so?

CodeBlanch commented 5 months ago

Adding information to telemetry is what we would call enrichment. The OTel way to do that is use a processor. There's an extension you could use: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.Extensions#baggageactivityprocessor. Here is how it is implemented using a processor: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs. If you want to stick with ActivityListener though no harm in doing that.

For propagation through message queues yes you'll most likely have to do things yourself using the propagation API. @alanwest made a demo app a while back demonstrating this you can check out here: https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/examples/MicroserviceExample/Utils/Messaging.

akakarikos commented 5 months ago

Thank you for your help @CodeBlanch , I got this working. Much appreciated.

cijothomas commented 5 months ago

@akakarikos Please use processors as @CodeBlanch suggested, instead of own ActivityListeners. With Processors, OTel ensures they are not invoked unless sampling in favorable to avoid unwanted costs.

akakarikos commented 5 months ago

@cijothomas thanks.

Yes, that is what I've done. A middleware to add the baggage, and a processor to attach it as tags to the activities. Also I've used the Propagator for the pub/sub invocations.