From fd788c5b9de0d2c4d7072545873e98a895929ead Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 18 Apr 2018 21:25:00 -0700 Subject: [PATCH 1/2] Introduces public setter for Activity #29207 --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 2 +- .../Diagnostics/Activity.Current.net45.cs | 29 ++++++---- .../Diagnostics/Activity.Current.net46.cs | 17 +++++- .../src/System/Diagnostics/Activity.cs | 19 ++++++- .../tests/ActivityTests.cs | 56 +++++++++++++++++++ ....Diagnostics.DiagnosticSource.Tests.csproj | 3 + 6 files changed, 108 insertions(+), 18 deletions(-) diff --git a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index 8ac4cec10959..f566a5150e47 100644 --- a/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -34,7 +34,7 @@ public static Activity Current #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - private set {} + set {} } } public abstract partial class DiagnosticSource { diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs index 21fbee09a603..3a93d00edfe2 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs @@ -11,7 +11,7 @@ namespace System.Diagnostics public partial class Activity { /// - /// Returns the current operation (Activity) for the current thread. This flows + /// Gets or sets the current operation (Activity) for the current thread. This flows /// across async calls. /// public static Activity Current @@ -22,7 +22,7 @@ public static Activity Current get { ObjectHandle activityHandle = (ObjectHandle)CallContext.LogicalGetData(FieldKey); - + // Unwrap the Activity if it was set in the same AppDomain (as FieldKey is AppDomain-specific). if (activityHandle != null) { @@ -30,29 +30,36 @@ public static Activity Current } return null; } - + #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - private set + set { - // Applications may implicitly or explicitly call other AppDomains - // that do not have DiagnosticSource DLL, therefore may not be able to resolve Activity type stored in the logical call context. - // To avoid it, we wrap Activity with ObjectHandle. - CallContext.LogicalSetData(FieldKey, new ObjectHandle(value)); + if (ValidateSetCurrent) + { + SetCurrent(value); + } } } #region private - private partial class KeyValueListNode +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + [System.Security.SecuritySafeCriticalAttribute] +#endif + private static void SetCurrent(Activity activity) { + // Applications may implicitly or explicitly call other AppDomains + // that do not have DiagnosticSource DLL, therefore may not be able to resolve Activity type stored in the logical call context. + // To avoid it, we wrap Activity with ObjectHandle. + CallContext.LogicalSetData(FieldKey, new ObjectHandle(activity)); } // Slot name depends on the AppDomain Id in order to prevent AppDomains to use the same Activity // Cross AppDomain calls are considered as 'external' i.e. only Activity Id and Baggage should be propagated and // new Activity should be started for the RPC calls (incoming and outgoing) private static readonly string FieldKey = $"{typeof(Activity).FullName}_{AppDomain.CurrentDomain.Id}"; -#endregion +#endregion //private } -} \ No newline at end of file +} diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs index 57795ebf55b0..735367287937 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs @@ -9,17 +9,28 @@ namespace System.Diagnostics public partial class Activity { /// - /// Returns the current operation (Activity) for the current thread. This flows + /// Gets or sets the current operation (Activity) for the current thread. This flows /// across async calls. /// public static Activity Current { get { return s_current.Value; } - private set { s_current.Value = value; } + set + { + if (ValidateSetCurrent(value)) + { + SetCurrent(value); + } + } } #region private + private static void SetCurrent(Activity activity) + { + s_current.Value = activity; + } + private static readonly AsyncLocal s_current = new AsyncLocal(); #endregion // private } -} \ No newline at end of file +} diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index 34f24635a634..c934eccc39dd 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -3,7 +3,9 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; -using System.Security; +#if ALLOW_PARTIALLY_TRUSTED_CALLERS + using System.Security; +#endif using System.Threading; namespace System.Diagnostics @@ -308,7 +310,7 @@ public Activity Start() } Id = GenerateId(); - Current = this; + SetCurrent(this); } return this; } @@ -337,7 +339,7 @@ public void Stop() SetEndTime(GetUtcNow()); } - Current = Parent; + SetCurrent(Parent); } } @@ -446,6 +448,17 @@ private static unsafe long GetRandomNumber() return *((long*)&g); } + private static bool ValidateSetCurrent(Activity activity) + { + bool canSet = activity == null || (activity.Id != null && !activity.isFinished); + if (!canSet) + { + NotifyError(new InvalidOperationException("Trying to set an Activity that is not running")); + } + + return canSet; + } + private string _rootId; private int _currentChildId; // A unique number for all children of this activity. diff --git a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 7f7744376ecc..390479226d05 100644 --- a/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -532,6 +532,62 @@ public async Task ActivityCurrentFlowsWithAsyncComplex() Assert.Same(originalActivity, Activity.Current); } + /// + /// Tests that Activity.Current could be set + /// + [Fact] + public async Task ActivityCurrentSet() + { + Activity activity = new Activity("activity"); + + // start Activity in the 'child' context + await Task.Run(() => activity.Start()); + + Assert.Null(Activity.Current); + Activity.Current = activity; + Assert.Same(activity, Activity.Current); + } + + /// + /// Tests that Activity.Current could be set to null + /// + [Fact] + public void ActivityCurrentSetToNull() + { + Activity started = new Activity("started").Start(); + + Activity.Current = null; + Assert.Null(Activity.Current); + } + + /// + /// Tests that Activity.Current could not be set to Activity + /// that has not been started yet + /// + [Fact] + public void ActivityCurrentNotSetToNotStarted() + { + Activity started = new Activity("started").Start(); + Activity notStarted = new Activity("notStarted"); + + Activity.Current = notStarted; + Assert.Same(started, Activity.Current); + } + + /// + /// Tests that Activity.Current could not be set to stopped Activity + /// + [Fact] + public void ActivityCurrentNotSetToStopped() + { + Activity started = new Activity("started").Start(); + Activity stopped = new Activity("stopped").Start(); + stopped.Stop(); + + Activity.Current = stopped; + Assert.Same(started, Activity.Current); + } + private class TestObserver : IObserver> { public string EventName { get; private set; } diff --git a/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj b/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj index 86316971ef75..82509a5f5c6c 100644 --- a/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj +++ b/src/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj @@ -36,5 +36,8 @@ Common\System\Net\Configuration.Http.cs + + + \ No newline at end of file From 2b198a2844f4fbfacff4d30f7036058b7f05576f Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Fri, 20 Apr 2018 13:19:47 -0700 Subject: [PATCH 2/2] fix build failure on .NET 4.5 --- .../src/System/Diagnostics/Activity.Current.net45.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs index 3a93d00edfe2..1c798c5455ae 100644 --- a/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs +++ b/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net45.cs @@ -36,7 +36,7 @@ public static Activity Current #endif set { - if (ValidateSetCurrent) + if (ValidateSetCurrent(value)) { SetCurrent(value); }