Skip to content

Commit d8c0170

Browse files
ManickaPManickaP
authored
Add ActivitySource support to DiagnosticsHandler (#64753)
* Brought back changes from #54437 * Fixed tests * feedback
1 parent dbd4cbb commit d8c0170

File tree

3 files changed

+171
-114
lines changed

3 files changed

+171
-114
lines changed

‎src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs

Lines changed: 51 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ namespace System.Net.Http
1515
/// </summary>
1616
internal sealed class DiagnosticsHandler : HttpMessageHandlerStage
1717
{
18-
private static readonly DiagnosticListener s_diagnosticListener =
19-
new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName);
18+
private static readonly DiagnosticListener s_diagnosticListener = new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName);
19+
private static readonly ActivitySource s_activitySource = new ActivitySource(DiagnosticsHandlerLoggingStrings.Namespace);
2020

2121
private readonly HttpMessageHandler _innerHandler;
2222
private readonly DistributedContextPropagator _propagator;
@@ -47,8 +47,29 @@ public DiagnosticsHandler(HttpMessageHandler innerHandler, DistributedContextPro
4747

4848
private static bool IsEnabled()
4949
{
50-
// check if there is a parent Activity or if someone listens to HttpHandlerDiagnosticListener
51-
return Activity.Current != null || s_diagnosticListener.IsEnabled();
50+
// check if there is a parent Activity or if someone listens to "System.Net.Http" ActivitySource or "HttpHandlerDiagnosticListener" DiagnosticListener.
51+
return Activity.Current != null ||
52+
s_activitySource.HasListeners() ||
53+
s_diagnosticListener.IsEnabled();
54+
}
55+
56+
private static Activity? CreateActivity(HttpRequestMessage requestMessage)
57+
{
58+
Activity? activity = null;
59+
if (s_activitySource.HasListeners())
60+
{
61+
activity = s_activitySource.CreateActivity(DiagnosticsHandlerLoggingStrings.ActivityName, ActivityKind.Client);
62+
}
63+
64+
if (activity is null)
65+
{
66+
if (Activity.Current is not null || s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, requestMessage))
67+
{
68+
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
69+
}
70+
}
71+
72+
return activity;
5273
}
5374

5475
internal static bool IsGloballyEnabled() => GlobalHttpSettings.DiagnosticsHandler.EnableActivityPropagation;
@@ -91,60 +112,38 @@ private async ValueTask<HttpResponseMessage> SendAsyncCore(HttpRequestMessage re
91112
}
92113
}
93114

94-
Activity? activity = null;
95115
DiagnosticListener diagnosticListener = s_diagnosticListener;
96116

97-
// if there is no listener, but propagation is enabled (with previous IsEnabled() check)
98-
// do not write any events just start/stop Activity and propagate Ids
99-
if (!diagnosticListener.IsEnabled())
100-
{
101-
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
102-
activity.Start();
103-
InjectHeaders(activity, request);
104-
105-
try
106-
{
107-
return async ?
108-
await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false) :
109-
_innerHandler.Send(request, cancellationToken);
110-
}
111-
finally
112-
{
113-
activity.Stop();
114-
}
115-
}
116-
117117
Guid loggingRequestId = Guid.Empty;
118+
Activity? activity = CreateActivity(request);
118119

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

124-
// Only send start event to users who subscribed for it, but start activity anyway
125+
// Only send start event to users who subscribed for it.
125126
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName))
126127
{
127-
StartActivity(diagnosticListener, activity, new ActivityStartData(request));
128-
}
129-
else
130-
{
131-
activity.Start();
128+
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ActivityStartName, new ActivityStartData(request));
132129
}
133130
}
134-
// try to write System.Net.Http.Request event (deprecated)
131+
132+
// Try to write System.Net.Http.Request event (deprecated)
135133
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated))
136134
{
137135
long timestamp = Stopwatch.GetTimestamp();
138136
loggingRequestId = Guid.NewGuid();
139137
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated,
140-
new RequestData(request, loggingRequestId, timestamp));
138+
new RequestData(
139+
request,
140+
loggingRequestId,
141+
timestamp));
141142
}
142143

143-
// If we are on at all, we propagate current activity information
144-
Activity? currentActivity = Activity.Current;
145-
if (currentActivity != null)
144+
if (activity is not null)
146145
{
147-
InjectHeaders(currentActivity, request);
146+
InjectHeaders(activity, request);
148147
}
149148

150149
HttpResponseMessage? response = null;
@@ -178,17 +177,20 @@ await _innerHandler.SendAsync(request, cancellationToken).ConfigureAwait(false)
178177
}
179178
finally
180179
{
181-
// always stop activity if it was started
182-
if (activity != null)
180+
// Always stop activity if it was started.
181+
if (activity is not null)
183182
{
184-
StopActivity(diagnosticListener, activity, new ActivityStopData(
185-
response,
186-
// If request is failed or cancelled, there is no response, therefore no information about request;
187-
// pass the request in the payload, so consumers can have it in Stop for failed/canceled requests
188-
// and not retain all requests in Start
189-
request,
190-
taskStatus));
183+
activity.SetEndTime(DateTime.UtcNow);
184+
185+
// Only send stop event to users who subscribed for it.
186+
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStopName))
187+
{
188+
Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ActivityStopName, new ActivityStopData(response, request, taskStatus));
189+
}
190+
191+
activity.Stop();
191192
}
193+
192194
// Try to write System.Net.Http.Response event (deprecated)
193195
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated))
194196
{
@@ -335,27 +337,6 @@ key is not null &&
335337
{
336338
diagnosticSource.Write(name, value);
337339
}
338-
339-
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
340-
Justification = "The args being passed into StartActivity have the commonly used properties being preserved with DynamicDependency.")]
341-
private static Activity StartActivity<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(
342-
DiagnosticSource diagnosticSource,
343-
Activity activity,
344-
T? args)
345-
{
346-
return diagnosticSource.StartActivity(activity, args);
347-
}
348-
349-
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern",
350-
Justification = "The args being passed into StopActivity have the commonly used properties being preserved with DynamicDependency.")]
351-
private static void StopActivity<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(
352-
DiagnosticSource diagnosticSource,
353-
Activity activity,
354-
T? args)
355-
{
356-
diagnosticSource.StopActivity(activity, args);
357-
}
358-
359340
#endregion
360341
}
361342
}

‎src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandlerLoggingStrings.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@ namespace System.Net.Http
88
/// </summary>
99
internal static class DiagnosticsHandlerLoggingStrings
1010
{
11-
public const string DiagnosticListenerName = "HttpHandlerDiagnosticListener";
12-
public const string RequestWriteNameDeprecated = "System.Net.Http.Request";
13-
public const string ResponseWriteNameDeprecated = "System.Net.Http.Response";
14-
15-
public const string ExceptionEventName = "System.Net.Http.Exception";
16-
public const string ActivityName = "System.Net.Http.HttpRequestOut";
17-
public const string ActivityStartName = "System.Net.Http.HttpRequestOut.Start";
11+
public const string DiagnosticListenerName = "HttpHandlerDiagnosticListener";
12+
public const string Namespace = "System.Net.Http";
13+
public const string RequestWriteNameDeprecated = Namespace + ".Request";
14+
public const string ResponseWriteNameDeprecated = Namespace + ".Response";
15+
public const string ExceptionEventName = Namespace + ".Exception";
16+
public const string ActivityName = Namespace + ".HttpRequestOut";
17+
public const string ActivityStartName = ActivityName + ".Start";
18+
public const string ActivityStopName = ActivityName + ".Stop";
1819
}
1920
}

‎src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs

Lines changed: 112 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.Diagnostics.Tracing;
8+
using System.IO;
89
using System.Linq;
910
using System.Net.Http.Headers;
1011
using System.Net.Test.Common;
@@ -802,43 +803,6 @@ public static IEnumerable<object[]> UseSocketsHttpHandler_WithIdFormat_MemberDat
802803
yield return new object[] { false, ActivityIdFormat.W3C };
803804
}
804805

805-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
806-
[MemberData(nameof(UseSocketsHttpHandler_WithIdFormat_MemberData))]
807-
public async Task SendAsync_ExpectedActivityPropagationWithoutListener(bool useSocketsHttpHandler, ActivityIdFormat idFormat)
808-
{
809-
Activity parent = new Activity("parent");
810-
parent.SetIdFormat(idFormat);
811-
parent.Start();
812-
813-
await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
814-
async uri =>
815-
{
816-
await GetAsync(UseVersion.ToString(), TestAsync.ToString(), uri, useSocketsHttpHandler: useSocketsHttpHandler);
817-
},
818-
async server =>
819-
{
820-
HttpRequestData requestData = await server.HandleRequestAsync();
821-
AssertHeadersAreInjected(requestData, parent);
822-
});
823-
}
824-
825-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
826-
[InlineData(true)]
827-
[InlineData(false)]
828-
public async Task SendAsync_ExpectedActivityPropagationWithoutListenerOrParentActivity(bool useSocketsHttpHandler)
829-
{
830-
await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
831-
async uri =>
832-
{
833-
await GetAsync(UseVersion.ToString(), TestAsync.ToString(), uri, useSocketsHttpHandler: useSocketsHttpHandler);
834-
},
835-
async server =>
836-
{
837-
HttpRequestData requestData = await server.HandleRequestAsync();
838-
AssertNoHeadersAreInjected(requestData);
839-
});
840-
}
841-
842806
[ConditionalTheory(nameof(EnableActivityPropagationEnvironmentVariableIsNotSetAndRemoteExecutorSupported))]
843807
[InlineData("true")]
844808
[InlineData("1")]
@@ -998,6 +962,117 @@ await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
998962
});
999963
}
1000964

965+
public static IEnumerable<object[]> SocketsHttpHandler_ActivityCreation_MemberData()
966+
{
967+
foreach (var currentActivitySet in new bool[] {
968+
true, // Activity was set
969+
false }) // No Activity is set
970+
{
971+
foreach (var diagnosticListenerActivityEnabled in new bool?[] {
972+
true, // DiagnosticListener requested an Activity
973+
false, // DiagnosticListener does not want an Activity
974+
null }) // There is no DiagnosticListener
975+
{
976+
foreach (var activitySourceCreatesActivity in new bool?[] {
977+
true, // ActivitySource created an Activity
978+
false, // ActivitySource chose not to create an Activity
979+
null }) // ActivitySource had no listeners
980+
{
981+
yield return new object[] { currentActivitySet, diagnosticListenerActivityEnabled, activitySourceCreatesActivity };
982+
}
983+
}
984+
}
985+
}
986+
987+
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
988+
[MemberData(nameof(SocketsHttpHandler_ActivityCreation_MemberData))]
989+
public void SendAsync_ActivityIsCreatedIfRequested(bool currentActivitySet, bool? diagnosticListenerActivityEnabled, bool? activitySourceCreatesActivity)
990+
{
991+
string parameters = $"{currentActivitySet},{diagnosticListenerActivityEnabled},{activitySourceCreatesActivity}";
992+
993+
RemoteExecutor.Invoke(async (useVersion, testAsync, parametersString) =>
994+
{
995+
bool?[] parameters = parametersString.Split(',').Select(p => p.Length == 0 ? (bool?)null : bool.Parse(p)).ToArray();
996+
bool currentActivitySet = parameters[0].Value;
997+
bool? diagnosticListenerActivityEnabled = parameters[1];
998+
bool? activitySourceCreatesActivity = parameters[2];
999+
1000+
bool madeASamplingDecision = false;
1001+
if (activitySourceCreatesActivity.HasValue)
1002+
{
1003+
ActivitySource.AddActivityListener(new ActivityListener
1004+
{
1005+
ShouldListenTo = _ => true,
1006+
Sample = (ref ActivityCreationOptions<ActivityContext> _) =>
1007+
{
1008+
madeASamplingDecision = true;
1009+
return activitySourceCreatesActivity.Value ? ActivitySamplingResult.AllData : ActivitySamplingResult.None;
1010+
}
1011+
});
1012+
}
1013+
1014+
bool listenerCallbackWasCalled = false;
1015+
IDisposable listenerSubscription = new MemoryStream(); // Dummy disposable
1016+
if (diagnosticListenerActivityEnabled.HasValue)
1017+
{
1018+
var diagnosticListenerObserver = new FakeDiagnosticListenerObserver(_ => listenerCallbackWasCalled = true);
1019+
1020+
diagnosticListenerObserver.Enable(name => !name.Contains("HttpRequestOut") || diagnosticListenerActivityEnabled.Value);
1021+
1022+
listenerSubscription = DiagnosticListener.AllListeners.Subscribe(diagnosticListenerObserver);
1023+
}
1024+
1025+
Activity parent = currentActivitySet ? new Activity("parent").Start() : null;
1026+
Activity activity = parent;
1027+
1028+
if (!currentActivitySet)
1029+
{
1030+
// Listen to new activity creations if an Activity was created without a parent
1031+
// (when a DiagnosticListener forced one to be created)
1032+
ActivitySource.AddActivityListener(new ActivityListener
1033+
{
1034+
ShouldListenTo = _ => true,
1035+
ActivityStarted = created =>
1036+
{
1037+
Assert.Null(parent);
1038+
activity = created;
1039+
}
1040+
});
1041+
}
1042+
1043+
using (listenerSubscription)
1044+
{
1045+
await GetFactoryForVersion(useVersion).CreateClientAndServerAsync(
1046+
async uri =>
1047+
{
1048+
await GetAsync(useVersion, testAsync, uri);
1049+
},
1050+
async server =>
1051+
{
1052+
HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync();
1053+
1054+
if (currentActivitySet || diagnosticListenerActivityEnabled == true || activitySourceCreatesActivity == true)
1055+
{
1056+
Assert.NotNull(activity);
1057+
AssertHeadersAreInjected(requestData, activity, parent is null);
1058+
}
1059+
else
1060+
{
1061+
AssertNoHeadersAreInjected(requestData);
1062+
1063+
if (!currentActivitySet)
1064+
{
1065+
Assert.Null(activity);
1066+
}
1067+
}
1068+
});
1069+
}
1070+
1071+
Assert.Equal(activitySourceCreatesActivity.HasValue, madeASamplingDecision);
1072+
Assert.Equal(diagnosticListenerActivityEnabled.HasValue, listenerCallbackWasCalled);
1073+
}, UseVersion.ToString(), TestAsync.ToString(), parameters).Dispose();
1074+
}
1075+
10011076
private static T GetProperty<T>(object obj, string propertyName)
10021077
{
10031078
Type t = obj.GetType();

0 commit comments

Comments
 (0)