Skip to content

Commit 6d09fb3

Browse files
andrewlockandrewlock
committed
Fixes for activity handler
- Split ParentSpanId access - Refactor AcitivityHandlerCommon to explicitly split W3C vs hierarchical IDs - Add handling for < .NET Core 3 activities
1 parent 4bcb8ec commit 6d09fb3

File tree

3 files changed

+60
-36
lines changed

3 files changed

+60
-36
lines changed

‎tracer/src/Datadog.Trace/Activity/DuckTypes/IW3CActivity.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,14 @@ internal interface IW3CActivity : IActivity
2020
[DuckField(Name = "_spanId")]
2121
string? SpanId { get; set; }
2222

23+
/// <summary>
24+
/// Gets the ParentSpanId or creates it if Parent is non-null.
25+
/// Only applies to W3C IDs, always null for hierarchical IDs
26+
/// </summary>
27+
string? ParentSpanId { get; }
28+
2329
[DuckField(Name = "_parentSpanId")]
24-
string? ParentSpanId { get; set; }
30+
string? RawParentSpanId { get; set; }
2531

2632
[DuckField(Name = "_id")]
2733
string? RawId { get; set; }

‎tracer/src/Datadog.Trace/Activity/Handlers/ActivityHandlerCommon.cs‎

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -57,45 +57,54 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
5757
var activityTraceId = w3cActivity.TraceId;
5858
var activitySpanId = w3cActivity.SpanId;
5959

60-
// If the user has specified a parent context, get the parent Datadog SpanContext
61-
// We avoid calling ParentId until we know we need it, as that will perform an expensive allocation
62-
// Instead, we use ParentSpanId (which looks at the private string property) and Parent (which is a linked object).
63-
// If either of these are non-null, then we have a parent
64-
if (!StringUtil.IsNullOrEmpty(activityTraceId) && w3cActivity is { ParentSpanId: { } parentSpanId })
60+
if (!StringUtil.IsNullOrEmpty(activityTraceId))
6561
{
66-
// We know that we have a parent context, but we use TraceId+ParentSpanId for the mapping.
67-
// This is a result of an issue with OTel v1.0.1 (unsure if OTel or us tbh) where the
68-
// ".ParentId" matched for the Trace+Span IDs but not for the flags portion.
69-
// Doing a lookup on just the TraceId+ParentSpanId seems to be more resilient.
70-
if (ActivityMappingById.TryGetValue(new ActivityKey(activityTraceId, parentSpanId), out ActivityMapping mapping))
62+
// W3C ID
63+
if (w3cActivity is { ParentSpanId: { } parentSpanId })
7164
{
72-
parent = mapping.Scope.Span.Context;
73-
}
74-
else
75-
{
76-
// create a new parent span context for the ActivityContext
77-
_ = HexString.TryParseTraceId(activityTraceId, out var newActivityTraceId);
78-
_ = HexString.TryParseUInt64(parentSpanId, out var newActivitySpanId);
79-
80-
parent = Tracer.Instance.CreateSpanContext(
81-
SpanContext.None,
82-
traceId: newActivityTraceId,
83-
spanId: newActivitySpanId,
84-
rawTraceId: activityTraceId,
85-
rawSpanId: parentSpanId);
65+
// This will be true for activities using W3C IDs which have a parent span.
66+
// The ParentSpanId will be created from the Parent if that is set (or parsed out of _parentId if necessary)
67+
//
68+
// We know that we have a parent context, but we use TraceId+ParentSpanId for the mapping.
69+
// This is a result of an issue with OTel v1.0.1 (unsure if OTel or us tbh) where the
70+
// ".ParentId" matched for the Trace+Span IDs but not for the flags portion.
71+
// Doing a lookup on just the TraceId+ParentSpanId seems to be more resilient.
72+
if (ActivityMappingById.TryGetValue(new ActivityKey(activityTraceId, parentSpanId), out ActivityMapping mapping))
73+
{
74+
parent = mapping.Scope.Span.Context;
75+
}
76+
else
77+
{
78+
// create a new parent span context for the ActivityContext
79+
_ = HexString.TryParseTraceId(activityTraceId, out var newActivityTraceId);
80+
_ = HexString.TryParseUInt64(parentSpanId, out var newActivitySpanId);
81+
82+
parent = Tracer.Instance.CreateSpanContext(
83+
SpanContext.None,
84+
traceId: newActivityTraceId,
85+
spanId: newActivitySpanId,
86+
rawTraceId: activityTraceId,
87+
rawSpanId: parentSpanId);
88+
}
8689
}
8790
}
88-
else if ((string.IsNullOrEmpty(activityTraceId) && w3cActivity is { ParentSpanId: not null })
89-
|| w3cActivity is { Parent: not null })
91+
else
9092
{
91-
// We know we have a parent context, but also that the traceID is (weirdly) null, so we fallback
92-
// to calling ParentId (which allocates) instead.
93-
var parentId = w3cActivity.ParentId;
94-
95-
// we have a ParentSpanId/ParentId, but no TraceId/SpanId, so default to use the ParentId for lookup
96-
if (!StringUtil.IsNullOrEmpty(parentId) && ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
93+
// No traceID, so much be Hierarchical ID
94+
if (w3cActivity.ParentSpanId is { } parentSpanId)
9795
{
98-
parent = mapping.Scope.Span.Context;
96+
// This is a weird scenario - we're in a hierarchical ID, we don't have a trace ID, but we _do_ have a _parentSpanID?!
97+
// should never hit this path unless we've gone wrong somewhere
98+
Log.Error("Activity with ID {ActivityId} had parent span ID {ParentSpanId} but TraceID was missing", activity.Id, parentSpanId);
99+
}
100+
else
101+
{
102+
// Since _parentSpanID is null, this either grabs _parentId, or Parent.Id, depending on what was set
103+
var parentId = w3cActivity.ParentId;
104+
if (!StringUtil.IsNullOrEmpty(parentId) && ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
105+
{
106+
parent = mapping.Scope.Span.Context;
107+
}
99108
}
100109
}
101110

@@ -112,7 +121,7 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
112121
activityTraceId = w3cActivity.TraceId;
113122

114123
// SpanId (always 16 chars long)
115-
w3cActivity.ParentSpanId = activeSpan.Context.RawSpanId;
124+
w3cActivity.RawParentSpanId = activeSpan.Context.RawSpanId;
116125

117126
// We clear internals Id and ParentId values to force recalculation.
118127
w3cActivity.RawId = null;
@@ -138,6 +147,15 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
138147
activityKey = new(traceId: activityTraceId, spanId: activitySpanId);
139148
}
140149
}
150+
else
151+
{
152+
// non-IW3CActivity, i.e. we're in .NET Core 2.x territory. Only have hierarchical IDs to worry about here
153+
var parentId = activity.ParentId;
154+
if (!StringUtil.IsNullOrEmpty(parentId) && ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
155+
{
156+
parent = mapping.Scope.Span.Context;
157+
}
158+
}
141159

142160
try
143161
{

‎tracer/src/Datadog.Trace/Activity/Handlers/IgnoreActivityHandler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static void IgnoreActivity<T>(T activity, Span? span)
6060
w3cActivity.TraceId = span.Context.RawTraceId;
6161

6262
// SpanId (always 16 chars long)
63-
w3cActivity.ParentSpanId = span.Context.RawSpanId;
63+
w3cActivity.RawParentSpanId = span.Context.RawSpanId;
6464

6565
// We clear internals Id and ParentId values to force recalculation.
6666
w3cActivity.RawId = null;

0 commit comments

Comments
 (0)