Skip to content

Commit

Permalink
Add ActivitySource support to DiagnosticsHandler (#64753)
Browse files Browse the repository at this point in the history
* Brought back changes from #54437

* Fixed tests

* feedback
  • Loading branch information
ManickaP committed Feb 5, 2022
1 parent dbd4cbb commit d8c0170
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 114 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace System.Net.Http
/// </summary>
internal sealed class DiagnosticsHandler : HttpMessageHandlerStage
{
private static readonly DiagnosticListener s_diagnosticListener =
new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName);
private static readonly DiagnosticListener s_diagnosticListener = new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName);
private static readonly ActivitySource s_activitySource = new ActivitySource(DiagnosticsHandlerLoggingStrings.Namespace);

private readonly HttpMessageHandler _innerHandler;
private readonly DistributedContextPropagator _propagator;
Expand Down Expand Up @@ -47,8 +47,29 @@ public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPro

private static bool IsEnabled()
{
// check if there is a parent Activity or if someone listens to HttpHandlerDiagnosticListener
return Activity.Current != null || s_diagnosticListener.IsEnabled();
// check if there is a parent Activity or if someone listens to "System.Net.Http" ActivitySource or "HttpHandlerDiagnosticListener" DiagnosticListener.
return Activity.Current != null ||
s_activitySource.HasListeners() ||
s_diagnosticListener.IsEnabled();
}

private static Activity? CreateActivity(HttpRequestMessage requestMessage)
{
Activity? activity = null;
if (s_activitySource.HasListeners())
{
activity = s_activitySource.CreateActivity(DiagnosticsHandlerLoggingStrings.ActivityName, ActivityKind.Client);
}

if (activity is null)
{
if (Activity.Current is not null || s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, requestMessage))
{
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
}
}

return activity;
}

internal static bool IsGloballyEnabled() => GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation;
Expand Down Expand Up @@ -91,60 +112,38 @@ internal override ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage re
}
}

Activity? activity = null;
DiagnosticListener diagnosticListener = s_diagnosticListener;

// if there is no listener, but propagation is enabled (with previous IsEnabled() check)
// do not write any events just start/stop Activity and propagate Ids
if (!diagnosticListener.IsEnabled())
{
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
activity.Start();
InjectHeaders(activity, request);

try
{
return async ?
await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false) :
_innerHandler.Send(request, cancellationToken);
}
finally
{
activity.Stop();
}
}

Guid loggingRequestId = Guid.Empty;
Activity? activity = CreateActivity(request);

// There is a listener. Check if listener wants to be notified about HttpClient Activities
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, request))
// Start activity anyway if it was created.
if (activity is not null)
{
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
activity.Start();

// Only send start event to users who subscribed for it, but start activity anyway
// Only send start event to users who subscribed for it.
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName))
{
StartActivity(diagnosticListener, activity, new ActivityStartData(request));
}
else
{
activity.Start();
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ActivityStartName, new ActivityStartData(request));
}
}
// try to write System.Net.Http.Request event (deprecated)

// Try to write System.Net.Http.Request event (deprecated)
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated))
{
long timestamp = Stopwatch.GetTimestamp();
loggingRequestId = Guid.NewGuid();
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated,
new RequestData(request, loggingRequestId, timestamp));
new RequestData(
request,
loggingRequestId,
timestamp));
}

// If we are on at all, we propagate current activity information
Activity? currentActivity = Activity.Current;
if (currentActivity != null)
if (activity is not null)
{
InjectHeaders(currentActivity, request);
InjectHeaders(activity, request);
}

HttpResponseMessage? response = null;
Expand Down Expand Up @@ -178,17 +177,20 @@ internal override ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage re
}
finally
{
// always stop activity if it was started
if (activity != null)
// Always stop activity if it was started.
if (activity is not null)
{
StopActivity(diagnosticListener, activity, new ActivityStopData(
response,
// If request is failed or cancelled, there is no response, therefore no information about request;
// pass the request in the payload, so consumers can have it in Stop for failed/canceled requests
// and not retain all requests in Start
request,
taskStatus));
activity.SetEndTime(DateTime.UtcNow);

// Only send stop event to users who subscribed for it.
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStopName))
{
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ActivityStopName, new ActivityStopData(response, request, taskStatus));
}

activity.Stop();
}

// Try to write System.Net.Http.Response event (deprecated)
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated))
{
Expand Down Expand Up @@ -335,27 +337,6 @@ private void InjectHeaders(Activity currentActivity, HttpRequestMessage request)
{
diagnosticSource.Write(name, value);
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
Justification = "The args being passed into StartActivity have the commonly used properties being preserved with DynamicDependency.")]
private static Activity StartActivity<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(
DiagnosticSource diagnosticSource,
Activity activity,
T? args)
{
return diagnosticSource.StartActivity(activity, args);
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
Justification = "The args being passed into StopActivity have the commonly used properties being preserved with DynamicDependency.")]
private static void StopActivity<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(
DiagnosticSource diagnosticSource,
Activity activity,
T? args)
{
diagnosticSource.StopActivity(activity, args);
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ namespace System.Net.Http
/// </summary>
internal static class DiagnosticsHandlerLoggingStrings
{
public const string DiagnosticListenerName = "HttpHandlerDiagnosticListener";
public const string RequestWriteNameDeprecated = "System.Net.Http.Request";
public const string ResponseWriteNameDeprecated = "System.Net.Http.Response";

public const string ExceptionEventName = "System.Net.Http.Exception";
public const string ActivityName = "System.Net.Http.HttpRequestOut";
public const string ActivityStartName = "System.Net.Http.HttpRequestOut.Start";
public const string DiagnosticListenerName = "HttpHandlerDiagnosticListener";
public const string Namespace = "System.Net.Http";
public const string RequestWriteNameDeprecated = Namespace + ".Request";
public const string ResponseWriteNameDeprecated = Namespace + ".Response";
public const string ExceptionEventName = Namespace + ".Exception";
public const string ActivityName = Namespace + ".HttpRequestOut";
public const string ActivityStartName = ActivityName + ".Start";
public const string ActivityStopName = ActivityName + ".Stop";
}
}
149 changes: 112 additions & 37 deletions src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.Tracing;
using System.IO;
using System.Linq;
using System.Net.Http.Headers;
using System.Net.Test.Common;
Expand Down Expand Up @@ -802,43 +803,6 @@ public static IEnumerable<object[]> UseSocketsHttpHandler_WithIdFormat_MemberDat
yield return new object[] { false, ActivityIdFormat.W3C };
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
[MemberData(nameof(UseSocketsHttpHandler_WithIdFormat_MemberData))]
public async Task SendAsync_ExpectedActivityPropagationWithoutListener(bool useSocketsHttpHandler, ActivityIdFormat idFormat)
{
Activity parent = new Activity("parent");
parent.SetIdFormat(idFormat);
parent.Start();

await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
async uri =>
{
await GetAsync(UseVersion.ToString(), TestAsync.ToString(), uri, useSocketsHttpHandler: useSocketsHttpHandler);
},
async server =>
{
HttpRequestData requestData = await server.HandleRequestAsync();
AssertHeadersAreInjected(requestData, parent);
});
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
[InlineData(true)]
[InlineData(false)]
public async Task SendAsync_ExpectedActivityPropagationWithoutListenerOrParentActivity(bool useSocketsHttpHandler)
{
await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
async uri =>
{
await GetAsync(UseVersion.ToString(), TestAsync.ToString(), uri, useSocketsHttpHandler: useSocketsHttpHandler);
},
async server =>
{
HttpRequestData requestData = await server.HandleRequestAsync();
AssertNoHeadersAreInjected(requestData);
});
}

[ConditionalTheory(nameof(EnableActivityPropagationEnvironmentVariableIsNotSetAndRemoteExecutorSupported))]
[InlineData("true")]
[InlineData("1")]
Expand Down Expand Up @@ -998,6 +962,117 @@ public async Task SendAsync_CustomSocketsHttpHandlerPropagator_PropagatorIsUsed(
});
}

public static IEnumerable<object[]> SocketsHttpHandler_ActivityCreation_MemberData()
{
foreach (var currentActivitySet in new bool[] {
true, // Activity was set
false }) // No Activity is set
{
foreach (var diagnosticListenerActivityEnabled in new bool?[] {
true, // DiagnosticListener requested an Activity
false, // DiagnosticListener does not want an Activity
null }) // There is no DiagnosticListener
{
foreach (var activitySourceCreatesActivity in new bool?[] {
true, // ActivitySource created an Activity
false, // ActivitySource chose not to create an Activity
null }) // ActivitySource had no listeners
{
yield return new object[] { currentActivitySet, diagnosticListenerActivityEnabled, activitySourceCreatesActivity };
}
}
}
}

[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[MemberData(nameof(SocketsHttpHandler_ActivityCreation_MemberData))]
public void SendAsync_ActivityIsCreatedIfRequested(bool currentActivitySet, bool? diagnosticListenerActivityEnabled, bool? activitySourceCreatesActivity)
{
string parameters = $"{currentActivitySet},{diagnosticListenerActivityEnabled},{activitySourceCreatesActivity}";

RemoteExecutor.Invoke(async (useVersion, testAsync, parametersString) =>
{
bool?[] parameters = parametersString.Split(',').Select(p => p.Length == 0 ? (bool?)null : bool.Parse(p)).ToArray();
bool currentActivitySet = parameters[0].Value;
bool? diagnosticListenerActivityEnabled = parameters[1];
bool? activitySourceCreatesActivity = parameters[2];
bool madeASamplingDecision = false;
if (activitySourceCreatesActivity.HasValue)
{
ActivitySource.AddActivityListener(new ActivityListener
{
ShouldListenTo = _ => true,
Sample = (ref ActivityCreationOptions<ActivityContext> _) =>
{
madeASamplingDecision = true;
return activitySourceCreatesActivity.Value ? ActivitySamplingResult.AllData : ActivitySamplingResult.None;
}
});
}
bool listenerCallbackWasCalled = false;
IDisposable listenerSubscription = new MemoryStream(); // Dummy disposable
if (diagnosticListenerActivityEnabled.HasValue)
{
var diagnosticListenerObserver = new FakeDiagnosticListenerObserver(_ => listenerCallbackWasCalled = true);
diagnosticListenerObserver.Enable(name => !name.Contains("HttpRequestOut") || diagnosticListenerActivityEnabled.Value);
listenerSubscription = DiagnosticListener.AllListeners.Subscribe(diagnosticListenerObserver);
}
Activity parent = currentActivitySet ? new Activity("parent").Start() : null;
Activity activity = parent;
if (!currentActivitySet)
{
// Listen to new activity creations if an Activity was created without a parent
// (when a DiagnosticListener forced one to be created)
ActivitySource.AddActivityListener(new ActivityListener
{
ShouldListenTo = _ => true,
ActivityStarted = created =>
{
Assert.Null(parent);
activity = created;
}
});
}
using (listenerSubscription)
{
await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
async uri =>
{
await GetAsync(useVersion, testAsync, uri);
},
async server =>
{
HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync();
if (currentActivitySet || diagnosticListenerActivityEnabled == true || activitySourceCreatesActivity == true)
{
Assert.NotNull(activity);
AssertHeadersAreInjected(requestData, activity, parent is null);
}
else
{
AssertNoHeadersAreInjected(requestData);
if (!currentActivitySet)
{
Assert.Null(activity);
}
}
});
}
Assert.Equal(activitySourceCreatesActivity.HasValue, madeASamplingDecision);
Assert.Equal(diagnosticListenerActivityEnabled.HasValue, listenerCallbackWasCalled);
}, UseVersion.ToString(), TestAsync.ToString(), parameters).Dispose();
}

private static T GetProperty<T>(object obj, string propertyName)
{
Type t = obj.GetType();
Expand Down

0 comments on commit d8c0170

Please sign in to comment.