dotnet/runtime

Duplicated activity baggage when calling SetBaggage on parent and current activity

Open

#59.496 aberto em 22 de set. de 2021

Ver no GitHub
 (13 comments) (0 reactions) (0 assignees)C# (5.445 forks)batch import
area-System.Diagnostics.Activitybughelp wanted

Métricas do repositório

Stars
 (17.886 stars)
Métricas de merge de PR
 (Mesclagem média 12d 11h) (661 fundiu PRs em 30d)

Description

I'm using System.Diagnostics.DiagnosticSource with version 6.0.0-rc.1.21451.13 for project that targets net5.0.

If I understand correctly SetBaggage suppose to replace value of the baggage with the same key if it's present. And this is also stated in the docs.

But when I'm trying to call SetBaggage for the key that was present in the baggage of parent activity it added as duplicated item.

For example, following code:

using System;
using System.Diagnostics;

using (var activity = new Activity("A")
	.Start()
	.SetBaggage("Test", "TA"))
{
	using (var subActivity = new Activity("B")
		.Start()
		.SetBaggage("Test", "TB"))
	{
		foreach (var pair in subActivity.Baggage)
		{
			Console.WriteLine("{0}:{1}", pair.Key, pair.Value);
		}
	}
}

Will output:

Test:TB
Test:TA

Probably more common case, is setting baggage in ActivityListener.ActivityStarted handler:

using System;
using System.Diagnostics;

var listener = new ActivityListener();
listener.ActivityStarted += a => { a.SetBaggage("Test", a.OperationName); };
listener.Sample += (ref ActivityCreationOptions<ActivityContext> context) => ActivitySamplingResult.AllData;
listener.SampleUsingParentId += (ref ActivityCreationOptions<string> context) => ActivitySamplingResult.AllData;
listener.ShouldListenTo += source => true;
ActivitySource.AddActivityListener(listener);

var source = new ActivitySource("TestSource");
using (var activity = source.StartActivity("SA"))
{
	using (var subActivity = source.StartActivity("SB"))
	{
		foreach (var pair in subActivity.Baggage)
		{
			Console.WriteLine("{0}:{1}", pair.Key, pair.Value);
		}
	}
}

It will output:

Test:SB
Test:SA

If this behavior is intentional, to avoid overriding parent baggage or due to implementation details, then at least docs should be updated, but I think it would be much better if behavior could be changed to override value with the same key as SetBaggage suppose to work, since current approach could cause a lot of confusions and situation when baggage grow more then it should.

Guia do colaborador