Instrumentation docs Part #2 - HTTP options

[alanwest]

Relates to #1233. Adds documentation of HTTP instrumentation options.

[reyang]

LGTM, well written! 👍

[alanwest]

Based on our conversation regarding HttpClient and HttpWebRequest, I took a stab at better wording for this section, though I'm not convinced that it is accurate because our conversation has made me a bit confused by this code

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs

Lines 30 to 42 in ce0bf3f

	#if NETFRAMEWORK
	/// <summary>
	/// Enables HttpClient and HttpWebRequest instrumentation.
	/// </summary>
	/// <param name="builder"><see cref="TracerProviderBuilder"/> being configured.</param>
	/// <param name="configureHttpClientInstrumentationOptions">HttpClient configuration options.</param>
	/// <param name="configureHttpWebRequestInstrumentationOptions">HttpWebRequest configuration options.</param>
	/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
	public static TracerProviderBuilder AddHttpClientInstrumentation(
	this TracerProviderBuilder builder,
	Action<HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions = null,
	Action<HttpWebRequestInstrumentationOptions> configureHttpWebRequestInstrumentationOptions = null)
	#else

This seems to suggest the ability to set options for HttpClient and HttpWebRequest, but if I'm understanding things correctly, the options passed to this method for HttpClient effectively remain unused. Right? If so, then it seems like we should remove the configureHttpClientInstrumentationOptions parameter from the .NET Framework compilation.

[cijothomas]

mm. The TracerProviderBuilderExtensions code seems bit puzzling to me as well. Let me take another close look and get back to you.

[CodeBlanch]

In .NET Framework, HttpClient uses HttpWebRequest. So really, only AddHttpWebRequestInstrumentation will be active. But if you use a .NET Standard package that pulls in .NET Standard HttpClient, what will happen? Could .NET Core bits get in there? Not totally sure about that but I think that's why both are there.

[alanwest]

Hmm. I'm pretty sure the guidance is to not consume HttpClient as a nuget package for .net framework. While it's technically possible it often leads to problems. I feel like I've read more "official" guidance on this somewhere but all I'm finding offhand is a tweet from Immo Landwerth https://twitter.com/terrajobst/status/997262020108926976?s=20.

I guess my thought is if there's a general consensus that this isn't best practice then should we seek to support it?

[cijothomas]

should we just remove AddHttpWebRequest to avoid confusion, and have a single method AddHttpClientInstrumentation for both .NET core and .NET framework?
And in the documentation for filter, state that, In .NET Core, HttpRequestMessage and .NET Framework, HttpWebRequest is the type passed to Filter?

[alanwest]

For now, I just removed this section distinguishing between the two add methods. I actually didn't realize until just now that AddHttpWebRequestInstrumentation is an internal method - sorry 🤦 - so in effect this was already true:

should we just remove AddHttpWebRequest to avoid confusion, and have a single method AddHttpClientInstrumentation for both .NET core and .NET framework?

Though, I am still curious about the behavior of things when the NuGet package is used in the context of .NET Framework. I'll spend a little time experimenting with this, because I feel if we can simplify and remove the configureHttpClientInstrumentationOptions for .NET Framework, that would be kinda nice.