HttpSemanticConventions - new metric in AspNetCore Instrumentation.

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,6 +2,26 @@
 
 ## Unreleased
 
+* Introduced a new metric, `http.server.request.duration`, for users who opt-in
+  to the new semantic convention by configuring the `OTEL_SEMCONV_STABILITY_OPT_IN`
+  environment variable. This metric measures time in seconds and uses unique
+  histogram buckets as defined by the semantic conventions for Http
+  [metrics](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md)
+  While the convention recommends using custom histogram buckets, 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 this metric.
+  ([#4802](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4802))
+  * New metric: `http.server.request.duration`
+    * Unit: `seconds`
+    * Histogram Buckets: `0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5,
+    0.75, 1,  2.5, 5, 7.5, 10`
+  * Replaces old metric: `http.server.duration`
+    * Unit: `miliseconds`
+    * Histogram Buckets: `0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500,
+    5000, 7500, 10000`
+
 ## 1.5.1-beta.1
 
 Released 2023-Jul-20

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

@@ -27,13 +27,16 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation;
 
 internal sealed class HttpInMetricsListener : ListenerHandler
 {
-    private const string HttpServerDurationMetricName = "http.server.duration";
+    internal const string HttpServerDurationMetricName = "http.server.duration";
+    internal const string HttpServerRequestDurationMetricName = "http.server.request.duration";
+
     private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop";
     private const string EventName = "OnStopActivity";
 
     private readonly Meter meter;
     private readonly AspNetCoreMetricsInstrumentationOptions options;
     private readonly Histogram<double> httpServerDuration;
+    private readonly Histogram<double> httpServerRequestDuration;
     private readonly bool emitOldAttributes;
     private readonly bool emitNewAttributes;
 
@@ -42,109 +45,172 @@ internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstru
     {
         this.meter = meter;
         this.options = options;
-        this.httpServerDuration = meter.CreateHistogram<double>(HttpServerDurationMetricName, "ms", "Measures the duration of inbound HTTP requests.");
 
         this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old);
 
         this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New);
+
+        if (this.emitOldAttributes)
+        {
+            this.httpServerDuration = meter.CreateHistogram<double>(HttpServerDurationMetricName, "ms", "Measures the duration of inbound HTTP requests.");
+        }
+
+        if (this.emitNewAttributes)
+        {
+            this.httpServerRequestDuration = meter.CreateHistogram<double>(HttpServerRequestDurationMetricName, "s", "Measures the duration of inbound HTTP requests.");
+        }
     }
 
     public override void OnEventWritten(string name, object payload)
     {
         if (name == OnStopEvent)
         {
-            var context = payload as HttpContext;
-            if (context == null)
+            if (this.emitOldAttributes)
             {
-                AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName);
-                return;
+                this.OnEventWritten_Old(name, payload);
             }
 
-            try
+            if (this.emitNewAttributes)
             {
-                if (this.options.Filter?.Invoke(HttpServerDurationMetricName, context) == false)
-                {
-                    AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName);
-                    return;
-                }
+                this.OnEventWritten_New(name, payload);
             }
-            catch (Exception ex)
+        }
+    }
+
+    public void OnEventWritten_Old(string name, object payload)
+    {
+        var context = payload as HttpContext;
+        if (context == null)
+        {
+            AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName);
+            return;
+        }
+
+        try
+        {
+            if (this.options.Filter?.Invoke(HttpServerDurationMetricName, context) == false)
             {
-                AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName, ex);
+                AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName);
                 return;
             }
+        }
+        catch (Exception ex)
+        {
+            AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName, ex);
+            return;
+        }
+
+        // TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
+        // Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
+        // If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
+        if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
+        {
+            return;
+        }
+
+        TagList tags = default;
 
-            // TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
-            // Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
-            // If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
-            if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, context.Request.Scheme));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, context.Request.Method));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
+
+        if (context.Request.Host.HasValue)
+        {
+            tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostName, context.Request.Host.Host));
+
+            if (context.Request.Host.Port is not null && context.Request.Host.Port != 80 && context.Request.Host.Port != 443)
             {
-                return;
+                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port));
             }
+        }
 
-            TagList tags = default;
-
-            // see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md
-            if (this.emitOldAttributes)
+#if NET6_0_OR_GREATER
+        var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
+        if (!string.IsNullOrEmpty(route))
+        {
+            tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, route));
+        }
+#endif
+        if (this.options.Enrich != null)
+        {
+            try
             {
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpScheme, context.Request.Scheme));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpMethod, context.Request.Method));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
-
-                if (context.Request.Host.HasValue)
-                {
-                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostName, context.Request.Host.Host));
-
-                    if (context.Request.Host.Port is not null && context.Request.Host.Port != 80 && context.Request.Host.Port != 443)
-                    {
-                        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port));
-                    }
-                }
+                this.options.Enrich(HttpServerDurationMetricName, context, ref tags);
             }
+            catch (Exception ex)
+            {
+                AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName, ex);
+            }
+        }
 
-            // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
-            if (this.emitNewAttributes)
+        // 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.
+        this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags);
+    }
+
+    public void OnEventWritten_New(string name, object payload)
+    {
+        var context = payload as HttpContext;
+        if (context == null)
+        {
+            AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), EventName, HttpServerRequestDurationMetricName);
+            return;
+        }
+
+        try
+        {
+            if (this.options.Filter?.Invoke(HttpServerRequestDurationMetricName, context) == false)
             {
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, context.Request.Method));
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
-
-                if (context.Request.Host.HasValue)
-                {
-                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerAddress, context.Request.Host.Host));
-
-                    if (context.Request.Host.Port is not null && context.Request.Host.Port != 80 && context.Request.Host.Port != 443)
-                    {
-                        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeServerPort, context.Request.Host.Port));
-                    }
-                }
+                AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(nameof(HttpInMetricsListener), EventName, HttpServerRequestDurationMetricName);
+                return;
             }
+        }
+        catch (Exception ex)
+        {
+            AspNetCoreInstrumentationEventSource.Log.RequestFilterException(nameof(HttpInMetricsListener), EventName, HttpServerRequestDurationMetricName, ex);
+            return;
+        }
+
+        // TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
+        // Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
+        // If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
+        if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
+        {
+            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, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, context.Request.Method));
+        tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
 
 #if NET6_0_OR_GREATER
-            var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
-            if (!string.IsNullOrEmpty(route))
+        var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
+        if (!string.IsNullOrEmpty(route))
+        {
+            tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, route));
+        }
+#endif
+        if (this.options.Enrich != null)
+        {
+            try
             {
-                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRoute, route));
+                this.options.Enrich(HttpServerRequestDurationMetricName, context, ref tags);
             }
-#endif
-            if (this.options.Enrich != null)
+            catch (Exception ex)
             {
-                try
-                {
-                    this.options.Enrich(HttpServerDurationMetricName, context, ref tags);
-                }
-                catch (Exception ex)
-                {
-                    AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInMetricsListener), EventName, HttpServerDurationMetricName, ex);
-                }
+                AspNetCoreInstrumentationEventSource.Log.EnrichmentException(nameof(HttpInMetricsListener), EventName, HttpServerRequestDurationMetricName, ex);
             }
-
-            // 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.
-            this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags);
         }
+
+        // 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.
+        this.httpServerRequestDuration.Record(Activity.Current.Duration.TotalSeconds, tags);
     }
 }

src/OpenTelemetry.Instrumentation.AspNetCore/README.md

@@ -90,13 +90,34 @@ public void ConfigureServices(IServiceCollection services)
 
 #### List of metrics produced
 
-The instrumentation is implemented based on [metrics semantic
-conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#metric-httpserverduration).
-Currently, the instrumentation supports the following metric.
+The instrumentation is implemented based on
+[metrics semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/docs/http/http-metrics.md).
 
-| Name  | Instrument Type | Unit | Description |
-|-------|-----------------|------|-------------|
-| `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. |
+A different metric is emitted depending if a user opts-in to the new Http
+Semantic Conventions using `OTEL_SEMCONV_STABILITY_OPT_IN`.
+
+* By default, the instrumentation emits the following metric.
+
+    | Name  | Instrument Type | Unit | Description |
+    |-------|-----------------|------|-------------|
+    | `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. |
+
+* If user sets the environment variable to `http`, the instrumentation emits
+  the following metric.
+
+    | Name  | Instrument Type | Unit | Description |
+    |-------|-----------------|------|-------------|
+    | `http.server.request.duration` | Histogram | `s` | Measures the duration of inbound HTTP requests. |
+
+    This 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/2bad9afad58fbd6b33cc683d1ad1f006e35e4a5d/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 this metric.
+
+* If user sets the environment variable to `http/dup`, the instrumentation
+  emits both `http.server.duration` and `http.server.request.duration`.
 
 ## Advanced configuration