[Instrumentation.AspNet] Add metric enrich functionality

[qhris]

Fixes #1226 by adding adding functionality.

Changes

• Adds enrich functionality for metrics

For significant contributions please make sure you have completed the following items:

• Appropriate CHANGELOG.md updated for non-trivial changes
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #1407 (10ff5b9) into main (71655ce) will decrease coverage by 1.35%.
Report is 51 commits behind head on main.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##             main    #1407      +/-   ##
==========================================
- Coverage   73.91%   72.56%   -1.35%     
==========================================
  Files         267       14     -253     
  Lines        9615      288    -9327     
==========================================
- Hits         7107      209    -6898     
+ Misses       2508       79    -2429     

Flag	Coverage Δ
unittests-Instrumentation.AspNet	72.56% <94.44%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ation.AspNet.TelemetryHttpModule/ActivityHelper.cs	85.71% <100.00%> (+0.46%)	⬆️
...nTelemetry.Instrumentation.AspNet/AspNetMetrics.cs	100.00% <100.00%> (ø)
...tion.AspNet/AspNetMetricsInstrumentationOptions.cs	100.00% <100.00%> (ø)
...ion.AspNet/Implementation/HttpInMetricsListener.cs	100.00% <100.00%> (ø)
...mentation.AspNet/MeterProviderBuilderExtensions.cs	83.33% <75.00%> (-16.67%)	⬇️

... and 251 files with indirect coverage changes

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[qhris]

Is there anything you want me to do with this?

Ideally I'd build upon this to introduce the new http.request.server.duration metric. I can do that in this PR or do it as a follow up. If you want me to introduce the new metric; maybe the old one should be removed? We can't exactly do a switch like .NET did between 7 and 8 where the OTEL_SEMCONV_STABILITY_OPT_IN became irrelevant.

[vishweshbankwar]

@qhris Do you need filter as well? If yes, could you explain the use case. ASP.NET Core has removed the filter capability open-telemetry/opentelemetry-dotnet#4981

[qhris]

@vishweshbankwar No particular reason, I was just matching the API's to the now removed functionality in .NET 8.
I'll update this to remove filtering.

[Kielek]

Sorry for late feedback. Any chance to split the PR into 2 parts - exactly as you have in CHANGE:OG:

1. Enriching functionality
2. Adding tags.

2 reasons behind it.

1. Changelog will have better links to the changes, so the end-user can easily check potential changes.
2. It is easier to verify smaller functionalities while reviewing.

[jdom]

If I read the specs, it does look like the http.server.request.duration metric in seconds is stable: https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md#metric-httpserverrequestduration
(I'm not an expert in OTel, so perhaps I'm not looking in the right spot)

[Kielek]

@jdom, you are looking correctly, but we should switch whole package to the new convention at once. IMO the worst option is mixing them.

I am fine with merging this as is, then make one more PR to switch convention.

[qhris]

@Kielek that sounds good to me, since I'd have to undo a lot of work just to add it back in with new values.
I like the idea of just removing the old conventions for metrics, if we are clear about it in the release notes.

So for now I'll just make the suggested changes to the changelog and fix the conflicts.
There's also probably no need to release a new version of the package until the 2nd PR is made.

[ZRich97]

@qhris Do you need filter as well? If yes, could you explain the use case. ASP.NET Core has removed the filter capability open-telemetry/opentelemetry-dotnet#4981

@vishweshbankwar - our team has used the filter logic in ASP.NET Core's instrumentation to avoid emitting metrics/traces for healthchecks and other unmonitored paths. We'd like to have the same functionality from the ASP.NET instrumentation (though I realize this has now been removed in the newer Core packages). Is this a valid use case that would merit adding filtering, or is there a separate OTel mechanism we should use? I can open a separate PR for filtering if it would be more appropriate to move the discussion there.

[Kielek]

Approving. Still I would prefer to have it as 2 PRs.

@vishweshbankwar, could you please check this?

[qhris]

@Kielek I can still split it if necessary but that would be later tonight. Either way works for me. If it doesn't get merged until then the plan is:

1. Make this PR only be the enrich functionality (with additional changes for tests and such).
2. Prepare a new PR with the new http.server.request.duration metrics that @jdom mentioned are stable.

[vishweshbankwar]

@Kielek I can still split it if necessary but that would be later tonight. Either way works for me. If it doesn't get merged until then the plan is:

1. Make this PR only be the enrich functionality (with additional changes for tests and such).
2. Prepare a new PR with the new http.server.request.duration metrics that @jdom mentioned are stable.

@qhris Do you mind updating this PR to just add Enrich functionality? Since we are not going to release the changes related to attributes, its best that we do not merge those. A separate PR could update the conventions to stable one. That will also make changelog much clear. Thanks!

[qhris]

@Kielek @vishweshbankwar

Sorry for the inconvenience.
I have retargeted this PR to only add the Enrich functionality now!

[vishweshbankwar]

LGTM - Thanks @qhris!

[Kielek]

Thanks for contribution!