Trim more Http DiagnosticsHandler code

[eerhardt]

Since the DiagnosticsHandler still gets instantiated in the HttpClientHandler, none of its overriden methods are getting trimmed. This leads to System.Diagnostics.DiagnosticListener still being preserved in a linked application.

This fix is split DiagnosticsHandler.IsEnabled() into two methods:

• IsGloballyEnabled() - checks the AppContext switch, and is replaced by the linker by a feature swtich.
• IsEnabled() - which checks IsGloballyEnabled() and if there is an Activity or listener available.

This allows all but the IsEnabled and IsGloballyEnabled methods to get trimmed on DiagnosticsHandler.

Contributes to #38765

This allows for ~20KB savings in the default Blazor WASM template app:

Build	Size
master	2,420,736 bytes
PR	2,400,768 bytes

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[marek-safar]

I like 4c690ff better as the prefered outcome is to have no trace of DiagnosticsHandler in the binary

[ManickaP]

Question about 4c690ff: it creates the DiagnosticsHandler only if it's enabled and than lives with this decision for the rest of HttpClientHandler live. And we recommend keeping HttpClient around and share it for multiple requests. So, if someone subscribes to the diagnostics after the HttpClient has been created, will they ever receive any events?

[eerhardt]

So, if someone subscribes to the diagnostics after the HttpClient has been created, will they ever receive any events?

I don't believe so. That's the behavior change that I mentioned in the above summary:

However, I was concerned that could affect non-trimmed behavior by moving the DiagnosticsHandler.IsEnabled() check into the constructor of HttpClient, and not during Send.

If we still wanted to take that approach, I believe we can address this by splitting DiagnosticsHandler.IsEnabled() into 2 different methods:

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

Lines 26 to 31 in a366d63

	internal static bool IsEnabled()
	{
	// check if there is a parent Activity (and propagation is not suppressed)
	// or if someone listens to HttpHandlerDiagnosticListener
	return Settings.s_activityPropagationEnabled && (Activity.Current != null || Settings.s_diagnosticListener.IsEnabled());
	}

1. One that just returns the first part: Settings.s_activityPropagationEnabled. This would be called from the constructor, and can never change for the lifetime of a process (since it is a static readonly bool).
2. The second method would return the second part of that check: (Activity.Current != null || Settings.s_diagnosticListener.IsEnabled()). This can be called inside Send and SendAsync to check whether it should call _diagnosticsHandler or not.

Thoughts?

[ManickaP]

I believe we can address this by splitting DiagnosticsHandler.IsEnabled() into 2 different methods

I like it! It addresses the problem while it keeps the original behavior. 👍

[eerhardt]

@ManickaP - I've updated the PR to use the above proposal. Let me know what you think.

[ManickaP]

LGTM, thanks.