Creating CompositePropagator (#923)

* Creating CompositePropagator

Adding ActivityContext to interface

Adding tests

Updating CompositePropagator + tests

updating comments

Improving performance when size is 0

updating tests

updating default tracestate for empty

MikeGoldsmith review

adding more tests to exemplify usage

Feature/composite propagator refactor (#1)

* Added TestPropagator and switched a couple tests to use it.

* removing extra classes

Co-authored-by: Mikel Blanchard <[email protected]>

updating changelog and constructor

MikeGoldsmith comments

updating logic

* checking value

* adding inverted test

* updating tests

* removing tracecontext and b3 tests

* testing only aspnet to merge

* Revert "testing only aspnet to merge"

This reverts commit 43ca7d0.

Co-authored-by: Cijo Thomas <[email protected]>

src/OpenTelemetry.Api/CHANGELOG.md

@@ -16,6 +16,10 @@ changed to match the
   gets removed.
     ([#954](https://github.com/open-telemetry/opentelemetry-dotnet/pull/954)).
 * HttpStatusCode in all spans attribute (http.status_code) to use int value.
+* `ITextFormatActivity` got replaced by `ITextFormat` with an additional method
+  to be implemented (`IsInjected`)
+* Added `CompositePropagator` that accepts a list of `ITextFormat` following
+  [specification](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#create-a-composite-propagator)
 
 ## 0.4.0-beta.2
 

src/OpenTelemetry.Api/Context/Propagation/CompositePropagator.cs

@@ -0,0 +1,74 @@
+// <copyright file="CompositePropagator.cs" company="OpenTelemetry Authors">
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+
+namespace OpenTelemetry.Context.Propagation
+{
+    /// <summary>
+    /// CompositePropagator provides a mechanism for combining multiple propagators into a single one.
+    /// </summary>
+    public class CompositePropagator : ITextFormat
+    {
+        private static readonly ISet<string> EmptyFields = new HashSet<string>();
+        private readonly List<ITextFormat> textFormats;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="CompositePropagator"/> class.
+        /// </summary>
+        /// <param name="textFormats">List of <see cref="ITextFormat"/> wire context propagator.</param>
+        public CompositePropagator(List<ITextFormat> textFormats)
+        {
+            this.textFormats = textFormats ?? throw new ArgumentNullException(nameof(textFormats));
+        }
+
+        /// <inheritdoc/>
+        public ISet<string> Fields => EmptyFields;
+
+        /// <inheritdoc/>
+        public ActivityContext Extract<T>(ActivityContext activityContext, T carrier, Func<T, string, IEnumerable<string>> getter)
+        {
+            foreach (var textFormat in this.textFormats)
+            {
+                activityContext = textFormat.Extract(activityContext, carrier, getter);
+                if (activityContext.IsValid())
+                {
+                    return activityContext;
+                }
+            }
+
+            return activityContext;
+        }
+
+        /// <inheritdoc/>
+        public void Inject<T>(ActivityContext activityContext, T carrier, Action<T, string, string> setter)
+        {
+            foreach (var textFormat in this.textFormats)
+            {
+                textFormat.Inject(activityContext, carrier, setter);
+            }
+        }
+
+        /// <inheritdoc/>
+        public bool IsInjected<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
+        {
+            return this.textFormats.All(textFormat => textFormat.IsInjected(carrier, getter));
+        }
+    }
+}

src/OpenTelemetry.Api/Context/Propagation/ITextFormat.cs

@@ -45,10 +45,11 @@ public interface ITextFormat
         /// Extracts activity context from textual representation.
         /// </summary>
         /// <typeparam name="T">Type of object to extract context from. Typically HttpRequest or similar.</typeparam>
+        /// <param name="activityContext">The default activity context to be used if Extract fails.</param>
         /// <param name="carrier">Object to extract context from. Instance of this object will be passed to the getter.</param>
         /// <param name="getter">Function that will return string value of a key with the specified name.</param>
         /// <returns>Activity context from it's text representation.</returns>
-        ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>> getter);
+        ActivityContext Extract<T>(ActivityContext activityContext, T carrier, Func<T, string, IEnumerable<string>> getter);
 
         /// <summary>
         /// Tests if an activity context has been injected into a carrier.

src/OpenTelemetry.Api/Context/Propagation/TraceContextFormat.cs

@@ -30,7 +30,6 @@ public class TraceContextFormat : ITextFormat
         private const string TraceParent = "traceparent";
         private const string TraceState = "tracestate";
 
-        private static readonly int VersionLength = "00".Length;
         private static readonly int VersionPrefixIdLength = "00-".Length;
         private static readonly int TraceIdLength = "0af7651916cd43dd8448eb211c80319c".Length;
         private static readonly int VersionAndTraceIdLength = "00-0af7651916cd43dd8448eb211c80319c-".Length;
@@ -74,18 +73,18 @@ public bool IsInjected<T>(T carrier, Func<T, string, IEnumerable<string>> getter
         }
 
         /// <inheritdoc/>
-        public ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>> getter)
+        public ActivityContext Extract<T>(ActivityContext activityContext, T carrier, Func<T, string, IEnumerable<string>> getter)
         {
             if (carrier == null)
             {
                 OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext("null carrier");
-                return default;
+                return activityContext;
             }
 
             if (getter == null)
             {
                 OpenTelemetryApiEventSource.Log.FailedToExtractContext("null getter");
-                return default;
+                return activityContext;
             }
 
             try
@@ -95,22 +94,22 @@ public ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>
                 // There must be a single traceparent
                 if (traceparentCollection == null || traceparentCollection.Count() != 1)
                 {
-                    return default;
+                    return activityContext;
                 }
 
                 var traceparent = traceparentCollection.First();
-                var traceparentParsed = this.TryExtractTraceparent(traceparent, out var traceId, out var spanId, out var traceoptions);
+                var traceparentParsed = TryExtractTraceparent(traceparent, out var traceId, out var spanId, out var traceoptions);
 
                 if (!traceparentParsed)
                 {
-                    return default;
+                    return activityContext;
                 }
 
-                string tracestate = null;
+                string tracestate = string.Empty;
                 var tracestateCollection = getter(carrier, TraceState);
-                if (tracestateCollection != null)
+                if (tracestateCollection?.Any() ?? false)
                 {
-                    this.TryExtractTracestate(tracestateCollection.ToArray(), out tracestate);
+                    TryExtractTracestate(tracestateCollection.ToArray(), out tracestate);
                 }
 
                 return new ActivityContext(traceId, spanId, traceoptions, tracestate, isRemote: true);
@@ -121,7 +120,7 @@ public ActivityContext Extract<T>(T carrier, Func<T, string, IEnumerable<string>
             }
 
             // in case of exception indicate to upstream that there is no parseable context from the top
-            return default;
+            return activityContext;
         }
 
         /// <inheritdoc/>
@@ -157,7 +156,7 @@ public void Inject<T>(ActivityContext activityContext, T carrier, Action<T, stri
             }
         }
 
-        private bool TryExtractTraceparent(string traceparent, out ActivityTraceId traceId, out ActivitySpanId spanId, out ActivityTraceFlags traceOptions)
+        internal static bool TryExtractTraceparent(string traceparent, out ActivityTraceId traceId, out ActivitySpanId spanId, out ActivityTraceFlags traceOptions)
         {
             // from https://github.com/w3c/distributed-tracing/blob/master/trace_context/HTTP_HEADER_FORMAT.md
             // traceparent: 00-0af7651916cd43dd8448eb211c80319c-00f067aa0ba902b7-01
@@ -179,8 +178,8 @@ private bool TryExtractTraceparent(string traceparent, out ActivityTraceId trace
             }
 
             // or version is not a hex (will throw)
-            var version0 = this.HexCharToByte(traceparent[0]);
-            var version1 = this.HexCharToByte(traceparent[1]);
+            var version0 = HexCharToByte(traceparent[0]);
+            var version1 = HexCharToByte(traceparent[1]);
 
             if (version0 == 0xf && version1 == 0xf)
             {
@@ -229,8 +228,8 @@ private bool TryExtractTraceparent(string traceparent, out ActivityTraceId trace
 
             try
             {
-                options0 = this.HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength]);
-                options1 = this.HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength + 1]);
+                options0 = HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength]);
+                options1 = HexCharToByte(traceparent[VersionAndTraceIdAndSpanIdLength + 1]);
             }
             catch (ArgumentOutOfRangeException)
             {
@@ -259,27 +258,7 @@ private bool TryExtractTraceparent(string traceparent, out ActivityTraceId trace
             return true;
         }
 
-        private byte HexCharToByte(char c)
-        {
-            if ((c >= '0') && (c <= '9'))
-            {
-                return (byte)(c - '0');
-            }
-
-            if ((c >= 'a') && (c <= 'f'))
-            {
-                return (byte)(c - 'a' + 10);
-            }
-
-            if ((c >= 'A') && (c <= 'F'))
-            {
-                return (byte)(c - 'A' + 10);
-            }
-
-            throw new ArgumentOutOfRangeException(nameof(c), $"Invalid character: {c}.");
-        }
-
-        private bool TryExtractTracestate(string[] tracestateCollection, out string tracestateResult)
+        internal static bool TryExtractTracestate(string[] tracestateCollection, out string tracestateResult)
         {
             tracestateResult = string.Empty;
 
@@ -304,5 +283,17 @@ private bool TryExtractTracestate(string[] tracestateCollection, out string trac
 
             return true;
         }
+
+        private static byte HexCharToByte(char c)
+        {
+            if (((c >= '0') && (c <= '9'))
+                || ((c >= 'a') && (c <= 'f'))
+                || ((c >= 'A') && (c <= 'F')))
+            {
+                return Convert.ToByte(c);
+            }
+
+            throw new ArgumentOutOfRangeException(nameof(c), $"Invalid character: {c}.");
+        }
     }
 }

src/OpenTelemetry.Api/Trace/SpanContext.cs

@@ -120,6 +120,12 @@ public IEnumerable<KeyValuePair<string, string>> TraceState
             }
         }
 
+        /// <summary>
+        /// Converts a <see cref="SpanContext"/> into an <see cref="ActivityContext"/>.
+        /// </summary>
+        /// <param name="spanContext"><see cref="SpanContext"/> source.</param>
+        public static implicit operator ActivityContext(SpanContext spanContext) => spanContext.ActivityContext;
+
         /// <summary>
         /// Compare two <see cref="SpanContext"/> for equality.
         /// </summary>

src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs

@@ -63,7 +63,7 @@ public override void OnStartActivity(Activity activity, object payload)
             {
                 // This requires to ignore the current activity and create a new one
                 // using the context extracted using the format TextFormat supports.
-                var ctx = this.options.TextFormat.Extract(request, HttpRequestHeaderValuesGetter);
+                var ctx = this.options.TextFormat.Extract(default, request, HttpRequestHeaderValuesGetter);
 
                 // Create a new activity with its parent set from the extracted context.
                 // This makes the new activity as a "sibling" of the activity created by

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs

@@ -50,9 +50,7 @@ public HttpInListener(string name, AspNetCoreInstrumentationOptions options, Act
 
         public override void OnStartActivity(Activity activity, object payload)
         {
-            var context = this.startContextFetcher.Fetch(payload) as HttpContext;
-
-            if (context == null)
+            if (!(this.startContextFetcher.Fetch(payload) is HttpContext context))
             {
                 AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity));
                 return;
@@ -72,7 +70,7 @@ public override void OnStartActivity(Activity activity, object payload)
                 // using the context extracted from w3ctraceparent header or
                 // using the format TextFormat supports.
 
-                var ctx = this.options.TextFormat.Extract(request, HttpRequestHeaderValuesGetter);
+                var ctx = this.options.TextFormat.Extract(default, request, HttpRequestHeaderValuesGetter);
 
                 // Create a new activity with its parent set from the extracted context.
                 // This makes the new activity as a "sibling" of the activity created by

src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs

@@ -81,7 +81,7 @@ IEnumerable<string> GetCarrierKeyValue(Dictionary<string, IEnumerable<string>> s
                     return value;
                 }
 
-                activityContext = this.textFormat.Extract(carrierMap, GetCarrierKeyValue);
+                activityContext = this.textFormat.Extract(default, carrierMap, GetCarrierKeyValue);
             }
 
             return !activityContext.IsValid() ? null : new SpanContextShim(new Trace.SpanContext(activityContext));
@@ -115,8 +115,7 @@ public void Inject<TCarrier>(
 
             if ((format == BuiltinFormats.TextMap || format == BuiltinFormats.HttpHeaders) && carrier is ITextMap textMapCarrier)
             {
-                // Remove comment after spanshim changes
-                // this.textFormat.Inject(shim.SpanContext, textMapCarrier, (instrumentation, key, value) => instrumentation.Set(key, value));
+                this.textFormat.Inject(shim.SpanContext, textMapCarrier, (instrumentation, key, value) => instrumentation.Set(key, value));
             }
         }
     }