Skip to content

Commit

Permalink
Add feature switch for diagnostics handler in System.Net.Http (#38765)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marek-safar
marek-safar authored Jul 9, 2020
1 parent 117c9d6 commit 8753b34
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 32 deletions.
7 changes: 7 additions & 0 deletions src/libraries/System.Net.Http/src/ILLink/ILLink.Substitutions.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<linker>
<assembly fullname="System.Net.Http">
<type fullname="System.Net.Http.DiagnosticsHandler">
<method signature="System.Boolean IsEnabled()" body="stub" value="false" feature="System.Net.Http.EnableActivityPropagation" featurevalue="false" />
</type>
</assembly>
</linker>
8 changes: 8 additions & 0 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
<PropertyGroup Condition=" '$(TargetsBrowser)' == 'true'">
<DefineConstants>$(DefineConstants);TARGETS_BROWSER</DefineConstants>
</PropertyGroup>
<!-- ILLinker settings -->
<PropertyGroup>
<ILLinkDirectory>$(MSBuildThisFileDirectory)ILLink\</ILLinkDirectory>
</PropertyGroup>
<ItemGroup>
<ILLinkSubstitutionsXmls Include="$(ILLinkDirectory)ILLink.Substitutions.xml" />
</ItemGroup>

<ItemGroup>
<Compile Include="System\Net\Http\ByteArrayContent.cs" />
<Compile Include="System\Net\Http\ByteArrayHelpers.cs" />
Expand Down
68 changes: 36 additions & 32 deletions src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal static bool IsEnabled()
{
// check if there is a parent Activity (and propagation is not suppressed)
// or if someone listens to HttpHandlerDiagnosticListener
return s_enableActivityPropagation && (Activity.Current != null || s_diagnosticListener.IsEnabled());
return Settings.s_activityPropagationEnabled && (Activity.Current != null || Settings.s_diagnosticListener.IsEnabled());
}

// SendAsyncCore returns already completed ValueTask for when async: false is passed.
Expand Down Expand Up @@ -58,10 +58,11 @@ private async ValueTask<HttpResponseMessage> SendAsyncCore(HttpRequestMessage re
}

Activity? activity = null;
DiagnosticListener diagnosticListener = Settings.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 (!s_diagnosticListener.IsEnabled())
if (!diagnosticListener.IsEnabled())
{
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);
activity.Start();
Expand All @@ -82,26 +83,26 @@ await base.SendAsync(request, cancellationToken).ConfigureAwait(false) :
Guid loggingRequestId = Guid.Empty;

// There is a listener. Check if listener wants to be notified about HttpClient Activities
if (s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, request))
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityName, request))
{
activity = new Activity(DiagnosticsHandlerLoggingStrings.ActivityName);

// Only send start event to users who subscribed for it, but start activity anyway
if (s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName))
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ActivityStartName))
{
s_diagnosticListener.StartActivity(activity, new ActivityStartData(request));
diagnosticListener.StartActivity(activity, new ActivityStartData(request));
}
else
{
activity.Start();
}
}
// try to write System.Net.Http.Request event (deprecated)
if (s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated))
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated))
{
long timestamp = Stopwatch.GetTimestamp();
loggingRequestId = Guid.NewGuid();
s_diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated,
diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.RequestWriteNameDeprecated,
new RequestData(request, loggingRequestId, timestamp));
}

Expand Down Expand Up @@ -132,12 +133,12 @@ await base.SendAsync(request, cancellationToken).ConfigureAwait(false) :
{
taskStatus = TaskStatus.Faulted;

if (s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ExceptionEventName))
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ExceptionEventName))
{
// 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
s_diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.ExceptionEventName, new ExceptionData(ex, request));
diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.ExceptionEventName, new ExceptionData(ex, request));
}
throw;
}
Expand All @@ -146,7 +147,7 @@ await base.SendAsync(request, cancellationToken).ConfigureAwait(false) :
// always stop activity if it was started
if (activity != null)
{
s_diagnosticListener.StopActivity(activity, new ActivityStopData(
diagnosticListener.StopActivity(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
Expand All @@ -155,10 +156,10 @@ await base.SendAsync(request, cancellationToken).ConfigureAwait(false) :
taskStatus));
}
// Try to write System.Net.Http.Response event (deprecated)
if (s_diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated))
if (diagnosticListener.IsEnabled(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated))
{
long timestamp = Stopwatch.GetTimestamp();
s_diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated,
diagnosticListener.Write(DiagnosticsHandlerLoggingStrings.ResponseWriteNameDeprecated,
new ResponseData(
response,
loggingRequestId,
Expand Down Expand Up @@ -246,29 +247,35 @@ internal ResponseData(HttpResponseMessage? response, Guid loggingRequestId, long
public override string ToString() => $"{{ {nameof(Response)} = {Response}, {nameof(LoggingRequestId)} = {LoggingRequestId}, {nameof(Timestamp)} = {Timestamp}, {nameof(RequestTaskStatus)} = {RequestTaskStatus} }}";
}

private const string EnableActivityPropagationEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION";
private const string EnableActivityPropagationAppCtxSettingName = "System.Net.Http.EnableActivityPropagation";
private static class Settings
{
private const string EnableActivityPropagationEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_ENABLEACTIVITYPROPAGATION";
private const string EnableActivityPropagationAppCtxSettingName = "System.Net.Http.EnableActivityPropagation";

private static readonly bool s_enableActivityPropagation = GetEnableActivityPropagationValue();
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))
private static bool GetEnableActivityPropagationValue()
{
return enableActivityPropagation;
}
// 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;
// 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;
}

// 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)
Expand Down Expand Up @@ -309,9 +316,6 @@ private static void InjectHeaders(Activity currentActivity, HttpRequestMessage r
}
}

private static readonly DiagnosticListener s_diagnosticListener =
new DiagnosticListener(DiagnosticsHandlerLoggingStrings.DiagnosticListenerName);

#endregion
}
}

0 comments on commit 8753b34

Please sign in to comment.