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

Rename of Propagators #1427

Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion examples/Console/TestOpenTracingWithConsoleExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal static object Run(OpenTracingShimOptions options)

// Following shows how to use the OpenTracing shim

var tracer = new TracerShim(TracerProvider.Default.GetTracer("MyCompany.MyProduct.MyWebServer"), new TextMapPropagator());
var tracer = new TracerShim(TracerProvider.Default.GetTracer("MyCompany.MyProduct.MyWebServer"), new TraceContextPropagator());

using (IScope parentScope = tracer.BuildSpan("Parent").StartActive(finishSpanOnDispose: true))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace Utils.Messaging
public class MessageReceiver : IDisposable
{
private static readonly ActivitySource ActivitySource = new ActivitySource(nameof(MessageReceiver));
private static readonly IPropagator Propagator = new TextMapPropagator();
private static readonly ITextMapPropagator Propagator = new TraceContextPropagator();

private readonly ILogger<MessageReceiver> logger;
private readonly IConnection connection;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace Utils.Messaging
public class MessageSender : IDisposable
{
private static readonly ActivitySource ActivitySource = new ActivitySource(nameof(MessageSender));
private static readonly IPropagator Propagator = new TextMapPropagator();
private static readonly ITextMapPropagator Propagator = new TraceContextPropagator();

private readonly ILogger<MessageSender> logger;
private readonly IConnection connection;
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* `B3Propagator` now supports the value `true` to be passed in for the header
`X-B3-Sampled`.
([#1413](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1413))
* Renamed IPropagator to ITextMapPropagator, TextMapPropagator to TraceContextPropagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))

## 0.7.0-beta.1

Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry.Api/Context/Propagation/B3Propagator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
namespace OpenTelemetry.Context.Propagation
{
/// <summary>
/// B3 text propagator. See https://github.com/openzipkin/b3-propagation for the specification.
/// A text map propagator for B3. See https://github.com/openzipkin/b3-propagation.
/// </summary>
public sealed class B3Propagator : IPropagator
public sealed class B3Propagator : ITextMapPropagator
Copy link
Member

@reyang reyang Oct 30, 2020

Choose a reason for hiding this comment

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

Do we want an interface ITextMapPropagator or an abstract class TextMapPropagator?
Interface gives benefit of multi-inheritance, which I don't see a useful scenario here (an extreme case would be having TraceContextPropagator implementing both the text and binary interface).
On the other side interface forbids us from adding optional methods in the future (will have to introduce ITextMapPropagator2, ITextMapPropagator3, ...).

Copy link
Member Author

Choose a reason for hiding this comment

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

yes i was thinking.. and agree :)

Copy link
Member

Choose a reason for hiding this comment

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

Base class seems like a good idea.

Just FYI as of C#8 you can add default implementation on an interface. Check out: https://devblogs.microsoft.com/dotnet/default-implementations-in-interfaces/

I haven't ever used it, but it's there 😄

Copy link
Member

Choose a reason for hiding this comment

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

Makes me wondering - if we have class Foobar: IFoo, IBar:

  1. Both IFoo and IBar decided to add a new default implementation and they share the same name 😂
  2. IFoo used to have IFoo.SayHello and now IBar decided to add the same thing 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to abstract base class now. We follow same approach for others part in SDK, like base processor, exporter etc.

{
internal const string XB3TraceId = "X-B3-TraceId";
internal const string XB3SpanId = "X-B3-SpanId";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
namespace OpenTelemetry.Context.Propagation
{
/// <summary>
/// W3C baggage: https://github.com/w3c/baggage/blob/master/baggage/HTTP_HEADER_FORMAT.md.
/// A text map propagator for W3C Baggage. See https://w3c.github.io/baggage/
/// </summary>
public class BaggagePropagator : IPropagator
public class BaggagePropagator : ITextMapPropagator
{
internal const string BaggageHeaderName = "Baggage";

Expand Down
10 changes: 5 additions & 5 deletions src/OpenTelemetry.Api/Context/Propagation/CompositePropagator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ namespace OpenTelemetry.Context.Propagation
/// <summary>
/// CompositePropagator provides a mechanism for combining multiple propagators into a single one.
/// </summary>
public class CompositePropagator : IPropagator
public class CompositePropagator : ITextMapPropagator
{
private static readonly ISet<string> EmptyFields = new HashSet<string>();
private readonly List<IPropagator> propagators;
private readonly List<ITextMapPropagator> propagators;

/// <summary>
/// Initializes a new instance of the <see cref="CompositePropagator"/> class.
/// </summary>
/// <param name="propagators">List of <see cref="IPropagator"/> wire context propagator.</param>
public CompositePropagator(IEnumerable<IPropagator> propagators)
/// <param name="propagators">List of <see cref="ITextMapPropagator"/> wire context propagator.</param>
public CompositePropagator(IEnumerable<ITextMapPropagator> propagators)
{
this.propagators = new List<IPropagator>(propagators ?? throw new ArgumentNullException(nameof(propagators)));
this.propagators = new List<ITextMapPropagator>(propagators ?? throw new ArgumentNullException(nameof(propagators)));
}

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="IPropagator.cs" company="OpenTelemetry Authors">
// <copyright file="ITextMapPropagator.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -20,10 +20,11 @@
namespace OpenTelemetry.Context.Propagation
{
/// <summary>
/// Defines an interface for Propagator, which is used to read and write
/// context data from and to message exchanges by the applications.
/// Defines an interface for a Propagator of TextMap type,
/// which uses string key/value pairs to inject and extract
/// propagation data.
/// </summary>
public interface IPropagator
public interface ITextMapPropagator
{
/// <summary>
/// Gets the list of headers used by propagator. The use cases of this are:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="TextMapPropagator.cs" company="OpenTelemetry Authors">
// <copyright file="TraceContextPropagator.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -24,9 +24,9 @@
namespace OpenTelemetry.Context.Propagation
{
/// <summary>
/// W3C trace context text wire protocol formatter. See https://github.com/w3c/distributed-tracing/.
/// A text map propagator for W3C trace context. See https://w3c.github.io/trace-context/.
/// </summary>
public class TextMapPropagator : IPropagator
public class TraceContextPropagator : ITextMapPropagator
{
private const string TraceParent = "traceparent";
private const string TraceState = "tracestate";
Expand All @@ -53,13 +53,13 @@ public PropagationContext Extract<T>(PropagationContext context, T carrier, Func

if (carrier == null)
{
OpenTelemetryApiEventSource.Log.FailedToExtractActivityContext(nameof(TextMapPropagator), "null carrier");
OpenTelemetryApiEventSource.Log.FailedToExtractActivityContext(nameof(TraceContextPropagator), "null carrier");
return context;
}

if (getter == null)
{
OpenTelemetryApiEventSource.Log.FailedToExtractActivityContext(nameof(TextMapPropagator), "null getter");
OpenTelemetryApiEventSource.Log.FailedToExtractActivityContext(nameof(TraceContextPropagator), "null getter");
return context;
}

Expand Down Expand Up @@ -94,7 +94,7 @@ public PropagationContext Extract<T>(PropagationContext context, T carrier, Func
}
catch (Exception ex)
{
OpenTelemetryApiEventSource.Log.ActivityContextExtractException(nameof(TextMapPropagator), ex);
OpenTelemetryApiEventSource.Log.ActivityContextExtractException(nameof(TraceContextPropagator), ex);
}

// in case of exception indicate to upstream that there is no parseable context from the top
Expand All @@ -106,19 +106,19 @@ public void Inject<T>(PropagationContext context, T carrier, Action<T, string, s
{
if (context.ActivityContext.TraceId == default || context.ActivityContext.SpanId == default)
{
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TextMapPropagator), "Invalid context");
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TraceContextPropagator), "Invalid context");
return;
}

if (carrier == null)
{
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TextMapPropagator), "null carrier");
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TraceContextPropagator), "null carrier");
return;
}

if (setter == null)
{
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TextMapPropagator), "null setter");
OpenTelemetryApiEventSource.Log.FailedToInjectActivityContext(nameof(TraceContextPropagator), "null setter");
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ namespace OpenTelemetry.Instrumentation.AspNet
public class AspNetInstrumentationOptions
{
/// <summary>
/// Gets or sets <see cref="IPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TextMapPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// Gets or sets <see cref="ITextMapPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TraceContextPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// </summary>
public IPropagator Propagator { get; set; } = new CompositePropagator(new IPropagator[]
public ITextMapPropagator Propagator { get; set; } = new CompositePropagator(new ITextMapPropagator[]
{
new TextMapPropagator(),
new TraceContextPropagator(),
new BaggagePropagator(),
});

Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Renamed IPropagator to ITextMapPropagator, TextMapPropagator to TraceContextPropagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))

## 0.7.0-beta.1

Released 2020-Oct-16
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public override void OnStartActivity(Activity activity, object payload)
var request = context.Request;
var requestValues = request.Unvalidated;

if (!(this.options.Propagator is TextMapPropagator))
if (!(this.options.Propagator is TraceContextPropagator))
{
var ctx = this.options.Propagator.Extract(default, request, HttpRequestHeaderValuesGetter);

Expand Down Expand Up @@ -139,7 +139,7 @@ public override void OnStopActivity(Activity activity, object payload)
Activity activityToEnrich = activity;
Activity createdActivity = null;

bool isCustomPropagator = !(this.options.Propagator is TextMapPropagator);
bool isCustomPropagator = !(this.options.Propagator is TraceContextPropagator);

if (isCustomPropagator)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ namespace OpenTelemetry.Instrumentation.AspNetCore
public class AspNetCoreInstrumentationOptions
{
/// <summary>
/// Gets or sets <see cref="IPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TextMapPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// Gets or sets <see cref="ITextMapPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TraceContextPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// </summary>
public IPropagator Propagator { get; set; } = new CompositePropagator(new IPropagator[]
public ITextMapPropagator Propagator { get; set; } = new CompositePropagator(new ITextMapPropagator[]
{
new TextMapPropagator(),
new TraceContextPropagator(),
new BaggagePropagator(),
});

Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
[Grpc.AspNetCore](https://www.nuget.org/packages/Grpc.AspNetCore/).
This option is enabled by default.
([#1423](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1423))
* Renamed IPropagator to ITextMapPropagator, TextMapPropagator to TraceContextPropagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))

## 0.7.0-beta.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public override void OnStartActivity(Activity activity, object payload)
}

var request = context.Request;
if (!this.hostingSupportsW3C || !(this.options.Propagator is TextMapPropagator))
if (!this.hostingSupportsW3C || !(this.options.Propagator is TraceContextPropagator))
{
var ctx = this.options.Propagator.Extract(default, request, HttpRequestHeaderValuesGetter);

Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
`HttpWebRequest` in Activity.CustomProperty. To enrich activity, use the
Enrich action on the instrumentation.
([#1407](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1407))
* Renamed IPropagator to ITextMapPropagator, TextMapPropagator to TraceContextPropagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))

## 0.7.0-beta.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ public class HttpClientInstrumentationOptions
public bool SetHttpFlavor { get; set; }

/// <summary>
/// Gets or sets <see cref="IPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TextMapPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// Gets or sets <see cref="ITextMapPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TraceContextPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// </summary>
public IPropagator Propagator { get; set; } = new CompositePropagator(new IPropagator[]
public ITextMapPropagator Propagator { get; set; } = new CompositePropagator(new ITextMapPropagator[]
{
new TextMapPropagator(),
new TraceContextPropagator(),
new BaggagePropagator(),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ public class HttpWebRequestInstrumentationOptions
public bool SetHttpFlavor { get; set; }

/// <summary>
/// Gets or sets <see cref="IPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TextMapPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// Gets or sets <see cref="ITextMapPropagator"/> for context propagation. Default value: <see cref="CompositePropagator"/> with <see cref="TraceContextPropagator"/> &amp; <see cref="BaggagePropagator"/>.
/// </summary>
public IPropagator Propagator { get; set; } = new CompositePropagator(new IPropagator[]
public ITextMapPropagator Propagator { get; set; } = new CompositePropagator(new ITextMapPropagator[]
{
new TextMapPropagator(),
new TraceContextPropagator(),
new BaggagePropagator(),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

if (!(this.httpClientSupportsW3C && this.options.Propagator is TextMapPropagator))
if (!(this.httpClientSupportsW3C && this.options.Propagator is TraceContextPropagator))
{
this.options.Propagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpRequestMessageHeaderValueSetter);
}
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Changelog

## Unreleased
* Renamed IPropagator to ITextMapPropagator, TextMapPropagator to TraceContextPropagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))

## 0.7.0-beta.1

Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ namespace OpenTelemetry.Shims.OpenTracing
public class TracerShim : global::OpenTracing.ITracer
{
private readonly Trace.Tracer tracer;
private readonly IPropagator propagator;
private readonly ITextMapPropagator propagator;

public TracerShim(Trace.Tracer tracer, IPropagator textFormat)
public TracerShim(Trace.Tracer tracer, ITextMapPropagator textFormat)
{
this.tracer = tracer ?? throw new ArgumentNullException(nameof(tracer));
this.propagator = textFormat ?? throw new ArgumentNullException(nameof(textFormat));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public void AspNetRequestsAreCollectedSuccessfully(

var expectedTraceId = ActivityTraceId.CreateRandom();
var expectedSpanId = ActivitySpanId.CreateRandom();
var propagator = new Mock<IPropagator>();
var propagator = new Mock<ITextMapPropagator>();
propagator.Setup(m => m.Extract<HttpRequest>(It.IsAny<PropagationContext>(), It.IsAny<HttpRequest>(), It.IsAny<Func<HttpRequest, string, IEnumerable<string>>>())).Returns(new PropagationContext(
new ActivityContext(
expectedTraceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public async Task CustomPropagator()
var expectedTraceId = ActivityTraceId.CreateRandom();
var expectedSpanId = ActivitySpanId.CreateRandom();

var propagator = new Mock<IPropagator>();
var propagator = new Mock<ITextMapPropagator>();
propagator.Setup(m => m.Extract(It.IsAny<PropagationContext>(), It.IsAny<HttpRequest>(), It.IsAny<Func<HttpRequest, string, IEnumerable<string>>>())).Returns(
new PropagationContext(
new ActivityContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync(bool shouldEnrich
parent.ActivityTraceFlags = ActivityTraceFlags.Recorded;

// Ensure that the header value func does not throw if the header key can't be found
var mockPropagator = new Mock<IPropagator>();
var mockPropagator = new Mock<ITextMapPropagator>();

// var isInjectedHeaderValueGetterThrows = false;
// mockTextFormat
Expand Down Expand Up @@ -133,7 +133,7 @@ public async Task HttpClientInstrumentationInjectsHeadersAsync(bool shouldEnrich
[InlineData(false)]
public async Task HttpClientInstrumentationInjectsHeadersAsync_CustomFormat(bool shouldEnrich)
{
var propagator = new Mock<IPropagator>();
var propagator = new Mock<ITextMapPropagator>();
propagator.Setup(m => m.Inject<HttpRequestMessage>(It.IsAny<PropagationContext>(), It.IsAny<HttpRequestMessage>(), It.IsAny<Action<HttpRequestMessage, string, string>>()))
.Callback<PropagationContext, HttpRequestMessage, Action<HttpRequestMessage, string, string>>((context, message, action) =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public async Task HttpWebRequestInstrumentationInjectsHeadersAsync()
[Fact]
public async Task HttpWebRequestInstrumentationInjectsHeadersAsync_CustomFormat()
{
var propagator = new Mock<IPropagator>();
var propagator = new Mock<ITextMapPropagator>();
propagator.Setup(m => m.Inject(It.IsAny<PropagationContext>(), It.IsAny<HttpWebRequest>(), It.IsAny<Action<HttpWebRequest, string, string>>()))
.Callback<PropagationContext, HttpWebRequest, Action<HttpWebRequest, string, string>>((context, message, action) =>
{
Expand Down
Loading