Prevent Span.CheckAndCaptureBaggage from failing - proposals

[fritz-net]

The original code is failing if a baggage already contains this key. This issue occurs for me when masstransit is retrying fail consume attempts of rabbitMq messages

Otel ??= new OTel() { Attributes = new Dictionary<string, object>() };
Otel.Attributes.Add(baggage.Key, baggage.Value);

Example + Stacktrace

An example to reproduce this issue can be found here:
https://github.com/fritz-net/ElasticFailingMasstransit

 T-FAULT rabbitmq://localhost/Dummy a4660000-5d0a-0015-47d4-08dbdd409ed2
      System.ArgumentException: An item with the same key has already been added. Key: messaging.message.conversation_id
         at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
         at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
         at Elastic.Apm.Model.Span.CheckAndCaptureBaggage()
         at Elastic.Apm.Model.Span..ctor(String name, String type, String parentId, String traceId, Transaction enclosingTransaction, IPayloadSender payloadSender, IApmLogger logger, ICurrentExecutionSegmentsContainer currentExecutionSegmentsContainer, IApmServerInfo apmServerInfo, Span parentSpan, InstrumentationFlag instrumentationFlag, Boolean captureStackTraceOnStart, Nullable`1 timestamp, Boolean isExitSpan, String id, IEnumerable`1 links, Activity current)
         at Elastic.Apm.Model.Transaction.StartSpanInternal(String name, String type, String subType, String action, InstrumentationFlag instrumentationFlag, Boolean captureStackTraceOnStart, Nullable`1 timestamp, String id, Boolean isExitSpan, IEnumerable`1 links, Activity current)
         at Elastic.Apm.OpenTelemetry.ElasticActivityListener.CreateSpanForActivity(Activity activity, Int64 timestamp, List`1 spanLinks)
         at Elastic.Apm.OpenTelemetry.ElasticActivityListener.<get_ActivityStarted>b__17_0(Activity activity)
         at System.Diagnostics.SynchronizedList`1.EnumWithAction(Action`2 action, Object arg)
         at System.Diagnostics.Activity.Start()
         ...

Proposal

my proposed solutions would be:

// A: add if not already added, fail if values differ
if (!Otel.Attributes.ContainsKey(baggage.Key))
	Otel.Attributes.Add(baggage.Key, baggage.Value);
else if (Otel.Attributes[baggage.Key].ToString() != baggage.Value)
	throw new Exception($"Baggage key {baggage.Key} already exists with a different value. Current value: '{Otel.Attributes[baggage.Key]}'; New Value: '{baggage.Value}'");

// B: overwrite if already added
Otel.Attributes[baggage.Key] = baggage.Value;

// C: if exists append to existing value
if (Otel.Attributes.ContainsKey(baggage.Key))
	Otel.Attributes[baggage.Key] = Otel.Attributes[baggage.Key] + "," + baggage.Value;
else
	Otel.Attributes.Add(baggage.Key, baggage.Value);

I would opt for C, since it does not loose any information while not failing like A
B has the benefit that it is not changing the format but it looses the old value.

[cla-checker-service]

❌ Author of the following commits did not sign a Contributor Agreement:
caf9e51

Please, read and sign the above mentioned agreement if you want to contribute to this project

[github-actions]

👋 @fritz-net Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

[Mpdreamz]

Thanks so much @fritz-net for bringing this to our attention!

I went with a slightly different fix in #2220 to bring our implementation in line with the other Elastic APM Agents.

[fritz-net]

@Mpdreamz you are welcome. I'm happy it got fixed. Looking forward for the next release to test it out :)