From 29490f236eb5e9ee4db2adfaed6fef2ec4208e9b Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 16 Nov 2023 15:28:02 +0100 Subject: [PATCH] Ensure baggage gets copied with baggage prefix (#2220) * Ensure baggage gets copied with baggage prefix This brings our agent in line with the wider agent specification on how to implement baggage propagation https://github.com/elastic/apm/blob/main/specs/agents/tracing-distributed-tracing.md#baggage-related-configuration * update failing testS --- src/Elastic.Apm/Api/Context.cs | 4 ++-- src/Elastic.Apm/Model/Error.cs | 4 +++- src/Elastic.Apm/Model/Span.cs | 6 ++++-- src/Elastic.Apm/Model/Transaction.cs | 6 ++++-- .../DistributedTracing/BaggageTests.cs | 18 ++++++++++++------ .../BaggageAspNetCoreTests.cs | 14 ++++++++++---- 6 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/Elastic.Apm/Api/Context.cs b/src/Elastic.Apm/Api/Context.cs index 448ac7a97..254d70d6e 100644 --- a/src/Elastic.Apm/Api/Context.cs +++ b/src/Elastic.Apm/Api/Context.cs @@ -12,12 +12,12 @@ namespace Elastic.Apm.Api { public class Context { - private Lazy> _custom = new Lazy>(); + private Lazy> _custom = new(); [JsonConverter(typeof(CustomJsonConverter))] public Dictionary Custom => _custom.Value; - internal Lazy InternalLabels = new Lazy(); + internal Lazy InternalLabels = new(); /// /// diff --git a/src/Elastic.Apm/Model/Error.cs b/src/Elastic.Apm/Model/Error.cs index 1e8a5d6bf..be282f45d 100644 --- a/src/Elastic.Apm/Model/Error.cs +++ b/src/Elastic.Apm/Model/Error.cs @@ -73,7 +73,9 @@ private void CheckAndCaptureBaggage() if (!WildcardMatcher.IsAnyMatch(Configuration.BaggageToAttach, baggage.Key)) continue; - Context.InternalLabels.Value.Add(baggage.Key, baggage.Value); + var newKey = $"baggage.{baggage.Key}"; + var labels = Context.InternalLabels.Value; + labels[newKey] = baggage.Value; } } diff --git a/src/Elastic.Apm/Model/Span.cs b/src/Elastic.Apm/Model/Span.cs index c31fbb203..9aff679cb 100644 --- a/src/Elastic.Apm/Model/Span.cs +++ b/src/Elastic.Apm/Model/Span.cs @@ -138,8 +138,10 @@ private void CheckAndCaptureBaggage() if (!WildcardMatcher.IsAnyMatch(Configuration.BaggageToAttach, baggage.Key)) continue; - Otel ??= new OTel() { Attributes = new Dictionary() }; - Otel.Attributes.Add(baggage.Key, baggage.Value); + Otel ??= new OTel { Attributes = new Dictionary() }; + + var newKey = $"baggage.{baggage.Key}"; + Otel.Attributes[newKey] = baggage.Value; } } diff --git a/src/Elastic.Apm/Model/Transaction.cs b/src/Elastic.Apm/Model/Transaction.cs index c80f962e2..7421091ea 100644 --- a/src/Elastic.Apm/Model/Transaction.cs +++ b/src/Elastic.Apm/Model/Transaction.cs @@ -347,8 +347,10 @@ private void CheckAndCaptureBaggage() if (!WildcardMatcher.IsAnyMatch(Configuration.BaggageToAttach, baggage.Key)) continue; - Otel ??= new OTel() { Attributes = new Dictionary() }; - Otel.Attributes.Add(baggage.Key, baggage.Value); + Otel ??= new OTel { Attributes = new Dictionary() }; + + var newKey = $"baggage.{baggage.Key}"; + Otel.Attributes[newKey] = baggage.Value; } } diff --git a/test/Elastic.Apm.Tests/DistributedTracing/BaggageTests.cs b/test/Elastic.Apm.Tests/DistributedTracing/BaggageTests.cs index 1662fbffa..07654abfe 100644 --- a/test/Elastic.Apm.Tests/DistributedTracing/BaggageTests.cs +++ b/test/Elastic.Apm.Tests/DistributedTracing/BaggageTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; +using Elastic.Apm.Api; using Elastic.Apm.Tests.Utilities; using FluentAssertions; using Xunit; @@ -41,14 +42,15 @@ public void CaptureBaggageWithDefaultConfig() RunSample(agent); payloadSender.FirstTransaction.Should().NotBeNull(); - payloadSender.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("baggage.foo", "bar")); payloadSender.FirstSpan.Should().NotBeNull(); - payloadSender.FirstSpan.Otel.Attributes.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstSpan.Otel.Attributes.Should().Contain(new KeyValuePair("baggage.foo", "bar")); + payloadSender.FirstSpan.Otel.Attributes.Should().NotContain(new KeyValuePair("foo", "bar")); payloadSender.FirstError.Should().NotBeNull(); payloadSender.FirstError.Context.InternalLabels.Should().NotBeNull(); - payloadSender.FirstError.Context.InternalLabels.Value.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstError.Context.InternalLabels.Value.Should().Contain(new KeyValuePair("baggage.foo", "bar")); } /// @@ -66,17 +68,21 @@ public void CaptureBaggageWithNonDefaultConfig() RunSample(agent); payloadSender.FirstTransaction.Should().NotBeNull(); - payloadSender.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("baggage.foo", "bar")); payloadSender.FirstTransaction.Otel.Attributes.Should().NotContainKey("key1"); + payloadSender.FirstTransaction.Otel.Attributes.Should().NotContainKey("baggage.key1"); payloadSender.FirstSpan.Should().NotBeNull(); - payloadSender.FirstSpan.Otel.Attributes.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstSpan.Otel.Attributes.Should().Contain(new KeyValuePair("baggage.foo", "bar")); payloadSender.FirstSpan.Otel.Attributes.Should().NotContainKey("key1"); + payloadSender.FirstSpan.Otel.Attributes.Should().NotContainKey("baggage.key1"); payloadSender.FirstError.Should().NotBeNull(); payloadSender.FirstError.Context.InternalLabels.Should().NotBeNull(); - payloadSender.FirstError.Context.InternalLabels.Value.Should().Contain(new KeyValuePair("foo", "bar")); + payloadSender.FirstError.Context.InternalLabels.Value.Should().Contain(new KeyValuePair("baggage.foo", "bar")); payloadSender.FirstError.Context.InternalLabels.Value.Should().NotContainKey("key1"); + payloadSender.FirstError.Context.InternalLabels.Value.Should().NotContainKey("foo"); + payloadSender.FirstError.Context.InternalLabels.Value.Should().NotContainKey("baggage.key1"); } private void RunSample(ApmAgent agent) diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs index be8fdf4c4..f668e462b 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Net.Http; using System.Threading.Tasks; +using Elastic.Apm.Model; using FluentAssertions; using Xunit; @@ -14,6 +15,10 @@ namespace Elastic.Apm.AspNetCore.Tests; [Collection("DiagnosticListenerTest")] public class BaggageAspNetCoreTests : MultiApplicationTestBase { + + private void ValidateOtelAttribute(Transaction transaction, string key, string value) => + transaction.Otel.Attributes.Should().Contain(new KeyValuePair($"baggage.{key}", value)); + [Fact] public async Task AccessBaggageFromUpstream() { @@ -35,11 +40,12 @@ public async Task AccessBaggageFromUpstream() .Should() .Be("key1=value1, key2 = value2, key3=value3"); - _payloadSender1.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("key1", "value1")); - _payloadSender1.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("key2", "value2")); - _payloadSender1.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("key3", "value3")); + ValidateOtelAttribute(_payloadSender1.FirstTransaction, "key1", "value1"); + ValidateOtelAttribute(_payloadSender1.FirstTransaction, "key2", "value2"); + ValidateOtelAttribute(_payloadSender1.FirstTransaction, "key3", "value3"); } + /// /// Calls the 1. service without any baggage, the /Home/WriteBaggage endpoint in the 1. service adds a baggage and then /// calls the 2. service as a downstream service. @@ -73,6 +79,6 @@ public async Task MultipleServices() .Should() .Be("foo=bar"); - _payloadSender2.FirstTransaction.Otel.Attributes.Should().Contain(new KeyValuePair("foo", "bar")); + ValidateOtelAttribute(_payloadSender2.FirstTransaction, "foo", "bar"); } }