diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs index 3622edfee0cb8..ce1a66171fc6e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHelper.cs @@ -3,12 +3,21 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.Metrics; using System.Threading; namespace System.Net.Http { internal static class DiagnosticsHelper { + // OTel bucket boundary recommendation for 'http.request.duration': + // https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpclientrequestduration + // We are using the same boundaries for durations which are not expected to be longer than an HTTP request. + public static InstrumentAdvice ShortHistogramAdvice { get; } = new() + { + HistogramBucketBoundaries = [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] + }; + internal static string GetRedactedUriString(Uri uri) { Debug.Assert(uri.IsAbsoluteUri); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs index 152ad063a5b8a..d08f2c1751f56 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Metrics/MetricsHandler.cs @@ -29,7 +29,8 @@ public MetricsHandler(HttpMessageHandler innerHandler, IMeterFactory? meterFacto _requestsDuration = meter.CreateHistogram( "http.client.request.duration", unit: "s", - description: "Duration of HTTP client requests."); + description: "Duration of HTTP client requests.", + advice: DiagnosticsHelper.ShortHistogramAdvice); } internal override ValueTask SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs index f654136a1001c..8933aa5c3fd23 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Metrics/SocketsHttpHandlerMetrics.cs @@ -16,12 +16,18 @@ internal sealed class SocketsHttpHandlerMetrics(Meter meter) public readonly Histogram ConnectionDuration = meter.CreateHistogram( name: "http.client.connection.duration", unit: "s", - description: "The duration of successfully established outbound HTTP connections."); + description: "The duration of successfully established outbound HTTP connections.", + advice: new InstrumentAdvice() + { + // These values are not based on a standard and may change in the future. + HistogramBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300] + }); public readonly Histogram RequestsQueueDuration = meter.CreateHistogram( name: "http.client.request.time_in_queue", unit: "s", - description: "The amount of time requests spent on a queue waiting for an available connection."); + description: "The amount of time requests spent on a queue waiting for an available connection.", + advice: DiagnosticsHelper.ShortHistogramAdvice); public void RequestLeftQueue(HttpRequestMessage request, HttpConnectionPool pool, TimeSpan duration, int versionMajor) { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs index b8dc01e63fdb7..53a716cd92045 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/MetricsTest.cs @@ -172,6 +172,8 @@ protected sealed class InstrumentRecorder : IDisposable where T : struct private Meter? _meter; public Action? MeasurementRecorded; + public Action> VerifyHistogramBucketBoundaries; + public int MeasurementCount => _values.Count; public InstrumentRecorder(string instrumentName) { @@ -204,6 +206,13 @@ private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnl { _values.Enqueue(new Measurement(measurement, tags)); MeasurementRecorded?.Invoke(); + if (VerifyHistogramBucketBoundaries is not null) + { + Histogram histogram = (Histogram)instrument; + IReadOnlyList boundaries = histogram.Advice.HistogramBucketBoundaries; + Assert.NotNull(boundaries); + VerifyHistogramBucketBoundaries(boundaries); + } } public IReadOnlyList> GetMeasurements() => _values.ToArray(); @@ -339,6 +348,7 @@ public Task RequestDuration_Success_Recorded(string method, HttpStatusCode statu { using HttpMessageInvoker client = CreateHttpMessageInvoker(); using InstrumentRecorder recorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); + using HttpRequestMessage request = new(HttpMethod.Parse(method), uri) { Version = UseVersion }; using HttpResponseMessage response = await SendAsync(client, request); @@ -927,6 +937,40 @@ await IgnoreExceptions(async () => }); }); } + + [Fact] + public Task DurationHistograms_HaveBucketSizeHints() + { + return LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + using HttpMessageInvoker client = CreateHttpMessageInvoker(); + using InstrumentRecorder requestDurationRecorder = SetupInstrumentRecorder(InstrumentNames.RequestDuration); + using InstrumentRecorder timeInQueueRecorder = SetupInstrumentRecorder(InstrumentNames.TimeInQueue); + using InstrumentRecorder connectionDurationRecorder = SetupInstrumentRecorder(InstrumentNames.ConnectionDuration); + + requestDurationRecorder.VerifyHistogramBucketBoundaries = b => + { + // Verify first and last value of the boundaries defined in + // https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpserverrequestduration + Assert.Equal(0.005, b.First()); + Assert.Equal(10, b.Last()); + }; + timeInQueueRecorder.VerifyHistogramBucketBoundaries = requestDurationRecorder.VerifyHistogramBucketBoundaries; + connectionDurationRecorder.VerifyHistogramBucketBoundaries = + b => Assert.True(b.Last() > 180); // At least 3 minutes for the highest bucket. + + using HttpRequestMessage request = new(HttpMethod.Get, uri) { Version = UseVersion }; + using HttpResponseMessage response = await SendAsync(client, request); + + Assert.Equal(1, requestDurationRecorder.MeasurementCount); + Assert.Equal(1, timeInQueueRecorder.MeasurementCount); + client.Dispose(); // terminate the connection + Assert.Equal(1, connectionDurationRecorder.MeasurementCount); + }, async server => + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + }); + } } [ActiveIssue("https://github.com/dotnet/runtime/issues/93754", TestPlatforms.Browser)] diff --git a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs index fe1048e90b22d..580254289f9f7 100644 --- a/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs +++ b/src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionMetrics.cs @@ -15,7 +15,14 @@ internal static class NameResolutionMetrics private static readonly Histogram s_lookupDuration = s_meter.CreateHistogram( name: "dns.lookup.duration", unit: "s", - description: "Measures the time taken to perform a DNS lookup."); + description: "Measures the time taken to perform a DNS lookup.", + advice: new InstrumentAdvice() + { + // OTel bucket boundary recommendation for 'http.request.duration': + // https://github.com/open-telemetry/semantic-conventions/blob/release/v1.23.x/docs/http/http-metrics.md#metric-httpclientrequestduration + // We are using these boundaries for all network requests that are expected to be short. + HistogramBucketBoundaries = [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] + }); public static bool IsEnabled() => s_lookupDuration.Enabled; diff --git a/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs b/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs index e72a7f2940a13..ef50997547dc2 100644 --- a/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs +++ b/src/libraries/System.Net.NameResolution/tests/FunctionalTests/MetricsTest.cs @@ -41,6 +41,22 @@ await RemoteExecutor.Invoke(async () => }).DisposeAsync(); } + [Fact] + public static async Task DurationHistogram_HasBucketSizeHints() + { + await RemoteExecutor.Invoke(async () => + { + const string ValidHostName = "localhost"; + + using var recorder = new InstrumentRecorder(DnsLookupDuration); + recorder.VerifyHistogramBucketBoundaries = b => Assert.True(b.Count > 2); + + await Dns.GetHostEntryAsync(ValidHostName); + + Assert.Equal(1, recorder.MeasurementCount); + }).DisposeAsync(); + } + [Fact] public static async Task ResolveInvalidHostName_MetricsRecorded() { @@ -86,6 +102,9 @@ private sealed class InstrumentRecorder : IDisposable where T : struct private readonly MeterListener _meterListener = new(); private readonly ConcurrentQueue> _values = new(); + public Action> VerifyHistogramBucketBoundaries; + public int MeasurementCount => _values.Count; + public InstrumentRecorder(string instrumentName) { _meterListener.InstrumentPublished = (instrument, listener) => @@ -99,7 +118,17 @@ public InstrumentRecorder(string instrumentName) _meterListener.Start(); } - private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnlySpan> tags, object? state) => _values.Enqueue(new Measurement(measurement, tags)); + private void OnMeasurementRecorded(Instrument instrument, T measurement, ReadOnlySpan> tags, object? state) + { + _values.Enqueue(new Measurement(measurement, tags)); + if (VerifyHistogramBucketBoundaries is not null) + { + Histogram histogram = (Histogram)instrument; + IReadOnlyList boundaries = histogram.Advice.HistogramBucketBoundaries; + Assert.NotNull(boundaries); + VerifyHistogramBucketBoundaries(boundaries); + } + } public IReadOnlyList> GetMeasurements() => _values.ToArray(); public void Dispose() => _meterListener.Dispose(); }