From: Ben Adams Date: Wed, 19 Jun 2019 14:21:25 +0000 (+0100) Subject: Shrink System.Diagnostics.Activity by 40 bytes (dotnet/corefx#37395) X-Git-Tag: submit/tizen/20210909.063632~11031^2~1244 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ae726460003eead34b8e82005a7405acd8ad0da5;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Shrink System.Diagnostics.Activity by 40 bytes (dotnet/corefx#37395) * Shrink Activity * fix threading issues * Handle parsing issues * Fix Commit migrated from https://github.com/dotnet/corefx/commit/edc88b0fe55278ee43c821ab05707d8bb0befded --- diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index e161ab6..f3a24b4 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -34,20 +34,20 @@ namespace System.Diagnostics public string OperationName { get { throw null; } } public System.Diagnostics.Activity Parent { get { throw null; } } public string ParentId { get { throw null; } } - public ref readonly System.Diagnostics.ActivitySpanId ParentSpanId { get { throw null; } } + public System.Diagnostics.ActivitySpanId ParentSpanId { get { throw null; } } public bool Recorded { get { throw null; } } public string RootId { get { throw null; } } - public ref readonly System.Diagnostics.ActivitySpanId SpanId { get { throw null; } } + public System.Diagnostics.ActivitySpanId SpanId { get { throw null; } } public System.DateTime StartTimeUtc { get { throw null; } } public System.Collections.Generic.IEnumerable> Tags { get { throw null; } } - public ref readonly System.Diagnostics.ActivityTraceId TraceId { get { throw null; } } + public System.Diagnostics.ActivityTraceId TraceId { get { throw null; } } public string TraceStateString { get { throw null; } set { } } public System.Diagnostics.ActivityTraceFlags ActivityTraceFlags { get { throw null; } set { } } public System.Diagnostics.Activity AddBaggage(string key, string value) { throw null; } public System.Diagnostics.Activity AddTag(string key, string value) { throw null; } public string GetBaggageItem(string key) { throw null; } public System.Diagnostics.Activity SetEndTime(System.DateTime endTimeUtc) { throw null; } - public System.Diagnostics.Activity SetParentId(in System.Diagnostics.ActivityTraceId traceId, in System.Diagnostics.ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None) { throw null; } + public System.Diagnostics.Activity SetParentId(System.Diagnostics.ActivityTraceId traceId, System.Diagnostics.ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None) { throw null; } public System.Diagnostics.Activity SetParentId(string parentId) { throw null; } public System.Diagnostics.Activity SetStartTime(System.DateTime startTimeUtc) { throw null; } public System.Diagnostics.Activity Start() { throw null; } @@ -65,7 +65,6 @@ namespace System.Diagnostics public readonly partial struct ActivitySpanId : System.IEquatable { private readonly object _dummy; - private readonly int _dummyPrimitive; public void CopyTo(System.Span destination) { } public static System.Diagnostics.ActivitySpanId CreateFromBytes(System.ReadOnlySpan idData) { throw null; } public static System.Diagnostics.ActivitySpanId CreateFromString(System.ReadOnlySpan idData) { throw null; } @@ -74,8 +73,8 @@ namespace System.Diagnostics public bool Equals(System.Diagnostics.ActivitySpanId spanId) { throw null; } public override bool Equals(object obj) { throw null; } public override int GetHashCode() { throw null; } - public static bool operator ==(in System.Diagnostics.ActivitySpanId spanId1, in System.Diagnostics.ActivitySpanId spandId2) { throw null; } - public static bool operator !=(in System.Diagnostics.ActivitySpanId spanId1, in System.Diagnostics.ActivitySpanId spandId2) { throw null; } + public static bool operator ==(System.Diagnostics.ActivitySpanId spanId1, System.Diagnostics.ActivitySpanId spandId2) { throw null; } + public static bool operator !=(System.Diagnostics.ActivitySpanId spanId1, System.Diagnostics.ActivitySpanId spandId2) { throw null; } public string ToHexString() { throw null; } public override string ToString() { throw null; } } @@ -85,7 +84,6 @@ namespace System.Diagnostics public readonly partial struct ActivityTraceId : System.IEquatable { private readonly object _dummy; - private readonly int _dummyPrimitive; public void CopyTo(System.Span destination) { } public static System.Diagnostics.ActivityTraceId CreateFromBytes(System.ReadOnlySpan idData) { throw null; } public static System.Diagnostics.ActivityTraceId CreateFromString(System.ReadOnlySpan idData) { throw null; } @@ -94,8 +92,8 @@ namespace System.Diagnostics public bool Equals(System.Diagnostics.ActivityTraceId traceId) { throw null; } public override bool Equals(object obj) { throw null; } public override int GetHashCode() { throw null; } - public static bool operator ==(in System.Diagnostics.ActivityTraceId traceId1, in System.Diagnostics.ActivityTraceId traceId2) { throw null; } - public static bool operator !=(in System.Diagnostics.ActivityTraceId traceId1, in System.Diagnostics.ActivityTraceId traceId2) { throw null; } + public static bool operator ==(System.Diagnostics.ActivityTraceId traceId1, System.Diagnostics.ActivityTraceId traceId2) { throw null; } + public static bool operator !=(System.Diagnostics.ActivityTraceId traceId1, System.Diagnostics.ActivityTraceId traceId2) { throw null; } public string ToHexString() { throw null; } public override string ToString() { throw null; } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs index 7353672..8f4435c 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.Current.net46.cs @@ -8,6 +8,8 @@ namespace System.Diagnostics { public partial class Activity { + private static readonly AsyncLocal s_current = new AsyncLocal(); + /// /// Gets or sets the current operation (Activity) for the current thread. This flows /// across async calls. @@ -24,13 +26,9 @@ namespace System.Diagnostics } } -#region private private static void SetCurrent(Activity activity) { s_current.Value = activity; } - - private static readonly AsyncLocal s_current = new AsyncLocal(); -#endregion // private } } 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 61b692c..0e020e6 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -6,6 +6,7 @@ using System.Buffers.Binary; using System.Buffers.Text; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Text; using System.Threading; @@ -32,6 +33,44 @@ namespace System.Diagnostics { private static readonly IEnumerable> s_emptyBaggageTags = new KeyValuePair[0]; // Array.Empty() doesn't exist in all configurations + private const byte ActivityTraceFlagsIsSet = 0b_1_0000000; // Internal flag to indicate if flags have been set + private const int RequestIdMaxLength = 1024; + + // Used to generate an ID it represents the machine and process we are in. + private static readonly string s_uniqSuffix = "-" + GetRandomNumber().ToString("x") + "."; + + // A unique number inside the appdomain, randomized between appdomains. + // Int gives enough randomization and keeps hex-encoded s_currentRootId 8 chars long for most applications + private static long s_currentRootId = (uint)GetRandomNumber(); + private static ActivityIdFormat s_defaultIdFormat; + /// + /// 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 + /// ID format will always be the DefaultIdFormat even if the ParentID is define and is + /// a different format. + /// + public static bool ForceDefaultIdFormat { get; set; } + + private string _traceState; + private State _state; + private int _currentChildId; // A unique number for all children of this activity. + + // State associated with ID. + private string _id; + private string _rootId; + // State associated with ParentId. + private string _parentId; + + // W3C formats + private string _parentSpanId; + private string _traceId; + private string _spanId; + + private byte _w3CIdFlags; + + private KeyValueListNode _tags; + private KeyValueListNode _baggage; + /// /// An operation name is a COARSEST name that is useful grouping/filtering. /// The name is typically a compile-time constant. Names of Rest APIs are @@ -41,6 +80,27 @@ namespace System.Diagnostics public string OperationName { get; } /// + /// If the Activity that created this activity is from the same process you can get + /// that Activity with Parent. However, this can be null if the Activity has no + /// parent (a root activity) or if the Parent is from outside the process. + /// + /// + public Activity Parent { get; private set; } + + /// + /// If the Activity has ended ( or was called) then this is the delta + /// between and end. If Activity is not ended and was not called then this is + /// . + /// + public TimeSpan Duration { get; private set; } + + /// + /// The time that operation started. It will typically be initialized when + /// is called, but you can set at any time via . + /// + public DateTime StartTimeUtc { get; private set; } + + /// /// This is an ID that is specific to a particular request. Filtering /// to a particular ID insures that you get only one request that matches. /// Id has a hierarchical structure: '|root-id.id1_id2.id3_' Id is generated when @@ -65,12 +125,14 @@ namespace System.Diagnostics { // if we represented it as a traceId-spanId, convert it to a string. // We can do this concatenation with a stackalloced Span if we actually used Id a lot. - if (_id == null && _spanIdSet) + if (_id == null && _spanId != null) { // Convert flags to binary. Span flagsChars = stackalloc char[2]; - ActivityTraceId.ByteToHexDigits(flagsChars, _w3CIdFlags); - _id = "00-" + _traceId.ToHexString() + "-" + _spanId.ToHexString() + "-" + flagsChars.ToString(); + ActivityTraceId.ByteToHexDigits(flagsChars, (byte)((~ActivityTraceFlagsIsSet) & _w3CIdFlags)); + string id = "00-" + _traceId + "-" + _spanId + "-" + flagsChars.ToString(); + + Interlocked.CompareExchange(ref _id, id, null); } return _id; @@ -78,20 +140,6 @@ namespace System.Diagnostics } /// - /// The time that operation started. It will typically be initialized when - /// is called, but you can set at any time via . - /// - public DateTime StartTimeUtc { get; private set; } - - /// - /// If the Activity that created this activity is from the same process you can get - /// that Activity with Parent. However, this can be null if the Activity has no - /// parent (a root activity) or if the Parent is from outside the process. - /// - /// - public Activity Parent { get; private set; } - - /// /// If the parent for this activity comes from outside the process, the activity /// does not have a Parent Activity but MAY have a ParentId (which was deserialized from /// from the parent). This accessor fetches the parent ID if it exists at all. @@ -106,11 +154,17 @@ namespace System.Diagnostics // if we represented it as a traceId-spanId, convert it to a string. if (_parentId == null) { - if (_parentSpanIdSet) - _parentId = "00-" + _traceId.ToHexString() + "-" + _parentSpanId.ToHexString() + "-00"; + if (_parentSpanId != null) + { + string parentId = "00-" + _traceId + "-" + _parentSpanId + "-00"; + Interlocked.CompareExchange(ref _parentId, parentId, null); + } else if (Parent != null) - _parentId = Parent.Id; + { + Interlocked.CompareExchange(ref _parentId, Parent.Id, null); + } } + return _parentId; } } @@ -130,15 +184,22 @@ namespace System.Diagnostics //Presumably, it will be called by logging systems for every log record, so we cache it. if (_rootId == null) { + string rootId = null; if (Id != null) { - _rootId = GetRootId(Id); + rootId = GetRootId(Id); } else if (ParentId != null) { - _rootId = GetRootId(ParentId); + rootId = GetRootId(ParentId); + } + + if (rootId != null) + { + Interlocked.CompareExchange(ref _rootId, rootId, null); } } + return _rootId; } } @@ -246,7 +307,14 @@ namespace System.Diagnostics /// 'this' for convenient chaining public Activity AddTag(string key, string value) { - _tags = new KeyValueListNode() { keyValue = new KeyValuePair(key, value), Next = _tags }; + KeyValueListNode currentTags = _tags; + KeyValueListNode newTags = new KeyValueListNode() { keyValue = new KeyValuePair(key, value) }; + do + { + newTags.Next = currentTags; + currentTags = Interlocked.CompareExchange(ref _tags, newTags, currentTags); + } while (!ReferenceEquals(newTags.Next, currentTags)); + return this; } @@ -261,7 +329,15 @@ namespace System.Diagnostics /// 'this' for convenient chaining public Activity AddBaggage(string key, string value) { - _baggage = new KeyValueListNode() { keyValue = new KeyValuePair(key, value), Next = _baggage }; + KeyValueListNode currentBaggage = _baggage; + KeyValueListNode newBaggage = new KeyValueListNode() { keyValue = new KeyValuePair(key, value) }; + + do + { + newBaggage.Next = currentBaggage; + currentBaggage = Interlocked.CompareExchange(ref _baggage, newBaggage, currentBaggage); + } while (!ReferenceEquals(newBaggage.Next, currentBaggage)); + return this; } @@ -280,7 +356,7 @@ namespace System.Diagnostics { NotifyError(new InvalidOperationException($"Trying to set {nameof(ParentId)} on activity which has {nameof(Parent)}")); } - else if (ParentId != null || _parentSpanIdSet) + else if (ParentId != null || _parentSpanId != null) { NotifyError(new InvalidOperationException($"{nameof(ParentId)} is already set")); } @@ -299,24 +375,21 @@ namespace System.Diagnostics /// Set the parent ID using the W3C convention using a TraceId and a SpanId. This /// constructor has the advantage that no string manipulation is needed to set the ID. /// - public Activity SetParentId(in ActivityTraceId traceId, in ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None) + public Activity SetParentId(ActivityTraceId traceId, ActivitySpanId spanId, ActivityTraceFlags activityTraceFlags = ActivityTraceFlags.None) { if (Parent != null) { NotifyError(new InvalidOperationException($"Trying to set {nameof(ParentId)} on activity which has {nameof(Parent)}")); } - else if (ParentId != null || _parentSpanIdSet) + else if (ParentId != null || _parentSpanId != null) { NotifyError(new InvalidOperationException($"{nameof(ParentId)} is already set")); } else { - _traceId = traceId; // The child will share the parent's traceId. - _traceIdSet = true; - _parentSpanId = spanId; - _parentSpanIdSet = true; - _w3CIdFlags = (byte)activityTraceFlags; - _w3CIdFlagsSet = true; + _traceId = traceId.ToHexString(); // The child will share the parent's traceId. + _parentSpanId = spanId.ToHexString(); + ActivityTraceFlags = activityTraceFlags; } return this; } @@ -362,13 +435,6 @@ namespace System.Diagnostics } /// - /// If the Activity has ended ( or was called) then this is the delta - /// between and end. If Activity is not ended and was not called then this is - /// . - /// - public TimeSpan Duration { get; private set; } - - /// /// Starts activity /// /// Sets to hold . @@ -383,13 +449,13 @@ namespace System.Diagnostics public Activity Start() { // Has the ID already been set (have we called Start()). - if (_id != null || _spanIdSet) + if (_id != null || _spanId != null) { NotifyError(new InvalidOperationException("Trying to start an Activity that was already started")); } else { - if (_parentId == null && !_parentSpanIdSet) + if (_parentId == null && _parentSpanId is null) { Activity parent = Current; if (parent != null) @@ -401,7 +467,7 @@ namespace System.Diagnostics } } - if (StartTimeUtc == default(DateTime)) + if (StartTimeUtc == default) StartTimeUtc = GetUtcNow(); // Figure out what format to use. @@ -409,7 +475,7 @@ namespace System.Diagnostics IdFormat = DefaultIdFormat; else if (Parent != null) IdFormat = Parent.IdFormat; - else if (_parentSpanIdSet) + else if (_parentSpanId != null) IdFormat = ActivityIdFormat.W3C; else if (_parentId != null) IdFormat = IsW3CId(_parentId) ? ActivityIdFormat.W3C : ActivityIdFormat.Hierarchical; @@ -442,9 +508,9 @@ namespace System.Diagnostics return; } - if (!isFinished) + if (!IsFinished) { - isFinished = true; + IsFinished = true; if (Duration == TimeSpan.Zero) { @@ -490,55 +556,43 @@ namespace System.Diagnostics /// /// If the Activity has the W3C format, this returns the ID for the SPAN part of the Id. - /// Otherwise it returns a zero SpanId. - /// - /// Note that this returns a readonly ref. This is because SpanId has a cache of the - /// Hex string. However for that cache to be available for subsequent conversions to - /// Hex, you have to be operating on the same instance. Thus if you need the Hex string - /// you should store the SpanId in a ref local variable and call the AsHexString property - /// which will have the effect of updating Activity's instance so all subsequent uses - /// share the same converted string. + /// Otherwise it returns a zero SpanId. /// - public ref readonly ActivitySpanId SpanId + public ActivitySpanId SpanId { #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif get { - if (!_spanIdSet) + if (_spanId is null) { if (_id != null && IdFormat == ActivityIdFormat.W3C) { - _spanId = ActivitySpanId.CreateFromString(_id.AsSpan(36, 16)); - _spanIdSet = true; + ActivitySpanId activitySpanId = ActivitySpanId.CreateFromString(_id.AsSpan(36, 16)); + string spanId = activitySpanId.ToHexString(); + + Interlocked.CompareExchange(ref _spanId, spanId, null); } } - return ref _spanId; + return new ActivitySpanId(_spanId); } } /// /// If the Activity has the W3C format, this returns the ID for the TraceId part of the Id. /// Otherwise it returns a zero TraceId. - /// - /// Note that this returns a readonly ref. This is because TraceId has a cache of the - /// Hex string. However for that cache to be available for subsequent conversions to - /// Hex, you have to be operating on the same instance. Thus if you need the Hex string - /// you should store the TraceId in a ref local variable and call the AsHexString property - /// which will have the effect of updating Activity's instance so all subsequent uses - /// share the same converted string. /// - public ref readonly ActivityTraceId TraceId + public ActivityTraceId TraceId { get { - if (!_traceIdSet) + if (_traceId is null) { TrySetTraceIdFromParent(); } - return ref _traceId; + return new ActivityTraceId(_traceId); } } @@ -547,9 +601,6 @@ namespace System.Diagnostics /// public bool Recorded { get => (ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0; } - byte _w3CIdFlags; - bool _w3CIdFlagsSet; - /// /// Return the flags (defined by the W3C ID specification) associated with the activity. /// @@ -557,16 +608,15 @@ namespace System.Diagnostics { get { - if (!_w3CIdFlagsSet) + if (!W3CIdFlagsSet) { - _w3CIdFlagsSet = TrySetTraceFlagsFromParent(); + TrySetTraceFlagsFromParent(); } - return (ActivityTraceFlags)_w3CIdFlags; + return (ActivityTraceFlags)((~ActivityTraceFlagsIsSet) & _w3CIdFlags); } set { - _w3CIdFlagsSet = true; - _w3CIdFlags = (byte)value; + _w3CIdFlags = (byte)(ActivityTraceFlagsIsSet | (byte)value); } } @@ -574,39 +624,38 @@ namespace System.Diagnostics /// If the parent Activity ID has the W3C format, this returns the ID for the SpanId part of the ParentId. /// Otherwise it returns a zero SpanId. /// - public ref readonly ActivitySpanId ParentSpanId + public ActivitySpanId ParentSpanId { #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif get { - if (!_parentSpanIdSet) + if (_parentSpanId is null) { + string parentSpanId = null; if (_parentId != null && IsW3CId(_parentId)) { try { - _parentSpanId = ActivitySpanId.CreateFromString(_parentId.AsSpan(36, 16)); + parentSpanId = ActivitySpanId.CreateFromString(_parentId.AsSpan(36, 16)).ToHexString(); } catch { } - _parentSpanIdSet = true; } else if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C) { - _parentSpanId = Parent.SpanId; - _parentSpanIdSet = true; + parentSpanId = Parent.SpanId.ToHexString(); + } + + if (parentSpanId != null) + { + Interlocked.CompareExchange(ref _parentSpanId, parentSpanId, null); } } - return ref _parentSpanId; + return new ActivitySpanId(_parentSpanId); } } - /// - /// Returns the format for the ID. - /// - public ActivityIdFormat IdFormat { get; private set; } - /* static state (configuration) */ /// /// Activity tries to use the same format for IDs as its parent. @@ -617,28 +666,19 @@ namespace System.Diagnostics { get { - if (s_DefaultIdFormat == ActivityIdFormat.Unknown) - s_DefaultIdFormat = ActivityIdFormat.Hierarchical; - return s_DefaultIdFormat; + if (s_defaultIdFormat == ActivityIdFormat.Unknown) + s_defaultIdFormat = ActivityIdFormat.Hierarchical; + return s_defaultIdFormat; } set { if (!(ActivityIdFormat.Hierarchical <= value && value <= ActivityIdFormat.W3C)) throw new ArgumentException($"value must be a valid ActivityIDFormat value"); - s_DefaultIdFormat = value; + s_defaultIdFormat = value; } } /// - /// 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 - /// ID format will always be the DefaultIdFormat even if the ParentID is define and is - /// a different format. - /// - public static bool ForceDefaultIdFormat { get; set; } - - #region private - /// /// Returns true if 'id' has the format of a WC3 id see https://w3c.github.io/trace-context /// private static bool IsW3CId(string id) @@ -663,24 +703,25 @@ namespace System.Diagnostics /// private void GenerateW3CId() { + // Called from .Start() + // Get the TraceId from the parent or make a new one. - if (!_traceIdSet) + if (_traceId is null) { if (!TrySetTraceIdFromParent()) { - _traceId = ActivityTraceId.CreateRandom(); - _traceIdSet = true; + _traceId = ActivityTraceId.CreateRandom().ToHexString(); } } - if (!_w3CIdFlagsSet) + if (!W3CIdFlagsSet) { - _w3CIdFlagsSet = TrySetTraceFlagsFromParent(); + TrySetTraceFlagsFromParent(); } // Create a new SpanID. - _spanId = ActivitySpanId.CreateRandom(); - _spanIdSet = true; + + _spanId = ActivitySpanId.CreateRandom().ToHexString(); } private static void NotifyError(Exception exception) @@ -701,6 +742,7 @@ namespace System.Diagnostics /// private string GenerateHierarchicalId() { + // Called from .Start() string ret; if (Parent != null) { @@ -789,7 +831,7 @@ namespace System.Diagnostics private static bool ValidateSetCurrent(Activity activity) { - bool canSet = activity == null || (activity.Id != null && !activity.isFinished); + bool canSet = activity == null || (activity.Id != null && !activity.IsFinished); if (!canSet) { NotifyError(new InvalidOperationException("Trying to set an Activity that is not running")); @@ -803,36 +845,34 @@ namespace System.Diagnostics #endif private bool TrySetTraceIdFromParent() { - Debug.Assert(!_traceIdSet); + Debug.Assert(_traceId is null); if (Parent != null && Parent.IdFormat == ActivityIdFormat.W3C) { - _traceId = Parent.TraceId; - _traceIdSet = true; + _traceId = Parent.TraceId.ToHexString(); } else if (_parentId != null && IsW3CId(_parentId)) { try { - _traceId = ActivityTraceId.CreateFromString(_parentId.AsSpan(3, 32)); - _traceIdSet = true; + _traceId = ActivityTraceId.CreateFromString(_parentId.AsSpan(3, 32)).ToHexString(); } catch { } } - return _traceIdSet; + return _traceId != null; } #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - private bool TrySetTraceFlagsFromParent() + private void TrySetTraceFlagsFromParent() { - Debug.Assert(!_w3CIdFlagsSet); + Debug.Assert(!W3CIdFlagsSet); - if (!_w3CIdFlagsSet) + if (!W3CIdFlagsSet) { if (Parent != null) { @@ -840,26 +880,40 @@ namespace System.Diagnostics } else if (_parentId != null && IsW3CId(_parentId)) { - _w3CIdFlags = ActivityTraceId.HexByteFromChars(_parentId[53], _parentId[54]); - _w3CIdFlagsSet = true; + _w3CIdFlags = (byte)(ActivityTraceId.HexByteFromChars(_parentId[53], _parentId[54]) | ActivityTraceFlagsIsSet); } } - - return _w3CIdFlagsSet; } + private bool W3CIdFlagsSet + { + get => (_w3CIdFlags & ActivityTraceFlagsIsSet) != 0; + } - private string _rootId; - private int _currentChildId; // A unique number for all children of this activity. - - // Used to generate an ID it represents the machine and process we are in. - private static readonly string s_uniqSuffix = "-" + GetRandomNumber().ToString("x") + "."; - - //A unique number inside the appdomain, randomized between appdomains. - //Int gives enough randomization and keeps hex-encoded s_currentRootId 8 chars long for most applications - private static long s_currentRootId = (uint)GetRandomNumber(); + private bool IsFinished + { + get => (_state & State.IsFinished) != 0; + set + { + if (value) + { + _state |= State.IsFinished; + } + else + { + _state &= ~State.IsFinished; + } + } + } - private const int RequestIdMaxLength = 1024; + /// + /// Returns the format for the ID. + /// + public ActivityIdFormat IdFormat + { + get => (ActivityIdFormat)(_state & State.FormatFlags); + private set => _state = (_state & ~State.FormatFlags) | (State)((byte)value & (byte)State.FormatFlags); + } /// /// Having our own key-value linked list allows us to be more efficient @@ -870,26 +924,18 @@ namespace System.Diagnostics public KeyValueListNode Next; } - private static ActivityIdFormat s_DefaultIdFormat; - - private KeyValueListNode _tags; - private KeyValueListNode _baggage; - private string _traceState; - - // State associated with ID. It can be represented by a string or by TraceId, SpanId. - private string _id; - private ActivityTraceId _traceId; - private bool _traceIdSet; - private ActivitySpanId _spanId; - private bool _spanIdSet; + [Flags] + private enum State : byte + { + None = 0, - // State associated with ParentId. It can be represented by a string or by TraceId, SpanId. - private string _parentId; - private ActivitySpanId _parentSpanId; - private bool _parentSpanIdSet; + FormatUnknown = 0b_0_00000_00, + FormatHierarchical = 0b_0_00000_01, + FormatW3C = 0b_0_00000_10, + FormatFlags = 0b_0_00000_11, - private bool isFinished; - #endregion // private + IsFinished = 0b_1_00000_00, + } } /// @@ -898,8 +944,8 @@ namespace System.Diagnostics [Flags] public enum ActivityTraceFlags { - None = 0, - Recorded = 1 // The Activity (or more likley its parents) has been marked as useful to record + None = 0b_0_0000000, + Recorded = 0b_0_0000001, // The Activity (or more likley its parents) has been marked as useful to record } /// @@ -907,9 +953,9 @@ namespace System.Diagnostics /// public enum ActivityIdFormat { - Unknown, // ID format is not known. - Hierarchical, //|XXXX.XX.X_X ... see https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/ActivityUserGuide.md#id-format - W3C, // 00-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX-XXXXXXXXXXXXXXXX-XX see https://w3c.github.io/trace-context/ + Unknown = 0, // ID format is not known. + Hierarchical = 1, //|XXXX.XX.X_X ... see https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/ActivityUserGuide.md#id-format + W3C = 2, // 00-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX-XXXXXXXXXXXXXXXX-XX see https://w3c.github.io/trace-context/ }; /// @@ -924,45 +970,36 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - public unsafe readonly struct ActivityTraceId : IEquatable + public readonly struct ActivityTraceId : IEquatable { + private readonly string _hexString; + + internal ActivityTraceId(string hexString) => _hexString = hexString; + /// /// Create a new TraceId with at random number in it (very likely to be unique) /// public static ActivityTraceId CreateRandom() { - ActivityTraceId ret = new ActivityTraceId(); - SetToRandomBytes(new Span(&ret._id1, sizeof(ulong) * 2)); - return ret; + Span span = stackalloc byte[sizeof(ulong) * 2]; + SetToRandomBytes(span); + return CreateFromBytes(span); } public static ActivityTraceId CreateFromBytes(ReadOnlySpan idData) { if (idData.Length != 16) throw new ArgumentOutOfRangeException(nameof(idData)); - ActivityTraceId ret = new ActivityTraceId(); - idData.CopyTo(new Span(&ret._id1, sizeof(ulong) * 2)); - return ret; + return new ActivityTraceId(SpanToHexString(idData)); } public static ActivityTraceId CreateFromUtf8String(ReadOnlySpan idData) => new ActivityTraceId(idData); public static ActivityTraceId CreateFromString(ReadOnlySpan idData) { - if (idData.Length != 32) + if (idData.Length != 32 || !ActivityTraceId.IsLowerCaseHexAndNotAllZeros(idData)) throw new ArgumentOutOfRangeException(nameof(idData)); - ActivityTraceId ret = new ActivityTraceId(); - ActivityTraceId.SetSpanFromHexChars(new Span(&ret._id1, sizeof(ulong) * 2), idData); - return ret; - } - - /// - /// Copy the bytes of the TraceId (16 total) into the 'destination' span. - /// - public void CopyTo(Span destination) - { - fixed (ulong* idPtr = &_id1) - new ReadOnlySpan(idPtr, sizeof(ulong) * 2).CopyTo(destination); + return new ActivityTraceId(idData.ToString()); } /// @@ -970,17 +1007,7 @@ namespace System.Diagnostics /// public string ToHexString() { - if (_asHexString == null) - { - fixed (ulong* idPtr = &_id1) - { - // Cast away the read-only-ness of _asHexString, and assign the converted value to it. - // We are OK with this because conceptually the class is still read-only. - ref string strRef = ref Unsafe.AsRef(in _asHexString); - Interlocked.CompareExchange(ref strRef, SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong) * 2)), null); - } - } - return _asHexString; + return _hexString ?? "00000000000000000000000000000000"; } /// @@ -988,57 +1015,76 @@ namespace System.Diagnostics /// public override string ToString() => ToHexString(); - #region equality operators - public static bool operator ==(in ActivityTraceId traceId1, in ActivityTraceId traceId2) + public static bool operator ==(ActivityTraceId traceId1, ActivityTraceId traceId2) { - return traceId1._id1 == traceId2._id1 && traceId1._id2 == traceId2._id2; + return traceId1._hexString == traceId2._hexString; } - public static bool operator !=(in ActivityTraceId traceId1, in ActivityTraceId traceId2) + public static bool operator !=(ActivityTraceId traceId1, ActivityTraceId traceId2) { - return traceId1._id1 != traceId2._id1 || traceId1._id2 != traceId2._id2; + return traceId1._hexString != traceId2._hexString; } public bool Equals(ActivityTraceId traceId) { - return _id1 == traceId._id1 && _id2 == traceId._id2; + return _hexString == traceId._hexString; } public override bool Equals(object obj) { if (obj is ActivityTraceId traceId) - return _id1 == traceId._id1 && _id2 == traceId._id2; + return _hexString == traceId._hexString; return false; } public override int GetHashCode() { - return _id1.GetHashCode() + _id2.GetHashCode(); + return ToHexString().GetHashCode(); } - #endregion - #region private /// /// This is exposed as CreateFromUtf8String, but we are modifying fields, so the code needs to be in a constructor. /// /// private ActivityTraceId(ReadOnlySpan idData) { - _id1 = 0; - _id2 = 0; - _asHexString = null; if (idData.Length != 32) throw new ArgumentOutOfRangeException(nameof(idData)); - Utf8Parser.TryParse(idData.Slice(0, 16), out _id1, out _, 'x'); - Utf8Parser.TryParse(idData.Slice(16, 16), out _id2, out _, 'x'); + + Span span = stackalloc ulong[2]; + + if (!Utf8Parser.TryParse(idData.Slice(0, 16), out span[0], out _, 'x')) + { + // Invalid Id, use random https://github.com/dotnet/corefx/issues/38486 + _hexString = CreateRandom()._hexString; + return; + } + + if (!Utf8Parser.TryParse(idData.Slice(16, 16), out span[1], out _, 'x')) + { + // Invalid Id, use random https://github.com/dotnet/corefx/issues/38486 + _hexString = CreateRandom()._hexString; + return; + } + if (BitConverter.IsLittleEndian) { - _id1 = BinaryPrimitives.ReverseEndianness(_id1); - _id2 = BinaryPrimitives.ReverseEndianness(_id2); + span[0] = BinaryPrimitives.ReverseEndianness(span[0]); + span[1] = BinaryPrimitives.ReverseEndianness(span[1]); } + + _hexString = ActivityTraceId.SpanToHexString(MemoryMarshal.AsBytes(span)); + } + + /// + /// Copy the bytes of the TraceId (16 total) into the 'destination' span. + /// + public void CopyTo(Span destination) + { + ActivityTraceId.SetSpanFromHexChars(ToHexString().AsSpan(), destination); } /// /// Sets the bytes in 'outBytes' to be random values. outBytes.Length must be less than or equal to 16 /// /// - internal static void SetToRandomBytes(Span outBytes) + internal unsafe static void SetToRandomBytes(Span outBytes) { Debug.Assert(outBytes.Length <= sizeof(Guid)); // Guid is 16 bytes, and so is TraceId Guid guid = Guid.NewGuid(); @@ -1046,7 +1092,7 @@ namespace System.Diagnostics guidBytes.Slice(0, outBytes.Length).CopyTo(outBytes); } - #region CONVERSION binary spans to hex spans, and hex spans to binary spans + // CONVERSION binary spans to hex spans, and hex spans to binary spans /* It would be nice to use generic Hex number conversion routines, but there * is nothing that is exposed publicly and efficient */ /// @@ -1070,7 +1116,7 @@ namespace System.Diagnostics /// Converts 'idData' which is assumed to be HEX Unicode characters to binary /// puts it in 'outBytes' /// - internal static void SetSpanFromHexChars(Span outBytes, ReadOnlySpan charData) + internal static void SetSpanFromHexChars(ReadOnlySpan charData, Span outBytes) { Debug.Assert(outBytes.Length * 2 == charData.Length); for (int i = 0; i < outBytes.Length; i++) @@ -1103,12 +1149,34 @@ namespace System.Diagnostics outChars[1] = BinaryToHexDigit(val & 0xF); } - #endregion + internal static bool IsLowerCaseHexAndNotAllZeros(ReadOnlySpan idData) + { + // Verify lower-case hex and not all zeros https://w3c.github.io/trace-context/#field-value + bool isNonZero = false; + int i = 0; + for (; i < idData.Length; i++) + { + char c = idData[i]; + if (!IsHexadecimalLowercaseChar(c)) + { + return false; + } + + if (c != '0') + { + isNonZero = true; + } + } + + return isNonZero; + } - readonly ulong _id1; - readonly ulong _id2; - readonly string _asHexString; // Caches the Hex string - #endregion + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsHexadecimalLowercaseChar(char c) + { + // Between 0 - 9 or lowercased between a - f + return (uint)(c - '0') <= 9 || (uint)(c - 'a') <= ('f' - 'a'); + } } /// @@ -1123,44 +1191,36 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - public unsafe readonly struct ActivitySpanId : IEquatable + public readonly struct ActivitySpanId : IEquatable { + private readonly string _hexString; + + internal ActivitySpanId(string hexString) => _hexString = hexString; + /// /// Create a new SpanId with at random number in it (very likely to be unique) /// - public static ActivitySpanId CreateRandom() + public unsafe static ActivitySpanId CreateRandom() { - ActivitySpanId ret = new ActivitySpanId(); - ActivityTraceId.SetToRandomBytes(new Span(&ret._id1, sizeof(ulong))); - return ret; + ulong id; + ActivityTraceId.SetToRandomBytes(new Span(&id, sizeof(ulong))); + return new ActivitySpanId(ActivityTraceId.SpanToHexString(new ReadOnlySpan(&id, sizeof(ulong)))); } public static ActivitySpanId CreateFromBytes(ReadOnlySpan idData) { if (idData.Length != 8) throw new ArgumentOutOfRangeException(nameof(idData)); - ActivitySpanId ret = new ActivitySpanId(); - idData.CopyTo(new Span(&ret._id1, sizeof(ulong))); - return ret; + return new ActivitySpanId(ActivityTraceId.SpanToHexString(idData)); } public static ActivitySpanId CreateFromUtf8String(ReadOnlySpan idData) => new ActivitySpanId(idData); + public static ActivitySpanId CreateFromString(ReadOnlySpan idData) { - if (idData.Length != 16) + if (idData.Length != 16 || !ActivityTraceId.IsLowerCaseHexAndNotAllZeros(idData)) throw new ArgumentOutOfRangeException(nameof(idData)); - ActivitySpanId ret = new ActivitySpanId(); - ActivityTraceId.SetSpanFromHexChars(new Span(&ret._id1, sizeof(ulong)), idData); - return ret; - } - - /// - /// Copy the bytes of the TraceId (8 bytes total) into the 'destination' span. - /// - public void CopyTo(Span destination) - { - fixed (ulong* idPtr = &_id1) - new ReadOnlySpan(idPtr, sizeof(ulong)).CopyTo(destination); + return new ActivitySpanId(idData.ToString()); } /// @@ -1169,17 +1229,7 @@ namespace System.Diagnostics /// public string ToHexString() { - if (_asHexString == null) - { - fixed (ulong* idPtr = &_id1) - { - // Cast away the read-only-ness of _asHexString, and assign the converted value to it. - // We are OK with this because conceptually the class is still read-only. - ref string strRef = ref Unsafe.AsRef(in _asHexString); - Interlocked.CompareExchange(ref strRef, ActivityTraceId.SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong))), null); - } - } - return _asHexString; + return _hexString ?? "0000000000000000"; } /// @@ -1187,47 +1237,61 @@ namespace System.Diagnostics /// public override string ToString() => ToHexString(); - #region equality operators - public static bool operator ==(in ActivitySpanId spanId1, in ActivitySpanId spandId2) + public static bool operator ==(ActivitySpanId spanId1, ActivitySpanId spandId2) { - return spanId1._id1 == spandId2._id1; + return spanId1._hexString == spandId2._hexString; } - public static bool operator !=(in ActivitySpanId spanId1, in ActivitySpanId spandId2) + public static bool operator !=(ActivitySpanId spanId1, ActivitySpanId spandId2) { - return spanId1._id1 != spandId2._id1; + return spanId1._hexString != spandId2._hexString; } public bool Equals(ActivitySpanId spanId) { - return _id1 == spanId._id1; + return _hexString == spanId._hexString; } public override bool Equals(object obj) { if (!(obj is ActivitySpanId)) + { return false; + } + ActivitySpanId spanId = (ActivitySpanId)obj; - return _id1 == spanId._id1; + return _hexString == spanId._hexString; } public override int GetHashCode() { - return _id1.GetHashCode(); + return ToHexString().GetHashCode(); } - #endregion - - #region private - private ActivitySpanId(ReadOnlySpan idData) + private unsafe ActivitySpanId(ReadOnlySpan idData) { - _id1 = 0; - _asHexString = null; if (idData.Length != 16) + { throw new ArgumentOutOfRangeException(nameof(idData)); - Utf8Parser.TryParse(idData, out _id1, out _, 'x'); + } + + if (!Utf8Parser.TryParse(idData, out ulong id, out _, 'x')) + { + // Invalid Id, use random https://github.com/dotnet/corefx/issues/38486 + _hexString = CreateRandom()._hexString; + return; + } + if (BitConverter.IsLittleEndian) - _id1 = BinaryPrimitives.ReverseEndianness(_id1); + { + id = BinaryPrimitives.ReverseEndianness(id); + } + + _hexString = ActivityTraceId.SpanToHexString(new ReadOnlySpan(&id, sizeof(ulong))); } - readonly ulong _id1; - readonly string _asHexString; // Caches the Hex string - #endregion + /// + /// Copy the bytes of the TraceId (8 bytes total) into the 'destination' span. + /// + public void CopyTo(Span destination) + { + ActivityTraceId.SetSpanFromHexChars(ToHexString().AsSpan(), destination); + } } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs index 178ff45..1c85225 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticListener.cs @@ -134,7 +134,7 @@ namespace System.Diagnostics Name = name; // Insert myself into the list of all Listeners. - lock (s_lock) + lock (s_allListenersLock) { // Issue the callback for this new diagnostic listener. var allListenerObservable = s_allListenerObservable; @@ -160,7 +160,7 @@ namespace System.Diagnostics virtual public void Dispose() { // Remove myself from the list of all listeners. - lock (s_lock) + lock (s_allListenersLock) { if (_disposed) { @@ -350,7 +350,7 @@ namespace System.Diagnostics { public IDisposable Subscribe(IObserver observer) { - lock (s_lock) + lock (s_allListenersLock) { // Call back for each existing listener on the new callback (catch-up). for (DiagnosticListener cur = s_allListeners; cur != null; cur = cur._next) @@ -368,7 +368,7 @@ namespace System.Diagnostics /// internal void OnNewDiagnosticListener(DiagnosticListener diagnosticListener) { - Debug.Assert(Monitor.IsEntered(s_lock)); // We should only be called when we hold this lock + Debug.Assert(Monitor.IsEntered(s_allListenersLock)); // We should only be called when we hold this lock // Simply send a callback to every subscriber that we have a new listener for (var cur = _subscriptions; cur != null; cur = cur.Next) @@ -382,7 +382,7 @@ namespace System.Diagnostics /// private bool Remove(AllListenerSubscription subscription) { - lock (s_lock) + lock (s_allListenersLock) { if (_subscriptions == subscription) { @@ -468,7 +468,7 @@ namespace System.Diagnostics private static DiagnosticListener s_allListeners; // linked list of all instances of DiagnosticListeners. private static AllListenerObservable s_allListenerObservable; // to make callbacks to this object when listeners come into existence. - private static object s_lock = new object(); // A lock for + private static readonly object s_allListenersLock = new object(); #if false private static readonly DiagnosticListener s_default = new DiagnosticListener("DiagnosticListener.DefaultListener"); #endif