From 5c3218ab1e2304e69a371b4b2f141e09a0ca2495 Mon Sep 17 00:00:00 2001 From: Maximo Bautista Date: Mon, 10 Jun 2024 18:11:44 -0400 Subject: [PATCH] [Tracer] SpanLinks Permissive null Clean Up (#5674) * Addressing null permissions * Addressing possible null sent to ParseTraceState --- .../src/Datadog.Trace/Activity/OtlpHelpers.cs | 113 +++++++++--------- .../Propagators/W3CTraceContextPropagator.cs | 4 +- 2 files changed, 60 insertions(+), 57 deletions(-) diff --git a/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs b/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs index 2df451c26e06..0eb79387436e 100644 --- a/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs +++ b/tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs @@ -218,75 +218,78 @@ private static void ExtractActivityLinks(Span span, IActivity5? activity foreach (var link in (activity5.Links)) { - if (link.TryDuckCast(out var duckLink)) + if (!link.TryDuckCast(out var duckLink) + || duckLink.Context.TraceId.TraceId is null + || duckLink.Context.SpanId.SpanId is null) { - if (duckLink.Context.TraceId.TraceId is null || duckLink.Context.SpanId.SpanId is null) - { - continue; - } + continue; + } - _ = HexString.TryParseTraceId(duckLink.Context.TraceId.TraceId, out var newActivityTraceId); - _ = HexString.TryParseUInt64(duckLink.Context.SpanId.SpanId, out var newActivitySpanId); - var traceParentSample = duckLink.Context.TraceFlags > 0; - var traceState = W3CTraceContextPropagator.ParseTraceState(duckLink.Context.TraceState ?? string.Empty); + var parsedTraceId = HexString.TryParseTraceId(duckLink.Context.TraceId.TraceId, out var newActivityTraceId); + var parsedSpanId = HexString.TryParseUInt64(duckLink.Context.SpanId.SpanId, out var newActivitySpanId); - var samplingPriority = traceParentSample switch - { - true when traceState.SamplingPriority is > 0 => traceState.SamplingPriority.Value, - true => SamplingPriorityValues.AutoKeep, - false when traceState.SamplingPriority is <= 0 => traceState.SamplingPriority.Value, - false => SamplingPriorityValues.AutoReject, - }; + if (!parsedTraceId || !parsedSpanId) + { + continue; + } - var spanContext = new SpanContext( - newActivityTraceId, - newActivitySpanId, - samplingPriority: samplingPriority, - serviceName: null, - origin: traceState.Origin, - isRemote: duckLink.Context.IsRemote); + var traceParentSample = duckLink.Context.TraceFlags > 0; + var traceState = W3CTraceContextPropagator.ParseTraceState(duckLink.Context.TraceState); + var traceTags = TagPropagation.ParseHeader(traceState.PropagatedTags); + var samplingPriority = traceParentSample switch + { + true when traceState.SamplingPriority != null && SamplingPriorityValues.IsKeep(traceState.SamplingPriority.Value) => traceState.SamplingPriority.Value, + true => SamplingPriorityValues.AutoKeep, + false when traceState.SamplingPriority != null && SamplingPriorityValues.IsDrop(traceState.SamplingPriority.Value) => traceState.SamplingPriority.Value, + false => SamplingPriorityValues.AutoReject + }; - var traceTags = TagPropagation.ParseHeader(traceState.PropagatedTags); + if (traceParentSample && SamplingPriorityValues.IsDrop(samplingPriority)) + { + traceTags.SetTag(Tags.Propagated.DecisionMaker, "-0"); + } + else if (!traceParentSample && SamplingPriorityValues.IsKeep(samplingPriority)) + { + traceTags.RemoveTag(Tags.Propagated.DecisionMaker); + } - if (traceParentSample && traceState.SamplingPriority <= 0) - { - traceTags.SetTag(Tags.Propagated.DecisionMaker, "-0"); - } - else if (!traceParentSample && traceState.SamplingPriority > 0) - { - traceTags.RemoveTag(Tags.Propagated.DecisionMaker); - } + var spanContext = new SpanContext( + newActivityTraceId, + newActivitySpanId, + samplingPriority: samplingPriority, + serviceName: null, + origin: traceState.Origin, + isRemote: duckLink.Context.IsRemote); - spanContext.AdditionalW3CTraceState = traceState.AdditionalValues; - spanContext.LastParentId = traceState.LastParent; - spanContext.PropagatedTags = traceTags; + spanContext.AdditionalW3CTraceState = traceState.AdditionalValues; + spanContext.LastParentId = traceState.LastParent; + spanContext.PropagatedTags = traceTags; - var extractedSpan = new Span(spanContext, DateTimeOffset.Now, new CommonTags()); - var spanLink = span.AddSpanLink(extractedSpan); + var extractedSpan = new Span(spanContext, DateTimeOffset.Now, new CommonTags()); + var spanLink = span.AddSpanLink(extractedSpan); - if (duckLink.Tags is not null) + if (duckLink.Tags is not null) + { + foreach (var kvp in duckLink.Tags) { - foreach (var kvp in duckLink.Tags) + if (!string.IsNullOrEmpty(kvp.Key) + && IsAllowedAtributeType(kvp.Value)) { - if (!string.IsNullOrEmpty(kvp.Key) - && IsAllowedAtributeType(kvp.Value)) + if (kvp.Value is Array array) { - if (kvp.Value is Array array) + int index = 0; + foreach (var item in array) { - int index = 0; - foreach (var item in array) + if (item?.ToString() is { } value) { - if (item is not null) - { - spanLink.AddAttribute($"{kvp.Key}.{index}", item.ToString()!); - index++; - } + spanLink.AddAttribute($"{kvp.Key}.{index}", value); + index++; } } - else - { - spanLink.AddAttribute(kvp.Key, kvp.Value!.ToString()!); - } + } + else if (kvp.Value?.ToString() is { } kvpValue) + { + spanLink.AddAttribute(kvp.Key, kvpValue); } } } @@ -376,7 +379,7 @@ internal static void SetTagObject(Span span, string key, object? value, bool all if (index == 0) { // indicates that it was an empty array, we need to add the tag - AgentSetOtlpTag(span, key, JsonConvert.SerializeObject(value)); + AgentSetOtlpTag(span, key, "[]"); } } else @@ -641,7 +644,7 @@ private static bool IsAllowedAtributeType(object? value) return false; } - value = array.GetValue(0)!; + value = array.GetValue(0); if (value is null) { diff --git a/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs b/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs index 59628f892b2b..abb49a5ffec4 100644 --- a/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs +++ b/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs @@ -322,7 +322,7 @@ internal static bool TryParseTraceParent(string header, out W3CTraceParent trace return true; } - internal static W3CTraceState ParseTraceState(string header) + internal static W3CTraceState ParseTraceState(string? header) { // header format: "[*,]dd=s:1;o:rum;t.dm:-4;t.usr.id:12345[,*]" if (string.IsNullOrWhiteSpace(header)) @@ -330,7 +330,7 @@ internal static W3CTraceState ParseTraceState(string header) return new W3CTraceState(samplingPriority: null, origin: null, lastParent: ZeroLastParent, propagatedTags: null, additionalValues: null); } - SplitTraceStateValues(header, out var ddValues, out var additionalValues); + SplitTraceStateValues(header!, out var ddValues, out var additionalValues); if (ddValues is null or { Length: < 6 }) {