-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Investigate if OOB metrics from ASP.NET Core and HttpClient can be enabled from instrumentation libraries #1753
Comments
Moving the discussion here open-telemetry/opentelemetry-dotnet#4437 (comment) |
My opinion: If breaking changes happen to OTel after .NET 8 ships, the .NET team is interested in staying compatible with OTel. We'll mitigate the breaking change by having a configuration setting to preserve old behavior for people who depend on it. |
@vishweshbankwar Please add to the list how do we handle Enrich/Filter. The instrumentation library has that feature (only for Asp.Net Core instrumentation), and .NET also has some mechanism to enrich. Need to ensure a smooth experience for enrich/filter exists. |
UpdateBackgroundThe request from the .NET team is to incorporate specific metrics as default features in the instrumentation libraries provided by this repository. This change will facilitate a seamless integration of metrics for users on To simplify comprehension, I've categorized these metrics into three groups: Category-1Metrics conforming to semantic conventions specification and marked as feature-freeze:
Category-2Metrics also adhering to the semantic conventions specification but categorized as Experimental:
Category-3Metrics not covered by semantic conventions specification, specifically designed for applications using
Option-1Include all the metrics from all three categories as default features for Pros:
Cons:
Option-2Include Category-1 and Category-3 as default features for Pros:
Technical WorkaroundSince metrics emitted in Category-1 and Category-2 belong to the same NOTE : Users will experience some changes when migrating from |
I think we should go with option-1. It brings considerable value to .NET users by adopting OOTB metrics. The additional metrics are designed specifically for .NET applications to provide deeper insights and complements the metrics defined by OpenTelemetry specification. In addition to this, adding them as part of instrumentation library provides consistent configuration experience for users on different .NET versions (i.e. .NET6.0, .NET7.0 and .NET8.0). It's important to note that any potential breaking changes would affect just one metric, namely |
Sounds reasonable to me 👍 |
If going with option1: Please share perf numbers too. We should be seeing a lot of improvement due to avoiding reflection/diagnostic listeners etc., but its possible that the extra metrics might kill the gains and could make things worse. If we find that the extra metircs is affecting perf, document a view recipe so users can easily turn them off. |
Well...irrespective of this, we still need to add the view recipe in the documentation! |
.NET8.0
added OOB support for metrics inASP.NET Core
andHttpClient
that aligns with OpenTelemetry semantic conventions. Needs further investigation to check if the instrumentation libraries can be updated to enable these by addingAddMeter
in.NET8.0
onwards. Things we need to check.NET8.0
from lower versions? Will there be significant difference in terms of metric dimensions, request duration value etc.?Microsoft.AspNetCore.Hosting
, there are two metrics that are emittedhttp.server.active_requests
andhttp.server.request.duration
. Onlyhttp.server.request.duration
is part of stability plan. How do we handle non-stable metrics that are emitted? As it cannot be part of stable instrumentation library release.NOTE: This is the initial list of things, may add more as we go through the investigation.
Reference PRs
HttpClient
https://github.com/dotnet/runtime/pull/89809/files
dotnet/runtime#87319
ASP.NET Core
dotnet/aspnetcore#48375
dotnet/aspnetcore#46834
The text was updated successfully, but these errors were encountered: