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

[.NET7.0] Http client activity source migration #3415

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
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,30 @@ internal class HttpClientInstrumentation : IDisposable
{
private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;

private readonly Func<string, object, object, bool> isEnabled = (activityName, obj1, obj2)
=> !activityName.Equals("System.Net.Http.HttpRequestOut");

/// <summary>
/// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
/// </summary>
/// <param name="options">Configuration options for HTTP client instrumentation.</param>
public HttpClientInstrumentation(HttpClientInstrumentationOptions options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), null);
// For .NET7.0 activity will be created using activitySource.
// https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
// However, in case when activity creation returns null (due to sampling)
// the framework will fall back to creating activity anyways due to active diagnostic source listener
// To prevent this, isEnabled is implemented which will return false always
// so that the sampler's decision is respected.
if (HttpHandlerDiagnosticListener.IsNet7OrGreater)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), this.isEnabled);
}
else
{
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 @@ -28,6 +28,10 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
{
internal static readonly AssemblyName AssemblyName = typeof(HttpHandlerDiagnosticListener).Assembly.GetName();
internal static readonly bool IsNet7OrGreater;

// https://github.com/dotnet/runtime/blob/main/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L19
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
internal static readonly string FrameworkActivitySourceName = "System.Net.Http";
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());
Expand All @@ -40,6 +44,18 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
private readonly PropertyFetcher<TaskStatus> stopRequestStatusFetcher = new("RequestTaskStatus");
private readonly HttpClientInstrumentationOptions options;

static HttpHandlerDiagnosticListener()
{
try
{
IsNet7OrGreater = typeof(HttpClient).Assembly.GetName().Version.Major >= 7;
}
catch (Exception)
{
IsNet7OrGreater = false;
}
}

public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options)
: base("HttpHandlerDiagnosticListener")
{
Expand All @@ -58,7 +74,11 @@ public override void OnStartActivity(Activity activity, object payload)
// By this time, samplers have already run and
// activity.IsAllDataRequested populated accordingly.

if (Sdk.SuppressInstrumentation)
// For .NET7.0 or higher versions, activity is created using activity source
// However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
// To prevent processing such activities we first check the source name to confirm if it was created using
// activity source or not.
if (Sdk.SuppressInstrumentation || (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name)))
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}
Expand Down Expand Up @@ -108,8 +128,11 @@ public override void OnStartActivity(Activity activity, object payload)

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

ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);
if (!IsNet7OrGreater)
{
ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);
}

activity.SetTag(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
Expand All @@ -129,6 +152,15 @@ public override void OnStartActivity(Activity activity, object payload)

public override void OnStopActivity(Activity activity, object payload)
{
// For .NET7.0 or higher versions, activity is created using activity source
// However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
// To prevent processing such activities we first check the source name to confirm if it was created using
// activity source or not.
if (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name))
{
return;
}

if (activity.IsAllDataRequested)
{
// https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
Expand Down Expand Up @@ -178,6 +210,15 @@ public override void OnStopActivity(Activity activity, object payload)

public override void OnException(Activity activity, object payload)
{
// For .NET7.0 or higher versions, activity is created using activity source
// However, the framework will fallback to creating activity if the sampler's decision is to drop and there is a active diagnostic listener.
// To prevent processing such activities we first check the source name to confirm if it was created using
// activity source or not.
if (IsNet7OrGreater && string.IsNullOrEmpty(activity.Source.Name))
{
return;
}

if (activity.IsAllDataRequested)
{
if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,16 @@ internal static TracerProviderBuilder AddHttpClientInstrumentation(
this TracerProviderBuilder builder,
HttpClientInstrumentation instrumentation)
{
builder.AddSource(HttpHandlerDiagnosticListener.ActivitySourceName);
builder.AddLegacySource("System.Net.Http.HttpRequestOut");
if (HttpHandlerDiagnosticListener.IsNet7OrGreater)
{
builder.AddSource(HttpHandlerDiagnosticListener.FrameworkActivitySourceName);
}
else
{
builder.AddSource(HttpHandlerDiagnosticListener.ActivitySourceName);
builder.AddLegacySource("System.Net.Http.HttpRequestOut");
}

return builder.AddInstrumentation(() => instrumentation);
}

Expand Down