From 43ffc382c4c6c703273ed59f94990b8421a44913 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 28 Jun 2024 14:18:16 +0800 Subject: [PATCH 1/3] Annotate histograms with boundaries --- src/Hosting/Hosting/src/Internal/HostingMetrics.cs | 3 ++- .../Hosting/src/Microsoft.AspNetCore.Hosting.csproj | 1 + .../src/Microsoft.AspNetCore.RateLimiting.csproj | 1 + .../RateLimiting/src/RateLimitingMetrics.cs | 4 +++- .../src/Internal/Infrastructure/KestrelMetrics.cs | 7 +++++-- .../Microsoft.AspNetCore.Server.Kestrel.Core.csproj | 1 + src/Shared/Metrics/MetricsConstants.cs | 13 +++++++++++++ .../src/Internal/HttpConnectionsMetrics.cs | 3 ++- .../Microsoft.AspNetCore.Http.Connections.csproj | 1 + 9 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 src/Shared/Metrics/MetricsConstants.cs diff --git a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs index b72f0c6d5f03..bdd83e5212be 100644 --- a/src/Hosting/Hosting/src/Internal/HostingMetrics.cs +++ b/src/Hosting/Hosting/src/Internal/HostingMetrics.cs @@ -29,7 +29,8 @@ public HostingMetrics(IMeterFactory meterFactory) _requestDuration = _meter.CreateHistogram( "http.server.request.duration", unit: "s", - description: "Duration of HTTP server requests."); + description: "Duration of HTTP server requests.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries }); } // Note: Calling code checks whether counter is enabled. diff --git a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj index 3caa14c4beb3..80948eacb708 100644 --- a/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj +++ b/src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj @@ -17,6 +17,7 @@ + diff --git a/src/Middleware/RateLimiting/src/Microsoft.AspNetCore.RateLimiting.csproj b/src/Middleware/RateLimiting/src/Microsoft.AspNetCore.RateLimiting.csproj index a15b1fe9bf8c..10ee122da393 100644 --- a/src/Middleware/RateLimiting/src/Microsoft.AspNetCore.RateLimiting.csproj +++ b/src/Middleware/RateLimiting/src/Microsoft.AspNetCore.RateLimiting.csproj @@ -18,6 +18,7 @@ + diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index 3a0078acce7c..8b0b66070098 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Diagnostics.Metrics; using System.Runtime.CompilerServices; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.RateLimiting; @@ -40,7 +41,8 @@ public RateLimitingMetrics(IMeterFactory meterFactory) _queuedRequestDurationCounter = _meter.CreateHistogram( "aspnetcore.rate_limiting.request.time_in_queue", unit: "s", - description: "The duration of HTTP requests in a queue, waiting to acquire a rate limiting lease."); + description: "The duration of HTTP requests in a queue, waiting to acquire a rate limiting lease.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries }); _requestsCounter = _meter.CreateCounter( "aspnetcore.rate_limiting.requests", diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs index f6997fc8bbc1..10768d162a21 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelMetrics.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using System.Security.Authentication; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; @@ -43,7 +44,8 @@ public KestrelMetrics(IMeterFactory meterFactory) _connectionDuration = _meter.CreateHistogram( "kestrel.connection.duration", unit: "s", - description: "The duration of connections on the server."); + description: "The duration of connections on the server.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.LongSecondsBucketBoundaries }); _rejectedConnectionsCounter = _meter.CreateCounter( "kestrel.rejected_connections", @@ -68,7 +70,8 @@ public KestrelMetrics(IMeterFactory meterFactory) _tlsHandshakeDuration = _meter.CreateHistogram( "kestrel.tls_handshake.duration", unit: "s", - description: "The duration of TLS handshakes on the server."); + description: "The duration of TLS handshakes on the server.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries }); _activeTlsHandshakesCounter = _meter.CreateUpDownCounter( "kestrel.active_tls_handshakes", diff --git a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj index 130870a922f9..1576c0f5a653 100644 --- a/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj +++ b/src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj @@ -32,6 +32,7 @@ + diff --git a/src/Shared/Metrics/MetricsConstants.cs b/src/Shared/Metrics/MetricsConstants.cs new file mode 100644 index 000000000000..17f8b8ced558 --- /dev/null +++ b/src/Shared/Metrics/MetricsConstants.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http; + +internal static class MetricsConstants +{ + // Follows boundaries from http.server.request.duration/http.client.request.duration + public static readonly IReadOnlyList ShortSecondsBucketBoundaries = [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]; + + // Not based on a standard. See https://github.com/open-telemetry/semantic-conventions/issues/336 + public static readonly IReadOnlyList LongSecondsBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300]; +} diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs index 46556872bf05..9d289b926d28 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionsMetrics.cs @@ -33,7 +33,8 @@ public HttpConnectionsMetrics(IMeterFactory meterFactory) _connectionDuration = _meter.CreateHistogram( "signalr.server.connection.duration", unit: "s", - description: "The duration of connections on the server."); + description: "The duration of connections on the server.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.LongSecondsBucketBoundaries }); _currentConnectionsCounter = _meter.CreateUpDownCounter( "signalr.server.active_connections", diff --git a/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj b/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj index 8fbe199512bf..6449f8900b1f 100644 --- a/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj +++ b/src/SignalR/common/Http.Connections/src/Microsoft.AspNetCore.Http.Connections.csproj @@ -25,6 +25,7 @@ + From 00015bf1f9ba1a3b8ce1f5b934044d2160769a04 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 28 Jun 2024 14:33:03 +0800 Subject: [PATCH 2/3] Missed histogram --- src/Middleware/RateLimiting/src/RateLimitingMetrics.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs index 8b0b66070098..9037887f10b6 100644 --- a/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs +++ b/src/Middleware/RateLimiting/src/RateLimitingMetrics.cs @@ -31,7 +31,8 @@ public RateLimitingMetrics(IMeterFactory meterFactory) _requestLeaseDurationCounter = _meter.CreateHistogram( "aspnetcore.rate_limiting.request_lease.duration", unit: "s", - description: "The duration of rate limiting leases held by HTTP requests on the server."); + description: "The duration of rate limiting leases held by HTTP requests on the server.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries }); _queuedRequestsCounter = _meter.CreateUpDownCounter( "aspnetcore.rate_limiting.queued_requests", From c26607b79bfac4a01caa2e1ac9f6ecca914cb55d Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 28 Jun 2024 16:11:52 +0800 Subject: [PATCH 3/3] Update src/Shared/Metrics/MetricsConstants.cs --- src/Shared/Metrics/MetricsConstants.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/Metrics/MetricsConstants.cs b/src/Shared/Metrics/MetricsConstants.cs index 17f8b8ced558..6cd103eb3d35 100644 --- a/src/Shared/Metrics/MetricsConstants.cs +++ b/src/Shared/Metrics/MetricsConstants.cs @@ -8,6 +8,6 @@ internal static class MetricsConstants // Follows boundaries from http.server.request.duration/http.client.request.duration public static readonly IReadOnlyList ShortSecondsBucketBoundaries = [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]; - // Not based on a standard. See https://github.com/open-telemetry/semantic-conventions/issues/336 + // Not based on a standard. Larger bucket sizes for longer lasting operations, e.g. HTTP connection duration. See https://github.com/open-telemetry/semantic-conventions/issues/336 public static readonly IReadOnlyList LongSecondsBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300]; }