From 3964894633756c01b9d5e4fb5aa578f75cd37553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Mon, 9 Dec 2024 08:55:34 +0100 Subject: [PATCH] [Instrumentation.AspNetCore] metrics - cleanup code after dropping support for .NET6 (#2360) --- ...mentationMeterProviderBuilderExtensions.cs | 28 +--- .../AspNetCoreMetrics.cs | 41 ------ .../CHANGELOG.md | 3 +- .../Implementation/HttpInMetricsListener.cs | 132 ------------------ .../README.md | 31 ---- .../MetricTests.cs | 11 -- 6 files changed, 8 insertions(+), 238 deletions(-) delete mode 100644 src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs delete mode 100644 src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs index 0815bef20e..9e8f3d9919 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs @@ -1,10 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if !NET -using OpenTelemetry.Instrumentation.AspNetCore; -using OpenTelemetry.Instrumentation.AspNetCore.Implementation; -#endif using OpenTelemetry.Internal; namespace OpenTelemetry.Metrics; @@ -22,27 +18,15 @@ public static class AspNetCoreInstrumentationMeterProviderBuilderExtensions public static MeterProviderBuilder AddAspNetCoreInstrumentation( this MeterProviderBuilder builder) { - Guard.ThrowIfNull(builder); - -#if NET - return builder.ConfigureMeters(); -#else - // Note: Warm-up the status code and method mapping. - _ = TelemetryHelper.BoxedStatusCodes; - _ = TelemetryHelper.RequestDataHelper; - - builder.AddMeter(HttpInMetricsListener.InstrumentationName); +#if NETSTANDARD2_0_OR_GREATER + if (Environment.Version.Major < 8) + { + throw new PlatformNotSupportedException("Metrics instrumentation is not supported when executing on .NET 7 and lower."); + } -#pragma warning disable CA2000 - builder.AddInstrumentation(new AspNetCoreMetrics()); -#pragma warning restore CA2000 - - return builder; #endif - } + Guard.ThrowIfNull(builder); - internal static MeterProviderBuilder ConfigureMeters(this MeterProviderBuilder builder) - { return builder .AddMeter("Microsoft.AspNetCore.Hosting") .AddMeter("Microsoft.AspNetCore.Server.Kestrel") diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs deleted file mode 100644 index 877f5ece38..0000000000 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#if !NET -using OpenTelemetry.Instrumentation.AspNetCore.Implementation; - -namespace OpenTelemetry.Instrumentation.AspNetCore; - -/// -/// Asp.Net Core Requests instrumentation. -/// -internal sealed class AspNetCoreMetrics : IDisposable -{ - private static readonly HashSet DiagnosticSourceEvents = - [ - "Microsoft.AspNetCore.Hosting.HttpRequestIn", - "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start", - "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop", - "Microsoft.AspNetCore.Diagnostics.UnhandledException", - "Microsoft.AspNetCore.Hosting.UnhandledException" - ]; - - private readonly Func isEnabled = (eventName, _, _) - => DiagnosticSourceEvents.Contains(eventName); - - private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; - - internal AspNetCoreMetrics() - { - var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore"); - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(metricsListener, this.isEnabled, AspNetCoreInstrumentationEventSource.Log.UnknownErrorProcessingEvent); - this.diagnosticSourceSubscriber.Subscribe(); - } - - /// - public void Dispose() - { - this.diagnosticSourceSubscriber?.Dispose(); - } -} -#endif diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index eb7c0d907b..477cb8e52e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -3,7 +3,8 @@ ## Unreleased * Drop support for .NET 6 as this target is no longer supported. - ([#2138](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2138)) + ([#2138](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2138), + ([#2360](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2360)) * Updated OpenTelemetry core component version(s) to `1.10.0`. ([#2317](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2317)) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs deleted file mode 100644 index c9040d0248..0000000000 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ /dev/null @@ -1,132 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Diagnostics.Metrics; -using System.Reflection; -using Microsoft.AspNetCore.Http; -#if NET -using Microsoft.AspNetCore.Diagnostics; -using Microsoft.AspNetCore.Routing; -#endif -using OpenTelemetry.Internal; -using OpenTelemetry.Trace; - -namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; - -internal sealed class HttpInMetricsListener : ListenerHandler -{ - internal const string HttpServerRequestDurationMetricName = "http.server.request.duration"; - - internal const string OnUnhandledHostingExceptionEvent = "Microsoft.AspNetCore.Hosting.UnhandledException"; - internal const string OnUnhandledDiagnosticsExceptionEvent = "Microsoft.AspNetCore.Diagnostics.UnhandledException"; - - internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName(); - internal static readonly string InstrumentationName = AssemblyName.Name!; - internal static readonly string InstrumentationVersion = AssemblyName.Version!.ToString(); - internal static readonly Meter Meter = new(InstrumentationName, InstrumentationVersion); - - private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop"; - - private static readonly PropertyFetcher ExceptionPropertyFetcher = new("Exception"); - private static readonly PropertyFetcher HttpContextPropertyFetcher = new("HttpContext"); - private static readonly object ErrorTypeHttpContextItemsKey = new(); - - private static readonly Histogram HttpServerRequestDuration = Meter.CreateHistogram(HttpServerRequestDurationMetricName, "s", "Duration of HTTP server requests."); - - internal HttpInMetricsListener(string name) - : base(name) - { - } - - public static void OnExceptionEventWritten(string name, object? payload) - { - // We need to use reflection here as the payload type is not a defined public type. - if (!TryFetchException(payload, out var exc) || !TryFetchHttpContext(payload, out var ctx)) - { - AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(OnExceptionEventWritten), HttpServerRequestDurationMetricName); - return; - } - - ctx.Items.Add(ErrorTypeHttpContextItemsKey, exc.GetType().FullName); - - // See https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L252 - // and https://github.com/dotnet/aspnetcore/blob/690d78279e940d267669f825aa6627b0d731f64c/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs#L174 - // this makes sure that top-level properties on the payload object are always preserved. -#if NET - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")] -#endif - static bool TryFetchException(object? payload, [NotNullWhen(true)] out Exception? exc) - { - return ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null; - } -#if NET - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The ASP.NET Core framework guarantees that top level properties are preserved")] -#endif - static bool TryFetchHttpContext(object? payload, [NotNullWhen(true)] out HttpContext? ctx) - { - return HttpContextPropertyFetcher.TryFetch(payload, out ctx) && ctx != null; - } - } - - public static void OnStopEventWritten(string name, object? payload) - { - if (payload is not HttpContext context) - { - AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(OnStopEventWritten), HttpServerRequestDurationMetricName); - return; - } - - TagList tags = default; - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - tags.Add(new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, RequestDataHelper.GetHttpProtocolVersion(context.Request.Protocol))); - tags.Add(new KeyValuePair(SemanticConventions.AttributeUrlScheme, context.Request.Scheme)); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode))); - - var httpMethod = TelemetryHelper.RequestDataHelper.GetNormalizedHttpMethod(context.Request.Method); - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, httpMethod)); - -#if NET - // Check the exception handler feature first in case the endpoint was overwritten - var route = (context.Features.Get()?.Endpoint as RouteEndpoint ?? - context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText; - if (!string.IsNullOrEmpty(route)) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRoute, route)); - } -#endif - if (context.Items.TryGetValue(ErrorTypeHttpContextItemsKey, out var errorType)) - { - tags.Add(new KeyValuePair(SemanticConventions.AttributeErrorType, errorType)); - } - - // We are relying here on ASP.NET Core to set duration before writing the stop event. - // https://github.com/dotnet/aspnetcore/blob/d6fa351048617ae1c8b47493ba1abbe94c3a24cf/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L449 - // TODO: Follow up with .NET team if we can continue to rely on this behavior. - HttpServerRequestDuration.Record(Activity.Current!.Duration.TotalSeconds, tags); - } - - public override void OnEventWritten(string name, object? payload) - { - switch (name) - { - case OnUnhandledDiagnosticsExceptionEvent: - case OnUnhandledHostingExceptionEvent: - { - OnExceptionEventWritten(name, payload); - } - - break; - case OnStopEvent: - { - OnStopEventWritten(name, payload); - } - - break; - default: - break; - } - } -} diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index 12a76ed607..74938b4592 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -113,29 +113,8 @@ public void ConfigureServices(IServiceCollection services) } ``` -Following list of attributes are added by default on -`http.server.request.duration` metric. See -[http-metrics](https://github.com/open-telemetry/semantic-conventions/tree/v1.23.0/docs/http/http-metrics.md) -for more details about each individual attribute. `.NET8.0` and above supports -additional metrics, see [list of metrics produced](#list-of-metrics-produced) for -more details. - -* `error.type` -* `http.response.status_code` -* `http.request.method` -* `http.route` -* `network.protocol.version` -* `url.scheme` - #### List of metrics produced -When the application targets `.NET6.0` or `.NET7.0`, the instrumentation emits -the following metric: - -| Name | Details | -|-----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------| -| `http.server.request.duration` | [Specification](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpserverrequestduration) | - Starting from `.NET8.0`, metrics instrumentation is natively implemented, and the ASP.NET Core library has incorporated support for [built-in metrics](https://learn.microsoft.com/dotnet/core/diagnostics/built-in-metrics-aspnetcore) @@ -164,16 +143,6 @@ to achieve this. > There is no difference in features or emitted metrics when enabling metrics using `AddMeter()` or `AddAspNetCoreInstrumentation()` on `.NET8.0` and newer versions. - -> [!NOTE] -> The `http.server.request.duration` metric is emitted in `seconds` as per the -semantic convention. While the convention [recommends using custom histogram -buckets](https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md) -, this feature is not yet available via .NET Metrics API. A -[workaround](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4820) -has been included in OTel SDK starting version `1.6.0` which applies recommended -buckets by default for `http.server.request.duration`. This applies to all -targeted frameworks. ## Advanced configuration diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index c3f80a9cee..804332f450 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -1,22 +1,13 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if NET using System.Threading.RateLimiting; using Microsoft.AspNetCore.Builder; -#endif using Microsoft.AspNetCore.Hosting; -#if NET using Microsoft.AspNetCore.Http; -#endif using Microsoft.AspNetCore.Mvc.Testing; -#if NET using Microsoft.AspNetCore.RateLimiting; -#endif -#if NET using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; -#endif using Microsoft.Extensions.Logging; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -38,7 +29,6 @@ public void AddAspNetCoreInstrumentation_BadArgs() Assert.Throws(builder!.AddAspNetCoreInstrumentation); } -#if NET [Fact] public async Task ValidateNet8MetricsAsync() { @@ -178,7 +168,6 @@ static string GetTicks() await app.DisposeAsync(); } -#endif [Theory] [InlineData("/api/values/2", "api/Values/{id}", null, 200)]