[Instrumentation.AspNet] Add Filter Support For ASP.NET Instrumented Metrics

[ZRich97]

Fixes open-telemetry/opentelemetry-dotnet#1226

Changes

This PR adds AspNetMetricsInstrumentationOptions with the ability to filter and enrich emitted metrics to the OpenTelemetry.Instrumentation.AspNet package. These changes are meant to match those already available in the AspNetCore instrumentation library and based on this PR: open-telemetry/opentelemetry-dotnet#3948

Our main use case for this functionality is that services can rely on the out-of-the-box instrumentation to emit the "http.server.duration" metric while adding custom tags (in our case these may be "Region", "Cluster", etc.). This would be preferred to building custom handlers and middleware to publish a separate metric with custom tags.

TODO: Documentation and changelog updates.

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ZRich97 / name: Zack Richardson (8676d02, b74ba53, 2488187, 7d3236a, 5eac03b, 19d3be9, ae71bd8, ee0321c, 724d7e0)

[Kielek]

@ZRich97, could you please review open-telemetry/opentelemetry-dotnet#1407? @ghris created PR few days ago with similar functionality (Enrich).
Based on the discussion Filtering probably will not be needed.

Thanksfor the contribution!

[ZRich97]

@ZRich97, could you please review open-telemetry/opentelemetry-dotnet#1407? @ghris created PR few days ago with similar functionality (Enrich). Based on the discussion Filtering probably will not be needed.

Thanksfor the contribution!

@Kielek / @vishweshbankwar - Given #1407 was merged as Enrich-only, would it make sense for me to update and publish this PR to enable filtering? I believe there's a valid use case for filtering (mentioned here), but let me know your thoughts. Thanks!

[vishweshbankwar]

@ZRich97, could you please review open-telemetry/opentelemetry-dotnet#1407? @ghris created PR few days ago with similar functionality (Enrich). Based on the discussion Filtering probably will not be needed.
Thanksfor the contribution!

@Kielek / @vishweshbankwar - Given open-telemetry/opentelemetry-dotnet#1407 was merged as Enrich-only, would it make sense for me to update and publish this PR to enable filtering? I believe there's a valid use case for filtering (mentioned here), but let me know your thoughts. Thanks!

@ZRich97 What issues do you face if you dont filter out the endpoints you mentioned in #1407 (comment)

[ZRich97]

@ZRich97, could you please review open-telemetry/opentelemetry-dotnet#1407? @ghris created PR few days ago with similar functionality (Enrich). Based on the discussion Filtering probably will not be needed.
Thanksfor the contribution!

@Kielek / @vishweshbankwar - Given open-telemetry/opentelemetry-dotnet#1407 was merged as Enrich-only, would it make sense for me to update and publish this PR to enable filtering? I believe there's a valid use case for filtering (mentioned here), but let me know your thoughts. Thanks!

@ZRich97 What issues do you face if you dont filter out the endpoints you mentioned in #1407 (comment)

@vishweshbankwar - The main use case in my mind is filtering out unused metrics (noise). Some of our supported services don't need to collect metrics for every route in the application (and they currently don't in their non-OTel monitoring solutions). This filtering could also be done at the alerting/monitoring stage (metrics we don't care about could be identified by the dimensions collected/enriched by this instrumentation and filtered out of any dashboards/alerts), but why not avoid collecting unused metrics altogether?

I've updated this PR to only include the Filtering changes I'd already made and better aligned it with the Enrich changes made in #1407. Let me know what you think, and I can add more documentation if this is something that might be supported. Thanks!

[cijothomas]

Metrics Filter was recently removed from AspNetCore instrumentation, so it may be better to hold off from this, and gather all feedback here: #1765

@ZRich97 Could you add your use-case to above as well?

[ZRich97]

Metrics Filter was recently removed from AspNetCore instrumentation, so it may be better to hold off from this, and gather all feedback here: open-telemetry/opentelemetry-dotnet-contrib#1765

@ZRich97 Could you add your use-case to above as well?

@cijothomas - sure, I'll add our use-case there as well. From what I understand filtering is present for traces, is there a reason it shouldn't also be added for metrics? I would imagine the use-cases would be similar (reduce noise, only monitor certain paths or types of requests, etc.) - let me know if we are missing something. Thanks!

[cijothomas]

Metrics Filter was recently removed from AspNetCore instrumentation, so it may be better to hold off from this, and gather all feedback here: open-telemetry/opentelemetry-dotnet-contrib#1765
@ZRich97 Could you add your use-case to above as well?

@cijothomas - sure, I'll add our use-case there as well. From what I understand filtering is present for traces, is there a reason it shouldn't also be added for metrics? I would imagine the use-cases would be similar (reduce noise, only monitor certain paths or types of requests, etc.) - let me know if we are missing something. Thanks!

For traces,logs -every telemetry adds cost - memory cost, serialization costs, network costs, and Vendor billing (typically based on volume).
For metrics, due to the pre-aggregation - the cost increase is less significant (I am not saying it is not there, but relatively low), so it may not be worthwhile to execute filter conditions in metric hot path! Happy to hear more feedback on this topic, which will also ultimately influence if Asp.Net Core want to offer some native filtering ability!

[ZRich97]

For traces,logs -every telemetry adds cost - memory cost, serialization costs, network costs, and Vendor billing (typically based on volume). For metrics, due to the pre-aggregation - the cost increase is less significant (I am not saying it is not there, but relatively low), so it may not be worthwhile to execute filter conditions in metric hot path! Happy to hear more feedback on this topic, which will also ultimately influence if Asp.Net Core want to offer some native filtering ability!

@cijothomas - Yep, it makes sense that this would be more important for traces/logs. I don't see a downside to including filtering for metrics as well - is there one from your perspective (or some general guidance against it)?

If this PR isn't likely to be merged soon, I can go ahead and close it. For our services to start using the already-merged Enrich() functionality, we'd need a new release of the package. Based on the Contribution Guide, should I create a PR requesting a new release or is there one already planned?

[github-actions]

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

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.