Skip to content

Commit 1efa681

Browse files
chore: clean up baggage span tags implementation (#7335)
## Summary of changes - Removed identical AddBaggageTagsToSpan methods - Added to SpanContextPropagator method AddBaggageToSpanAsTags with centralized exception handling and logging - Replaced all method calls with direct calls to the propagator - Optimized loop consolidation for better performance - Original PR: #7020 ## Reason for change ## Implementation details ## Test coverage ## Other details <!-- Fixes #{issue} --> <!-- ⚠️ Note: Where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. MergeQueue is NOT enabled in this repository. If you have write access to the repo, the PR has 1-2 approvals (see above), and all of the required checks have passed, you can use the Squash and Merge button to merge the PR. If you don't have write access, or you need help, reach out in the #apm-dotnet channel in Slack. -->
1 parent 6b2aed9 commit 1efa681

File tree

6 files changed

+22
-81
lines changed

6 files changed

+22
-81
lines changed

tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -125,21 +125,6 @@ internal static void AddHeaderTagsFromHttpResponse(HttpContext httpContext, Scop
125125
}
126126
}
127127

128-
private static void AddBaggageTagsToSpan(ISpan span, Baggage baggage, Tracer tracer)
129-
{
130-
if (baggage != null)
131-
{
132-
try
133-
{
134-
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(span, baggage, tracer.Settings.BaggageTagKeys);
135-
}
136-
catch (Exception ex)
137-
{
138-
Log.Error(ex, "Error adding baggage tags to span.");
139-
}
140-
}
141-
}
142-
143128
private void OnBeginRequest(object sender, EventArgs eventArgs)
144129
{
145130
Scope scope = null;
@@ -213,7 +198,7 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
213198
scope.Span.DecorateWebServerSpan(resourceName: resourceName, httpMethod, host, url, userAgent, tags);
214199
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(scope.Span, headers, tracer.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpRequestHeadersTagPrefix);
215200

216-
AddBaggageTagsToSpan(scope.Span, extractedContext.Baggage, tracer);
201+
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(scope.Span, extractedContext.Baggage, tracer.Settings.BaggageTagKeys);
217202

218203
if (tracer.Settings.IpHeaderEnabled || Security.Instance.AppsecEnabled)
219204
{

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetMvcIntegration.cs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,6 @@ internal static class AspNetMvcIntegration
4040
private const IntegrationId IntegrationId = Configuration.IntegrationId.AspNetMvc;
4141
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(AspNetMvcIntegration));
4242

43-
private static void AddBaggageTagsToSpan(ISpan span, Baggage baggage, Tracer tracer)
44-
{
45-
if (baggage != null)
46-
{
47-
try
48-
{
49-
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(span, baggage, tracer.Settings.BaggageTagKeys);
50-
}
51-
catch (Exception ex)
52-
{
53-
Log.Error(ex, "Error adding baggage tags to span.");
54-
}
55-
}
56-
}
57-
5843
/// <summary>
5944
/// Creates a scope used to instrument an MVC action and populates some common details.
6045
/// </summary>
@@ -198,7 +183,7 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
198183
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(span, headers.Value, tracer.Settings.HeaderTags, SpanContextPropagator.HttpRequestHeadersTagPrefix);
199184
}
200185

201-
AddBaggageTagsToSpan(span, extractedContext.Baggage, tracer);
186+
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(span, extractedContext.Baggage, tracer.Settings.BaggageTagKeys);
202187

203188
if (tracer.Settings.IpHeaderEnabled || Security.Instance.AppsecEnabled)
204189
{

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetWebApi2Integration.cs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,6 @@ internal static class AspNetWebApi2Integration
3333
private const IntegrationId IntegrationId = Configuration.IntegrationId.AspNetWebApi2;
3434
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(AspNetWebApi2Integration));
3535

36-
private static void AddBaggageTagsToSpan(ISpan span, Baggage baggage, Tracer tracer)
37-
{
38-
if (baggage != null)
39-
{
40-
try
41-
{
42-
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(span, baggage, tracer.Settings.BaggageTagKeys);
43-
}
44-
catch (Exception ex)
45-
{
46-
Log.Error(ex, "Error adding baggage tags to span.");
47-
}
48-
}
49-
}
50-
5136
internal static Scope CreateScope(IHttpControllerContext controllerContext, out AspNetTags tags)
5237
{
5338
Scope scope = null;
@@ -119,7 +104,7 @@ internal static Scope CreateScope(IHttpControllerContext controllerContext, out
119104
tracer.TracerManager.SpanContextPropagator.AddHeadersToSpanAsTags(scope.Span, headersCollection.Value, tracer.Settings.HeaderTags, SpanContextPropagator.HttpRequestHeadersTagPrefix, request.Headers.UserAgent.ToString());
120105
}
121106

122-
AddBaggageTagsToSpan(scope.Span, extractedContext.Baggage, tracer);
107+
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(scope.Span, extractedContext.Baggage, tracer.Settings.BaggageTagKeys);
123108

124109
tags.SetAnalyticsSampleRate(IntegrationId, tracer.Settings, enabledWithGlobalSetting: true);
125110
tracer.TracerManager.Telemetry.IntegrationGeneratedSpan(IntegrationId);

tracer/src/Datadog.Trace/Configuration/TracerSettings.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,10 +637,11 @@ _ when x.ToBoolean() is { } boolean => boolean,
637637
.WithKeys(ConfigurationKeys.BaggageMaximumBytes)
638638
.AsInt32(defaultValue: W3CBaggagePropagator.DefaultMaximumBaggageBytes);
639639

640-
BaggageTagKeys = config
640+
BaggageTagKeys = new HashSet<string>(
641+
config
641642
.WithKeys(ConfigurationKeys.BaggageTagKeys)
642643
.AsString(defaultValue: "user.id,session.id,account.id")
643-
?.Split([','], StringSplitOptions.RemoveEmptyEntries) ?? [];
644+
?.Split([','], StringSplitOptions.RemoveEmptyEntries) ?? []);
644645

645646
LogSubmissionSettings = new DirectLogSubmissionSettings(source, _telemetry);
646647

@@ -1156,7 +1157,7 @@ public bool DiagnosticSourceEnabled
11561157
/// Default value is "user.id,session.id,account.id".
11571158
/// </summary>
11581159
/// <seealso cref="ConfigurationKeys.BaggageTagKeys"/>
1159-
internal string[] BaggageTagKeys { get; }
1160+
internal HashSet<string> BaggageTagKeys { get; }
11601161

11611162
/// <summary>
11621163
/// Gets a value indicating whether runtime metrics

tracer/src/Datadog.Trace/PlatformHelpers/AspNetCoreHttpRequestHandler.cs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,21 +98,6 @@ private void AddHeaderTagsToSpan(ISpan span, HttpRequest request, Tracer tracer)
9898
}
9999
}
100100

101-
private void AddBaggageTagsToSpan(ISpan span, Baggage baggage, Tracer tracer)
102-
{
103-
if (baggage != null)
104-
{
105-
try
106-
{
107-
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(span, baggage, tracer.Settings.BaggageTagKeys);
108-
}
109-
catch (Exception ex)
110-
{
111-
_log.Error(ex, "Error adding baggage tags to span.");
112-
}
113-
}
114-
}
115-
116101
public Scope StartAspNetCorePipelineScope(Tracer tracer, Security security, HttpContext httpContext, string resourceName)
117102
{
118103
var request = httpContext.Request;
@@ -140,7 +125,7 @@ public Scope StartAspNetCorePipelineScope(Tracer tracer, Security security, Http
140125
var scope = tracer.StartActiveInternal(_requestInOperationName, extractedContext.SpanContext, tags: tags, links: extractedContext.Links);
141126
scope.Span.DecorateWebServerSpan(resourceName, httpMethod, host, url, userAgent, tags);
142127
AddHeaderTagsToSpan(scope.Span, request, tracer);
143-
AddBaggageTagsToSpan(scope.Span, extractedContext.Baggage, tracer);
128+
tracer.TracerManager.SpanContextPropagator.AddBaggageToSpanAsTags(scope.Span, extractedContext.Baggage, tracer.Settings.BaggageTagKeys);
144129

145130
var originalPath = request.PathBase.HasValue ? request.PathBase.Add(request.Path) : request.Path;
146131
var requestTrackingFeature = new RequestTrackingFeature(originalPath, scope);

tracer/src/Datadog.Trace/Propagators/SpanContextPropagator.cs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Collections.Generic;
1111
using System.Linq;
1212
using Datadog.Trace.Headers;
13+
using Datadog.Trace.Logging;
1314
using Datadog.Trace.Tagging;
1415
using Datadog.Trace.Util;
1516

@@ -20,6 +21,8 @@ internal class SpanContextPropagator
2021
internal const string HttpRequestHeadersTagPrefix = "http.request.headers";
2122
internal const string HttpResponseHeadersTagPrefix = "http.response.headers";
2223

24+
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<SpanContextPropagator>();
25+
2326
private readonly ConcurrentDictionary<Key, string?> _defaultTagMappingCache = new();
2427
private readonly IContextInjector[] _injectors;
2528
private readonly IContextExtractor[] _extractors;
@@ -310,37 +313,34 @@ internal void ExtractHeaderTags<THeaders, TProcessor>(ref TProcessor processor,
310313
}
311314
}
312315

313-
internal void AddBaggageToSpanAsTags(ISpan span, Baggage? baggage, string[] baggageTagKeys)
316+
internal void AddBaggageToSpanAsTags(ISpan span, Baggage? baggage, HashSet<string> baggageTagKeys)
314317
{
315318
if (baggage is null or { Count: 0 })
316319
{
317320
return;
318321
}
319322

320-
if (baggageTagKeys.Length == 0)
323+
if (baggageTagKeys.Count == 0)
321324
{
322325
// feature disabled
323326
return;
324327
}
325328

326-
if (baggageTagKeys.Length == 1 && baggageTagKeys[0] == "*")
329+
try
327330
{
328-
// add all baggage items as tags
331+
var addAllItems = baggageTagKeys.Count == 1 && baggageTagKeys.Contains("*");
332+
329333
foreach (var item in baggage)
330334
{
331-
span.SetTag("baggage." + item.Key, item.Value);
335+
if (addAllItems || baggageTagKeys.Contains(item.Key))
336+
{
337+
span.SetTag("baggage." + item.Key, item.Value);
338+
}
332339
}
333-
334-
return;
335340
}
336-
337-
// add only specified baggage items as tags
338-
foreach (var key in baggageTagKeys)
341+
catch (Exception ex)
339342
{
340-
if (baggage.TryGetValue(key, out var value))
341-
{
342-
span.SetTag("baggage." + key, value);
343-
}
343+
Log.Error(ex, "Error adding baggage tags to span.");
344344
}
345345
}
346346

0 commit comments

Comments
 (0)