Instrument HttpClientHandler on netfx with DiagnosticSource/Activity

[lmolkova]

This changes enables instrumentation for outgoing Http requests with DiagnosticSource and Activity API on netfx.

It was previously introduced for .net core in #16393.
/cc @vancem

[stephentoub]

@davidsh, are we using the build in this package for desktop? I was under the impression we were changing things to use the one built into the framework instead?

[davidsh]

@davidsh, are we using the build in this package for desktop? I was under the impression we were changing things to use the one built into the framework instead?

Right now, we're building and using this for 'net46' as OOB for System.Net.Http on .NET Framework.

However, the plan (#16805) is to stop doing OOB. So, these code changes in HttpClientHandler.net46.cs will be thrown away.

[lmolkova]

@davidsh do I understand correctly that the plan is to facade the System.Net.Http.dll provided with the framework, so even when the new version of nuget package is installed, it actually reference locally installed package?
And is it planned for the next System.Net.Http 4.4.0?

What about bug fixes, how will they ship?

[karelz]

@lmolkova bug fixes will ship with .NET Framework updates.
Shipping OOB packages for inbox .NET Framework assemblies is troublesome - see #11100 and the pain it caused. It is also very costly (basically we do not have any test coverage now, so new infra would have to be created for that). We are trying to avoid that in future as much as we can.

[karelz]

For context, here's tracking issue to list and stop shipping OOB replacements of inbox components: #16823.

[karelz]

We should not touch net46 code at all. Are there any changes valuable for .NET Core?

[davidsh]

We should not touch net46 code at all. Are there any changes valuable for .NET Core?
Agreed. We should not merge this PR. This NET46 code here was copied as-is basically from Desktop and we shouldn't be changing it, else we introduce higher risk.

@lmolkova Did you manually test this on NETFX app? Because I don't think it will work anyways since the DiagnosticSource stuff might not be a proper dependency on NETFX.

[weshaggard]

We should not do ProjectReferences here, there should be just a Reference.

[stephentoub]

So @karelz, @davidsh, @lmolkova, should this PR just be closed?

[karelz]

I think so - @lmolkova @vancem is there anything in the PR that is useful for .NET Core (not 'net46')?

[lmolkova]

@karelz @stephentoub
No, there is nothing useful in this PR for net core.
I'm closing it, thanks