Skip to content

Commit

Permalink
Remove ActivitySourceAdapter (#1836)
Browse files Browse the repository at this point in the history
Removed ActivitySourceAdapater and made TracerProviderSdk natively support legacy activites
  • Loading branch information
utpilla authored Feb 25, 2021
1 parent 39841e5 commit 0581ce2
Show file tree
Hide file tree
Showing 32 changed files with 969 additions and 732 deletions.
2 changes: 1 addition & 1 deletion docs/trace/getting-started/getting-started.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<ItemGroup>
<!---
<PackageReference Include="OpenTelemetry.Exporter.Console" Version="$(OpenTelemetryExporterConsolePkgVer)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ internal class AspNetInstrumentation : IDisposable
/// <summary>
/// Initializes a new instance of the <see cref="AspNetInstrumentation"/> class.
/// </summary>
/// <param name="activitySource">ActivitySource adapter instance.</param>
/// <param name="options">Configuration options for ASP.NET instrumentation.</param>
public AspNetInstrumentation(ActivitySourceAdapter activitySource, AspNetInstrumentationOptions options)
public AspNetInstrumentation(AspNetInstrumentationOptions options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
name => new HttpInListener(name, options, activitySource),
name => new HttpInListener(name, options),
listener => listener.Name == AspNetDiagnosticListenerName,
null);
this.diagnosticSourceSubscriber.Subscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,11 @@ internal class HttpInListener : ListenerHandler
private readonly PropertyFetcher<object> routeFetcher = new PropertyFetcher<object>("Route");
private readonly PropertyFetcher<string> routeTemplateFetcher = new PropertyFetcher<string>("RouteTemplate");
private readonly AspNetInstrumentationOptions options;
private readonly ActivitySourceAdapter activitySource;

public HttpInListener(string name, AspNetInstrumentationOptions options, ActivitySourceAdapter activitySource)
public HttpInListener(string name, AspNetInstrumentationOptions options)
: base(name)
{
this.options = options ?? throw new ArgumentNullException(nameof(options));
this.activitySource = activitySource;
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Activity is retrieved from Activity.Current later and disposed.")]
Expand Down Expand Up @@ -98,6 +96,9 @@ public override void OnStartActivity(Activity activity, object payload)
// correctly stop and restore Activity.Current.
newOne.SetCustomProperty("OTel.ActivityByAspNet", activity);
activity.SetCustomProperty("OTel.ActivityByHttpInListener", newOne);

// Set IsAllDataRequested to false for the activity created by the framework to only export the sibling activity and not the framework activity
activity.IsAllDataRequested = false;
activity = newOne;
}

Expand All @@ -111,7 +112,8 @@ public override void OnStartActivity(Activity activity, object payload)
var path = requestValues.Path;
activity.DisplayName = path;

this.activitySource.Start(activity, ActivityKind.Server, ActivitySource);
ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server);

if (activity.IsAllDataRequested)
{
Expand Down Expand Up @@ -244,8 +246,6 @@ public override void OnStopActivity(Activity activity, object payload)
Activity.Current = activity;
}
}

this.activitySource.Stop(activityToEnrich);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using OpenTelemetry.Instrumentation.AspNet;
using OpenTelemetry.Instrumentation.AspNet.Implementation;

namespace OpenTelemetry.Trace
{
Expand All @@ -42,7 +43,10 @@ public static TracerProviderBuilder AddAspNetInstrumentation(
var aspnetOptions = new AspNetInstrumentationOptions();
configureAspNetInstrumentationOptions?.Invoke(aspnetOptions);

builder.AddDiagnosticSourceInstrumentation((activitySource) => new AspNetInstrumentation(activitySource, aspnetOptions));
builder.AddInstrumentation(() => new AspNetInstrumentation(aspnetOptions));
builder.AddSource(HttpInListener.ActivitySourceName);
builder.AddLegacyActivity("Microsoft.AspNet.HttpReqIn"); // for the activities created by AspNetCore
builder.AddLegacyActivity("ActivityCreatedByHttpInListener"); // for the sibling activities created by the instrumentation library

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ internal class AspNetCoreInstrumentation : IDisposable
/// <summary>
/// Initializes a new instance of the <see cref="AspNetCoreInstrumentation"/> class.
/// </summary>
/// <param name="activitySource">ActivitySource adapter instance.</param>
/// <param name="options">Configuration options for ASP.NET Core instrumentation.</param>
public AspNetCoreInstrumentation(ActivitySourceAdapter activitySource, AspNetCoreInstrumentationOptions options)
public AspNetCoreInstrumentation(AspNetCoreInstrumentationOptions options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInListener("Microsoft.AspNetCore", options, activitySource), null);
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInListener("Microsoft.AspNetCore", options), null);
this.diagnosticSourceSubscriber.Subscribe();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
internal class HttpInListener : ListenerHandler
{
internal const string ActivityOperationName = "Microsoft.AspNetCore.Hosting.HttpRequestIn";
internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
private const string UnknownHostName = "UNKNOWN-HOST";
private const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
private readonly PropertyFetcher<HttpContext> startContextFetcher = new PropertyFetcher<HttpContext>("HttpContext");
private readonly PropertyFetcher<HttpContext> stopContextFetcher = new PropertyFetcher<HttpContext>("HttpContext");
Expand All @@ -45,14 +46,12 @@ internal class HttpInListener : ListenerHandler
private readonly PropertyFetcher<string> beforeActionTemplateFetcher = new PropertyFetcher<string>("Template");
private readonly bool hostingSupportsW3C;
private readonly AspNetCoreInstrumentationOptions options;
private readonly ActivitySourceAdapter activitySource;

public HttpInListener(string name, AspNetCoreInstrumentationOptions options, ActivitySourceAdapter activitySource)
public HttpInListener(string name, AspNetCoreInstrumentationOptions options)
: base(name)
{
this.hostingSupportsW3C = typeof(HttpRequest).Assembly.GetName().Version.Major >= 3;
this.options = options ?? throw new ArgumentNullException(nameof(options));
this.activitySource = activitySource;
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The objects should not be disposed.")]
Expand Down Expand Up @@ -99,6 +98,9 @@ public override void OnStartActivity(Activity activity, object payload)

// Starting the new activity make it the Activity.Current one.
newOne.Start();

// Set IsAllDataRequested to false for the activity created by the framework to only export the sibling activity and not the framework activity
activity.IsAllDataRequested = false;
activity = newOne;
}

Expand All @@ -108,7 +110,8 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

this.activitySource.Start(activity, ActivityKind.Server, ActivitySource);
ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server);

if (activity.IsAllDataRequested)
{
Expand Down Expand Up @@ -208,8 +211,6 @@ public override void OnStopActivity(Activity activity, object payload)
// the one created by the instrumentation.
// And retrieve it here, and set it to Current.
}

this.activitySource.Stop(activity);
}

public override void OnCustom(string name, Activity activity, object payload)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using OpenTelemetry.Instrumentation.AspNetCore;
using OpenTelemetry.Instrumentation.AspNetCore.Implementation;

namespace OpenTelemetry.Trace
{
Expand All @@ -41,7 +42,10 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation(

var aspnetCoreOptions = new AspNetCoreInstrumentationOptions();
configureAspNetCoreInstrumentationOptions?.Invoke(aspnetCoreOptions);
builder.AddDiagnosticSourceInstrumentation((activitySource) => new AspNetCoreInstrumentation(activitySource, aspnetCoreOptions));
builder.AddInstrumentation(() => new AspNetCoreInstrumentation(aspnetCoreOptions));
builder.AddSource(HttpInListener.ActivitySourceName);
builder.AddLegacyActivity(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore
builder.AddLegacyActivity(HttpInListener.ActivityNameByHttpInListener); // for the sibling activities created by the instrumentation library

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,10 @@ internal class GrpcClientInstrumentation : IDisposable
/// <summary>
/// Initializes a new instance of the <see cref="GrpcClientInstrumentation"/> class.
/// </summary>
/// <param name="activitySource">ActivitySource adapter instance.</param>
/// <param name="options">Configuration options for Grpc client instrumentation.</param>
public GrpcClientInstrumentation(ActivitySourceAdapter activitySource, GrpcClientInstrumentationOptions options = null)
public GrpcClientInstrumentation(GrpcClientInstrumentationOptions options = null)
{
if (activitySource == null)
{
throw new ArgumentNullException(nameof(activitySource));
}

this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new GrpcClientDiagnosticListener(activitySource, options), null);
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new GrpcClientDiagnosticListener(options), null);
this.diagnosticSourceSubscriber.Subscribe();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,13 @@ internal class GrpcClientDiagnosticListener : ListenerHandler
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

private readonly GrpcClientInstrumentationOptions options;
private readonly ActivitySourceAdapter activitySource;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new PropertyFetcher<HttpRequestMessage>("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopRequestFetcher = new PropertyFetcher<HttpResponseMessage>("Response");

public GrpcClientDiagnosticListener(ActivitySourceAdapter activitySource, GrpcClientInstrumentationOptions options)
public GrpcClientDiagnosticListener(GrpcClientInstrumentationOptions options)
: base("Grpc.Net.Client")
{
if (activitySource == null)
{
throw new ArgumentNullException(nameof(activitySource));
}

this.options = options;
this.activitySource = activitySource;
}

public override void OnStartActivity(Activity activity, object payload)
Expand Down Expand Up @@ -89,7 +82,8 @@ public override void OnStartActivity(Activity activity, object payload)

activity.DisplayName = grpcMethod?.Trim('/');

this.activitySource.Start(activity, ActivityKind.Client, ActivitySource);
ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);

if (activity.IsAllDataRequested)
{
Expand Down Expand Up @@ -158,8 +152,6 @@ public override void OnStopActivity(Activity activity, object payload)
}
}
}

this.activitySource.Stop(activity);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using OpenTelemetry.Instrumentation.GrpcNetClient;
using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation;

namespace OpenTelemetry.Trace
{
Expand Down Expand Up @@ -43,7 +44,10 @@ public static TracerProviderBuilder AddGrpcClientInstrumentation(
var grpcOptions = new GrpcClientInstrumentationOptions();
configure?.Invoke(grpcOptions);

builder.AddDiagnosticSourceInstrumentation((activitySource) => new GrpcClientInstrumentation(activitySource, grpcOptions));
builder.AddInstrumentation(() => new GrpcClientInstrumentation(grpcOptions));
builder.AddSource(GrpcClientDiagnosticListener.ActivitySourceName);
builder.AddLegacyActivity("Grpc.Net.Client.GrpcOut");

return builder;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ internal class HttpClientInstrumentation : IDisposable
/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
/// </summary>
/// <param name="activitySourceAdapter">ActivitySource adapter instance.</param>
/// <param name="options">Configuration options for HTTP client instrumentation.</param>
public HttpClientInstrumentation(ActivitySourceAdapter activitySourceAdapter, HttpClientInstrumentationOptions options)
public HttpClientInstrumentation(HttpClientInstrumentationOptions options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options, activitySourceAdapter), null);
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), null);
this.diagnosticSourceSubscriber.Subscribe();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ internal class HttpHandlerDiagnosticListener : ListenerHandler

private static readonly Regex CoreAppMajorVersionCheckRegex = new Regex("^\\.NETCoreApp,Version=v(\\d+)\\.", RegexOptions.Compiled | RegexOptions.IgnoreCase);

private readonly ActivitySourceAdapter activitySource;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new PropertyFetcher<HttpRequestMessage>("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopResponseFetcher = new PropertyFetcher<HttpResponseMessage>("Response");
private readonly PropertyFetcher<Exception> stopExceptionFetcher = new PropertyFetcher<Exception>("Exception");
private readonly PropertyFetcher<TaskStatus> stopRequestStatusFetcher = new PropertyFetcher<TaskStatus>("RequestTaskStatus");
private readonly bool httpClientSupportsW3C;
private readonly HttpClientInstrumentationOptions options;

public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options, ActivitySourceAdapter activitySource)
public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options)
: base("HttpHandlerDiagnosticListener")
{
var framework = Assembly
Expand All @@ -64,7 +63,6 @@ public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options, A
}

this.options = options;
this.activitySource = activitySource;
}

public override void OnStartActivity(Activity activity, object payload)
Expand Down Expand Up @@ -107,7 +105,8 @@ public override void OnStartActivity(Activity activity, object payload)

activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);

this.activitySource.Start(activity, ActivityKind.Client, ActivitySource);
ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);

if (activity.IsAllDataRequested)
{
Expand Down Expand Up @@ -178,8 +177,6 @@ public override void OnStopActivity(Activity activity, object payload)
}
}
}

this.activitySource.Stop(activity);
}

public override void OnException(Activity activity, object payload)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

using System;
using OpenTelemetry.Instrumentation.Http;
#if NETFRAMEWORK
using OpenTelemetry.Instrumentation.Http.Implementation;
#endif

namespace OpenTelemetry.Trace
{
Expand Down Expand Up @@ -60,7 +58,9 @@ public static TracerProviderBuilder AddHttpClientInstrumentation(

configureHttpClientInstrumentationOptions?.Invoke(httpClientOptions);

builder.AddDiagnosticSourceInstrumentation((activitySource) => new HttpClientInstrumentation(activitySource, httpClientOptions));
builder.AddInstrumentation(() => new HttpClientInstrumentation(httpClientOptions));
builder.AddSource(HttpHandlerDiagnosticListener.ActivitySourceName);
builder.AddLegacyActivity("System.Net.Http.HttpRequestOut");

#if NETFRAMEWORK
builder.AddHttpWebRequestInstrumentation(configureHttpWebRequestInstrumentationOptions);
Expand Down
3 changes: 2 additions & 1 deletion src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
1 change: 1 addition & 0 deletions src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool
Loading

0 comments on commit 0581ce2

Please sign in to comment.