Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make exponential histogram negative buckets internal #4494

Merged
merged 4 commits into from
May 16, 2023
Merged

Make exponential histogram negative buckets internal #4494

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7194ec8
Mark negative buckets internal
alanwest May 16, 2023
4913e0f
Do not record negative values for exponential histograms
alanwest May 16, 2023
0e63693
OTLP exporter negative buckets
alanwest May 16, 2023
dcb4d88
Fix a couple more tests
alanwest May 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,6 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric)
dataPoint.Positive.BucketCounts.Add((ulong)bucketCount);
}

dataPoint.Negative = new OtlpMetrics.ExponentialHistogramDataPoint.Types.Buckets();
dataPoint.Negative.Offset = exponentialHistogramData.NegativeBuckets.Offset;
foreach (var bucketCount in exponentialHistogramData.NegativeBuckets)
{
dataPoint.Negative.BucketCounts.Add((ulong)bucketCount);
}

// TODO: exemplars.

histogram.DataPoints.Add(dataPoint);
Expand Down
1 change: 0 additions & 1 deletion src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.ExponentialHistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator
OpenTelemetry.Metrics.ExponentialHistogramBuckets.Offset.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData
OpenTelemetry.Metrics.ExponentialHistogramData.NegativeBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.PositiveBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.Scale.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData.ZeroCount.get -> long
Expand Down
1 change: 0 additions & 1 deletion src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.ExponentialHistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator
OpenTelemetry.Metrics.ExponentialHistogramBuckets.Offset.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData
OpenTelemetry.Metrics.ExponentialHistogramData.NegativeBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.PositiveBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.Scale.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData.ZeroCount.get -> long
Expand Down
1 change: 0 additions & 1 deletion src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.ExponentialHistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator
OpenTelemetry.Metrics.ExponentialHistogramBuckets.Offset.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData
OpenTelemetry.Metrics.ExponentialHistogramData.NegativeBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.PositiveBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.Scale.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData.ZeroCount.get -> long
Expand Down
1 change: 0 additions & 1 deletion src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.ExponentialHistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.ExponentialHistogramBuckets.Enumerator
OpenTelemetry.Metrics.ExponentialHistogramBuckets.Offset.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData
OpenTelemetry.Metrics.ExponentialHistogramData.NegativeBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.PositiveBuckets.get -> OpenTelemetry.Metrics.ExponentialHistogramBuckets!
OpenTelemetry.Metrics.ExponentialHistogramData.Scale.get -> int
OpenTelemetry.Metrics.ExponentialHistogramData.ZeroCount.get -> long
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Metrics/ExponentialHistogramData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal ExponentialHistogramData()

public ExponentialHistogramBuckets PositiveBuckets { get; private set; }

public ExponentialHistogramBuckets NegativeBuckets { get; private set; }
internal ExponentialHistogramBuckets NegativeBuckets { get; private set; }

internal ExponentialHistogramData Copy()
{
Expand Down
10 changes: 10 additions & 0 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,11 @@ private void UpdateHistogramWithBucketsAndMinMax(double number, ReadOnlySpan<Key
private void UpdateBase2ExponentialHistogram(double number, ReadOnlySpan<KeyValuePair<string, object>> tags = default, bool reportExemplar = false)
#pragma warning restore IDE0060 // Remove unused parameter
{
if (number < 0)
{
return;
}

var histogram = this.mpComponents!.Base2ExponentialBucketHistogram;

var sw = default(SpinWait);
Expand Down Expand Up @@ -1607,6 +1612,11 @@ private void UpdateBase2ExponentialHistogram(double number, ReadOnlySpan<KeyValu
private void UpdateBase2ExponentialHistogramWithMinMax(double number, ReadOnlySpan<KeyValuePair<string, object>> tags = default, bool reportExemplar = false)
#pragma warning restore IDE0060 // Remove unused parameter
{
if (number < 0)
{
return;
}

var histogram = this.mpComponents!.Base2ExponentialBucketHistogram;

var sw = default(SpinWait);
Expand Down
29 changes: 16 additions & 13 deletions test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,43 +544,46 @@ public void TestExponentialHistogramToOtlpMetric(string name, string description
Assert.True(dataPoint.TimeUnixNano > 0);

Assert.Equal(20, dataPoint.Scale);
Assert.Equal(2UL, dataPoint.Count);
Assert.Equal(1UL, dataPoint.ZeroCount);
if (longValue > 0 || doubleValue > 0)
{
Assert.Equal(2UL, dataPoint.Count);
}
else
{
Assert.Equal(1UL, dataPoint.Count);
}

if (longValue.HasValue)
{
// Known issue: Negative measurements affect the Sum. Per the spec, they should not.
Assert.Equal((double)longValue, dataPoint.Sum);
if (longValue > 0)
{
Assert.Equal((double)longValue, dataPoint.Sum);
Assert.Null(dataPoint.Negative);
Assert.True(dataPoint.Positive.Offset > 0);
Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]);
Assert.True(dataPoint.Negative.Offset == 0);
Assert.Empty(dataPoint.Negative.BucketCounts);
}
else
{
Assert.True(dataPoint.Negative.Offset > 0);
Assert.Equal(1UL, dataPoint.Negative.BucketCounts[0]);
Assert.Equal(0, dataPoint.Sum);
Assert.Null(dataPoint.Negative);
Assert.True(dataPoint.Positive.Offset == 0);
Assert.Empty(dataPoint.Positive.BucketCounts);
}
}
else
{
// Known issue: Negative measurements affect the Sum. Per the spec, they should not.
Assert.Equal(doubleValue, dataPoint.Sum);
if (doubleValue > 0)
{
Assert.Equal(doubleValue, dataPoint.Sum);
Assert.Null(dataPoint.Negative);
Assert.True(dataPoint.Positive.Offset > 0);
Assert.Equal(1UL, dataPoint.Positive.BucketCounts[0]);
Assert.True(dataPoint.Negative.Offset == 0);
Assert.Empty(dataPoint.Negative.BucketCounts);
}
else
{
Assert.True(dataPoint.Negative.Offset > 0);
Assert.Equal(1UL, dataPoint.Negative.BucketCounts[0]);
Assert.Equal(0, dataPoint.Sum);
Assert.Null(dataPoint.Negative);
Assert.True(dataPoint.Positive.Offset == 0);
Assert.Empty(dataPoint.Positive.BucketCounts);
}
Expand Down
18 changes: 11 additions & 7 deletions test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,11 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega
foreach (var value in valuesToRecord)
{
aggregatorStore.Update(value, Array.Empty<KeyValuePair<string, object>>());
expectedHistogram.Record(value);

if (value >= 0)
utpilla marked this conversation as resolved.
Show resolved Hide resolved
{
expectedHistogram.Record(value);
}
}

aggregatorStore.Snapshot();
Expand All @@ -311,13 +315,13 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega
var hasMinMax = metricPoint.TryGetHistogramMinMaxValues(out var min, out var max);

AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData());
Assert.Equal(40, sum);
Assert.Equal(7, count);
Assert.Equal(50, sum);
Assert.Equal(6, count);

if (aggregationType == AggregationType.Base2ExponentialHistogramWithMinMax)
{
Assert.True(hasMinMax);
Assert.Equal(-10, min);
Assert.Equal(0, min);
Assert.Equal(19, max);
}
else
Expand All @@ -334,13 +338,13 @@ internal void ExponentialHistogramTests(AggregationType aggregationType, Aggrega
if (aggregationTemporality == AggregationTemporality.Cumulative)
{
AssertExponentialBucketsAreCorrect(expectedHistogram, metricPoint.GetExponentialHistogramData());
Assert.Equal(40, sum);
Assert.Equal(7, count);
Assert.Equal(50, sum);
Assert.Equal(6, count);

if (aggregationType == AggregationType.Base2ExponentialHistogramWithMinMax)
{
Assert.True(hasMinMax);
Assert.Equal(-10, min);
Assert.Equal(0, min);
Assert.Equal(19, max);
}
else
Expand Down