Change Activity Default IdFormat to W3C (#37686)

* Change Activity Default IdFormat to W3C

* Remove the prefix from the switch and fix a test

* address some of the feedback

* Scope the breaking change to .NET 5

* Fix running the test on NetFX

* Changed the define NET5 to W3C_DEFAULT_ID_FORMAT

* Add net5 and remove netstandard2.0 configuration in the package

* Return back netstandard 2.0 targetting

When removing it, caused a  CI source build failure

diff --git a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.Common.cs b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs
similarity index 88%
rename from src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.Common.cs
rename to src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs
index a892b21..5acaf03 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/LocalAppContextSwitches.Common.cs
+++ b/src/libraries/Common/src/System/LocalAppContextSwitches.Common.cs
@@ -13,6 +13,11 @@ namespace System
     {
         // Returns value of given switch using provided cache.
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal static bool GetSwitchValue(string switchName, ref bool switchValue) =>
+            AppContext.TryGetSwitch(switchName, out switchValue);
+
+        // Returns value of given switch using provided cache.
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static bool GetCachedSwitchValue(string switchName, ref int cachedSwitchValue)
         {
             // The cached switch value has 3 states: 0 - unknown, 1 - true, -1 - false
@@ -24,7 +29,6 @@ namespace System
 
         private static bool GetCachedSwitchValueInternal(string switchName, ref int cachedSwitchValue)
         {
-
             bool hasSwitch = AppContext.TryGetSwitch(switchName, out bool isSwitchEnabled);
             if (!hasSwitch)
             {

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
index 4619c95..52f340c 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
@@ -5,7 +5,6 @@
     <NoWarn>$(NoWarn);SA1205</NoWarn>
     <Nullable>enable</Nullable>
     <TargetFrameworks>$(NetCoreAppCurrent);netstandard1.1;netstandard1.3;net45;net46;netstandard2.0;$(NetFrameworkCurrent)</TargetFrameworks>
-    <ExcludeCurrentNetCoreAppFromPackage>true</ExcludeCurrentNetCoreAppFromPackage>
     <ExcludeCurrentFullFrameworkFromPackage>true</ExcludeCurrentFullFrameworkFromPackage>
   </PropertyGroup>
   <!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
@@ -20,6 +19,7 @@
     <DefineConstants Condition="'$(TargetFramework)' != 'netstandard1.1' and '$(TargetFramework)' != 'netstandard1.3'">$(DefineConstants);EVENTSOURCE_ENUMERATE_SUPPORT</DefineConstants>
     <DefineConstants Condition="$(TargetFramework.StartsWith('net4'))">$(DefineConstants);ALLOW_PARTIALLY_TRUSTED_CALLERS;ENABLE_HTTP_HANDLER</DefineConstants>
     <ExcludeFromPackage Condition="'$(TargetFramework)' == 'netstandard2.0'">true</ExcludeFromPackage>
+    <DefineConstants Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'">$(DefineConstants);W3C_DEFAULT_ID_FORMAT</DefineConstants>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="System\Diagnostics\DiagnosticSource.cs" />
@@ -48,6 +48,12 @@
     <Reference Include="System.Runtime.CompilerServices.Unsafe" />
     <None Include="ActivityUserGuide.md" />
   </ItemGroup>
+  <ItemGroup Condition=" '$(TargetFramework)' == '$(NetCoreAppCurrent)'">
+    <Compile Include="System\Diagnostics\LocalAppContextSwitches.cs" />
+    <Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs">
+      <Link>Common\System\LocalAppContextSwitches.Common.cs</Link>
+    </Compile>
+  </ItemGroup>
   <ItemGroup Condition="'$(TargetFramework)' != 'net45' And '$(TargetFramework)' != 'netstandard1.1'">
     <Compile Include="System\Diagnostics\Activity.Current.net46.cs" />
   </ItemGroup>

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 e3a7f04..ffb8ef8 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
@@ -51,7 +51,7 @@ namespace System.Diagnostics
         private static ActivityIdFormat s_defaultIdFormat;
         /// <summary>
         /// Normally if the ParentID is defined, the format of that is used to determine the
-        /// format used by the Activity.   However if ForceDefaultFormat is set to true, the
+        /// format used by the Activity. However if ForceDefaultFormat is set to true, the
         /// ID format will always be the DefaultIdFormat even if the ParentID is define and is
         /// a different format.
         /// </summary>
@@ -735,7 +735,13 @@ namespace System.Diagnostics
             get
             {
                 if (s_defaultIdFormat == ActivityIdFormat.Unknown)
+                {
+#if W3C_DEFAULT_ID_FORMAT
+                    s_defaultIdFormat = LocalAppContextSwitches.DefaultActivityIdFormatIsHierarchial ? ActivityIdFormat.Hierarchical : ActivityIdFormat.W3C;
+#else
                     s_defaultIdFormat = ActivityIdFormat.Hierarchical;
+#endif // W3C_DEFAULT_ID_FORMAT
+                }
                 return s_defaultIdFormat;
             }
             set

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LocalAppContextSwitches.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LocalAppContextSwitches.cs
new file mode 100644 (file)
index 0000000..6a3ed63
--- /dev/null
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LocalAppContextSwitches.cs
@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Runtime.CompilerServices;
+
+namespace System
+{
+    internal static partial class LocalAppContextSwitches
+    {
+        public static bool DefaultActivityIdFormatIsHierarchial { get; } = InitializeDefaultActivityIdFormat();
+
+        private static bool InitializeDefaultActivityIdFormat()
+        {
+            bool defaultActivityIdFormatIsHierarchial = false;
+
+            if (!LocalAppContextSwitches.GetSwitchValue("System.Diagnostics.DefaultActivityIdFormatIsHierarchial", ref defaultActivityIdFormatIsHierarchial))
+            {
+                string? switchValue = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_DIAGNOSTICS_DEFAULTACTIVITYIDFORMATISHIERARCHIAL");
+                if (switchValue != null)
+                {
+                    defaultActivityIdFormatIsHierarchial = IsTrueStringIgnoreCase(switchValue) || switchValue.Equals("1");
+                }
+            }
+
+            return defaultActivityIdFormatIsHierarchial;
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static bool IsTrueStringIgnoreCase(string value)
+        {
+            return value.Length == 4 &&
+                    (value[0] == 't' || value[0] == 'T') &&
+                    (value[1] == 'r' || value[1] == 'R') &&
+                    (value[2] == 'u' || value[2] == 'U') &&
+                    (value[3] == 'e' || value[3] == 'E');
+        }
+    }
+}

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
index d79c701..8e7c58f 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
@@ -188,32 +188,6 @@ namespace System.Diagnostics.Tests
             Assert.Equal('#', activity.Id[activity.Id.Length - 1]);
         }
 
-        /// <summary>
-        /// Tests overflow in Id generation when parentId has a single (root) node
-        /// </summary>
-        [Fact]
-        public void ActivityIdNonHierarchicalOverflow()
-        {
-            // find out Activity Id length on this platform in this AppDomain
-            Activity testActivity = new Activity("activity")
-                .Start();
-            var expectedIdLength = testActivity.Id.Length;
-            testActivity.Stop();
-
-            // check that if parentId '|aaa...a' 1024 bytes long is set with single node (no dots or underscores in the Id)
-            // it causes overflow during Id generation, and new root Id is generated for the new Activity
-            var parentId = '|' + new string('a', 1022) + '.';
-
-            var activity = new Activity("activity")
-                .SetParentId(parentId)
-                .Start();
-
-            Assert.Equal(parentId, activity.ParentId);
-
-            // With probability 1/MaxLong, Activity.Id length may be expectedIdLength + 1
-            Assert.InRange(activity.Id.Length, expectedIdLength, expectedIdLength + 1);
-            Assert.DoesNotContain('#', activity.Id);
-        }
 
         /// <summary>
         /// Tests activity start and stop
@@ -258,6 +232,7 @@ namespace System.Diagnostics.Tests
         public void IdGenerationInternalParent()
         {
             var parent = new Activity("parent");
+            parent.SetIdFormat(ActivityIdFormat.Hierarchical);
             parent.Start();
             var child1 = new Activity("child1");
             var child2 = new Activity("child2");
@@ -535,11 +510,11 @@ namespace System.Diagnostics.Tests
         /****** WC3 Format tests *****/
 
         [Fact]
-        public void IdFormat_HierarchicalIsDefault()
+        public void IdFormat_W3CIsDefaultForNet5()
         {
             Activity activity = new Activity("activity1");
             activity.Start();
-            Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
+            Assert.Equal(PlatformDetection.IsNetCore ? ActivityIdFormat.W3C : ActivityIdFormat.Hierarchical, activity.IdFormat);
         }
 
         [Fact]
@@ -618,6 +593,20 @@ namespace System.Diagnostics.Tests
         }
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void IdFormat_WithTheEnvironmentSwitch()
+        {
+            var psi = new ProcessStartInfo();
+            psi.Environment.Add("DOTNET_SYSTEM_DIAGNOSTICS_DEFAULTACTIVITYIDFORMATISHIERARCHIAL", "true");
+
+            RemoteExecutor.Invoke(() =>
+            {
+                Activity activity = new Activity("activity15");
+                activity.Start();
+                 Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
+            }, new RemoteInvokeOptions() { StartInfo = psi }).Dispose();
+        }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public void IdFormat_HierarchicalWhenDefaultIsW3CButHierarchicalParentId()
         {
             RemoteExecutor.Invoke(() =>
@@ -633,13 +622,23 @@ namespace System.Diagnostics.Tests
         }
 
         [Fact]
-        public void IdFormat_ZeroTraceIdAndSpanIdWithHierarchicalFormat()
+        public void IdFormat_ZeroTraceIdAndSpanIdWithW3CFormat()
         {
             Activity activity = new Activity("activity");
             activity.Start();
-            Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
-            Assert.Equal("00000000000000000000000000000000", activity.TraceId.ToHexString());
-            Assert.Equal("0000000000000000", activity.SpanId.ToHexString());
+
+            if (PlatformDetection.IsNetCore)
+            {
+                Assert.Equal(ActivityIdFormat.W3C, activity.IdFormat);
+                Assert.NotEqual("00000000000000000000000000000000", activity.TraceId.ToHexString());
+                Assert.NotEqual("0000000000000000", activity.SpanId.ToHexString());
+            }
+            else
+            {
+                Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
+                Assert.Equal("00000000000000000000000000000000", activity.TraceId.ToHexString());
+                Assert.Equal("0000000000000000", activity.SpanId.ToHexString());
+            }
         }
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/ActivityTests.cs
new file mode 100644 (file)
index 0000000..e3b64f2
--- /dev/null
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/ActivityTests.cs
@@ -0,0 +1,106 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Threading.Tasks;
+using Xunit;
+
+namespace System.Diagnostics.Tests
+{
+    public class ActivityTests : IDisposable
+    {
+        [Fact]
+        public void ActivityIdNonHierarchicalOverflow()
+        {
+            // find out Activity Id length on this platform in this AppDomain
+            Activity testActivity = new Activity("activity")
+                .Start();
+            var expectedIdLength = testActivity.Id.Length;
+            testActivity.Stop();
+
+            // check that if parentId '|aaa...a' 1024 bytes long is set with single node (no dots or underscores in the Id)
+            // it causes overflow during Id generation, and new root Id is generated for the new Activity
+            var parentId = '|' + new string('a', 1022) + '.';
+
+            var activity = new Activity("activity")
+                .SetParentId(parentId)
+                .Start();
+
+            Assert.Equal(parentId, activity.ParentId);
+
+            // With probability 1/MaxLong, Activity.Id length may be expectedIdLength + 1
+            Assert.InRange(activity.Id.Length, expectedIdLength, expectedIdLength + 1);
+            Assert.DoesNotContain('#', activity.Id);
+        }
+
+        [Fact]
+        public void IdGenerationInternalParent()
+        {
+            var parent = new Activity("parent");
+            parent.Start();
+            var child1 = new Activity("child1");
+            var child2 = new Activity("child2");
+            //start 2 children in different execution contexts
+            Task.Run(() => child1.Start()).Wait();
+            Task.Run(() => child2.Start()).Wait();
+
+            // In Debug builds of System.Diagnostics.DiagnosticSource, the child operation Id will be constructed as follows
+            // "|parent.RootId.<child.OperationName.Replace(., -)>-childCount.".
+            // This is for debugging purposes to know which operation the child Id is comming from.
+            //
+            // In Release builds of System.Diagnostics.DiagnosticSource, it will not contain the operation name to keep it simple and it will be as
+            // "|parent.RootId.childCount.".
+
+            string child1DebugString = $"|{parent.RootId}.{child1.OperationName}-1.";
+            string child2DebugString = $"|{parent.RootId}.{child2.OperationName}-2.";
+            string child1ReleaseString = $"|{parent.RootId}.1.";
+            string child2ReleaseString = $"|{parent.RootId}.2.";
+
+            AssertExtensions.AtLeastOneEquals(child1DebugString, child1ReleaseString, child1.Id);
+            AssertExtensions.AtLeastOneEquals(child2DebugString, child2ReleaseString, child2.Id);
+
+            Assert.Equal(parent.RootId, child1.RootId);
+            Assert.Equal(parent.RootId, child2.RootId);
+            child1.Stop();
+            child2.Stop();
+            var child3 = new Activity("child3");
+            child3.Start();
+
+            string child3DebugString = $"|{parent.RootId}.{child3.OperationName}-3.";
+            string child3ReleaseString = $"|{parent.RootId}.3.";
+
+            AssertExtensions.AtLeastOneEquals(child3DebugString, child3ReleaseString, child3.Id);
+
+            var grandChild = new Activity("grandChild");
+            grandChild.Start();
+
+            child3DebugString = $"{child3.Id}{grandChild.OperationName}-1.";
+            child3ReleaseString = $"{child3.Id}1.";
+
+            AssertExtensions.AtLeastOneEquals(child3DebugString, child3ReleaseString, grandChild.Id);
+        }
+
+        [Fact]
+        public void IdFormat_HierarchicalIsDefault()
+        {
+            Activity activity = new Activity("activity1");
+            activity.Start();
+            Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
+        }
+
+        [Fact]
+        public void IdFormat_ZeroTraceIdAndSpanIdWithHierarchicalFormat()
+        {
+            Activity activity = new Activity("activity");
+            activity.Start();
+            Assert.Equal(ActivityIdFormat.Hierarchical, activity.IdFormat);
+            Assert.Equal("00000000000000000000000000000000", activity.TraceId.ToHexString());
+            Assert.Equal("0000000000000000", activity.SpanId.ToHexString());
+        }
+
+        public void Dispose()
+        {
+            Activity.Current = null;
+        }
+    }
+}

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/System.Diagnostics.DiagnosticSource.Switches.Tests.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/System.Diagnostics.DiagnosticSource.Switches.Tests.csproj
new file mode 100644 (file)
index 0000000..04a992f
--- /dev/null
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/System.Diagnostics.DiagnosticSource.Switches.Tests.csproj
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
+    <TestRuntime>true</TestRuntime>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="ActivityTests.cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/runtimeconfig.template.json b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/runtimeconfig.template.json
new file mode 100644 (file)
index 0000000..1b600a9
--- /dev/null
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/TestWithConfigSwitches/runtimeconfig.template.json
@@ -0,0 +1,5 @@
+{
+  "configProperties": {
+    "System.Diagnostics.DefaultActivityIdFormatIsHierarchial": true
+  }
+}
\ No newline at end of file

diff --git a/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj b/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj
index 0291615..7bb92b1 100644 (file)
--- a/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj
+++ b/src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj
@@ -219,7 +219,7 @@
     <Compile Include="misc\GDI\DeviceContextType.cs" />
     <Compile Include="misc\GDI\WindowsGraphics.cs" />
     <Compile Include="misc\GDI\WindowsRegion.cs" />
-    <Compile Include="$(CoreLibSharedDir)System\LocalAppContextSwitches.Common.cs"
+    <Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs"
              Link="System\LocalAppContextSwitches.Common.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs"
              Link="Common\Interop\Windows\Interop.Libraries.cs" />

diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
index d9c25f1..fcb68d7 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
+++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
@@ -403,7 +403,6 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\LazyOfTTMetadata.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\LoaderOptimization.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\LoaderOptimizationAttribute.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\LocalAppContextSwitches.Common.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\LocalAppContextSwitches.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\LocalDataStoreSlot.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\MarshalByRefObject.cs" />
@@ -1084,6 +1083,9 @@
     <Compile Include="$(CommonPath)SkipLocalsInit.cs">
       <Link>Common\SkipLocalsInit.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs">
+      <Link>Common\System\LocalAppContextSwitches.Common.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)System\HResults.cs">
       <Link>Common\System\HResults.cs</Link>
     </Compile>

diff --git a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
index 833bf50..1c9982e 100644 (file)
--- a/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
+++ b/src/libraries/System.Private.Xml/src/System.Private.Xml.csproj
@@ -791,7 +791,7 @@
     <Compile Include="System\Xml\Xsl\Xslt\XsltLoader.cs" />
     <Compile Include="System\Xml\Xsl\Xslt\XsltQilFactory.cs" />
     <Compile Include="System\Xml\Xsl\Xslt\XslVisitor.cs" />
-    <Compile Include="$(CoreLibSharedDir)System\LocalAppContextSwitches.Common.cs"
+    <Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs"
              Link="System\LocalAppContextSwitches.Common.cs" />
     <Compile Include="System\Xml\Core\LocalAppContextSwitches.cs" />
     <Compile Include="$(CommonPath)System\CSharpHelpers.cs" />