HttpSemanticConventions - new metric in AspNetCore Instrumentation.

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* TODO: UPDATE THIS
+
 ## 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,14 +45,38 @@ 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)
+        {
+            // TODO: This needs to be changed to  "s" (seconds). This is blocked until we can change the default histogram.
+            // See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/4797
+            this.httpServerRequestDuration = meter.CreateHistogram<double>(HttpServerRequestDurationMetricName, "ms", "Measures the duration of inbound HTTP requests.");
+        }
     }
 
     public override void OnEventWritten(string name, object payload)
+    {
+        if (this.emitOldAttributes)
+        {
+            this.OnEventWritten_Old(name, payload);
+        }
+
+        if (this.emitNewAttributes)
+        {
+            this.OnEventWritten_New(name, payload);
+        }
+    }
+
+    public void OnEventWritten_Old(string name, object payload)
     {
         if (name == OnStopEvent)
         {
@@ -84,43 +111,87 @@ public override void OnEventWritten(string name, object payload)
 
             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)
+            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.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)));
+                tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostName, context.Request.Host.Host));
 
-                if (context.Request.Host.HasValue)
+                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.AttributeNetHostName, context.Request.Host.Host));
+                    tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetHostPort, context.Request.Host.Port));
+                }
+            }
 
-                    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));
-                    }
+#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
+                {
+                    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)
+    {
+        if (name == OnStopEvent)
+        {
+            var context = payload as HttpContext;
+            if (context == null)
             {
-                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)));
+                AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), EventName, HttpServerRequestDurationMetricName);
+                return;
+            }
 
-                if (context.Request.Host.HasValue)
+            try
+            {
+                if (this.options.Filter?.Invoke(HttpServerRequestDurationMetricName, context) == false)
                 {
-                    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;
@@ -133,18 +204,21 @@ public override void OnEventWritten(string name, object payload)
             {
                 try
                 {
-                    this.options.Enrich(HttpServerDurationMetricName, context, ref tags);
+                    this.options.Enrich(HttpServerRequestDurationMetricName, 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);
+
+            // TODO: This needs to be changed to TotalSeconds. This is blocked until we can change the default histogram.
+            // See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/4797
+            this.httpServerRequestDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags);
         }
     }
 }

src/OpenTelemetry.Instrumentation.AspNetCore/README.md

@@ -90,13 +90,22 @@ 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/4bbb8c907402caa90bc077214e8a2c78807c1ab9/docs/http/http-metrics.md).
 
-| Name  | Instrument Type | Unit | Description |
-|-------|-----------------|------|-------------|
-| `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. |
+Have you opt-ed into the new Http Semantic Conventions?
+
+* If yes, the instrumentation supports the following metric.
+
+    | Name  | Instrument Type | Unit | Description |
+    |-------|-----------------|------|-------------|
+    | `http.server.request.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. |
+
+* If no, the instrumentation supports the following metric.
+
+    | Name  | Instrument Type | Unit | Description |
+    |-------|-----------------|------|-------------|
+    | `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. |
 
 ## Advanced configuration