Skip to content

Commit

Permalink
Throw on invalid histogram bounds (#2573)
Browse files Browse the repository at this point in the history
  • Loading branch information
mic-max authored Nov 9, 2021
1 parent 8029751 commit 05990ae
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Histogram bounds are validated when added to a View.
([#2573](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2573))

* Changed `BatchExportActivityProcessorOptions` constructor to throw
`FormatException` if it fails to parse any of the supported environment
variables.
Expand Down
6 changes: 6 additions & 0 deletions src/OpenTelemetry/Metrics/HistogramConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ public class HistogramConfiguration : MetricStreamConfiguration
{
private Aggregation aggregation = Aggregation.Histogram;

/// <summary>
/// Gets or sets the custom histogram bounds.
/// </summary>
/// <remarks>
/// The array must be in ascending order with distinct values.
/// </remarks>
public double[] BucketBounds { get; set; }

public override Aggregation Aggregation
Expand Down
22 changes: 22 additions & 0 deletions src/OpenTelemetry/Metrics/MeterProviderBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ public static MeterProviderBuilder AddView(this MeterProviderBuilder meterProvid
throw new ArgumentException($"Custom view name {metricStreamConfiguration.Name} is invalid.", nameof(metricStreamConfiguration.Name));
}

if (metricStreamConfiguration is HistogramConfiguration histogramConfiguration)
{
// Validate histogram bounds
if (histogramConfiguration.BucketBounds != null && !IsSortedAndDistinct(histogramConfiguration.BucketBounds))
{
throw new ArgumentException($"Histogram bounds must be in ascending order with distinct values", nameof(histogramConfiguration.BucketBounds));
}
}

if (meterProviderBuilder is MeterProviderBuilderBase meterProviderBuilderBase)
{
return meterProviderBuilderBase.AddView(instrumentName, metricStreamConfiguration);
Expand Down Expand Up @@ -148,5 +157,18 @@ public static MeterProvider Build(this MeterProviderBuilder meterProviderBuilder

return null;
}

private static bool IsSortedAndDistinct(double[] values)
{
for (int i = 1; i < values.Length; i++)
{
if (values[i] <= values[i - 1])
{
return false;
}
}

return true;
}
}
}
10 changes: 10 additions & 0 deletions test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ public void AddViewWithNullMetricStreamConfigurationThrowsArgumentnullException(
.Build());
}

[Theory]
[MemberData(nameof(MetricsTestData.InvalidHistogramBounds), MemberType = typeof(MetricsTestData))]
public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] bounds)
{
var ex = Assert.Throws<ArgumentException>(() => Sdk.CreateMeterProviderBuilder()
.AddView("name1", new HistogramConfiguration { BucketBounds = bounds }));

Assert.Contains("Histogram bounds must be in ascending order with distinct values", ex.Message);
}

[Theory]
[MemberData(nameof(MetricsTestData.ValidInstrumentNames), MemberType = typeof(MetricsTestData))]
public void ViewWithValidNameExported(string viewNewName)
Expand Down
9 changes: 9 additions & 0 deletions test/OpenTelemetry.Tests/Metrics/MetricsTestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,14 @@ public static IEnumerable<object[]> ValidInstrumentNames
new object[] { "my_metric2" },
new object[] { new string('m', 63) },
};

public static IEnumerable<object[]> InvalidHistogramBounds
=> new List<object[]>
{
new object[] { new double[] { 0, 0 } },
new object[] { new double[] { 1, 0 } },
new object[] { new double[] { 0, 1, 1, 2 } },
new object[] { new double[] { 0, 1, 2, -1 } },
};
}
}

0 comments on commit 05990ae

Please sign in to comment.