Fix: TraceId is not available before Activity starts (dotnet/corefx#36345)

* Fix: TraceId is not available before Activity starts

* minor fixes

* review: Move TraceId parsing from parent to helper method

* fix netfx

Commit migrated from https://github.com/dotnet/corefx/commit/b1f81d6480909bfe41fa6299fbfd38376df6f240

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 55be7e3..ffd45e5 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
@@ -496,12 +496,9 @@ namespace System.Diagnostics
             {
                 if (!_traceIdSet)
                 {
-                    if (_id != null && IdFormat == ActivityIdFormat.W3C)
-                    {
-                        _traceId = ActivityTraceId.CreateFromString(_id.AsSpan(3, 32));
-                        _traceIdSet = true;
-                    }
+                    TrySetTraceIdFromParent();
                 }
+
                 return ref _traceId;
             }
         }
@@ -605,27 +602,11 @@ namespace System.Diagnostics
             // Get the TraceId from the parent or make a new one.  
             if (!_traceIdSet)
             {
-                if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C)
-                {
-                    _traceId = Parent.TraceId;
-                }
-                else if (_parentId != null && IsW3CId(_parentId))
-                {
-                    try
-                    {
-                        _traceId = ActivityTraceId.CreateFromString(_parentId.AsSpan(3, 32));
-                    }
-                    catch
-                    {
-                        _traceId = ActivityTraceId.CreateRandom();
-                    }
-                }
-                else
+                if (!TrySetTraceIdFromParent())
                 {
                     _traceId = ActivityTraceId.CreateRandom();
+                    _traceIdSet = true;
                 }
-
-                _traceIdSet = true;
             }
             // Create a new SpanID. 
             _spanId = ActivitySpanId.CreateRandom();
@@ -755,6 +736,33 @@ namespace System.Diagnostics
             return canSet;
         }
 
+#if ALLOW_PARTIALLY_TRUSTED_CALLERS
+        [SecuritySafeCritical]
+#endif
+        private bool TrySetTraceIdFromParent()
+        {
+            Debug.Assert(!_traceIdSet);
+
+            if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C)
+            {
+                _traceId = Parent.TraceId;
+                _traceIdSet = true;
+            }
+            else if (_parentId != null && IsW3CId(_parentId))
+            {
+                try
+                {
+                    _traceId = ActivityTraceId.CreateFromString(_parentId.AsSpan(3, 32));
+                    _traceIdSet = true;
+                }
+                catch
+                {
+                }
+            }
+
+            return _traceIdSet;
+        }
+
         private string _rootId;
         private int _currentChildId;  // A unique number for all children of this activity.  
 

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
index 06d4a92..393a085 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
@@ -263,7 +263,6 @@ namespace System.Diagnostics.Tests
         [Fact]
         public void RootId()
         {
-
             var parentIds = new[]{
                 "123",   //Parent does not start with '|' and does not contain '.'
                 "123.1", //Parent does not start with '|' but contains '.'
@@ -612,6 +611,65 @@ namespace System.Diagnostics.Tests
             }
         }
 
+        [Fact]
+        public void TraceIdBeforeStartTests()
+        {
+            try
+            {
+                Activity activity;
+
+                // from traceparent header
+                activity = new Activity("activity1");
+                activity.SetParentId("00-0123456789abcdef0123456789abcdef-0123456789abcdef-01");
+                Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString());
+
+                // from explicit TraceId and SpanId
+                activity = new Activity("activity2");
+                activity.SetParentId(
+                    ActivityTraceId.CreateFromString("0123456789abcdef0123456789abcdef".AsSpan()),
+                    ActivitySpanId.CreateFromString("0123456789abcdef".AsSpan()));
+
+                Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString());
+
+                // from in-proc parent
+                Activity parent = new Activity("parent");
+                parent.SetParentId("00-0123456789abcdef0123456789abcdef-0123456789abcdef-01");
+                parent.Start();
+
+                activity = new Activity("child");
+                activity.Start();
+                Assert.Equal("0123456789abcdef0123456789abcdef", activity.TraceId.ToHexString());
+                parent.Stop();
+                activity.Stop();
+
+                // no parent
+                Activity.DefaultIdFormat = ActivityIdFormat.W3C;
+                Activity.ForceDefaultIdFormat = true;
+
+                activity = new Activity("activity3");
+                Assert.Equal("00000000000000000000000000000000", activity.TraceId.ToHexString());
+
+                // from invalid traceparent header
+                activity.SetParentId("123");
+                Assert.Equal("00000000000000000000000000000000", activity.TraceId.ToHexString());
+            }
+            finally
+            {
+                Activity.ForceDefaultIdFormat = false;
+                Activity.DefaultIdFormat = ActivityIdFormat.Hierarchical;
+                Activity.Current = null;
+            }
+        }
+
+        [Fact]
+        public void RootIdBeforeStartTests()
+        {
+            Activity activity = new Activity("activity1");
+            Assert.Null(activity.RootId);
+            activity.SetParentId("|0123456789abcdef0123456789abcdef.0123456789abcdef.");
+            Assert.Equal("0123456789abcdef0123456789abcdef", activity.RootId);
+        }
+
         /// <summary>
         /// Tests Activity Start and Stop with timestamp
         /// </summary>