Add HttpClient metrics for .NET Framework

[matt-hensley]

Fixes #4764

Changes

Extends the existing tracing implementation to also generate metrics. This requires tracing to be enabled - while not ideal, this significantly limits the scope of changes to get metrics working.

To enable metrics, you must call AddHttpClientInstrumentation() on TracerProviderBuilder.

A single metric is published: http.client.duration, same as the .NET Core/.NET metrics implementation.

Unit tests have been modified to cover HttpClient metrics on .NET Framework. All unit test are passing.

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: matt-hensley / name: Matt Hensley (85331a6, c565eff, de98635, b4405a0, 3a208b9, f52b469, 56e2203, dadfeb4, 364b1c2, 95c433b, 1b4c3cd, 06c704b)

[codecov]

Codecov Report

Merging #4768 (dadfeb4) into main (77b3a1e) will decrease coverage by 3.17%.
The diff coverage is 100.00%.

❗ Current head dadfeb4 differs from pull request most recent head 06c704b. Consider uploading reports for the commit 06c704b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4768      +/-   ##
==========================================
- Coverage   85.46%   82.30%   -3.17%     
==========================================
  Files         314      314              
  Lines       12940    12956      +16     
==========================================
- Hits        11059    10663     -396     
- Misses       1881     2293     +412     

Files Changed	Coverage Δ
...plementation/HttpWebRequestActivitySource.netfx.cs	81.41% <100.00%> (+0.55%)	⬆️
...rumentation.Http/MeterProviderBuilderExtensions.cs	100.00% <100.00%> (ø)

... and 22 files with indirect coverage changes

[cijothomas]

nits:
"If a sampler" is not right, as there is always a sampler! Suggestion:
The sampler must return RecordAndSample for ever Activity in order for the Metrics to be accurate. If some spans (Activity) are sampled, but others not, it'll affect Metrics accuracy, as Metric Tags (Attributes) are generated based on the Activity.

[cijothomas]

also, the user provided Filters can also affect this,

opentelemetry-dotnet/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs

Line 274 in 06c704b

if (!WebRequestActivitySource.HasListeners() || !Options.EventFilterHttpWebRequest(request))

.

1. Enable tracing.
2. Sampler returns RecordAndSample
3. No user defined filters.

[cijothomas]

I suggest revisit the design of relying on Activity for generating metrics, and instead produce Metrics independently.
The approach done in this PR is more like a "Span2Metrics" processor, and can done as a regular ActivityProcessor instead, so it'll be easy to convey that Metrics are only as good as the underlying Activity.

Could you check how big of a change it'd to produce metrics independently? (It may not be as bad to rely on Activity for duration alone...but not for the whole tags.)

[matt-hensley]

Did some research and have an idea. Could follow a similar pattern to HttpClient. The idea would be to:

• Call ActivitySource.CreateActivity if tracing is enabled. This Activity would be used by metrics and traces
• Instantiate a standalone Activity if only metrics are enabled. This wouldn't cause a trace to be emitted
• Null Activity if neither metrics nor traces are enabled

If metrics or traces are enabled, the existing tags would still be set, with the only difference being whether the Activity ends up processed by an exporter.

Does this approach sound OK? There are some details to work through, but this looks feasible to me. Also welcome to other ideas if anyone has thoughts.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.