diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 353b2db356f..2e6b2a17876 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -17,6 +17,10 @@ ([#5004](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5004)) ([#5016](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5016)) +* Update `AggregatorStore` to provide known connection metrics with larger + histogram buckets. + ([#5008](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5008)) + ## 1.7.0-alpha.1 Released 2023-Oct-16 diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 69343fc952f..9bbceacf6d6 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -372,10 +372,19 @@ internal MetricPointsAccessor GetMetricPoints() private static double[] FindDefaultHistogramBounds(in MetricStreamIdentity metricStreamIdentity) { - if (metricStreamIdentity.Unit == "s" && Metric.DefaultHistogramBoundMappings - .Contains((metricStreamIdentity.MeterName, metricStreamIdentity.InstrumentName))) + if (metricStreamIdentity.Unit == "s") { - return Metric.DefaultHistogramBoundsSeconds; + if (Metric.DefaultHistogramBoundShortMappings + .Contains((metricStreamIdentity.MeterName, metricStreamIdentity.InstrumentName))) + { + return Metric.DefaultHistogramBoundsShortSeconds; + } + + if (Metric.DefaultHistogramBoundLongMappings + .Contains((metricStreamIdentity.MeterName, metricStreamIdentity.InstrumentName))) + { + return Metric.DefaultHistogramBoundsLongSeconds; + } } return Metric.DefaultHistogramBounds; diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index 148c9dc8bfc..636755ed3b4 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -28,23 +28,31 @@ public sealed class Metric internal const int DefaultExponentialHistogramMaxScale = 20; internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 }; - internal static readonly double[] DefaultHistogramBoundsSeconds = new double[] { 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 }; - internal static readonly HashSet<(string, string)> DefaultHistogramBoundMappings = new() + + // Short default histogram bounds. Based on the recommended semantic convention values for http.server.request.duration. + internal static readonly double[] DefaultHistogramBoundsShortSeconds = new double[] { 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 }; + internal static readonly HashSet<(string, string)> DefaultHistogramBoundShortMappings = new() { ("Microsoft.AspNetCore.Hosting", "http.server.request.duration"), - ("Microsoft.AspNetCore.Http.Connections", "signalr.server.connection.duration"), ("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request.time_in_queue"), ("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request_lease.duration"), - ("Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration"), ("Microsoft.AspNetCore.Server.Kestrel", "kestrel.tls_handshake.duration"), ("OpenTelemetry.Instrumentation.AspNetCore", "http.server.request.duration"), ("OpenTelemetry.Instrumentation.Http", "http.client.request.duration"), - ("System.Net.Http", "http.client.connection.duration"), ("System.Net.Http", "http.client.request.duration"), ("System.Net.Http", "http.client.request.time_in_queue"), ("System.Net.NameResolution", "dns.lookups.duration"), }; + // Long default histogram bounds. Not based on a standard. May change in the future. + internal static readonly double[] DefaultHistogramBoundsLongSeconds = new double[] { 0, 0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300 }; + internal static readonly HashSet<(string, string)> DefaultHistogramBoundLongMappings = new() + { + ("Microsoft.AspNetCore.Http.Connections", "signalr.server.connection.duration"), + ("Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration"), + ("System.Net.Http", "http.client.connection.duration"), + }; + private readonly AggregatorStore aggStore; internal Metric( diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs index 9e51d2ed09b..a93d0156f0f 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTestsBase.cs @@ -239,47 +239,49 @@ public void MultiThreadedHistogramUpdateAndSnapShotTest() } [Theory] - [InlineData("Microsoft.AspNetCore.Hosting", "http.server.request.duration")] - [InlineData("Microsoft.AspNetCore.Http.Connections", "signalr.server.connection.duration")] - [InlineData("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request_lease.duration")] - [InlineData("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request.time_in_queue")] - [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration")] - [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.tls_handshake.duration")] - [InlineData("OpenTelemetry.Instrumentation.AspNetCore", "http.server.duration")] - [InlineData("OpenTelemetry.Instrumentation.Http", "http.client.duration")] - [InlineData("System.Net.Http", "http.client.connection.duration")] - [InlineData("System.Net.Http", "http.client.request.duration")] - [InlineData("System.Net.Http", "http.client.request.time_in_queue")] - [InlineData("System.Net.NameResolution", "dns.lookups.duration")] - [InlineData("General.App", "simple.alternative.counter")] - public void HistogramBucketsDefaultUpdatesForSecondsTest(string meterName, string instrumentName) + [InlineData("Microsoft.AspNetCore.Hosting", "http.server.request.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("Microsoft.AspNetCore.Hosting", "http.server.request.duration", "ms", KnownHistogramBuckets.Default)] + [InlineData("Microsoft.AspNetCore.Hosting", "http.server.request.duration", "By", KnownHistogramBuckets.Default)] + [InlineData("Microsoft.AspNetCore.Hosting", "http.server.request.duration", null, KnownHistogramBuckets.Default)] + [InlineData("Microsoft.AspNetCore.Http.Connections", "signalr.server.connection.duration", "s", KnownHistogramBuckets.DefaultLongSeconds)] + [InlineData("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request_lease.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("Microsoft.AspNetCore.RateLimiting", "aspnetcore.rate_limiting.request.time_in_queue", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.connection.duration", "s", KnownHistogramBuckets.DefaultLongSeconds)] + [InlineData("Microsoft.AspNetCore.Server.Kestrel", "kestrel.tls_handshake.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("OpenTelemetry.Instrumentation.AspNetCore", "http.server.duration", "ms", KnownHistogramBuckets.Default)] + [InlineData("OpenTelemetry.Instrumentation.Http", "http.client.duration", "ms", KnownHistogramBuckets.Default)] + [InlineData("System.Net.Http", "http.client.connection.duration", "s", KnownHistogramBuckets.DefaultLongSeconds)] + [InlineData("System.Net.Http", "http.client.request.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("System.Net.Http", "http.client.request.time_in_queue", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("System.Net.NameResolution", "dns.lookups.duration", "s", KnownHistogramBuckets.DefaultShortSeconds)] + [InlineData("General.App", "simple.alternative.counter", "s", KnownHistogramBuckets.Default)] + public void HistogramBucketsDefaultUpdatesForSecondsTest(string meterName, string instrumentName, string unit, KnownHistogramBuckets expectedHistogramBuckets) { - RunTest(meterName, instrumentName, unit: "s"); - RunTest(meterName, instrumentName, unit: "ms"); - RunTest(meterName, instrumentName, unit: "By"); - RunTest(meterName, instrumentName, unit: null); + using var meter = new Meter(meterName); - void RunTest(string meterName, string instrumentName, string unit) - { - using var meter = new Meter(meterName); - - var instrument = meter.CreateHistogram(instrumentName, unit); + var instrument = meter.CreateHistogram(instrumentName, unit); - var metricStreamIdentity = new MetricStreamIdentity(instrument, metricStreamConfiguration: null); + var metricStreamIdentity = new MetricStreamIdentity(instrument, metricStreamConfiguration: null); - AggregatorStore aggregatorStore = new( - metricStreamIdentity, - AggregationType.Histogram, - AggregationTemporality.Cumulative, - maxMetricPoints: 1024, - this.emitOverflowAttribute); + AggregatorStore aggregatorStore = new( + metricStreamIdentity, + AggregationType.Histogram, + AggregationTemporality.Cumulative, + maxMetricPoints: 1024, + this.emitOverflowAttribute); - Assert.NotNull(aggregatorStore.HistogramBounds); - Assert.Equal( - unit == "s" && Metric.DefaultHistogramBoundMappings.Contains((meterName, instrumentName)) ? - Metric.DefaultHistogramBoundsSeconds : Metric.DefaultHistogramBounds, - aggregatorStore.HistogramBounds); + KnownHistogramBuckets actualHistogramBounds = KnownHistogramBuckets.Default; + if (aggregatorStore.HistogramBounds == Metric.DefaultHistogramBoundsShortSeconds) + { + actualHistogramBounds = KnownHistogramBuckets.DefaultShortSeconds; } + else if (aggregatorStore.HistogramBounds == Metric.DefaultHistogramBoundsLongSeconds) + { + actualHistogramBounds = KnownHistogramBuckets.DefaultLongSeconds; + } + + Assert.NotNull(aggregatorStore.HistogramBounds); + Assert.Equal(expectedHistogramBuckets, actualHistogramBounds); } internal static void AssertExponentialBucketsAreCorrect(Base2ExponentialBucketHistogram expectedHistogram, ExponentialHistogramData data) diff --git a/test/OpenTelemetry.Tests/Metrics/KnownHistogramBuckets.cs b/test/OpenTelemetry.Tests/Metrics/KnownHistogramBuckets.cs new file mode 100644 index 00000000000..e64cc4bcc09 --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/KnownHistogramBuckets.cs @@ -0,0 +1,35 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +namespace OpenTelemetry.Metrics.Tests; + +public enum KnownHistogramBuckets +{ + /// + /// Default OpenTelemetry semantic convention buckets. + /// + Default, + + /// + /// Buckets for up to 10 seconds. + /// + DefaultShortSeconds, + + /// + /// Buckets for up to 300 seconds. + /// + DefaultLongSeconds, +}