From 0a60aaa06f9615368afba833f9111beec97db9c1 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 8 Jul 2021 10:24:15 -0700 Subject: [PATCH 1/3] Propagators Support --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 15 +- ...System.Diagnostics.DiagnosticSource.csproj | 4 + .../System/Diagnostics/LegacyPropagator.cs | 190 ++++++++ .../System/Diagnostics/NoOutputPropagator.cs | 23 + .../Diagnostics/PassThroughPropagator.cs | 62 +++ .../System/Diagnostics/TextMapPropagator.cs | 139 ++++++ .../tests/PropagatorTests.cs | 458 ++++++++++++++++++ ....Diagnostics.DiagnosticSource.Tests.csproj | 1 + 8 files changed, 891 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/tests/PropagatorTests.cs 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 482a58bb414ea9..79751ec1a9e0f6 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -253,7 +253,20 @@ public sealed class ActivityListener : IDisposable public System.Diagnostics.SampleActivity? SampleUsingParentId { get { throw null; } set { throw null; } } public System.Diagnostics.SampleActivity? Sample { get { throw null; } set { throw null; } } public void Dispose() { throw null; } - } + } + public abstract class TextMapPropagator + { + public delegate void PropagatorGetterCallback(object carrier, string fieldName, out string? fieldValue, out System.Collections.Generic.IEnumerable? fieldValues); + public delegate void PropagatorSetterCallback(object carrier, string fieldName, string fieldValue); + public abstract System.Collections.Generic.IReadOnlyCollection Fields { get; } + public abstract void Inject(Activity activity, object carrier, PropagatorSetterCallback setter); + public abstract void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState); + public abstract System.Collections.Generic.IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter); + public static TextMapPropagator Current { get; set; } + public static TextMapPropagator CreateDefaultPropagator() { throw null; } + public static TextMapPropagator CreatePassThroughPropagator() { throw null; } + public static TextMapPropagator CreateNoOutputPropagator() { throw null; } + } } namespace System.Diagnostics.Metrics 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 e1dca02a19d4ae..e4a9cbd28d7ff5 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -39,7 +39,11 @@ + + + + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs new file mode 100644 index 00000000000000..bf521c29ace299 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs @@ -0,0 +1,190 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Collections.Generic; + +namespace System.Diagnostics +{ + internal class LegacyPropagator : TextMapPropagator + { + internal static TextMapPropagator Instance { get; } = new LegacyPropagator(); + + public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + + public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + { + if (activity is null || setter is null) + { + return; + } + + string? id = activity.Id; + if (id is null) + { + return; + } + + if (activity.IdFormat == ActivityIdFormat.W3C) + { + setter(carrier, TraceParent, id); + if (activity.TraceStateString is not null) + { + setter(carrier, TraceState, activity.TraceStateString); + } + } + else + { + setter(carrier, RequestId, id); + } + + InjectBaggage(carrier, activity.Baggage, setter); + } + + public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) + { + if (getter is null) + { + traceId = null; + traceState = null; + return; + } + + getter(carrier, TraceParent, out traceId, out _); + if (traceId is null) + { + getter(carrier, RequestId, out traceId, out _); + } + + getter(carrier, TraceState, out traceState, out _); + } + + public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) + { + IEnumerable>? baggage = null; + if (getter is null) + { + return null; + } + + getter(carrier, Baggage, out string? theBaggage, out _); + if (theBaggage is null || !TryExtractBaggage(theBaggage, out baggage)) + { + getter(carrier, CorrelationContext, out theBaggage, out _); + if (theBaggage is not null) + { + TryExtractBaggage(theBaggage, out baggage); + } + } + + return baggage; + } + + internal static bool TryExtractBaggage(string baggagestring, out IEnumerable>? baggage) + { + baggage = null; + List>? baggageList = null; + + if (string.IsNullOrEmpty(baggagestring)) + { + return true; + } + + int currentIndex = 0; + + do + { + // Skip spaces + while (currentIndex < baggagestring.Length && (baggagestring[currentIndex] == Space || baggagestring[currentIndex] == Tab)) + { + currentIndex++; + } + + if (currentIndex >= baggagestring.Length) + { + break; // No Key exist + } + + int keyStart = currentIndex; + + // Search end of the key + while (currentIndex < baggagestring.Length && baggagestring[currentIndex] != Space && baggagestring[currentIndex] != Tab && baggagestring[currentIndex] != '=') + { + currentIndex++; + } + + if (currentIndex >= baggagestring.Length) + { + break; + } + + int keyEnd = currentIndex; + + if (baggagestring[currentIndex] != '=') + { + // Skip Spaces + while (currentIndex < baggagestring.Length && (baggagestring[currentIndex] == Space || baggagestring[currentIndex] == Tab)) + { + currentIndex++; + } + + if (currentIndex >= baggagestring.Length) + { + break; // Wrong key format + } + + if (baggagestring[currentIndex] != '=') + { + break; // wrong key format. + } + } + + currentIndex++; + + // Skip spaces + while (currentIndex < baggagestring.Length && (baggagestring[currentIndex] == Space || baggagestring[currentIndex] == Tab)) + { + currentIndex++; + } + + if (currentIndex >= baggagestring.Length) + { + break; // Wrong value format + } + + int valueStart = currentIndex; + + // Search end of the value + while (currentIndex < baggagestring.Length && baggagestring[currentIndex] != Space && baggagestring[currentIndex] != Tab && + baggagestring[currentIndex] != Comma && baggagestring[currentIndex] != Semicolon) + { + currentIndex++; + } + + if (keyStart < keyEnd && valueStart < currentIndex) + { + if (baggageList is null) + { + baggageList = new(); + } + + // Insert in reverse order for asp.net compatability. + baggageList.Insert(0, new KeyValuePair( + WebUtility.UrlDecode(baggagestring.Substring(keyStart, keyEnd - keyStart)).Trim(s_trimmingSpaceCharacters), + WebUtility.UrlDecode(baggagestring.Substring(valueStart, currentIndex - valueStart)).Trim(s_trimmingSpaceCharacters))); + } + + // Skip to end of values + while (currentIndex < baggagestring.Length && baggagestring[currentIndex] != Comma) + { + currentIndex++; + } + + currentIndex++; // Move to next key-value entry + } while (currentIndex < baggagestring.Length); + + baggage = baggageList; + return baggageList != null; + } + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs new file mode 100644 index 00000000000000..ce610e0fdbb7f1 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +namespace System.Diagnostics +{ + internal class NoOutputPropagator : TextMapPropagator + { + internal static TextMapPropagator Instance { get; } = new NoOutputPropagator(); + + public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + + public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + { + // nothing to do. + } + + public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); + + public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs new file mode 100644 index 00000000000000..e1dc9d30bfc011 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +namespace System.Diagnostics +{ + internal class PassThroughPropagator : TextMapPropagator + { + internal static TextMapPropagator Instance { get; } = new PassThroughPropagator(); + + public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + + public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + { + GetRootId(out string? parentId, out string? traceState, out bool isW3c, out IEnumerable>? baggage); + if (parentId is null) + { + return; + } + + setter(carrier, isW3c ? TraceParent : RequestId, parentId); + + if (traceState is not null) + { + setter(carrier, TraceState, traceState); + } + + if (baggage is not null) + { + InjectBaggage(carrier, baggage, setter); + } + } + + public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); + + public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); + + private static void GetRootId(out string? parentId, out string? traceState, out bool isW3c, out IEnumerable>? baggage) + { + Activity? activity = Activity.Current; + if (activity is null) + { + parentId = null; + traceState = null; + isW3c = false; + baggage = null; + return; + } + + while (activity is not null && activity.Parent is not null) + { + activity = activity.Parent; + } + + traceState = activity?.TraceStateString; + parentId = activity?.ParentId ?? activity?.Id; + isW3c = activity?.IdFormat == ActivityIdFormat.W3C; + baggage = activity?.Baggage; + } + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs new file mode 100644 index 00000000000000..8d45da5c7c4537 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs @@ -0,0 +1,139 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Text; +using System.Diagnostics; +using System.Collections.Generic; + +namespace System.Diagnostics +{ + /// + /// Propagator defines the restrictions imposed by a specific transport and is bound to a data type, in order to propagate in-band context data across process boundaries. + /// TextMapPropagator inject values into and extracts values from carriers as string key/value pairs. + /// + public abstract class TextMapPropagator + { + private static TextMapPropagator s_current = CreateDefaultPropagator(); + + /// + /// Define the callback that can be used with the propagators extract methods. This callback will be invoked for each propagation key to get. + /// + /// Carrier is the medium used by Propagators to read values from. + /// The propagation field name. + /// An output string to receive the value corresponds to the input fieldName. This should return non null value if there is only one value for the input field name. + /// An output collection of strings to receive the values corresponds to the input fieldName. This should return non null value if there are more than one value for the input field name. + public delegate void PropagatorGetterCallback(object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues); + + /// + /// Define the callback that can be used with the propagators inject methods. This callback will be invoked to set a propagation key/value pair. + /// Propagators may invoke it multiple times in order to set multiple pairs. + /// + /// Carrier is the medium used by Propagators to write values to. + /// The propagation field name. + /// The value corresponds to the input fieldName. + public delegate void PropagatorSetterCallback(object carrier, string fieldName, string fieldValue); + + /// + /// The predefined propagation fields + /// + /// Returns list of fields that will be used by the TextMapPropagator. + public abstract IReadOnlyCollection Fields { get; } + + /// + /// Injects the trace values stroed in the object into a carrier. For example, into the headers of an HTTP request. + /// + /// The Activity object has the trace context to inject to the carrier. + /// Carrier is the medium used by the propagators to write values to. + /// The callback will be invoked to set a propagation key/value pair to the carrier. + public abstract void Inject(Activity activity, object carrier, PropagatorSetterCallback setter); + + /// + /// Extracts the value from an incoming request represented by the carrier. For example, from the headers of an HTTP request. + /// + /// Carrier is the medium used by the propagators to read values from. + /// The callback will be invoked to get the propagation trace Id and trace state from carrier. + /// The extracted trace Id from the carrier. + /// The extracted trace state from the carrier. + public abstract void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState); + + /// + /// Extracts the baggage key-value pairs list from an incoming request represented by the carrier. For example, from the headers of an HTTP request. + /// + /// Carrier is the medium used by the propagators to read values from. + /// The callback will be invoked to get the propagation baggage list from carrier. + /// Returns the extracted key-value pair list from teh carrier. + public abstract IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter); + + /// + /// Get or set the process wide propagator object which used as the current selected propagator. + /// + public static TextMapPropagator Current + { + get + { + Debug.Assert(s_current is not null); + return s_current; + } + + set + { + if (value is not null) + { + s_current = value; + } + } + } + + /// + /// returns the default propagator object which Current property will be initialized with. + /// + public static TextMapPropagator CreateDefaultPropagator() => LegacyPropagator.Instance; + + /// + /// returns the propagator which can propagate the trace context data using the root Activity and ignore any created child activities. + /// + public static TextMapPropagator CreatePassThroughPropagator() => PassThroughPropagator.Instance; + + /// + /// returns the propagator which suppress injecting any data to teh carriers. + /// + public static TextMapPropagator CreateNoOutputPropagator() => NoOutputPropagator.Instance; + + // internal stuff + + internal static void InjectBaggage(object carrier, IEnumerable> baggage, PropagatorSetterCallback setter, bool injectAsW3C = false) + { + using (IEnumerator> e = baggage.GetEnumerator()) + { + if (e.MoveNext()) + { + StringBuilder baggageList = new StringBuilder(); + + KeyValuePair item = e.Current; + baggageList.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)); + + while (e.MoveNext()) + { + item = e.Current; + baggageList.Append(Comma).Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)); + } + + setter(carrier, injectAsW3C ? Baggage : CorrelationContext, baggageList.ToString()); + } + } + } + + internal const string TraceParent = "traceparent"; + internal const string RequestId = "Request-Id"; + internal const string TraceState = "tracestate"; + internal const string Baggage = "baggage"; + internal const string CorrelationContext = "Correlation-Context"; + internal const char Space = ' '; + internal const char Tab = (char)9; + internal const char Comma = ','; + internal const char Semicolon = ';'; + + internal static readonly char [] s_trimmingSpaceCharacters = new char[] { Space, Tab }; + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/PropagatorTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/PropagatorTests.cs new file mode 100644 index 00000000000000..626df9ec03be5b --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/PropagatorTests.cs @@ -0,0 +1,458 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Collections.Generic; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class PropagatorTests + { + internal const string TraceParent = "traceparent"; + internal const string RequestId = "Request-Id"; + internal const string TraceState = "tracestate"; + internal const string Baggage = "baggage"; + internal const string CorrelationContext = "Correlation-Context"; + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TestAllPropagators() + { + RemoteExecutor.Invoke(() => { + Assert.NotNull(TextMapPropagator.Current); + + // + // Default Propagator + // + + Assert.Same(TextMapPropagator.CreateDefaultPropagator(), TextMapPropagator.Current); + + TestLegacyPropagatorUsingW3CActivity( + TextMapPropagator.Current, + "Legacy1=true", + new List>() { new KeyValuePair(" LegacyKey1 ", " LegacyValue1 ") }); + + TestLegacyPropagatorUsingHierarchicalActivity( + TextMapPropagator.Current, + "Legacy2=true", + new List>() { new KeyValuePair("LegacyKey2", "LegacyValue2") }); + + // + // NoOutput Propagator + // + + TextMapPropagator.Current = TextMapPropagator.CreateNoOutputPropagator(); + Assert.NotNull(TextMapPropagator.Current); + TestNoOutputPropagatorUsingHierarchicalActivity( + TextMapPropagator.Current, + "ActivityState=1", + new List>() { new KeyValuePair("B1", "V1"), new KeyValuePair(" B2 ", " V2 ")}); + + TestNoOutputPropagatorUsingHierarchicalActivity( + TextMapPropagator.Current, + "ActivityState=2", + null); + + TestNoOutputPropagatorUsingW3CActivity( + TextMapPropagator.Current, + "ActivityState=1", + new List>() { new KeyValuePair(" B3 ", " V3"), new KeyValuePair(" B4 ", " V4 "), new KeyValuePair("B5", "V5")}); + + TestNoOutputPropagatorUsingW3CActivity( + TextMapPropagator.Current, + "ActivityState=2", + null); + + // + // Pass Through Propagator + // + + TextMapPropagator.Current = TextMapPropagator.CreatePassThroughPropagator(); + Assert.NotNull(TextMapPropagator.Current); + TestPassThroughPropagatorUsingHierarchicalActivityWithParentChain( + TextMapPropagator.Current, + "PassThrough=true", + new List>() { new KeyValuePair("PassThroughKey1", "PassThroughValue1"), new KeyValuePair("PassThroughKey2", "PassThroughValue2")}); + + TestPassThroughPropagatorUsingHierarchicalActivityWithParentId( + TextMapPropagator.Current, + "PassThrough1=true", + new List>() { new KeyValuePair("PassThroughKey3", "PassThroughValue3"), new KeyValuePair(" PassThroughKey4 ", " PassThroughValue4 ")}); + + TestPassThroughPropagatorUsingW3CActivity( + TextMapPropagator.Current, + "PassThrough2=1", + new List>() { new KeyValuePair(" PassThroughKey4 ", " PassThroughValue4 ") }); + + }).Dispose(); + } + + private void TestLegacyPropagatorUsingW3CActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateW3CActivity("LegacyW3C1", "LegacyW3CState=1", baggage); + using Activity b = CreateW3CActivity("LegacyW3C2", "LegacyW3CState=2", baggage); + + Assert.NotSame(Activity.Current, a); + + TestLegacyPropagatorUsing(a, propagator, state, baggage); + + Assert.Same(Activity.Current, b); + + TestLegacyPropagatorUsing(Activity.Current, propagator, state, baggage); + } + + private void TestLegacyPropagatorUsingHierarchicalActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateHierarchicalActivity("LegacyHierarchical1", null, "LegacyHierarchicalState=1", baggage); + using Activity b = CreateHierarchicalActivity("LegacyHierarchical2", null, "LegacyHierarchicalState=2", baggage); + + Assert.NotSame(Activity.Current, a); + + TestLegacyPropagatorUsing(a, propagator, state, baggage); + + Assert.Same(Activity.Current, b); + + TestLegacyPropagatorUsing(Activity.Current, propagator, state, baggage); + } + + private void TestLegacyPropagatorUsing(Activity a, TextMapPropagator propagator, string state, IEnumerable> baggage) + { + // Test with non-current + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == TraceParent && a.IdFormat == ActivityIdFormat.W3C) + { + Assert.Equal(a.Id, value); + return; + } + + if (fieldName == RequestId && a.IdFormat != ActivityIdFormat.W3C) + { + Assert.Equal(a.Id, value); + return; + } + + if (fieldName == TraceState) + { + Assert.Equal(a.TraceStateString, value); + return; + } + + if (fieldName == CorrelationContext) + { + Assert.Equal(GetFormattedBaggage(a.Baggage), value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + + TestDefaultExtraction(propagator, a); + TestBaggageExtraction(propagator, a); + } + + private void TestNoOutputPropagatorUsingHierarchicalActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateHierarchicalActivity("NoOutputHierarchical", null, state, baggage); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + Assert.False(true, $"Not expected to have the setter callback be called in the NoOutput propgator."); + }); + + TestDefaultExtraction(propagator, a); + + TestBaggageExtraction(propagator, a); + } + + private void TestNoOutputPropagatorUsingW3CActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateW3CActivity("NoOutputW3C", state, baggage); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + Assert.False(true, $"Not expected to have the setter callback be called in the NoOutput propgator."); + }); + + TestDefaultExtraction(propagator, a); + + TestBaggageExtraction(propagator, a); + } + + private void TestPassThroughPropagatorUsingHierarchicalActivityWithParentChain(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateHierarchicalActivity("PassThrough", null, state, baggage); + using Activity b = CreateHierarchicalActivity("PassThroughChild1", null, state + "1", new List>() { new KeyValuePair("Child1Key", "Child1Value") } ); + using Activity c = CreateHierarchicalActivity("PassThroughChild2", null, state + "2", new List>() { new KeyValuePair("Child2Key", "Child2Value") } ); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == TraceParent) + { + Assert.False(true, $"Unexpected to inject a TraceParent with Hierarchical Activity."); + return; + } + + if (fieldName == RequestId) + { + Assert.Equal(a.Id, value); + return; + } + + if (fieldName == TraceState) + { + Assert.Equal(a.TraceStateString, value); + return; + } + + if (fieldName == CorrelationContext) + { + Assert.Equal(GetFormattedBaggage(a.Baggage), value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + + TestDefaultExtraction(propagator, a); + TestDefaultExtraction(propagator, b); + TestDefaultExtraction(propagator, c); + + TestBaggageExtraction(propagator, a); + TestBaggageExtraction(propagator, b); + TestBaggageExtraction(propagator, c); + } + + private void TestPassThroughPropagatorUsingHierarchicalActivityWithParentId(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateHierarchicalActivity("PassThrough", "Parent1", state, baggage); + using Activity b = CreateHierarchicalActivity("PassThroughChild1", "Parent2", state + "1", new List>() { new KeyValuePair("Child1Key", "Child1Value") } ); + using Activity c = CreateHierarchicalActivity("PassThroughChild2", "Parent3", state + "2", new List>() { new KeyValuePair("Child2Key", "Child2Value") } ); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == TraceParent) + { + Assert.False(true, $"Unexpected to inject a TraceParent with Hierarchical Activity."); + return; + } + + if (fieldName == RequestId) + { + Assert.Equal(c.ParentId, value); + return; + } + + if (fieldName == TraceState) + { + Assert.Equal(c.TraceStateString, value); + return; + } + + if (fieldName == CorrelationContext) + { + Assert.Equal(GetFormattedBaggage(c.Baggage), value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + + TestDefaultExtraction(propagator, a); + TestDefaultExtraction(propagator, b); + TestDefaultExtraction(propagator, c); + + TestBaggageExtraction(propagator, a); + TestBaggageExtraction(propagator, b); + TestBaggageExtraction(propagator, c); + } + + private void TestPassThroughPropagatorUsingW3CActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + { + using Activity a = CreateW3CActivity("PassThroughW3C", "PassThroughW3CState=1", baggage); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == TraceParent) + { + Assert.Equal(a.Id, value); + return; + } + + if (fieldName == TraceState) + { + Assert.Equal(a.TraceStateString, value); + return; + } + + if (fieldName == CorrelationContext) + { + Assert.Equal(GetFormattedBaggage(a.Baggage), value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + + TestDefaultExtraction(propagator, a); + TestBaggageExtraction(propagator, a); + } + + private void TestDefaultExtraction(TextMapPropagator propagator, Activity a) + { + bool traceParentEncountered = false; + + propagator.ExtractTraceIdAndState(null, (object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues) => + { + Assert.Null(carrier); + fieldValues = null; + fieldValue = null; + + if (fieldName == TraceParent) + { + if (a.IdFormat == ActivityIdFormat.W3C) + { + fieldValue = a.Id; + } + else + { + traceParentEncountered = true; + } + return; + } + + if (fieldName == RequestId) + { + if (a.IdFormat == ActivityIdFormat.W3C) + { + Assert.True(false, $"Not expected to get RequestId as we expect the request handled using TraceParenet."); + } + else + { + Assert.True(traceParentEncountered, $"Expected to get TraceParent request before getting RequestId."); + fieldValue = a.Id; + } + + return; + } + + if (fieldName == TraceState) + { + fieldValue = a.TraceStateString; + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }, out string? traceId, out string? traceState); + + Assert.Equal(a.Id, traceId); + Assert.Equal(a.TraceStateString, traceState); + } + + private void TestBaggageExtraction(TextMapPropagator propagator, Activity a) + { + bool baggageEncountered = false; + + IEnumerable>? b = propagator.ExtractBaggage(null, (object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues) => + { + Assert.Null(carrier); + fieldValue = null; + fieldValues = null; + + if (fieldName == Baggage) + { + if (a.IdFormat == ActivityIdFormat.W3C) + { + fieldValue = GetFormattedBaggage(a.Baggage); + } + else + { + baggageEncountered = true; + } + + return; + } + + if (fieldName == CorrelationContext && a.IdFormat != ActivityIdFormat.W3C) + { + Assert.True(baggageEncountered, $"Expected to get Baggage request before getting Correlation-Context."); + fieldValue = GetFormattedBaggage(a.Baggage); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + + Assert.Equal(GetFormattedBaggage(a.Baggage, false, true), GetFormattedBaggage(b, true)); + } + + private static string GetFormattedBaggage(IEnumerable>? b, bool flipOrder = false, bool trimSpaces = false) + { + string formattedBaggage = ""; + + if (b is null) + { + return formattedBaggage; + } + List> list = new List>(b); + + int startIndex = flipOrder ? list.Count - 1 : 0; + int exitIndex = flipOrder ? -1 : list.Count; + int step = flipOrder ? -1 : 1; + + for (int i = startIndex; i != exitIndex; i += step) + { + string key = trimSpaces ? list[i].Key.Trim() : list[i].Key; + string value = trimSpaces ? list[i].Value.Trim() : list[i].Value; + + formattedBaggage += (formattedBaggage.Length > 0 ? "," : "") + WebUtility.UrlEncode(key) + "=" + WebUtility.UrlEncode(value); + } + + return formattedBaggage; + } + + private Activity CreateHierarchicalActivity(string name, string parentId, string state, IEnumerable>? baggage) + { + Activity a = new Activity(name); + a.SetIdFormat(ActivityIdFormat.Hierarchical); + + if (baggage is not null) + { + foreach (KeyValuePair kvp in baggage) + { + a.SetBaggage(kvp.Key, kvp.Value); + } + } + + a.TraceStateString = state; + + if (parentId is not null) + { + a.SetParentId(parentId); + } + a.Start(); + + return a; + } + + private Activity CreateW3CActivity(string name, string state, IEnumerable>? baggage) + { + Activity a = new Activity(name); + a.SetIdFormat(ActivityIdFormat.W3C); + + if (baggage is not null) + { + foreach (KeyValuePair kvp in baggage) + { + a.SetBaggage(kvp.Key, kvp.Value); + } + } + + a.TraceStateString = state; + a.Start(); + + return a; + } + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj index 3d38a94931789f..24c84423e66384 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj @@ -28,6 +28,7 @@ + From 00bf7309ea928502e63d5310cce4b33be1bbd1c4 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Fri, 9 Jul 2021 17:58:40 -0700 Subject: [PATCH 2/3] Address the feedback and adding more tests --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 10 +- .../System/Diagnostics/LegacyPropagator.cs | 57 +++-- .../System/Diagnostics/NoOutputPropagator.cs | 10 +- .../Diagnostics/PassThroughPropagator.cs | 29 +-- .../System/Diagnostics/TextMapPropagator.cs | 31 +-- .../tests/PropagatorTests.cs | 228 +++++++++++++++++- 6 files changed, 281 insertions(+), 84 deletions(-) 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 79751ec1a9e0f6..7e50f732e42175 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -256,12 +256,12 @@ public sealed class ActivityListener : IDisposable } public abstract class TextMapPropagator { - public delegate void PropagatorGetterCallback(object carrier, string fieldName, out string? fieldValue, out System.Collections.Generic.IEnumerable? fieldValues); - public delegate void PropagatorSetterCallback(object carrier, string fieldName, string fieldValue); + public delegate void PropagatorGetterCallback(object? carrier, string fieldName, out string? fieldValue, out System.Collections.Generic.IEnumerable? fieldValues); + public delegate void PropagatorSetterCallback(object? carrier, string fieldName, string fieldValue); public abstract System.Collections.Generic.IReadOnlyCollection Fields { get; } - public abstract void Inject(Activity activity, object carrier, PropagatorSetterCallback setter); - public abstract void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState); - public abstract System.Collections.Generic.IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter); + public abstract void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter); + public abstract void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState); + public abstract System.Collections.Generic.IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter); public static TextMapPropagator Current { get; set; } public static TextMapPropagator CreateDefaultPropagator() { throw null; } public static TextMapPropagator CreatePassThroughPropagator() { throw null; } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs index bf521c29ace299..a469dd5b56b6b9 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/LegacyPropagator.cs @@ -3,16 +3,17 @@ using System.Net; using System.Collections.Generic; +using System.Collections.ObjectModel; namespace System.Diagnostics { - internal class LegacyPropagator : TextMapPropagator + internal sealed class LegacyPropagator : TextMapPropagator { internal static TextMapPropagator Instance { get; } = new LegacyPropagator(); - public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + public override IReadOnlyCollection Fields { get; } = new ReadOnlyCollection(new[] { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }); - public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) { if (activity is null || setter is null) { @@ -28,7 +29,7 @@ public override void Inject(Activity activity, object carrier, PropagatorSetterC if (activity.IdFormat == ActivityIdFormat.W3C) { setter(carrier, TraceParent, id); - if (activity.TraceStateString is not null) + if (!string.IsNullOrEmpty(activity.TraceStateString)) { setter(carrier, TraceState, activity.TraceStateString); } @@ -41,7 +42,7 @@ public override void Inject(Activity activity, object carrier, PropagatorSetterC InjectBaggage(carrier, activity.Baggage, setter); } - public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) + public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState) { if (getter is null) { @@ -59,15 +60,16 @@ public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCall getter(carrier, TraceState, out traceState, out _); } - public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) + public override IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) { - IEnumerable>? baggage = null; if (getter is null) { return null; } getter(carrier, Baggage, out string? theBaggage, out _); + + IEnumerable>? baggage = null; if (theBaggage is null || !TryExtractBaggage(theBaggage, out baggage)) { getter(carrier, CorrelationContext, out theBaggage, out _); @@ -80,12 +82,12 @@ public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCall return baggage; } - internal static bool TryExtractBaggage(string baggagestring, out IEnumerable>? baggage) + internal static bool TryExtractBaggage(string baggageString, out IEnumerable>? baggage) { baggage = null; List>? baggageList = null; - if (string.IsNullOrEmpty(baggagestring)) + if (string.IsNullOrEmpty(baggageString)) { return true; } @@ -95,12 +97,12 @@ internal static bool TryExtractBaggage(string baggagestring, out IEnumerable= baggagestring.Length) + if (currentIndex >= baggageString.Length) { break; // No Key exist } @@ -108,32 +110,32 @@ internal static bool TryExtractBaggage(string baggagestring, out IEnumerable= baggagestring.Length) + if (currentIndex >= baggageString.Length) { break; } int keyEnd = currentIndex; - if (baggagestring[currentIndex] != '=') + if (baggageString[currentIndex] != '=') { // Skip Spaces - while (currentIndex < baggagestring.Length && (baggagestring[currentIndex] == Space || baggagestring[currentIndex] == Tab)) + while (currentIndex < baggageString.Length && (baggageString[currentIndex] == Space || baggageString[currentIndex] == Tab)) { currentIndex++; } - if (currentIndex >= baggagestring.Length) + if (currentIndex >= baggageString.Length) { break; // Wrong key format } - if (baggagestring[currentIndex] != '=') + if (baggageString[currentIndex] != '=') { break; // wrong key format. } @@ -142,12 +144,12 @@ internal static bool TryExtractBaggage(string baggagestring, out IEnumerable= baggagestring.Length) + if (currentIndex >= baggageString.Length) { break; // Wrong value format } @@ -155,33 +157,30 @@ internal static bool TryExtractBaggage(string baggagestring, out IEnumerable( - WebUtility.UrlDecode(baggagestring.Substring(keyStart, keyEnd - keyStart)).Trim(s_trimmingSpaceCharacters), - WebUtility.UrlDecode(baggagestring.Substring(valueStart, currentIndex - valueStart)).Trim(s_trimmingSpaceCharacters))); + WebUtility.UrlDecode(baggageString.Substring(keyStart, keyEnd - keyStart)).Trim(s_trimmingSpaceCharacters), + WebUtility.UrlDecode(baggageString.Substring(valueStart, currentIndex - valueStart)).Trim(s_trimmingSpaceCharacters))); } // Skip to end of values - while (currentIndex < baggagestring.Length && baggagestring[currentIndex] != Comma) + while (currentIndex < baggageString.Length && baggageString[currentIndex] != Comma) { currentIndex++; } currentIndex++; // Move to next key-value entry - } while (currentIndex < baggagestring.Length); + } while (currentIndex < baggageString.Length); baggage = baggageList; return baggageList != null; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs index ce610e0fdbb7f1..f9d503a8f1c9a3 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/NoOutputPropagator.cs @@ -5,19 +5,19 @@ namespace System.Diagnostics { - internal class NoOutputPropagator : TextMapPropagator + internal sealed class NoOutputPropagator : TextMapPropagator { internal static TextMapPropagator Instance { get; } = new NoOutputPropagator(); - public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + public override IReadOnlyCollection Fields { get; } = LegacyPropagator.Instance.Fields; - public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) { // nothing to do. } - public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); + public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); - public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); + public override IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs index e1dc9d30bfc011..b92f636ec43556 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs @@ -5,14 +5,19 @@ namespace System.Diagnostics { - internal class PassThroughPropagator : TextMapPropagator + internal sealed class PassThroughPropagator : TextMapPropagator { internal static TextMapPropagator Instance { get; } = new PassThroughPropagator(); - public override IReadOnlyCollection Fields { get; } = new HashSet() { TraceParent, RequestId, TraceState, Baggage, CorrelationContext }; + public override IReadOnlyCollection Fields { get; } = LegacyPropagator.Instance.Fields; - public override void Inject(Activity activity, object carrier, PropagatorSetterCallback setter) + public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) { + if (setter is null) + { + return; + } + GetRootId(out string? parentId, out string? traceState, out bool isW3c, out IEnumerable>? baggage); if (parentId is null) { @@ -21,7 +26,7 @@ public override void Inject(Activity activity, object carrier, PropagatorSetterC setter(carrier, isW3c ? TraceParent : RequestId, parentId); - if (traceState is not null) + if (!string.IsNullOrEmpty(traceState)) { setter(carrier, TraceState, traceState); } @@ -32,25 +37,17 @@ public override void Inject(Activity activity, object carrier, PropagatorSetterC } } - public override void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); + public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState) => LegacyPropagator.Instance.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState); - public override IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); + public override IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) => LegacyPropagator.Instance.ExtractBaggage(carrier, getter); private static void GetRootId(out string? parentId, out string? traceState, out bool isW3c, out IEnumerable>? baggage) { Activity? activity = Activity.Current; - if (activity is null) - { - parentId = null; - traceState = null; - isW3c = false; - baggage = null; - return; - } - while (activity is not null && activity.Parent is not null) + while (activity?.Parent is Activity parent) { - activity = activity.Parent; + activity = parent; } traceState = activity?.TraceStateString; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs index 8d45da5c7c4537..0e044952a52294 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs @@ -23,7 +23,7 @@ public abstract class TextMapPropagator /// The propagation field name. /// An output string to receive the value corresponds to the input fieldName. This should return non null value if there is only one value for the input field name. /// An output collection of strings to receive the values corresponds to the input fieldName. This should return non null value if there are more than one value for the input field name. - public delegate void PropagatorGetterCallback(object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues); + public delegate void PropagatorGetterCallback(object? carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues); /// /// Define the callback that can be used with the propagators inject methods. This callback will be invoked to set a propagation key/value pair. @@ -32,7 +32,7 @@ public abstract class TextMapPropagator /// Carrier is the medium used by Propagators to write values to. /// The propagation field name. /// The value corresponds to the input fieldName. - public delegate void PropagatorSetterCallback(object carrier, string fieldName, string fieldValue); + public delegate void PropagatorSetterCallback(object? carrier, string fieldName, string fieldValue); /// /// The predefined propagation fields @@ -46,7 +46,7 @@ public abstract class TextMapPropagator /// The Activity object has the trace context to inject to the carrier. /// Carrier is the medium used by the propagators to write values to. /// The callback will be invoked to set a propagation key/value pair to the carrier. - public abstract void Inject(Activity activity, object carrier, PropagatorSetterCallback setter); + public abstract void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter); /// /// Extracts the value from an incoming request represented by the carrier. For example, from the headers of an HTTP request. @@ -55,7 +55,7 @@ public abstract class TextMapPropagator /// The callback will be invoked to get the propagation trace Id and trace state from carrier. /// The extracted trace Id from the carrier. /// The extracted trace state from the carrier. - public abstract void ExtractTraceIdAndState(object carrier, PropagatorGetterCallback getter, out string? traceId, out string? traceState); + public abstract void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState); /// /// Extracts the baggage key-value pairs list from an incoming request represented by the carrier. For example, from the headers of an HTTP request. @@ -63,7 +63,7 @@ public abstract class TextMapPropagator /// Carrier is the medium used by the propagators to read values from. /// The callback will be invoked to get the propagation baggage list from carrier. /// Returns the extracted key-value pair list from teh carrier. - public abstract IEnumerable>? ExtractBaggage(object carrier, PropagatorGetterCallback getter); + public abstract IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter); /// /// Get or set the process wide propagator object which used as the current selected propagator. @@ -78,10 +78,7 @@ public static TextMapPropagator Current set { - if (value is not null) - { - s_current = value; - } + s_current = value ?? throw new ArgumentNullException(nameof(value)); } } @@ -102,7 +99,7 @@ public static TextMapPropagator Current // internal stuff - internal static void InjectBaggage(object carrier, IEnumerable> baggage, PropagatorSetterCallback setter, bool injectAsW3C = false) + internal static void InjectBaggage(object? carrier, IEnumerable> baggage, PropagatorSetterCallback setter) { using (IEnumerator> e = baggage.GetEnumerator()) { @@ -110,16 +107,13 @@ internal static void InjectBaggage(object carrier, IEnumerable item = e.Current; - baggageList.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)); - - while (e.MoveNext()) + do { - item = e.Current; - baggageList.Append(Comma).Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)); - } + KeyValuePair item = e.Current; + baggageList.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)).Append(CommaWithSpace); + } while (e.MoveNext()); - setter(carrier, injectAsW3C ? Baggage : CorrelationContext, baggageList.ToString()); + setter(carrier, CorrelationContext, baggageList.ToString(0, baggageList.Length - 2)); } } } @@ -133,6 +127,7 @@ internal static void InjectBaggage(object carrier, IEnumerable>() { new KeyValuePair(" LegacyKey1 ", " LegacyValue1 ") }); - TestLegacyPropagatorUsingHierarchicalActivity( + TestDefaultPropagatorUsingHierarchicalActivity( TextMapPropagator.Current, "Legacy2=true", new List>() { new KeyValuePair("LegacyKey2", "LegacyValue2") }); + TestFields(TextMapPropagator.Current); + // // NoOutput Propagator // @@ -64,6 +67,8 @@ public void TestAllPropagators() "ActivityState=2", null); + TestFields(TextMapPropagator.Current); + // // Pass Through Propagator // @@ -85,38 +90,48 @@ public void TestAllPropagators() "PassThrough2=1", new List>() { new KeyValuePair(" PassThroughKey4 ", " PassThroughValue4 ") }); + TestPassThroughPropagatorWithNullCurrent(TextMapPropagator.Current); + + TestFields(TextMapPropagator.Current); + + // + // Test Current + // + + Assert.Throws(() => TextMapPropagator.Current = null); + }).Dispose(); } - private void TestLegacyPropagatorUsingW3CActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + private void TestDefaultPropagatorUsingW3CActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) { using Activity a = CreateW3CActivity("LegacyW3C1", "LegacyW3CState=1", baggage); using Activity b = CreateW3CActivity("LegacyW3C2", "LegacyW3CState=2", baggage); Assert.NotSame(Activity.Current, a); - TestLegacyPropagatorUsing(a, propagator, state, baggage); + TestDefaultPropagatorUsing(a, propagator, state, baggage); Assert.Same(Activity.Current, b); - TestLegacyPropagatorUsing(Activity.Current, propagator, state, baggage); + TestDefaultPropagatorUsing(Activity.Current, propagator, state, baggage); } - private void TestLegacyPropagatorUsingHierarchicalActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) + private void TestDefaultPropagatorUsingHierarchicalActivity(TextMapPropagator propagator, string state, IEnumerable> baggage) { using Activity a = CreateHierarchicalActivity("LegacyHierarchical1", null, "LegacyHierarchicalState=1", baggage); using Activity b = CreateHierarchicalActivity("LegacyHierarchical2", null, "LegacyHierarchicalState=2", baggage); Assert.NotSame(Activity.Current, a); - TestLegacyPropagatorUsing(a, propagator, state, baggage); + TestDefaultPropagatorUsing(a, propagator, state, baggage); Assert.Same(Activity.Current, b); - TestLegacyPropagatorUsing(Activity.Current, propagator, state, baggage); + TestDefaultPropagatorUsing(Activity.Current, propagator, state, baggage); } - private void TestLegacyPropagatorUsing(Activity a, TextMapPropagator propagator, string state, IEnumerable> baggage) + private void TestDefaultPropagatorUsing(Activity a, TextMapPropagator propagator, string state, IEnumerable> baggage) { // Test with non-current propagator.Inject(a, null, (object carrier, string fieldName, string value) => @@ -299,6 +314,29 @@ private void TestPassThroughPropagatorUsingW3CActivity(TextMapPropagator propaga TestBaggageExtraction(propagator, a); } + private void TestPassThroughPropagatorWithNullCurrent(TextMapPropagator propagator) + { + Activity.Current = null; + + propagator.Inject(null, null, (object carrier, string fieldName, string value) => + { + Assert.False(true, $"PassThroughPropagator shouldn't inject anything if the Activity.Current is null"); + }); + + using Activity a = CreateW3CActivity("PassThroughNotNull", "", null); + + propagator.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == TraceParent) + { + Assert.Equal(a.Id, value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}'"); + }); + } + private void TestDefaultExtraction(TextMapPropagator propagator, Activity a) { bool traceParentEncountered = false; @@ -387,7 +425,16 @@ private void TestBaggageExtraction(TextMapPropagator propagator, Activity a) Assert.Equal(GetFormattedBaggage(a.Baggage, false, true), GetFormattedBaggage(b, true)); } - private static string GetFormattedBaggage(IEnumerable>? b, bool flipOrder = false, bool trimSpaces = false) + private void TestFields(TextMapPropagator propagator) + { + Assert.True(propagator.Fields.Contains(TraceParent)); + Assert.True(propagator.Fields.Contains(RequestId)); + Assert.True(propagator.Fields.Contains(TraceState)); + Assert.True(propagator.Fields.Contains(Baggage)); + Assert.True(propagator.Fields.Contains(CorrelationContext)); + } + + internal static string GetFormattedBaggage(IEnumerable>? b, bool flipOrder = false, bool trimSpaces = false) { string formattedBaggage = ""; @@ -406,7 +453,7 @@ private static string GetFormattedBaggage(IEnumerable 0 ? "," : "") + WebUtility.UrlEncode(key) + "=" + WebUtility.UrlEncode(value); + formattedBaggage += (formattedBaggage.Length > 0 ? ", " : "") + WebUtility.UrlEncode(key) + "=" + WebUtility.UrlEncode(value); } return formattedBaggage; @@ -454,5 +501,164 @@ private Activity CreateW3CActivity(string name, string state, IEnumerable>? ParseBaggage(string baggageString) + { + if (baggageString is null) + { + return null; + } + + List> list = new(); + string [] parts = baggageString.Split(','); + + foreach (string part in parts) + { + string [] baggageItem = part.Split('='); + + if (baggageItem.Length != 2) + { + return null; // Invalid format + } + + list.Add(new KeyValuePair(WebUtility.UrlDecode(baggageItem[0]).Trim(), WebUtility.UrlDecode(baggageItem[1]).Trim())); + } + + return list; + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TestCustomPropagator() + { + RemoteExecutor.Invoke(() => { + + TextMapPropagator.Current = new CustomPropagator(); + using Activity a = CreateW3CActivity("CustomW3C1", "CustomW3CState=1", new List>() { new KeyValuePair(" CustomKey1 ", " CustomValue1 ") }); + + string traceParent = "x-" + a.Id ; + string traceState = "x-" + a.TraceStateString; + string baggageString = "x=y, " + GetFormattedBaggage(a.Baggage); + + TextMapPropagator.Current.Inject(a, null, (object carrier, string fieldName, string value) => + { + if (fieldName == CustomPropagator.XTraceParent) + { + Assert.Equal(traceParent, value); + return; + } + + if (fieldName == CustomPropagator.XTraceState) + { + Assert.Equal(traceState, value); + return; + } + + if (fieldName == CustomPropagator.XBaggage) + { + Assert.Equal(baggageString, value); + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}' in the Custom Propagator"); + }); + + TextMapPropagator.Current.ExtractTraceIdAndState(null, (object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues) => + { + fieldValues = null; + fieldValue = null; + + if (fieldName == CustomPropagator.XTraceParent) + { + fieldValue = traceParent; + return; + } + + if (fieldName == CustomPropagator.XTraceState) + { + fieldValue = traceState; + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}' in the Custom propagator"); + }, out string? traceId, out string? state); + + Assert.Equal(traceParent, traceId); + Assert.Equal(traceState, state); + + IEnumerable>? b = TextMapPropagator.Current.ExtractBaggage(null, (object carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues) => + { + Assert.Null(carrier); + fieldValue = null; + fieldValues = null; + + if (fieldName == CustomPropagator.XBaggage) + { + fieldValue = baggageString; + return; + } + + Assert.False(true, $"Encountered wrong header name '{fieldName}' in custom propagator"); + }); + + Assert.Equal(2, b.Count()); + Assert.Equal(new KeyValuePair("x", "y"), b.ElementAt(0)); + Assert.Equal(new KeyValuePair("CustomKey1", "CustomValue1"), b.ElementAt(1)); + + }).Dispose(); + } + + internal class CustomPropagator : TextMapPropagator + { + internal const string XTraceParent = "x-traceparent"; + internal const string XTraceState = "x-tracestate"; + internal const string XBaggage = "x-baggage"; + + public override IReadOnlyCollection Fields { get; } = new[] { XTraceParent, XTraceState, XBaggage}; + + public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) + { + if (activity is null || carrier is null) + { + return; + } + + setter(carrier, XTraceParent, "x-" + activity.Id); + + if (!string.IsNullOrEmpty(activity.TraceStateString)) + { + setter(carrier, XTraceState, "x-" + activity.TraceStateString); + } + + if (activity.Baggage.Count() > 0) + { + setter(carrier, XBaggage, "x=y, " + PropagatorTests.GetFormattedBaggage(activity.Baggage)); + } + } + + public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState) + { + if (getter is null) + { + traceId = null; + traceState = null; + return; + } + + getter(carrier, XTraceParent, out traceId, out _); + getter(carrier, XTraceState, out traceState, out _); + } + + public override IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) + { + if (getter is null) + { + return null; + } + + getter(carrier, XBaggage, out string? theBaggage, out _); + + return PropagatorTests.ParseBaggage(theBaggage); + } + } } } From 5e033b4012399d9bd0da577a5b249d19534887d6 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Sat, 10 Jul 2021 11:59:09 -0700 Subject: [PATCH 3/3] More Feedback --- .../Diagnostics/PassThroughPropagator.cs | 2 +- .../System/Diagnostics/TextMapPropagator.cs | 37 +++++++++++-------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs index b92f636ec43556..01bf821a5cbc94 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/PassThroughPropagator.cs @@ -52,7 +52,7 @@ private static void GetRootId(out string? parentId, out string? traceState, out traceState = activity?.TraceStateString; parentId = activity?.ParentId ?? activity?.Id; - isW3c = activity?.IdFormat == ActivityIdFormat.W3C; + isW3c = parentId is not null ? Activity.TryConvertIdToContext(parentId, traceState, out _) : false; baggage = activity?.Baggage; } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs index 0e044952a52294..0dab622a29833d 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/TextMapPropagator.cs @@ -9,7 +9,8 @@ namespace System.Diagnostics { /// - /// Propagator defines the restrictions imposed by a specific transport and is bound to a data type, in order to propagate in-band context data across process boundaries. + /// An implementation of TextMapPropagator determines if and how distributed context information is encoded and decoded as it traverses the network. + /// The encoding can be transported over any network protocol that supports string key-value pairs. For example when using HTTP, each key value pair is an HTTP header. /// TextMapPropagator inject values into and extracts values from carriers as string key/value pairs. /// public abstract class TextMapPropagator @@ -17,7 +18,7 @@ public abstract class TextMapPropagator private static TextMapPropagator s_current = CreateDefaultPropagator(); /// - /// Define the callback that can be used with the propagators extract methods. This callback will be invoked for each propagation key to get. + /// The callback that is used in propagators' extract methods. The callback is invoked to lookup the value of a named field. /// /// Carrier is the medium used by Propagators to read values from. /// The propagation field name. @@ -26,8 +27,8 @@ public abstract class TextMapPropagator public delegate void PropagatorGetterCallback(object? carrier, string fieldName, out string? fieldValue, out IEnumerable? fieldValues); /// - /// Define the callback that can be used with the propagators inject methods. This callback will be invoked to set a propagation key/value pair. - /// Propagators may invoke it multiple times in order to set multiple pairs. + /// The callback that is used in propagators' inject methods. This callback is invoked to set the value of a named field. + /// Propagators may invoke it multiple times in order to set multiple fields. /// /// Carrier is the medium used by Propagators to write values to. /// The propagation field name. @@ -35,7 +36,7 @@ public abstract class TextMapPropagator public delegate void PropagatorSetterCallback(object? carrier, string fieldName, string fieldValue); /// - /// The predefined propagation fields + /// The set of field names this propagator is likely to read or write. /// /// Returns list of fields that will be used by the TextMapPropagator. public abstract IReadOnlyCollection Fields { get; } @@ -43,26 +44,26 @@ public abstract class TextMapPropagator /// /// Injects the trace values stroed in the object into a carrier. For example, into the headers of an HTTP request. /// - /// The Activity object has the trace context to inject to the carrier. - /// Carrier is the medium used by the propagators to write values to. - /// The callback will be invoked to set a propagation key/value pair to the carrier. + /// The Activity object has the distributed context to inject to the carrier. + /// Carrier is the medium in which the distributed context will be stored. + /// The callback will be invoked to set a named key/value pair on the carrier. public abstract void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter); /// - /// Extracts the value from an incoming request represented by the carrier. For example, from the headers of an HTTP request. + /// Extracts trace Id and trace state from an incoming request represented by the carrier. For example, from the headers of an HTTP request. /// - /// Carrier is the medium used by the propagators to read values from. + /// Carrier is the medium from which values will be read. /// The callback will be invoked to get the propagation trace Id and trace state from carrier. /// The extracted trace Id from the carrier. /// The extracted trace state from the carrier. public abstract void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState); /// - /// Extracts the baggage key-value pairs list from an incoming request represented by the carrier. For example, from the headers of an HTTP request. + /// Extracts the baggage key-value pair list from an incoming request represented by the carrier. For example, from the headers of an HTTP request. /// - /// Carrier is the medium used by the propagators to read values from. + /// Carrier is the medium from which values will be read. /// The callback will be invoked to get the propagation baggage list from carrier. - /// Returns the extracted key-value pair list from teh carrier. + /// Returns the extracted key-value pair list from the carrier. public abstract IEnumerable>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter); /// @@ -85,15 +86,21 @@ public static TextMapPropagator Current /// /// returns the default propagator object which Current property will be initialized with. /// + /// + /// CreateDefaultPropagator will create a propagator instance that can inject and extract the headers with field names "tarcestate", + /// "traceparent" of the identifiers which are formatted as W3C trace parent, "Request-Id" of the identifiers which are formatted as a hierarchical identifier. + /// The returned propagator can inject the baggage key-value pair list with header name "Correlation-Context" and it can extract the baggage values mapped to header names "Correlation-Context" and "baggage". + /// public static TextMapPropagator CreateDefaultPropagator() => LegacyPropagator.Instance; /// - /// returns the propagator which can propagate the trace context data using the root Activity and ignore any created child activities. + /// Returns a propagator which attempts to act transparently, emitting the same data on outbound network requests that was received on the in-bound request. + /// When encoding the outbound message, this propagator uses information from the request's root Activity, ignoring any intermediate Activities that may have been created while processing the request. /// public static TextMapPropagator CreatePassThroughPropagator() => PassThroughPropagator.Instance; /// - /// returns the propagator which suppress injecting any data to teh carriers. + /// Returns a propagator which does not transmit any distributed context information in outbound network messages. /// public static TextMapPropagator CreateNoOutputPropagator() => NoOutputPropagator.Instance;