Update Id generation schema

2 files changed, 78 insertions, 7 deletions

diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
index 55a8c583d3..ff385c3b6c 100644
--- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
+++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
@@ -265,27 +265,60 @@ namespace System.Diagnostics
             {
                 // Normal start within the process
                 Debug.Assert(!string.IsNullOrEmpty(Parent.Id));
-                ret = $"{Parent.Id}.{Interlocked.Increment(ref Parent._currentChildId)}";
+                ret = appendSuffix(Parent.Id, $"{Interlocked.Increment(ref Parent._currentChildId)}");
             }
             else if (ParentId != null)
             {
                 // Start from outside the process (e.g. incoming HTTP)
                 Debug.Assert(ParentId.Length != 0);
-                ret = $"{ParentId}.{Interlocked.Increment(ref s_currentRootId)}";
+                ret = appendSuffix(ParentId, $"{Interlocked.Increment(ref s_currentRootId):x}");
             }
             else
             {
-                byte[] bytes = new byte[8];
-                s_random.Value.NextBytes(bytes);
-                ret = $"{BitConverter.ToUInt64(bytes, 0)}";
+                ret = generateRootId();
             }
+
             // Useful place to place a conditional breakpoint.  
             return ret;
         }
 
+        private string appendSuffix(string parentId, string suffix)
+        {
+            if (parentId.Length + suffix.Length <= 127)
+                return $"{parentId}.{suffix}";
+
+            //Id overflow:
+            //find position in RequestId to trim
+            int trimPosition = parentId.Length - 1;
+            while (trimPosition > 0)
+            {
+                if ((parentId[trimPosition] == '.' || parentId[trimPosition] == '#')
+                    && trimPosition <= 119) //overflow suffix length is 8 + 1 for #.
+                    break;
+                trimPosition--;
+            }
+
+            //ParentId is not valid Request-Id, let's generate proper one.
+            if (trimPosition == 0)
+                return generateRootId();
+
+            //generate overflow suffix
+            byte[] bytes = new byte[4];
+            s_random.Value.NextBytes(bytes);
+
+            return $"{parentId.Substring(0, trimPosition)}#{BitConverter.ToUInt32(bytes, 0):x8}";
+        }
+
+        private string generateRootId()
+        {
+            byte[] bytes = new byte[8];
+            s_random.Value.NextBytes(bytes);
+            return $"/{BitConverter.ToUInt64(bytes, 0):x}";
+        }
+
         // Used to generate an ID 
-        long _currentChildId;            // A unique number for all children of this activity.  
-        static long s_currentRootId;      // A unique number inside the appdomain.
+        int _currentChildId;            // A unique number for all children of this activity.  
+        static int s_currentRootId;      // A unique number inside the appdomain.
         private static readonly Lazy<Random> s_random = new Lazy<Random>();
         /// <summary>
         /// Having our own key-value linked list allows us to be more efficient  
diff --git a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
index 0f6a2c95a1..a1bda7c883 100644
--- a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
+++ b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -76,6 +77,43 @@ namespace System.Diagnostics.Tests
         }
 
         /// <summary>
+        /// Tests activity SetParentId
+        /// </summary>
+        [Fact]
+        public void ActivityIdOverflow()
+        {
+            //check parentId /abc.1.1...1.1.1.1.1 (126 bytes) and check last .1.1.1.1 is replaced with #overflow_suffix 8 bytes long
+            var parentId = new StringBuilder("/abc");
+            while (parentId.Length < 126)
+                parentId.Append(".1");
+
+            var activity = new Activity("activity")
+                .SetParentId(parentId.ToString())
+                .Start();
+
+            Assert.Equal(
+                parentId.ToString().Substring(0, parentId.Length - 8),
+                activity.Id.Substring(0, activity.Id.Length - 9));
+            Assert.Equal('#', activity.Id[activity.Id.Length - 9]);
+
+            //check parentId /abc.1.1...1.012345678 (128 bytes) and check last .012345678 is replaced with #overflow_suffix 8 bytes long
+            parentId = new StringBuilder("/abc");
+            while (parentId.Length < 118)
+                parentId.Append(".1");
+            parentId.Append(".012345678");
+
+            activity = new Activity("activity")
+                .SetParentId(parentId.ToString())
+                .Start();
+
+            //last .012345678 will be replaced with #overflow_suffix 8 bytes long
+            Assert.Equal(
+                parentId.ToString().Substring(0, parentId.Length - 10),
+                activity.Id.Substring(0, activity.Id.Length - 9));
+            Assert.Equal('#', activity.Id[activity.Id.Length - 9]);
+        }
+
+        /// <summary>
         /// Tests activity start and stop 
         /// Checks Activity.Current correctness, Id generation
         /// </summary>