From bba486db40f5bb1ef7e3f656c35508cac80306b2 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 12 Jan 2022 15:32:35 -0800 Subject: [PATCH 1/4] Introduce Activity.IsStopped Property --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 1 + .../src/System/Diagnostics/Activity.cs | 5 ++++ .../tests/ActivityTests.cs | 26 ++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index fd218a7dbf9df1..f82cadcde38e11 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -25,6 +25,7 @@ public string? Id } public bool IsAllDataRequested { get { throw null; } set { throw null; } } + public bool IsStopped { get { throw null; } } public System.Diagnostics.ActivityIdFormat IdFormat { get { throw null; } } public System.Diagnostics.ActivityKind Kind { get { throw null; } } public string OperationName { get { throw null; } } 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 72c8a20bb77453..085dca3ece693e 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -784,6 +784,11 @@ public ActivityTraceId TraceId /// public bool IsAllDataRequested { get; set;} + /// + /// Indicate if the this Activity object is stopped + /// + public bool IsStopped { get => IsFinished; } + /// /// Return the flags (defined by the W3C ID specification) associated with the activity. /// diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 2ac5efa2d9fad5..33c243b6f7cad8 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -299,7 +299,6 @@ public void ActivityIdOverflow() Assert.Equal('#', activity.Id[activity.Id.Length - 1]); } - /// /// Tests activity start and stop /// Checks Activity.Current correctness, Id generation @@ -320,6 +319,31 @@ public void StartStop() Assert.Null(Activity.Current); } + /// + /// Tests Activity.IsStopped + /// + [Fact] + public void IsStoppedTest() + { + var activity = new Activity("activity"); + Assert.False(activity.IsStopped); + activity.Start(); + Assert.False(activity.IsStopped); + Assert.Equal(TimeSpan.Zero, activity.Duration); + activity.Stop(); + Assert.NotEqual(TimeSpan.Zero, activity.Duration); + Assert.True(activity.IsStopped); + + activity = new Activity("activity"); + Assert.False(activity.IsStopped); + activity.Start(); + Assert.False(activity.IsStopped); + activity.SetEndTime(DateTime.UtcNow.AddMinutes(1)); // Setting end time shouldn't mark the activity as stopped + Assert.False(activity.IsStopped); + activity.Stop(); + Assert.True(activity.IsStopped); + } + /// /// Tests Id generation /// From 7fe9e89ee0b40ddfbce308c6335fe7c6ede760a0 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 12 Jan 2022 18:55:00 -0800 Subject: [PATCH 2/4] Rename IsFinished to IsStopped --- .../src/System/Diagnostics/Activity.cs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 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 085dca3ece693e..57533d397ac725 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -684,9 +684,9 @@ public void Stop() return; } - if (!IsFinished) + if (!IsStopped) { - IsFinished = true; + IsStopped = true; if (Duration == TimeSpan.Zero) { @@ -784,11 +784,6 @@ public ActivityTraceId TraceId /// public bool IsAllDataRequested { get; set;} - /// - /// Indicate if the this Activity object is stopped - /// - public bool IsStopped { get => IsFinished; } - /// /// Return the flags (defined by the W3C ID specification) associated with the activity. /// @@ -956,7 +951,7 @@ internal static bool TryConvertIdToContext(string traceParent, string? traceStat /// public void Dispose() { - if (!IsFinished) + if (!IsStopped) { Stop(); } @@ -1237,7 +1232,7 @@ private static unsafe long GetRandomNumber() private static bool ValidateSetCurrent(Activity? activity) { - bool canSet = activity == null || (activity.Id != null && !activity.IsFinished); + bool canSet = activity == null || (activity.Id != null && !activity.IsStopped); if (!canSet) { NotifyError(new InvalidOperationException(SR.ActivityNotRunning)); @@ -1303,18 +1298,21 @@ private bool W3CIdFlagsSet get => (_w3CIdFlags & ActivityTraceFlagsIsSet) != 0; } - private bool IsFinished + /// + /// Indicate if the this Activity object is stopped + /// + public bool IsStopped { - get => (_state & State.IsFinished) != 0; - set + get => (_state & State.IsStopped) != 0; + private set { if (value) { - _state |= State.IsFinished; + _state |= State.IsStopped; } else { - _state &= ~State.IsFinished; + _state &= ~State.IsStopped; } } } @@ -1628,7 +1626,7 @@ private enum State : byte FormatW3C = 0b_0_00000_10, FormatFlags = 0b_0_00000_11, - IsFinished = 0b_1_00000_00, + IsStopped = 0b_1_00000_00, } } From 8030692d0f8dc8eb4bafe70855ba44f81ec34b12 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 12 Jan 2022 19:14:56 -0800 Subject: [PATCH 3/4] Update src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs Co-authored-by: Dan Moseley --- .../src/System/Diagnostics/Activity.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 57533d397ac725..5b741cf1a97be4 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -1299,7 +1299,7 @@ private bool W3CIdFlagsSet } /// - /// Indicate if the this Activity object is stopped + /// Indicates whether this Activity object is stopped /// public bool IsStopped { From 647acbca401c6aa9de1f7927f3c2b1279610bfca Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 13 Jan 2022 13:59:29 -0800 Subject: [PATCH 4/4] Address the feedback --- .../src/System/Diagnostics/Activity.cs | 5 ++- .../tests/ActivityTests.cs | 38 ++++++++++++++----- 2 files changed, 33 insertions(+), 10 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 5b741cf1a97be4..1bffa9b98f07bf 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -1299,8 +1299,11 @@ private bool W3CIdFlagsSet } /// - /// Indicates whether this Activity object is stopped + /// Indicates whether this object is stopped /// + /// + /// When subscribing to stop event using , the received object in the event callback will have as true. + /// public bool IsStopped { get => (_state & State.IsStopped) != 0; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 33c243b6f7cad8..6b2a0c8aecf5ec 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -325,7 +325,7 @@ public void StartStop() [Fact] public void IsStoppedTest() { - var activity = new Activity("activity"); + using var activity = new Activity("activity"); Assert.False(activity.IsStopped); activity.Start(); Assert.False(activity.IsStopped); @@ -334,14 +334,34 @@ public void IsStoppedTest() Assert.NotEqual(TimeSpan.Zero, activity.Duration); Assert.True(activity.IsStopped); - activity = new Activity("activity"); - Assert.False(activity.IsStopped); - activity.Start(); - Assert.False(activity.IsStopped); - activity.SetEndTime(DateTime.UtcNow.AddMinutes(1)); // Setting end time shouldn't mark the activity as stopped - Assert.False(activity.IsStopped); - activity.Stop(); - Assert.True(activity.IsStopped); + using var activity1 = new Activity("activity"); + Assert.False(activity1.IsStopped); + activity1.Start(); + Assert.False(activity1.IsStopped); + activity1.SetEndTime(DateTime.UtcNow.AddMinutes(1)); // Setting end time shouldn't mark the activity as stopped + Assert.False(activity1.IsStopped); + activity1.Stop(); + Assert.True(activity1.IsStopped); + + // + // Validate when receiving Start/Stop Activity events + // + + using ActivitySource aSource = new ActivitySource("TestActivityIsStopped"); + using ActivityListener listener = new ActivityListener(); + + listener.ShouldListenTo = (activitySource) => activitySource.Name == "TestActivityIsStopped"; + listener.Sample = (ref ActivityCreationOptions activityOptions) => ActivitySamplingResult.AllData; + listener.ActivityStarted = a => Assert.False(a.IsStopped); + listener.ActivityStopped = a => Assert.True(a.IsStopped); + ActivitySource.AddActivityListener(listener); + Activity sourceActivity; + using (sourceActivity = aSource.StartActivity("a1")) + { + Assert.NotNull(sourceActivity); + Assert.False(sourceActivity.IsStopped); + } + Assert.True(sourceActivity.IsStopped); } ///