Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating CompositePropagator #923

Merged
merged 18 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
Setting an attribute with an existing key now results in overwriting it.
Setting null value has no impact except if null is set to an existing key,
it gets removed.
* `ITextFormatActivity` got replaced by `ITextFormat` with an additional method
to be implemented (`IsInjected`)
* Added `CompositePropagator` that accepts a list of `ITextFormat` following
eddynaka marked this conversation as resolved.
Show resolved Hide resolved
[specification](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#create-a-composite-propagator)

## 0.4.0-beta.2

Expand Down
74 changes: 74 additions & 0 deletions src/OpenTelemetry.Api/Context/Propagation/CompositePropagator.cs
Original file line number Diff line number Diff line change
@@ -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);
eddynaka marked this conversation as resolved.
Show resolved Hide resolved
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));
}
}
}
3 changes: 2 additions & 1 deletion src/OpenTelemetry.Api/Context/Propagation/ITextFormat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
65 changes: 28 additions & 37 deletions src/OpenTelemetry.Api/Context/Propagation/TraceContextFormat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
{
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
Expand All @@ -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);
Expand All @@ -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/>
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks to @CodeBlanch we'll get better help from .NET Activity API to do the parsing etc in the Rc1 release. We can refactor some of the code here at that time.

{
// from https://github.com/w3c/distributed-tracing/blob/master/trace_context/HTTP_HEADER_FORMAT.md
// traceparent: 00-0af7651916cd43dd8448eb211c80319c-00f067aa0ba902b7-01
Expand All @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;

Expand All @@ -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}.");
}
}
}
6 changes: 6 additions & 0 deletions src/OpenTelemetry.Api/Trace/SpanContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cijothomas I'm thinking about dropping implicit casts on all the shim classes. Why do that? Not sure. It just feels like a good thing to have. Could reduce bugs in places where we don't consider the Span case because we focus more on the Activity case? The idea is to allow Span version to be passed anywhere Activity version can be passed. How do you feel about that? 🤷


/// <summary>
/// Compare two <see cref="SpanContext"/> for equality.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
}
}
}
Expand Down
Loading