From 56c732745d94487bad514d4b833572a75e5897cf Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Thu, 1 Jul 2021 12:49:02 +0200 Subject: [PATCH] Revert "Add ActivitySource support to DiagnosticsHandler (#54437)" This reverts commit c88da2905317c0b199c5814594f1ea5d079e0760. --- .../src/ILLink/ILLink.Substitutions.xml | 2 +- .../src/System.Net.Http.csproj | 3 +- .../src/System/Net/Http/DiagnosticsHandler.cs | 276 ++++++++++-------- .../Http/DiagnosticsHandlerLoggingStrings.cs | 25 ++ .../src/System/Net/Http/HttpClientHandler.cs | 26 +- .../tests/FunctionalTests/DiagnosticsTests.cs | 158 +++------- 6 files changed, 252 insertions(+), 238 deletions(-) create mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandlerLoggingStrings.cs diff --git a/src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml b/src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml index d6aea3dbf37e4..314469c96d777 100644 --- a/src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml +++ b/src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml @@ -1,7 +1,7 @@ - + diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index 1423025f7c169..0fee14684e2ea 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -1,4 +1,4 @@ - + win true @@ -494,6 +494,7 @@ + diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs index 14c1464e77a70..4e4d730c4c188 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs @@ -15,121 +15,103 @@ namespace System.Net.Http /// internal sealed class DiagnosticsHandler : DelegatingHandler { - private const string Namespace = "System.Net.Http"; - private const string RequestWriteNameDeprecated = Namespace + ".Request"; - private const string ResponseWriteNameDeprecated = Namespace + ".Response"; - private const string ExceptionEventName = Namespace + ".Exception"; - private const string ActivityName = Namespace + ".HttpRequestOut"; - private const string ActivityStartName = ActivityName + ".Start"; - private const string ActivityStopName = ActivityName + ".Stop"; - - private static readonly DiagnosticListener s_diagnosticListener = new("HttpHandlerDiagnosticListener"); - private static readonly ActivitySource s_activitySource = new(Namespace); - - public static bool IsGloballyEnabled { get; } = GetEnableActivityPropagationValue(); - - private static bool GetEnableActivityPropagationValue() + /// + /// DiagnosticHandler constructor + /// + /// Inner handler: Windows or Unix implementation of HttpMessageHandler. + /// Note that DiagnosticHandler is the latest in the pipeline + public DiagnosticsHandler(HttpMessageHandler innerHandler) : base(innerHandler) { - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(Namespace + ".EnableActivityPropagation", out bool enableActivityPropagation)) - { - return enableActivityPropagation; - } - - // AppContext switch wasn't used. Check the environment variable to determine which handler should be used. - string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION"); - if (envVar != null && (envVar.Equals("false", StringComparison.OrdinalIgnoreCase) || envVar.Equals("0"))) - { - // Suppress Activity propagation. - return false; - } + } - // Defaults to enabling Activity propagation. - return true; + internal static bool IsEnabled() + { + // check if there is a parent Activity (and propagation is not suppressed) + // or if someone listens to HttpHandlerDiagnosticListener + return IsGloballyEnabled() && (Activity.Current != null || Settings.s_diagnosticListener.IsEnabled()); } - public DiagnosticsHandler(HttpMessageHandler innerHandler) : base(innerHandler) + internal static bool IsGloballyEnabled() { - Debug.Assert(IsGloballyEnabled); + return Settings.s_activityPropagationEnabled; } - private static bool ShouldLogDiagnostics(HttpRequestMessage request, out Activity? activity) + // SendAsyncCore returns already completed ValueTask for when async: false is passed. + // Internally, it calls the synchronous Send method of the base class. + protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => + SendAsyncCore(request, async: false, cancellationToken).AsTask().GetAwaiter().GetResult(); + + protected internal override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + SendAsyncCore(request, async: true, cancellationToken).AsTask(); + + private async ValueTask SendAsyncCore(HttpRequestMessage request, bool async, + CancellationToken cancellationToken) { - if (request is null) + // HttpClientHandler is responsible to call static DiagnosticsHandler.IsEnabled() before forwarding request here. + // It will check if propagation is on (because parent Activity exists or there is a listener) or off (forcibly disabled) + // This code won't be called unless consumer unsubscribes from DiagnosticListener right after the check. + // So some requests happening right after subscription starts might not be instrumented. Similarly, + // when consumer unsubscribes, extra requests might be instrumented + + if (request == null) { throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest); } - activity = null; - - if (s_activitySource.HasListeners()) - { - activity = s_activitySource.CreateActivity(ActivityName, ActivityKind.Client); - } + Activity? activity = null; + DiagnosticListener diagnosticListener = Settings.s_diagnosticListener; - if (activity is null) + // 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()) { - bool diagnosticListenerEnabled = s_diagnosticListener.IsEnabled(); + activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName); + activity.Start(); + InjectHeaders(activity, request); - if (Activity.Current is not null || (diagnosticListenerEnabled && s_diagnosticListener.IsEnabled(ActivityName, request))) + try { - // If a diagnostics listener is enabled for the Activity, always create one - activity = new Activity(ActivityName); + return async ? + await base.SendAsync(request, cancellationToken).ConfigureAwait(false) : + base.Send(request, cancellationToken); } - else + finally { - // There is no Activity, but we may still want to use the instrumented SendAsyncCore if diagnostic listeners are interested in other events - return diagnosticListenerEnabled; + activity.Stop(); } } - activity.Start(); + Guid loggingRequestId = Guid.Empty; - if (s_diagnosticListener.IsEnabled(ActivityStartName)) + // There is a listener. Check if listener wants to be notified about HttpClient Activities + if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, request)) { - Write(ActivityStartName, new ActivityStartData(request)); - } + activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName); - InjectHeaders(activity, request); - - return true; - } - - protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) - { - if (ShouldLogDiagnostics(request, out Activity? activity)) - { - ValueTask sendTask = SendAsyncCore(request, activity, async: false, cancellationToken); - return sendTask.IsCompleted ? - sendTask.Result : - sendTask.AsTask().GetAwaiter().GetResult(); + // Only send start event to users who subscribed for it, but start activity anyway + if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName)) + { + StartActivity(diagnosticListener, activity, new ActivityStartData(request)); + } + else + { + activity.Start(); + } } - else + // try to write System.Net.Http.Request event (deprecated) + if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated)) { - return base.Send(request, cancellationToken); + long timestamp = Stopwatch.GetTimestamp(); + loggingRequestId = Guid.NewGuid(); + Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated, + new RequestData(request, loggingRequestId, timestamp)); } - } - protected internal override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - if (ShouldLogDiagnostics(request, out Activity? activity)) - { - return SendAsyncCore(request, activity, async: true, cancellationToken).AsTask(); - } - else + // If we are on at all, we propagate current activity information + Activity? currentActivity = Activity.Current; + if (currentActivity != null) { - return base.SendAsync(request, cancellationToken); - } - } - - private async ValueTask SendAsyncCore(HttpRequestMessage request, Activity? activity, bool async, CancellationToken cancellationToken) - { - Guid loggingRequestId = default; - - if (s_diagnosticListener.IsEnabled(RequestWriteNameDeprecated)) - { - loggingRequestId = Guid.NewGuid(); - Write(RequestWriteNameDeprecated, new RequestData(request, loggingRequestId, Stopwatch.GetTimestamp())); + InjectHeaders(currentActivity, request); } HttpResponseMessage? response = null; @@ -144,39 +126,52 @@ private async ValueTask SendAsyncCore(HttpRequestMessage re catch (OperationCanceledException) { taskStatus = TaskStatus.Canceled; + + // we'll report task status in HttpRequestOut.Stop throw; } catch (Exception ex) { - if (s_diagnosticListener.IsEnabled(ExceptionEventName)) + taskStatus = TaskStatus.Faulted; + + if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ExceptionEventName)) { - Write(ExceptionEventName, new ExceptionData(ex, request)); + // If request was initially instrumented, Activity.Current has all necessary context for logging + // Request is passed to provide some context if instrumentation was disabled and to avoid + // extensive Activity.Tags usage to tunnel request properties + Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ExceptionEventName, new ExceptionData(ex, request)); } - - taskStatus = TaskStatus.Faulted; throw; } finally { - if (activity is not null) + // always stop activity if it was started + if (activity != null) { - activity.SetEndTime(DateTime.UtcNow); - - if (s_diagnosticListener.IsEnabled(ActivityStopName)) - { - Write(ActivityStopName, new ActivityStopData(response, request, taskStatus)); - } - - activity.Stop(); + 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)); } - - if (s_diagnosticListener.IsEnabled(ResponseWriteNameDeprecated)) + // Try to write System.Net.Http.Response event (deprecated) + if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated)) { - Write(ResponseWriteNameDeprecated, new ResponseData(response, loggingRequestId, Stopwatch.GetTimestamp(), taskStatus)); + long timestamp = Stopwatch.GetTimestamp(); + Write(diagnosticListener, DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated, + new ResponseData( + response, + loggingRequestId, + timestamp, + taskStatus)); } } } + #region private + private sealed class ActivityStartData { // matches the properties selected in https://github.com/dotnet/diagnostics/blob/ffd0254da3bcc47847b1183fa5453c0877020abd/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/HttpRequestSourceConfiguration.cs#L36-L40 @@ -274,29 +269,55 @@ internal ResponseData(HttpResponseMessage? response, Guid loggingRequestId, long public override string ToString() => $"{{ {nameof(Response)} = {Response}, {nameof(LoggingRequestId)} = {LoggingRequestId}, {nameof(Timestamp)} = {Timestamp}, {nameof(RequestTaskStatus)} = {RequestTaskStatus} }}"; } - private static void InjectHeaders(Activity currentActivity, HttpRequestMessage request) + private static class Settings { - const string TraceParentHeaderName = "traceparent"; - const string TraceStateHeaderName = "tracestate"; - const string RequestIdHeaderName = "Request-Id"; - const string CorrelationContextHeaderName = "Correlation-Context"; + private const string EnableActivityPropagationEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION"; + private const string EnableActivityPropagationAppCtxSettingName = "System.Net.Http.EnableActivityPropagation"; + + public static readonly bool s_activityPropagationEnabled = GetEnableActivityPropagationValue(); + + private static bool GetEnableActivityPropagationValue() + { + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(EnableActivityPropagationAppCtxSettingName, out bool enableActivityPropagation)) + { + return enableActivityPropagation; + } + + // AppContext switch wasn't used. Check the environment variable to determine which handler should be used. + string? envVar = Environment.GetEnvironmentVariable(EnableActivityPropagationEnvironmentVariableSettingName); + if (envVar != null && (envVar.Equals("false", StringComparison.OrdinalIgnoreCase) || envVar.Equals("0"))) + { + // Suppress Activity propagation. + return false; + } + // Defaults to enabling Activity propagation. + return true; + } + + public static readonly DiagnosticListener s_diagnosticListener = + new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName); + } + + private static void InjectHeaders(Activity currentActivity, HttpRequestMessage request) + { if (currentActivity.IdFormat == ActivityIdFormat.W3C) { - if (!request.Headers.Contains(TraceParentHeaderName)) + if (!request.Headers.Contains(DiagnosticsHandlerLoggingStrings.TraceParentHeaderName)) { - request.Headers.TryAddWithoutValidation(TraceParentHeaderName, currentActivity.Id); + request.Headers.TryAddWithoutValidation(DiagnosticsHandlerLoggingStrings.TraceParentHeaderName, currentActivity.Id); if (currentActivity.TraceStateString != null) { - request.Headers.TryAddWithoutValidation(TraceStateHeaderName, currentActivity.TraceStateString); + request.Headers.TryAddWithoutValidation(DiagnosticsHandlerLoggingStrings.TraceStateHeaderName, currentActivity.TraceStateString); } } } else { - if (!request.Headers.Contains(RequestIdHeaderName)) + if (!request.Headers.Contains(DiagnosticsHandlerLoggingStrings.RequestIdHeaderName)) { - request.Headers.TryAddWithoutValidation(RequestIdHeaderName, currentActivity.Id); + request.Headers.TryAddWithoutValidation(DiagnosticsHandlerLoggingStrings.RequestIdHeaderName, currentActivity.Id); } } @@ -312,16 +333,41 @@ private static void InjectHeaders(Activity currentActivity, HttpRequestMessage r baggage.Add(new NameValueHeaderValue(WebUtility.UrlEncode(item.Key), WebUtility.UrlEncode(item.Value)).ToString()); } while (e.MoveNext()); - request.Headers.TryAddWithoutValidation(CorrelationContextHeaderName, baggage); + request.Headers.TryAddWithoutValidation(DiagnosticsHandlerLoggingStrings.CorrelationContextHeaderName, baggage); } } } [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern", Justification = "The values being passed into Write have the commonly used properties being preserved with DynamicDependency.")] - private static void Write<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(string name, T value) + private static void Write<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>( + DiagnosticSource diagnosticSource, + string name, + T value) + { + 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) { - s_diagnosticListener.Write(name, value); + 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 } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandlerLoggingStrings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandlerLoggingStrings.cs new file mode 100644 index 0000000000000..0fa57394c1cc3 --- /dev/null +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandlerLoggingStrings.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Net.Http +{ + /// + /// Defines names of DiagnosticListener and Write events for DiagnosticHandler + /// + 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 RequestIdHeaderName = "Request-Id"; + public const string CorrelationContextHeaderName = "Correlation-Context"; + + public const string TraceParentHeaderName = "traceparent"; + public const string TraceStateHeaderName = "tracestate"; + } +} diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs index bfb8f6cae7834..2e3289643cfbe 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs @@ -20,17 +20,17 @@ namespace System.Net.Http public partial class HttpClientHandler : HttpMessageHandler { private readonly HttpHandlerType _underlyingHandler; - private readonly HttpMessageHandler _handler; + private readonly DiagnosticsHandler? _diagnosticsHandler; private ClientCertificateOption _clientCertificateOptions; private volatile bool _disposed; public HttpClientHandler() { - _handler = _underlyingHandler = new HttpHandlerType(); - if (DiagnosticsHandler.IsGloballyEnabled) + _underlyingHandler = new HttpHandlerType(); + if (DiagnosticsHandler.IsGloballyEnabled()) { - _handler = new DiagnosticsHandler(_handler); + _diagnosticsHandler = new DiagnosticsHandler(_underlyingHandler); } ClientCertificateOptions = ClientCertificateOption.Manual; } @@ -288,11 +288,21 @@ public SslProtocols SslProtocols public IDictionary Properties => _underlyingHandler.Properties; [UnsupportedOSPlatform("browser")] - protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => - _handler.Send(request, cancellationToken); + protected internal override HttpResponseMessage Send(HttpRequestMessage request, + CancellationToken cancellationToken) + { + return DiagnosticsHandler.IsEnabled() && _diagnosticsHandler != null ? + _diagnosticsHandler.Send(request, cancellationToken) : + _underlyingHandler.Send(request, cancellationToken); + } - protected internal override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => - _handler.SendAsync(request, cancellationToken); + protected internal override Task SendAsync(HttpRequestMessage request, + CancellationToken cancellationToken) + { + return DiagnosticsHandler.IsEnabled() && _diagnosticsHandler != null ? + _diagnosticsHandler.SendAsync(request, cancellationToken) : + _underlyingHandler.SendAsync(request, cancellationToken); + } // lazy-load the validator func so it can be trimmed by the ILLinker if it isn't used. private static Func? s_dangerousAcceptAnyServerCertificateValidator; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs index 66b60e800b8a7..1fb6fd925fd33 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs @@ -5,7 +5,6 @@ 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; @@ -257,7 +256,7 @@ public void SendAsync_ExpectedDiagnosticCancelledLogging() GetProperty(kvp.Value, "Request"); TaskStatus status = GetProperty(kvp.Value, "RequestTaskStatus"); Assert.Equal(TaskStatus.Canceled, status); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } }); @@ -345,7 +344,7 @@ public void SendAsync_ExpectedDiagnosticSourceActivityLogging(ActivityIdFormat i activityStopResponseLogged = GetProperty(kvp.Value, "Response"); TaskStatus requestStatus = GetProperty(kvp.Value, "RequestTaskStatus"); Assert.Equal(TaskStatus.RanToCompletion, requestStatus); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } }); @@ -410,7 +409,7 @@ public void SendAsync_ExpectedDiagnosticSourceActivityLogging_InvalidBaggage() Assert.Contains("goodkey=bad%2Fvalue", correlationContext); TaskStatus requestStatus = GetProperty(kvp.Value, "RequestTaskStatus"); Assert.Equal(TaskStatus.RanToCompletion, requestStatus); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } else if (kvp.Key.Equals("System.Net.Http.Exception")) { @@ -468,7 +467,7 @@ public void SendAsync_ExpectedDiagnosticSourceActivityLoggingDoesNotOverwriteHea Assert.False(request.Headers.TryGetValues("traceparent", out var _)); Assert.False(request.Headers.TryGetValues("tracestate", out var _)); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } }); @@ -520,7 +519,7 @@ public void SendAsync_ExpectedDiagnosticSourceActivityLoggingDoesNotOverwriteW3C } else if (kvp.Key.Equals("System.Net.Http.HttpRequestOut.Stop")) { - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } }); @@ -609,7 +608,7 @@ public void SendAsync_ExpectedDiagnosticExceptionActivityLogging() GetProperty(kvp.Value, "Request"); TaskStatus requestStatus = GetProperty(kvp.Value, "RequestTaskStatus"); Assert.Equal(TaskStatus.Faulted, requestStatus); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } else if (kvp.Key.Equals("System.Net.Http.Exception")) { @@ -648,7 +647,7 @@ public void SendAsync_ExpectedDiagnosticSynchronousExceptionActivityLogging() GetProperty(kvp.Value, "Request"); TaskStatus requestStatus = GetProperty(kvp.Value, "RequestTaskStatus"); Assert.Equal(TaskStatus.Faulted, requestStatus); - activityStopTcs.SetResult(); + activityStopTcs.SetResult();; } else if (kvp.Key.Equals("System.Net.Http.Exception")) { @@ -797,6 +796,44 @@ public void SendAsync_ExpectedDiagnosticExceptionOnlyActivityLogging() }, UseVersion.ToString(), TestAsync.ToString()).Dispose(); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void SendAsync_ExpectedActivityPropagationWithoutListener() + { + RemoteExecutor.Invoke(async (useVersion, testAsync) => + { + Activity parent = new Activity("parent").Start(); + + await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( + async uri => + { + await GetAsync(useVersion, testAsync, uri); + }, + async server => + { + HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync(); + AssertHeadersAreInjected(requestData, parent); + }); + }, UseVersion.ToString(), TestAsync.ToString()).Dispose(); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void SendAsync_ExpectedActivityPropagationWithoutListenerOrParentActivity() + { + RemoteExecutor.Invoke(async (useVersion, testAsync) => + { + await GetFactoryForVersion(useVersion).CreateClientAndServerAsync( + async uri => + { + await GetAsync(useVersion, testAsync, uri); + }, + async server => + { + HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync(); + AssertNoHeadersAreInjected(requestData); + }); + }, UseVersion.ToString(), TestAsync.ToString()).Dispose(); + } + [ConditionalTheory(nameof(EnableActivityPropagationEnvironmentVariableIsNotSetAndRemoteExecutorSupported))] [InlineData("true")] [InlineData("1")] @@ -862,111 +899,6 @@ public void SendAsync_SuppressedGlobalStaticPropagationNoListenerAppCtx(bool swi }, UseVersion.ToString(), TestAsync.ToString(), switchValue.ToString()).Dispose(); } - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true, true, true)] // Activity was set and ActivitySource created an Activity - [InlineData(true, false, true)] // Activity was set and ActivitySource created an Activity - [InlineData(true, null, true)] // Activity was set and ActivitySource created an Activity - [InlineData(true, true, false)] // Activity was set, ActivitySource chose not to create an Activity, so one was manually created - [InlineData(true, false, false)] // Activity was set, ActivitySource chose not to create an Activity, so one was manually created - [InlineData(true, null, false)] // Activity was set, ActivitySource chose not to create an Activity, so one was manually created - [InlineData(true, true, null)] // Activity was set, ActivitySource had no listeners, so an Activity was manually created - [InlineData(true, false, null)] // Activity was set, ActivitySource had no listeners, so an Activity was manually created - [InlineData(true, null, null)] // Activity was set, ActivitySource had no listeners, so an Activity was manually created - [InlineData(false, true, true)] // DiagnosticListener requested an Activity and ActivitySource created an Activity - [InlineData(false, true, false)] // DiagnosticListener requested an Activity, ActivitySource chose not to create an Activity, so one was manually created - [InlineData(false, true, null)] // DiagnosticListener requested an Activity, ActivitySource had no listeners, so an Activity was manually created - [InlineData(false, false, true)] // No Activity is set, DiagnosticListener does not want one, but ActivitySource created an Activity - [InlineData(false, false, false)] // No Activity is set, DiagnosticListener does not want one and ActivitySource chose not to create an Activity - [InlineData(false, false, null)] // No Activity is set, DiagnosticListener does not want one and ActivitySource has no listeners - [InlineData(false, null, true)] // No Activity is set, there is no DiagnosticListener, but ActivitySource created an Activity - [InlineData(false, null, false)] // No Activity is set, there is no DiagnosticListener and ActivitySource chose not to create an Activity - [InlineData(false, null, null)] // No Activity is set, there is no DiagnosticListener and ActivitySource has no listeners - 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 _) => - { - 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 activity = currentActivitySet ? new Activity("parent").Start() : null; - - 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(activity); - 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); - } - 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(object obj, string propertyName) { Type t = obj.GetType();