Skip to content

Commit

Permalink
Remove zero bucket for the histogram buckets for ASP.NET Core and Htt…
Browse files Browse the repository at this point in the history
…pClient metrics (#5021)
  • Loading branch information
utpilla authored Nov 3, 2023
1 parent d91be77 commit 299a6c1
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 10 deletions.
16 changes: 14 additions & 2 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,21 @@
([#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.
* Update Metrics SDK to override the default histogram buckets for the following
metrics from ASP.NET Core and HttpClient runtime:
* `signalr.server.connection.duration`
* `kestrel.connection.duration`
* `http.client.connection.duration`

These histogram metrics which have their `Unit` as `s` (second) will have
their default histogram buckets as `[ 0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2,
5, 10, 30, 60, 120, 300 ]`.
([#5008](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5008))
([#5021](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5021))

* Remove the bucket with value `0` for histogram buckets for all metrics from
ASP.NET Core and HttpClient.
([#5021](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5021))

## 1.7.0-alpha.1

Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Metrics/Metric.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public sealed class Metric
internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000 };

// 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 double[] DefaultHistogramBoundsShortSeconds = new double[] { 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"),
Expand All @@ -45,7 +45,7 @@ public sealed class Metric
};

// 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 double[] DefaultHistogramBoundsLongSeconds = new double[] { 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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,15 @@ private static KeyValuePair<string, object>[] AssertMetricPoint_New(
histogramBounds.Add(t.ExplicitBound);
}

Assert.Equal(
expected: new List<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, double.PositiveInfinity },
actual: histogramBounds);
// TODO: Remove the check for the older bounds once 1.7.0 is released. This is a temporary fix for instrumentation libraries CI workflow.

var expectedHistogramBoundsOld = new List<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, double.PositiveInfinity };
var expectedHistogramBoundsNew = new List<double> { 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, double.PositiveInfinity };

var histogramBoundsMatchCorrectly = Enumerable.SequenceEqual(expectedHistogramBoundsOld, histogramBounds) ||
Enumerable.SequenceEqual(expectedHistogramBoundsNew, histogramBounds);

Assert.True(histogramBoundsMatchCorrectly);

return attributes;
}
Expand Down
12 changes: 9 additions & 3 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -545,9 +545,15 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync(
histogramBounds.Add(t.ExplicitBound);
}

Assert.Equal(
expected: new List<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, double.PositiveInfinity },
actual: histogramBounds);
// TODO: Remove the check for the older bounds once 1.7.0 is released. This is a temporary fix for instrumentation libraries CI workflow.

var expectedHistogramBoundsOld = new List<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, double.PositiveInfinity };
var expectedHistogramBoundsNew = new List<double> { 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, double.PositiveInfinity };

var histogramBoundsMatchCorrectly = Enumerable.SequenceEqual(expectedHistogramBoundsOld, histogramBounds) ||
Enumerable.SequenceEqual(expectedHistogramBoundsNew, histogramBounds);

Assert.True(histogramBoundsMatchCorrectly);
}
}
}
Expand Down

0 comments on commit 299a6c1

Please sign in to comment.