From 35c6f582b7dfd52a0cc740f49cd8bb4dfbb1fee0 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 6 Oct 2026 15:47:36 +0100 Subject: [PATCH 1/3] Avoid copying tags and links when sampling Do not copy links and tags when creating a new `Activity` when `ActivitySamplingResult.PropagationData` is specified. Fixes #135007. --- .../src/System/Diagnostics/Activity.cs | 9 +- .../tests/ActivitySourceTests.cs | 185 ++++++++++++++++++ 2 files changed, 191 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index 982a4d45b44cf9..d81b4ec8ef7d78 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -1197,7 +1197,10 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind activity.IdFormat = idFormat; activity._traceState = traceState; - if (links != null) + // Links and tags are unnecessary for activities sampled as PropagationData, so skip copying them. + bool copyData = request != ActivitySamplingResult.PropagationData; + + if (copyData && links != null) { using (IEnumerator enumerator = links.GetEnumerator()) { @@ -1208,7 +1211,7 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind } } - if (tags != null) + if (copyData && tags != null) { using (IEnumerator> enumerator = tags.GetEnumerator()) { @@ -1219,7 +1222,7 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind } } - if (samplerTags != null) + if (copyData && samplerTags != null) { if (activity._tags == null) { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs index ce501189bb3d53..d2ac9f5f85e2a7 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs @@ -734,6 +734,191 @@ public void PropagationDataSamplingTest() }).Dispose(); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void PropagationDataSamplingDoesNotCopyTagsAndLinksTest() + { + RemoteExecutor.Invoke(() => { + Activity.ForceDefaultIdFormat = true; + Activity.DefaultIdFormat = ActivityIdFormat.W3C; + + using ActivitySource aSource = new ActivitySource("PropagationDataTagsAndLinksTest"); + + ActivitySamplingResult result = ActivitySamplingResult.PropagationData; + int sampledTags = 0; + int sampledLinks = 0; + + using ActivityListener listener = new ActivityListener + { + ShouldListenTo = (activitySource) => ReferenceEquals(activitySource, aSource), + Sample = (ref ActivityCreationOptions activityOptions) => + { + sampledTags = activityOptions.Tags.Count(); + sampledLinks = activityOptions.Links.Count(); + return result; + } + }; + + ActivitySource.AddActivityListener(listener); + + KeyValuePair[] tags = [new("tag1", "value1"), new("tag2", "value2")]; + ActivityLink[] links = [new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None))]; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags, links)) + { + Assert.NotNull(a); + Assert.False(a.IsAllDataRequested); + Assert.Empty(a.TagObjects); + Assert.Empty(a.Links); + Assert.Equal(2, sampledTags); + Assert.Equal(1, sampledLinks); + } + + result = ActivitySamplingResult.AllData; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags, links)) + { + Assert.NotNull(a); + Assert.True(a.IsAllDataRequested); + Assert.Equal(2, a.TagObjects.Count()); + Assert.Single(a.Links); + } + }).Dispose(); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void PropagationDataSamplingWithParentIdDoesNotCopyTagsAndLinksTest() + { + RemoteExecutor.Invoke(() => { + Activity.ForceDefaultIdFormat = true; + Activity.DefaultIdFormat = ActivityIdFormat.W3C; + + using ActivitySource aSource = new ActivitySource("PropagationDataParentIdTest"); + + ActivitySamplingResult result = ActivitySamplingResult.PropagationData; + int sampledTags = 0; + int sampledLinks = 0; + + using ActivityListener listener = new ActivityListener + { + ShouldListenTo = (activitySource) => ReferenceEquals(activitySource, aSource), + SampleUsingParentId = (ref ActivityCreationOptions activityOptions) => + { + sampledTags = activityOptions.Tags.Count(); + sampledLinks = activityOptions.Links.Count(); + return result; + } + }; + + ActivitySource.AddActivityListener(listener); + + string parentId = "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01"; + KeyValuePair[] tags = [new("tag1", "value1")]; + ActivityLink[] links = [new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None))]; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, parentId, tags, links)) + { + Assert.NotNull(a); + Assert.False(a.IsAllDataRequested); + Assert.Equal(parentId, a.ParentId); + Assert.Empty(a.TagObjects); + Assert.Empty(a.Links); + Assert.Equal(1, sampledTags); + Assert.Equal(1, sampledLinks); + } + + result = ActivitySamplingResult.AllData; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, parentId, tags, links)) + { + Assert.NotNull(a); + Assert.True(a.IsAllDataRequested); + Assert.Single(a.TagObjects); + Assert.Single(a.Links); + } + }).Dispose(); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void PropagationDataSamplingDoesNotCopySamplingTagsTest() + { + RemoteExecutor.Invoke(() => { + using ActivitySource aSource = new ActivitySource("PropagationDataSamplingTagsTest"); + + ActivitySamplingResult result = ActivitySamplingResult.PropagationData; + + using ActivityListener listener = new ActivityListener + { + ShouldListenTo = (activitySource) => ReferenceEquals(activitySource, aSource), + Sample = (ref ActivityCreationOptions activityOptions) => + { + activityOptions.SamplingTags.Add("sampler.tag", "value"); + return result; + } + }; + + ActivitySource.AddActivityListener(listener); + + using (Activity a = aSource.StartActivity("a")) + { + Assert.NotNull(a); + Assert.False(a.IsAllDataRequested); + Assert.Empty(a.TagObjects); + } + + result = ActivitySamplingResult.AllData; + + using (Activity a = aSource.StartActivity("a")) + { + Assert.NotNull(a); + Assert.True(a.IsAllDataRequested); + Assert.Contains(a.TagObjects, t => t.Key == "sampler.tag" && (string)t.Value == "value"); + } + }).Dispose(); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void PropagationDataSamplingWithMultipleListenersCopiesTagsAndLinksIfAnyRequestsDataTest() + { + RemoteExecutor.Invoke(() => { + using ActivitySource aSource = new ActivitySource("PropagationDataMultipleListenersTest"); + + using ActivityListener propagationListener = new ActivityListener + { + ShouldListenTo = (activitySource) => ReferenceEquals(activitySource, aSource), + Sample = (ref ActivityCreationOptions activityOptions) => ActivitySamplingResult.PropagationData + }; + + ActivitySource.AddActivityListener(propagationListener); + + KeyValuePair[] tags = [new("tag1", "value1")]; + ActivityLink[] links = [new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None))]; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags, links)) + { + Assert.NotNull(a); + Assert.False(a.IsAllDataRequested); + Assert.Empty(a.TagObjects); + Assert.Empty(a.Links); + } + + using ActivityListener allDataListener = new ActivityListener + { + ShouldListenTo = (activitySource) => ReferenceEquals(activitySource, aSource), + Sample = (ref ActivityCreationOptions activityOptions) => ActivitySamplingResult.AllData + }; + + ActivitySource.AddActivityListener(allDataListener); + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags, links)) + { + Assert.NotNull(a); + Assert.True(a.IsAllDataRequested); + Assert.Single(a.TagObjects); + Assert.Single(a.Links); + } + }).Dispose(); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void TestExpectedListenersReturnValues() { From 7e5b732b112e38ce40739823518360b8fe9f050d Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 6 Oct 2026 19:25:54 +0100 Subject: [PATCH 2/3] Address feedback - Harden check for skipping tags. - Don't skip sampler tags. --- .../src/System/Diagnostics/Activity.cs | 6 +++--- .../tests/ActivitySourceTests.cs | 11 +++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index d81b4ec8ef7d78..fd64e889e249d4 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -1197,8 +1197,8 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind activity.IdFormat = idFormat; activity._traceState = traceState; - // Links and tags are unnecessary for activities sampled as PropagationData, so skip copying them. - bool copyData = request != ActivitySamplingResult.PropagationData; + // Links and creation tags are unnecessary for activities sampled as PropagationData, so skip copying them. + bool copyData = request is ActivitySamplingResult.AllData or ActivitySamplingResult.AllDataAndRecorded; if (copyData && links != null) { @@ -1222,7 +1222,7 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind } } - if (copyData && samplerTags != null) + if (samplerTags != null) { if (activity._tags == null) { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs index d2ac9f5f85e2a7..3ec52136b48c95 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs @@ -839,7 +839,7 @@ public void PropagationDataSamplingWithParentIdDoesNotCopyTagsAndLinksTest() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void PropagationDataSamplingDoesNotCopySamplingTagsTest() + public void PropagationDataSamplingCopiesSamplingTagsButNotCreationTagsTest() { RemoteExecutor.Invoke(() => { using ActivitySource aSource = new ActivitySource("PropagationDataSamplingTagsTest"); @@ -858,19 +858,22 @@ public void PropagationDataSamplingDoesNotCopySamplingTagsTest() ActivitySource.AddActivityListener(listener); - using (Activity a = aSource.StartActivity("a")) + KeyValuePair[] tags = [new("creation.tag", "value")]; + + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags)) { Assert.NotNull(a); Assert.False(a.IsAllDataRequested); - Assert.Empty(a.TagObjects); + Assert.Equal([new KeyValuePair("sampler.tag", "value")], a.TagObjects); } result = ActivitySamplingResult.AllData; - using (Activity a = aSource.StartActivity("a")) + using (Activity a = aSource.StartActivity("a", ActivityKind.Server, default(ActivityContext), tags)) { Assert.NotNull(a); Assert.True(a.IsAllDataRequested); + Assert.Contains(a.TagObjects, t => t.Key == "creation.tag" && (string)t.Value == "value"); Assert.Contains(a.TagObjects, t => t.Key == "sampler.tag" && (string)t.Value == "value"); } }).Dispose(); From abc422c1b3ce754680b635fb095d3d6478dfa464 Mon Sep 17 00:00:00 2001 From: martincostello Date: Wed, 7 Oct 2026 09:35:13 +0100 Subject: [PATCH 3/3] Address feedback Update `ActivitySource,CreateActivity()` parameter documentation for `tags` and `links` for new behaviour. --- .../src/System/Diagnostics/ActivitySource.cs | 60 +++++++++++++++---- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs index f5ca608d2ae976..e14fa042526f95 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs @@ -153,8 +153,16 @@ public bool HasListeners() /// The operation name of the Activity. /// The /// The parent object to initialize the created Activity object with. - /// The optional tags list to initialize the created Activity object with. - /// The optional list to initialize the created Activity object with. + /// + /// The optional tags list to initialize the created Activity object with. The tags are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// + /// + /// The optional list to initialize the created Activity object with. The links are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// /// The default Id format to use. /// The created object or null if there is no any listener. /// @@ -169,8 +177,16 @@ public bool HasListeners() /// The operation name of the Activity. /// The /// The parent Id to initialize the created Activity object with. - /// The optional tags list to initialize the created Activity object with. - /// The optional list to initialize the created Activity object with. + /// + /// The optional tags list to initialize the created Activity object with. The tags are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// + /// + /// The optional list to initialize the created Activity object with. The links are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// /// The default Id format to use. /// The created object or null if there is no any listener. /// @@ -194,8 +210,16 @@ public bool HasListeners() /// The operation name of the Activity. /// The /// The parent object to initialize the created Activity object with. - /// The optional tags list to initialize the created Activity object with. - /// The optional list to initialize the created Activity object with. + /// + /// The optional tags list to initialize the created Activity object with. The tags are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// + /// + /// The optional list to initialize the created Activity object with. The links are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// /// The optional start timestamp to set on the created Activity object. /// The created object or null if there is no any listener. public Activity? StartActivity(string name, ActivityKind kind, ActivityContext parentContext, IEnumerable>? tags = null, IEnumerable? links = null, DateTimeOffset startTime = default) @@ -207,8 +231,16 @@ public bool HasListeners() /// The operation name of the Activity. /// The /// The parent Id to initialize the created Activity object with. - /// The optional tags list to initialize the created Activity object with. - /// The optional list to initialize the created Activity object with. + /// + /// The optional tags list to initialize the created Activity object with. The tags are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// + /// + /// The optional list to initialize the created Activity object with. The links are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// /// The optional start timestamp to set on the created Activity object. /// The created object or null if there is no any listener. public Activity? StartActivity(string name, ActivityKind kind, string? parentId, IEnumerable>? tags = null, IEnumerable? links = null, DateTimeOffset startTime = default) @@ -219,8 +251,16 @@ public bool HasListeners() /// /// The /// The parent object to initialize the created Activity object with. - /// The optional tags list to initialize the created Activity object with. - /// The optional list to initialize the created Activity object with. + /// + /// The optional tags list to initialize the created Activity object with. The tags are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// + /// + /// The optional list to initialize the created Activity object with. The links are made available to sampling callbacks, + /// but are only copied to the created Activity if the combined sampling result is + /// or . + /// /// The optional start timestamp to set on the created Activity object. /// The operation name of the Activity. /// The created object or null if there is no any listener.