Skip to content

Commit 7132268

Browse files
authored
[System.Diagnostics.DiagnosticSource.Activity] Reset recorded flag when creating activity when not requested by sampler(s) (#111289)
* Reset recorded flag when creating activity when not requested by samplers. * Additional test coverage. * Test fixes. * Code review.
1 parent 00506eb commit 7132268

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1233,7 +1233,9 @@ internal static Activity Create(ActivitySource source, string name, ActivityKind
12331233
activity._parentSpanId = parentContext.SpanId.ToString();
12341234
}
12351235

1236-
activity.ActivityTraceFlags = parentContext.TraceFlags;
1236+
// Note: Don't inherit Recorded from parent as it is set below
1237+
// based on sampling decision
1238+
activity.ActivityTraceFlags = parentContext.TraceFlags & ~ActivityTraceFlags.Recorded;
12371239
activity._parentTraceFlags = (byte)parentContext.TraceFlags;
12381240
activity.HasRemoteParent = parentContext.IsRemote;
12391241
}

src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivitySourceTests.cs

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ public void TestListeningToConstructedActivityEvents()
256256
}
257257

258258
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
259-
public void EnsureRecordingTest()
259+
public void AllDataAndRecordedSamplingTest()
260260
{
261261
RemoteExecutor.Invoke(() => {
262262
Activity.ForceDefaultIdFormat = true;
@@ -278,14 +278,76 @@ public void EnsureRecordingTest()
278278

279279
ActivitySource.AddActivityListener(listener);
280280

281-
Activity a = aSource.StartActivity("RecordedActivity");
281+
// Note: Remote parent is set as NOT recorded
282+
var parentContext = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None, isRemote: true);
283+
284+
Activity a = aSource.StartActivity("RecordedActivity", ActivityKind.Internal, parentContext);
282285
Assert.NotNull(a);
283286

287+
Assert.True(a.IsAllDataRequested);
284288
Assert.True(a.Recorded);
285289
Assert.True((a.Context.TraceFlags & ActivityTraceFlags.Recorded) != 0);
286290
}).Dispose();
287291
}
288292

293+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
294+
public void AllDataSamplingTest()
295+
{
296+
RemoteExecutor.Invoke(() => {
297+
Activity.ForceDefaultIdFormat = true;
298+
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
299+
300+
ActivitySource aSource = new ActivitySource("EnsureRecordingTest");
301+
302+
ActivityListener listener = new ActivityListener
303+
{
304+
ShouldListenTo = (activitySource) => true,
305+
Sample = (ref ActivityCreationOptions<ActivityContext> activityOptions) => ActivitySamplingResult.AllData
306+
};
307+
308+
ActivitySource.AddActivityListener(listener);
309+
310+
// Note: Remote parent is set as recorded
311+
var parentContext = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded, isRemote: true);
312+
313+
Activity a = aSource.StartActivity("RecordedActivity", ActivityKind.Internal, parentContext);
314+
Assert.NotNull(a);
315+
316+
Assert.True(a.IsAllDataRequested);
317+
Assert.False(a.Recorded);
318+
Assert.False((a.Context.TraceFlags & ActivityTraceFlags.Recorded) != 0);
319+
}).Dispose();
320+
}
321+
322+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
323+
public void PropagationDataSamplingTest()
324+
{
325+
RemoteExecutor.Invoke(() => {
326+
Activity.ForceDefaultIdFormat = true;
327+
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
328+
329+
ActivitySource aSource = new ActivitySource("EnsureRecordingTest");
330+
331+
ActivityListener listener = new ActivityListener
332+
{
333+
ShouldListenTo = (activitySource) => true,
334+
Sample = (ref ActivityCreationOptions<ActivityContext> activityOptions) => ActivitySamplingResult.PropagationData
335+
};
336+
337+
ActivitySource.AddActivityListener(listener);
338+
339+
// Note: Remote parent is set as recorded
340+
var parentContext = new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded, isRemote: true);
341+
342+
Activity a = aSource.StartActivity("RecordedActivity", ActivityKind.Internal, parentContext);
343+
Assert.NotNull(a);
344+
345+
Assert.False(a.IsAllDataRequested);
346+
Assert.False(a.Recorded);
347+
Assert.False((a.Context.TraceFlags & ActivityTraceFlags.Recorded) != 0);
348+
}).Dispose();
349+
}
350+
289351
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
290352
public void TestExpectedListenersReturnValues()
291353
{
@@ -424,7 +486,10 @@ public void TestActivityCreationProperties()
424486

425487
Assert.Equal(ctx.TraceId, activity.TraceId);
426488
Assert.Equal(ctx.SpanId, activity.ParentSpanId);
427-
Assert.Equal(ctx.TraceFlags, activity.ActivityTraceFlags);
489+
Assert.NotEqual(ctx.TraceFlags, activity.ActivityTraceFlags);
490+
Assert.True(ctx.TraceFlags.HasFlag(ActivityTraceFlags.Recorded));
491+
Assert.False(activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded));
492+
Assert.False(activity.Recorded);
428493
Assert.Equal(ctx.TraceState, activity.TraceStateString);
429494
Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat);
430495

0 commit comments

Comments
 (0)