[api] Add Baggage.Attach API

[CodeBlanch]

Fixes #3257
Alternative design #5208

[Note: The "attach" name is taken from Otel Specification > Context > Optional Global operations]

Goals

The goal here is to introduce an API which makes the Baggage ambient context flow behavior explicit. The current APIs are marked as obsolete to encourage users to migrate to the new safe API but nothing has changed and nothing will break.

Changes

• Adds nullable annotations for the Baggage API.
• Adds Baggage.Attach API which can be used to establish a new Baggage context which will flow on the current thread and across tasks until detached/disposed (AsyncLocal behavior).
• Obsolete existing static APIs on Baggage which interact with Baggage.Current.

Public API Changes

public readonly struct Baggage : IEquatable<Baggage>
{
+    public static IDisposable Attach() {}
+    public static IDisposable Attach(Baggage baggage) {}
}

Obsoletions

public readonly struct Baggage : IEquatable<Baggage>
{
    public static Baggage Current
    {
        get {}
+       [Obsolete(BaggageCurrentSetterObsoleteMessage)]
        set {}
    }

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static IReadOnlyDictionary<string, string> GetBaggage(Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static Dictionary<string, string>.Enumerator GetEnumerator(Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static string? GetBaggage(string name, Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static Baggage SetBaggage(string name, string? value, Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static Baggage SetBaggage(IEnumerable<KeyValuePair<string, string?>> baggageItems, Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static Baggage RemoveBaggage(string name, Baggage baggage = default) {}

+   [Obsolete(BaggageStaticMethodObsoleteMessage)]
    public static Baggage ClearBaggage(Baggage baggage = default) {}
}

Usage of Baggage.Attach

Simple case where user doesn't want to pass the current baggage on a specific call

// Create a new downstream empty baggage context
using (Baggage.Attach())
{
    await AuthenticateUserAsync();
}

More complex case where the user wants to fork the current baggage and add something for each call

using var baggageScope = Baggage.Attach(Baggage.Create().SetBaggage("TenantId", "1234"));

List<Task> jobTasks = new();
foreach (var job in jobs)
{
   jobTasks.Add(LaunchJob(job));
}

await Task.WhenAll(jobTasks);

static async Task LaunchJob(Job job)
{
   // Create a new downstream baggage context from the current one with a new key added for the job
   using (Baggage.Attach(Baggage.Current.SetBaggage("JobId", job.Id)))
   {
       // Will see "TenantId" & "JobId" on Baggage
       await RunJob(job);
   }
}

How to migrate to the new API

Setting Baggage for an AspNetCore request via middleware

// Before
public class BaggageMiddleware
{
    private readonly RequestDelegate next;

    public BaggageMiddleware(RequestDelegate next)
    {
        this.next = next;
    }

    public async Task InvokeAsync(HttpContext context)
    {
        await this.ReadTenantIdFromDatabase();

        await this.next(context);
    }

    private async Task ReadTenantIdFromDatabase()
    {
        // Simulate async call to DB
        await Task.Yield();

        Baggage.SetBaggage("tenantId", "1234");
    }
}

// After
public class BaggageMiddleware
{
    private readonly RequestDelegate next;

    public BaggageMiddleware(RequestDelegate next)
    {
        this.next = next;
    }

    public async Task InvokeAsync(HttpContext context)
    {
        var tenantId = await this.ReadTenantIdFromDatabase();

        using (Baggage.Attach(Baggage.Current.SetBaggage("tenantId", tenantId)))
        {
            await this.next(context);
        }
    }

    private async Task<string> ReadTenantIdFromDatabase()
    {
        // Simulate async call to DB
        await Task.Yield();

        return "1234";
    }
}

Prevent Baggage.Current from flowing on a specific downstream call

// Before
private async Task AuthenticateAsync()
{
    var currentBaggageContainer = RuntimeContext.GetValue("otel.baggage");

    RuntimeContext.SetValue("otel.baggage", null);

    try
    {
        await CallAuthenticationServiceAsync();
    }
    finally
    {
        RuntimeContext.SetValue("otel.baggage", currentBaggageContainer);
    }
}

// After
private async Task AuthenticateAsync()
{
    using var baggageScope = Baggage.Attach();

    await CallAuthenticationServiceAsync();
}

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6250307) 83.38% compared to head (8877b5b) 83.20%.
Report is 68 commits behind head on main.

❗ Current head 8877b5b differs from pull request most recent head 0ab6917. Consider uploading reports for the commit 0ab6917 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5218      +/-   ##
==========================================
- Coverage   83.38%   83.20%   -0.19%     
==========================================
  Files         297      271      -26     
  Lines       12531    11983     -548     
==========================================
- Hits        10449     9970     -479     
+ Misses       2082     2013      -69     

Flag	Coverage Δ
unittests	?
unittests-Solution-Experimental	83.13% <100.00%> (?)
unittests-Solution-Stable	83.12% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/OpenTelemetry.Api/Baggage.cs	100.00% <100.00%> (ø)

... and 34 files with indirect coverage changes

[CodeBlanch]

RE: #5208 (comment)

@alanwest's proposed API is this:

void Main() {
  using (var scope1 = ContextBuilder.NewContext().WithBaggage("key1", "key1").StartScope()) {
    // Baggage.Current contains key1/value1
    using (var scope2 = ContextBuilder.NewContext().WithBaggage({"key2", "value2"}).StartScope()) {
      // Baggage.Current only contains key2/value2 here.
      
      // If you also wanted key1/value1 in baggage here, then it would need to be explicit.
      // Something like:
      // var scope2 = ContextBuilder.FromContext(scope1).WithBaggage({"key2", "value2"})
      // The context of scope2 would be derived from scope1 and key2/value2 would be additive
    }
    
    // scope2 has been disposed and scope1 has been restored, and
    // Baggage.Current contains key1/value1
  }  
}

The API being proposed here is similar:

void Main() {
  using (Baggage.Attach(Baggage.Create().SetBaggage("key1", "key1")) {
    // Baggage.Current contains key1/value1
    using (Baggage.Attach(Baggage.Create().SetBaggage("key2", "key2")) {
      // Baggage.Current only contains key2/value2 here.
      
      // If you also wanted key1/value1 in baggage here, then it would need to be explicit.
      // Something like:
      // var scope2 = ContextBuilder.FromContext(scope1).WithBaggage({"key2", "value2"})
      // The context of scope2 would be derived from scope1 and key2/value2 would be additive
    }
    
    // scope2 has been disposed and scope1 has been restored, and
    // Baggage.Current contains key1/value1
  }  
}

But with less new artifacts / smaller public API.

I think it would be great to have a fully spec-compliant "Context" API. However, we couldn't do it strictly in OTel .NET because we don't own SpanContext (ActivityContext). That is part of System.Diagnostics.DiagnosticSource (.NET Runtime).

My thinking is we work with .NET on what that API should be. Once .NET has that API available, we can [Obsolete] public readonly struct Baggage and recommend the .NET API instead.

The changes here are good IMO to make the current OTel Baggage API more safe / spec-complaint in the meantime until that is available.

[reyang]

The spec requires several operations (e.g. Set Value, Remove Value), how to achieve these with the proposal from this PR?

[CodeBlanch]

@reyang

Those APIs are still there on readonly struct Baggage. The things being obsoleted on this PR are the static APIs which mutate the ambient context.

Here are the untouched APIs which I think are pretty spec-compliant at the moment:

public readonly struct Baggage : IEquatable<Baggage>
{
   public int Count { get; }
   public IReadOnlyDictionary<string, string> GetBaggage() {}
   public string? GetBaggage(string name) {}
   public Baggage SetBaggage(string name, string? value) {}
   public Baggage SetBaggage(params KeyValuePair<string, string?>[] baggageItems) {}
   public Baggage SetBaggage(IEnumerable<KeyValuePair<string, string?>> baggageItems) {}
   public Baggage RemoveBaggage(string name) {}
   public Baggage ClearBaggage() {}

   // Helper for creating Baggage instances but new Baggage() or default(Baggage) work equally well
   public static Baggage Create(Dictionary<string, string>? baggageItems = null) {}

   // Ceremony stuff
   public Dictionary<string, string>.Enumerator GetEnumerator() {}
   public bool Equals(Baggage other) {}
   public override bool Equals(object? obj) {}
   public override int GetHashCode() {}
   public static bool operator ==(Baggage left, Baggage right) {}
   public static bool operator !=(Baggage left, Baggage right) {}
}

When it comes to working with a Baggage instance those methods follow the immutability rules of the spec. Meaning they always return something new, the instance invoked is never modified.

var b1 = Baggage.Create().SetBaggage("k1, "v1");
var b2 = b1.SetBaggage("k2", "v2");
var b3 = b2.RemoveBaggage("k2").SetBaggage("k3, "v3");

// b1 has k1
// b2 has k1 & k2
// b3 has k1 & k3

How those work with the new API is you build up some Baggage and then attach it:

var newBaggage = Baggage.Create().SetBaggage("k1", "v1");

using var scope = Baggage.Attach(newBaggage);

// Everything running under scope has Baggage.Current with "k1"

If you want to work off the existing Baggage.Current:

var newBaggage = Baggage.Current.RemoveBaggage("k1").SetBaggage("newThing", "v1");

using var scope = Baggage.Attach(newBaggage);

// Everything running under scope has whatever was in Baggage.Current without "k1" and with "newThing"

[reyang]

How those work with the new API is you build up some Baggage and then attach it:

var newBaggage = Baggage.Create().SetBaggage("k1", "v1");

using var scope = Baggage.Attach(newBaggage);

// Everything running under scope has Baggage.Current with "k1"

If you want to work off the existing Baggage.Current:

var newBaggage = Baggage.Current.RemoveBaggage("k1").SetBaggage("newThing", "v1");

using var scope = Baggage.Attach(newBaggage);

// Everything running under scope has whatever was in Baggage.Current without "k1" and with "newThing"

I still don't understand, for example, how to use this in the following situation:

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs

Line 62 in dfd27c5

public void OnStartActivity(Activity activity, object payload)

[CodeBlanch]

@reyang

I still don't understand, for example, how to use this in the following situation:

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs

Line 62 in dfd27c5

public void OnStartActivity(Activity activity, object payload)

You wouldn't use the Attach API here.

If you are writing outgoing instrumentation you use Baggage.Current to inject whatever the current Baggage is (may be empty):

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs

Lines 108 to 112 in dfd27c5

	var textMapPropagator = Propagators.DefaultTextMapPropagator;
	textMapPropagator.Inject(
	new PropagationContext(activity.Context, Baggage.Current),
	request,
	HttpRequestMessageContextPropagation.HeaderValueSetter);

Users who want to send some Baggage on a Grpc.Net call would use the Attach API:

private async Task CallService()
{
   using var scope = Baggage.Attach(Baggage.Create().SetBaggage("MyKey", "MyValue"));

   await client.SayHelloAsync(
       new HelloRequest { Name = "World" });
}

Incoming instrumentation is more interesting (probably what you meant to link to 😄).

If I'm catching your drift, you are looking for a way to do this that doesn't involve holding onto something which must be disposed.

I just updated the AspNetCore instrumentation on this PR. What I did there is if we detect AsyncLocal in play, we don't dispose anything. We know AspNetCore does the right thing with ExecutionContext. But if we're not doing AsyncLocal, we will manage the lifetime of the scope.

We could possibly make a different API but I'm a bit hesistant to do that because the spec has a MUST for the Detach:

The API MUST accept the following parameters:

A Token that was returned by a previous call to attach a Context.

I took a look at some of the other instrumentation in contrib (Owin, AspNet, WCF, etc.) and it won't take much effort to fix them up becuase they already have container things to track the Activity to clean up when finished.

If you have any ideas though I'm happy to explore them.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[czadokonradnetigate]

Any updates on that one?