Disallow setting Parent on the started Activity (#81394)
authorTarek Mahmoud Sayed <tarekms@microsoft.com>
Tue, 31 Jan 2023 23:02:39 +0000 (15:02 -0800)
committerGitHub <noreply@github.com>
Tue, 31 Jan 2023 23:02:39 +0000 (15:02 -0800)
Fixes https://github.com/dotnet/runtime/issues/81371

src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs

index f57c9d3..93a345d 100644 (file)
   <data name="ActivityStartAlreadyStarted" xml:space="preserve">
     <value>"Can not start an Activity that was already started"</value>
   </data>
+  <data name="ActivitySetParentAlreadyStarted" xml:space="preserve">
+    <value>"Can not set parent on already started Activity"</value>
+  </data>
   <data name="EndTimeNotUtc" xml:space="preserve">
     <value>"EndTime is not UTC"</value>
   </data>
index 581d710..68c3445 100644 (file)
@@ -564,7 +564,12 @@ namespace System.Diagnostics
         /// <param name="parentId">The id of the parent operation.</param>
         public Activity SetParentId(string parentId)
         {
-            if (Parent != null)
+            if (_id != null || _spanId != null)
+            {
+                // Cannot set the parent on already started Activity.
+                NotifyError(new InvalidOperationException(SR.ActivitySetParentAlreadyStarted));
+            }
+            else if (Parent != null)
             {
                 NotifyError(new InvalidOperationException(SR.SetParentIdOnActivityWithParent));
             }
@@ -589,7 +594,12 @@ namespace System.Diagnostics
         /// </summary>
         public Activity SetParentId(ActivityTraceId traceId, ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None)
         {
-            if (Parent != null)
+            if (_id != null || _spanId != null)
+            {
+                // Cannot set the parent on already started Activity.
+                NotifyError(new InvalidOperationException(SR.ActivitySetParentAlreadyStarted));
+            }
+            else if (Parent != null)
             {
                 NotifyError(new InvalidOperationException(SR.SetParentIdOnActivityWithParent));
             }
index 066f17f..ae3c138 100644 (file)
@@ -239,7 +239,23 @@ namespace System.Diagnostics.Tests
         [Fact]
         public void SetParentId()
         {
-            var parent = new Activity("parent");
+            using (var a = new Activity("foo"))
+            {
+                a.Start();
+                string parentId = a.ParentId;
+                a.SetParentId("00-6e76af18746bae4eadc3581338bbe8b1-2899ebfdbdce904b-00"); // Error does nothing
+                Assert.Equal(parentId, a.ParentId);
+            }
+
+            using (var a = new Activity("foo"))
+            {
+                a.Start();
+                string parentId = a.ParentId;
+                a.SetParentId(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom()); // Nothing will happen
+                Assert.Equal(parentId, a.ParentId);
+            }
+
+            using var parent = new Activity("parent");
             parent.SetParentId(null);  // Error does nothing
             Assert.Null(parent.ParentId);
 
@@ -255,7 +271,7 @@ namespace System.Diagnostics.Tests
             Assert.Equal(parent.ParentId, parent.RootId);
             parent.Start();
 
-            var child = new Activity("child");
+            using var child = new Activity("child");
             child.Start();
 
             Assert.Equal(parent.Id, child.ParentId);
@@ -730,7 +746,7 @@ namespace System.Diagnostics.Tests
         [Fact]
         public void IdFormat_W3CWhenTraceIdAndSpanIdProvided()
         {
-            Activity activity = new Activity("activity3");
+            using Activity activity = new Activity("activity3");
             ActivityTraceId activityTraceId = ActivityTraceId.CreateRandom();
             activity.SetParentId(activityTraceId, ActivitySpanId.CreateRandom());
             activity.Start();