Skip to content

Various optimizations for Activity handling #8040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrewlock merged 10 commits into from
Jan 12, 2026
Merged

Various optimizations for Activity handling #8040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
797255e
Don't allocate for ParentId if you don't need to
andrewlock Jan 5, 2026
c0f469f
Don't call Tracer.Instance.ActiveScope if we don't need to
andrewlock Jan 8, 2026
824fd8c
Extract StopActivitySlow as it's a rarely-called code path
andrewlock Jan 8, 2026
764f8c0
Simplify ShouldIGnoreBeOperationName because it's ~15 times faster
andrewlock Jan 6, 2026
1fd31df
Fixes for activity handler
andrewlock Jan 9, 2026
f517c5f
Rename w3cActivity to activity3
andrewlock Jan 9, 2026
3968330
Fix docs
andrewlock Jan 11, 2026
a022e64
Fix duck typing for ParentSpanId
andrewlock Jan 12, 2026
a66aef3
Fix ActivityTests to dispose the tracer
andrewlock Jan 12, 2026
982d0dc
Remove ParentSpanId again, as we actually _don't_ want to cause that …
andrewlock Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tracer/src/Datadog.Trace/Activity/DuckTypes/IActivity.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ internal interface IActivity : IDuckType
/// unless you know that you're not using W3C activities (because <see cref="IW3CActivity.SpanId"/> is null</remarks>
string Id { get; }

/// <summary>
/// Gets the ParentId, as a string calculated from <see cref="Parent"/> or, if an <see cref="IW3CActivity"/>,
/// from the <see cref="IW3CActivity.RawParentSpanId"/>.
/// </summary>
/// <remarks>NOTE: this property allocates, and so should generally not be called unless the specific returned value is required</remarks>
string? ParentId { get; }

// The corresponding property on the Activity object is nullable,
Expand Down
2 changes: 1 addition & 1 deletion tracer/src/Datadog.Trace/Activity/DuckTypes/IW3CActivity.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal interface IW3CActivity : IActivity
string? SpanId { get; set; }

[DuckField(Name = "_parentSpanId")]
string ParentSpanId { get; set; }
string? RawParentSpanId { get; set; }

[DuckField(Name = "_id")]
string? RawId { get; set; }
Expand Down
110 changes: 68 additions & 42 deletions tracer/src/Datadog.Trace/Activity/Handlers/ActivityHandlerCommon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
where T : IActivity
{
Tracer.Instance.TracerManager.Telemetry.IntegrationRunning(IntegrationId);
var activeSpan = Tracer.Instance.ActiveScope?.Span as Span;

// Propagate Trace and Parent Span ids
SpanContext? parent = null;
Expand All @@ -48,25 +47,28 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
string? rawTraceId = null;
string? rawSpanId = null;

// for non-IW3CActivity interfaces we'll use Activity.Id and string.Empty as the key as they don't have a guaranteed TraceId+SpanId
// for IW3CActivity interfaces we'll use the Activity.TraceId + Activity.SpanId as the key
// have to also validate that the TraceId and SpanId actually exist and aren't null - as they can be in some cases
// for non-W3C activities using Hierarchical IDs (both IW3CActivity and IActivity) we use Activity.Id and string.Empty
// for W3C ID activities (always IW3CActivity) we'll use the Activity.TraceId + Activity.SpanId as the key
ActivityKey? activityKey = null;

if (activity is IW3CActivity w3cActivity)
if (activity is IW3CActivity activity3)
Copy link
Member

@lucaspimentel lucaspimentel Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level of care low, but I actually prefer w3cActivity here because activity3 could be taken to mean it's an IActivity3 reference (even though IActivity3 doesn't exist, but IActivity5 and IActivity6 do).

Copy link
Member Author

@andrewlock andrewlock Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Level of care low, but I actually prefer w3cActivity here because activity3 could be taken to mean it's an IActivity3 reference

I have a pretty strong opinion on not using w3cActivity, because it comes with implicit assumptions (I know, because the code made them, as did I 😅)

even though IActivity3 doesn't exist, but IActivity5 and IActivity6 do)

Yeah I'm going to rename IW3CActitvity to IActivity3 later, when it's less conflicting 😉

{
var activityTraceId = w3cActivity.TraceId;
var activitySpanId = w3cActivity.SpanId;
var activityTraceId = activity3.TraceId;
var activitySpanId = activity3.SpanId;

// If the user has specified a parent context, get the parent Datadog SpanContext
if (w3cActivity is { ParentSpanId: { } parentSpanId, ParentId: { } parentId })
if (!StringUtil.IsNullOrEmpty(activityTraceId))
{
// We know that we have a parent context, but we use TraceId+ParentSpanId for the mapping.
// This is a result of an issue with OTel v1.0.1 (unsure if OTel or us tbh) where the
// ".ParentId" matched for the Trace+Span IDs but not for the flags portion.
// Doing a lookup on just the TraceId+ParentSpanId seems to be more resilient.
if (activityTraceId != null!)
// W3C ID
if (activity3 is { RawParentSpanId: { } parentSpanId })
{
// This will be true for activities using W3C IDs which have a "remote" parent span ID
// We explicitly don't check the case where we _do_ have a Parent object (i.e. in-process activity)
// as in that scenario we may need to remap the parent instead (see below).
//
// We know that we have a parent context, but we use TraceId+ParentSpanId for the mapping.
// This is a result of an issue with OTel v1.0.1 (unsure if OTel or us tbh) where the
// ".ParentId" matched for the Trace+Span IDs but not for the flags portion.
// Doing a lookup on just the TraceId+ParentSpanId seems to be more resilient.
if (ActivityMappingById.TryGetValue(new ActivityKey(activityTraceId, parentSpanId), out ActivityMapping mapping))
{
parent = mapping.Scope.Span.Context;
Expand All @@ -85,43 +87,55 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
rawSpanId: parentSpanId);
}
}
}
else
{
// No traceID, so must be Hierarchical ID
if (activity3 is { RawParentSpanId: { } parentSpanId })
{
// This is a weird scenario - we're in a hierarchical ID, we don't have a trace ID, but we _do_ have a _parentSpanID?!
Copy link
Member

@lucaspimentel lucaspimentel Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about activities with hierarchical IDs? AFAIK those are not compatible with OpenTelemetry. Could we just ignore them? (general question, not specific to this PR)

except special cases like the aspnetcore observer, if we still use that without DD_TRACE_OTEL_ENABLED.

Copy link
Member

@lucaspimentel lucaspimentel Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, I see the comment below

non-IW3CActivity, i.e. we're in .NET Core 2.x territory. Only have hierarchical IDs to worry about here

Copy link
Member Author

@andrewlock andrewlock Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just ignore them? (general question, not specific to this PR)

Great question, maybe we could 🤔 Big breaking change though...

// should never hit this path unless we've gone wrong somewhere
Log.Error("Activity with ID {ActivityId} had parent span ID {ParentSpanId} but TraceID was missing", activity.Id, parentSpanId);
}
else
{
// we have a ParentSpanId/ParentId, but no TraceId/SpanId, so default to use the ParentId for lookup
if (ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
// Since _parentSpanID is null, this either grabs _parentId, or Parent.Id, depending on what was set
var parentId = activity3.ParentId;
if (!StringUtil.IsNullOrEmpty(parentId) && ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
{
parent = mapping.Scope.Span.Context;
}
}
}

if (parent is null && activeSpan is not null)
// If we don't have a remote context, then we may need to remap the current activity to
// reparent it with a datadog span
if (parent is null
&& activitySpanId is not null
&& activityTraceId is not null
&& Tracer.Instance.ActiveScope?.Span is Span activeSpan
&& (activity.Parent is null || activity.Parent.StartTimeUtc <= activeSpan.StartTime.UtcDateTime))
{
// We ensure the activity follows the same TraceId as the span
// And marks the ParentId the current spanId
if ((activity.Parent is null || activity.Parent.StartTimeUtc <= activeSpan.StartTime.UtcDateTime)
&& activitySpanId is not null
&& activityTraceId is not null)
{
// TraceId (always 32 chars long even when using 64-bit ids)
w3cActivity.TraceId = activeSpan.Context.RawTraceId;
activityTraceId = w3cActivity.TraceId;
// TraceId (always 32 chars long even when using 64-bit ids)
activity3.TraceId = activeSpan.Context.RawTraceId;
activityTraceId = activity3.TraceId;

// SpanId (always 16 chars long)
w3cActivity.ParentSpanId = activeSpan.Context.RawSpanId;
// SpanId (always 16 chars long)
activity3.RawParentSpanId = activeSpan.Context.RawSpanId;

// We clear internals Id and ParentId values to force recalculation.
w3cActivity.RawId = null;
w3cActivity.RawParentId = null;
// We clear internal IDs to force recalculation.
activity3.RawId = null;
activity3.RawParentId = null;

// Avoid recalculation of the traceId.
traceId = activeSpan.TraceId128;
}
// Avoid recalculation of the traceId.
traceId = activeSpan.TraceId128;
}

// if there's an existing Activity we try to use its TraceId and SpanId,
// but if Activity.IdFormat is not ActivityIdFormat.W3C, they may be null or unparsable
if (activityTraceId != null! && activitySpanId != null!)
if (activityTraceId != null && activitySpanId != null)
{
if (traceId == TraceId.Zero)
{
Expand All @@ -132,11 +146,16 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet

rawTraceId = activityTraceId;
rawSpanId = activitySpanId;
activityKey = new(traceId: activityTraceId, spanId: activitySpanId);
}

if (activityTraceId != null! && activitySpanId != null!)
}
else
{
// non-IW3CActivity, i.e. we're in .NET Core 2.x territory. Only have hierarchical IDs to worry about here
var parentId = activity.ParentId;
if (!StringUtil.IsNullOrEmpty(parentId) && ActivityMappingById.TryGetValue(new ActivityKey(parentId), out ActivityMapping mapping))
{
activityKey = new(traceId: activityTraceId, spanId: activitySpanId);
parent = mapping.Scope.Span.Context;
}
}

Expand All @@ -158,8 +177,9 @@ public static void ActivityStarted<T>(string sourceName, T activity, OpenTelemet
}

// We check if we have to ignore the activity by the operation name value
if (IgnoreActivityHandler.IgnoreByOperationName(activity, activeSpan))
if (IgnoreActivityHandler.ShouldIgnoreByOperationName(activity.OperationName))
{
IgnoreActivityHandler.IgnoreActivity(activity, Tracer.Instance.ActiveScope?.Span as Span);
activityMapping = default;
return;
}
Expand Down Expand Up @@ -216,7 +236,7 @@ public static void ActivityStopped<T>(string sourceName, T activity)
{
if (activity.Instance is not null)
{
if (IgnoreActivityHandler.ShouldIgnoreByOperationName(activity))
if (IgnoreActivityHandler.ShouldIgnoreByOperationName(activity.OperationName))
{
return;
}
Expand Down Expand Up @@ -255,6 +275,16 @@ public static void ActivityStopped<T>(string sourceName, T activity)
}
}

StopActivitySlow(sourceName, activity);
}
catch (Exception ex)
{
Log.Error(ex, "Error processing the DefaultActivityHandler.ActivityStopped callback");
}

static void StopActivitySlow<TInner>(string sourceName, TInner activity)
where TInner : IActivity
{
// The listener didn't send us the Activity or the scope instance was not found
// In this case we are going go through the dictionary to check if we have an activity that
// has been closed and then close the associated scope.
Expand Down Expand Up @@ -319,10 +349,6 @@ public static void ActivityStopped<T>(string sourceName, T activity)
}
}
}
catch (Exception ex)
{
Log.Error(ex, "Error processing the DefaultActivityHandler.ActivityStopped callback");
}

static void CloseActivityScope<TInner>(string sourceName, TInner activity, Scope scope)
where TInner : IActivity
Expand Down
80 changes: 29 additions & 51 deletions tracer/src/Datadog.Trace/Activity/Handlers/IgnoreActivityHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#nullable enable

using System;
using Datadog.Trace.Activity.DuckTypes;
using Datadog.Trace.Util;

Expand All @@ -30,36 +31,43 @@ internal sealed class IgnoreActivityHandler : IActivityHandler
"Experimental.System.Net.Sockets",
};

private static readonly string[] IgnoreOperationNamesStartingWith =
public static bool ShouldIgnoreByOperationName(string? operationName)
{
"System.Net.Http.",
"Microsoft.AspNetCore.",
};
// We only have two ignored operation names for now, if we get more, we can be more
// generalized, but this is called twice in hot path creation
return operationName is not null
&& (operationName.StartsWith("System.Net.Http.", StringComparison.Ordinal)
|| operationName.StartsWith("Microsoft.AspNetCore.", StringComparison.Ordinal));
}

public static bool ShouldIgnoreByOperationName<T>(T activity)
public static void IgnoreActivity<T>(T activity, Span? span)
where T : IActivity
{
foreach (var ignoreSourceName in IgnoreOperationNamesStartingWith)
if (span is not null && activity is IW3CActivity w3cActivity)
{
if (activity.OperationName?.StartsWith(ignoreSourceName) == true)
#pragma warning disable DDDUCK001 // Checking IDuckType for null
if ((activity.Parent is null || activity.Parent.StartTimeUtc < span.StartTime.UtcDateTime)
&& w3cActivity.SpanId is not null
&& w3cActivity.TraceId is not null)
{
return true;
}
}
// If we ignore the activity and there's an existing active span
// We modify the activity spanId with the one in the span
// The reason for that is in case this ignored activity is used
// for propagation then the current active span will appear as parentId
// in the context propagation, and we will keep the entire trace.

return false;
}
// TraceId (always 32 chars long even when using 64-bit ids)
w3cActivity.TraceId = span.Context.RawTraceId;

public static bool IgnoreByOperationName<T>(T activity, Span? span)
where T : IActivity
{
if (ShouldIgnoreByOperationName(activity))
{
IgnoreActivity(activity, span);
return true;
}
// SpanId (always 16 chars long)
w3cActivity.RawParentSpanId = span.Context.RawSpanId;

return false;
// We clear internal Ids to force recalculation.
w3cActivity.RawId = null;
w3cActivity.RawParentId = null;
}
#pragma warning restore DDDUCK001 // Checking IDuckType for null
}
}

public bool ShouldListenTo(string sourceName, string? version)
Expand Down Expand Up @@ -87,35 +95,5 @@ public void ActivityStopped<T>(string sourceName, T activity)
{
// Do nothing
}

private static void IgnoreActivity<T>(T activity, Span? span)
where T : IActivity
{
if (span is not null && activity is IW3CActivity w3cActivity)
{
#pragma warning disable DDDUCK001 // Checking IDuckType for null
if ((activity.Parent is null || activity.Parent.StartTimeUtc < span.StartTime.UtcDateTime)
&& w3cActivity.SpanId is not null
&& w3cActivity.TraceId is not null)
{
// If we ignore the activity and there's an existing active span
// We modify the activity spanId with the one in the span
// The reason for that is in case this ignored activity is used
// for propagation then the current active span will appear as parentId
// in the context propagation, and we will keep the entire trace.

// TraceId (always 32 chars long even when using 64-bit ids)
w3cActivity.TraceId = span.Context.RawTraceId;

// SpanId (always 16 chars long)
w3cActivity.ParentSpanId = span.Context.RawSpanId;

// We clear internals Id and ParentId values to force recalculation.
w3cActivity.RawId = null;
w3cActivity.RawParentId = null;
}
#pragma warning restore DDDUCK001 // Checking IDuckType for null
}
}
}
}
9 changes: 5 additions & 4 deletions tracer/test/Datadog.Trace.Tests/ActivityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// </copyright>

using System;
using System.Threading.Tasks;
using Datadog.Trace.Configuration;
using Datadog.Trace.Propagators;
using Datadog.Trace.TestHelpers;
Expand Down Expand Up @@ -32,14 +33,14 @@ public ActivityTests(ActivityFixture fixture)
}

[SkippableFact]
public void SimpleActivitiesAndSpansTest()
public async Task SimpleActivitiesAndSpansTest()
{
// macos 12 is crazy flaky around timings
// We should unskip this once we have resolved the issues around Hierarchical IDs
SkipOn.Platform(SkipOn.PlatformValue.MacOs);

var settings = new TracerSettings();
var tracer = TracerHelper.CreateWithFakeAgent(settings);
await using var tracer = TracerHelper.CreateWithFakeAgent(settings);
Tracer.UnsafeSetTracerInstance(tracer);

Tracer.Instance.ActiveScope.Should().BeNull();
Expand Down Expand Up @@ -136,10 +137,10 @@ public void SimpleActivitiesAndSpansTest()
}

[Fact]
public void SimpleSpansAndActivitiesTest()
public async Task SimpleSpansAndActivitiesTest()
{
var settings = new TracerSettings();
var tracer = TracerHelper.CreateWithFakeAgent(settings);
await using var tracer = TracerHelper.CreateWithFakeAgent(settings);
Tracer.UnsafeSetTracerInstance(tracer);

Tracer.Instance.ActiveScope.Should().BeNull();
Expand Down
Loading