[aspnetcore] Restore metrics instrumentation in netstandard builds (#…

…2403)

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs

@@ -1,6 +1,10 @@
 // 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;
@@ -18,15 +22,27 @@ public static class AspNetCoreInstrumentationMeterProviderBuilderExtensions
     public static MeterProviderBuilder AddAspNetCoreInstrumentation(
         this MeterProviderBuilder builder)
     {
-#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.");
-        }
+        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);
 
+#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")

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs

@@ -0,0 +1,41 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#if !NET
+using OpenTelemetry.Instrumentation.AspNetCore.Implementation;
+
+namespace OpenTelemetry.Instrumentation.AspNetCore;
+
+/// <summary>
+/// Asp.Net Core Requests instrumentation.
+/// </summary>
+internal sealed class AspNetCoreMetrics : IDisposable
+{
+    private static readonly HashSet<string> 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<string, object?, object?, bool> 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();
+    }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        this.diagnosticSourceSubscriber?.Dispose();
+    }
+}
+#endif

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,13 +2,16 @@
 
 ## Unreleased
 
+* Metric support for the .NET Standard target was removed by mistake in 1.10.0.
+  This functionality has been restored.
+  ([#2403](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2403))
+
 ## 1.10.0
 
 Released 2024-Dec-09
 
 * Drop support for .NET 6 as this target is no longer supported.
-  ([#2138](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2138),
-  ([#2360](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2360))
+  ([#2138](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2138))
 
 * Updated OpenTelemetry core component version(s) to `1.10.0`.
   ([#2317](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2317))

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs

@@ -0,0 +1,132 @@
+// 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<Exception> ExceptionPropertyFetcher = new("Exception");
+    private static readonly PropertyFetcher<HttpContext> HttpContextPropertyFetcher = new("HttpContext");
+    private static readonly object ErrorTypeHttpContextItemsKey = new();
+
+    private static readonly Histogram<double> HttpServerRequestDuration = Meter.CreateHistogram<double>(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<string, object?>(SemanticConventions.AttributeNetworkProtocolVersion, RequestDataHelper.GetHttpProtocolVersion(context.Request.Protocol)));
+        tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
+        tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
+
+        var httpMethod = TelemetryHelper.RequestDataHelper.GetNormalizedHttpMethod(context.Request.Method);
+        tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
+
+#if NET
+        // Check the exception handler feature first in case the endpoint was overwritten
+        var route = (context.Features.Get<IExceptionHandlerPathFeature>()?.Endpoint as RouteEndpoint ??
+                     context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
+        if (!string.IsNullOrEmpty(route))
+        {
+            tags.Add(new KeyValuePair<string, object?>(SemanticConventions.AttributeHttpRoute, route));
+        }
+#endif
+        if (context.Items.TryGetValue(ErrorTypeHttpContextItemsKey, out var errorType))
+        {
+            tags.Add(new KeyValuePair<string, object?>(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;
+        }
+    }
+}

src/OpenTelemetry.Instrumentation.AspNetCore/README.md

@@ -113,8 +113,29 @@ 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)
@@ -143,6 +164,16 @@ 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.
+<!-- This comment is to make sure the two notes above and below are not merged -->
+> [!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
 

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs

@@ -1,13 +1,22 @@
 // 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;
@@ -29,6 +38,7 @@ public void AddAspNetCoreInstrumentation_BadArgs()
         Assert.Throws<ArgumentNullException>(builder!.AddAspNetCoreInstrumentation);
     }
 
+#if NET
     [Fact]
     public async Task ValidateNet8MetricsAsync()
     {
@@ -168,6 +178,7 @@ static string GetTicks()
 
         await app.DisposeAsync();
     }
+#endif
 
     [Theory]
     [InlineData("/api/values/2", "api/Values/{id}", null, 200)]