Skip to content

Commit

Permalink
Add public method GetHistogramBuckets
Browse files Browse the repository at this point in the history
  • Loading branch information
utpilla committed Nov 23, 2021
1 parent 9ba6bb7 commit e41e84e
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public override ExportResult Export(in Batch<Metric> batch)

bool isFirstIteration = true;
double previousExplicitBound = default;
foreach (var histogramMeasurement in metricPoint.HistogramBuckets)
foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets())
{
if (isFirstIteration)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric)
dataPoint.Count = (ulong)metricPoint.LongValue;
dataPoint.Sum = metricPoint.DoubleValue;

foreach (var histogramMeasurement in metricPoint.HistogramBuckets)
foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets())
{
dataPoint.BucketCounts.Add((ulong)histogramMeasurement.BucketCount);
if (histogramMeasurement.ExplicitBound != double.PositiveInfinity)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric)
var timestamp = metricPoint.EndTime.ToUnixTimeMilliseconds();

long totalCount = 0;
foreach (var histogramMeasurement in metricPoint.HistogramBuckets)
foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets())
{
totalCount += histogramMeasurement.BucketCount;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Current.get -> OpenTelemetry.M
OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Enumerator() -> void
OpenTelemetry.Metrics.HistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.HistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.HistogramBuckets.Enumerator
OpenTelemetry.Metrics.MetricPoint.HistogramBuckets.get -> OpenTelemetry.Metrics.HistogramBuckets
OpenTelemetry.Metrics.MetricPoint.GetHistogramBuckets() -> OpenTelemetry.Metrics.HistogramBuckets
OpenTelemetry.Metrics.MetricPointsAccessor
OpenTelemetry.Metrics.MetricPointsAccessor.MetricPointsAccessor() -> void
OpenTelemetry.Metrics.MetricPointsAccessor.Dispose() -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Current.get -> OpenTelemetry.M
OpenTelemetry.Metrics.HistogramBuckets.Enumerator.Enumerator() -> void
OpenTelemetry.Metrics.HistogramBuckets.Enumerator.MoveNext() -> bool
OpenTelemetry.Metrics.HistogramBuckets.GetEnumerator() -> OpenTelemetry.Metrics.HistogramBuckets.Enumerator
OpenTelemetry.Metrics.MetricPoint.HistogramBuckets.get -> OpenTelemetry.Metrics.HistogramBuckets
OpenTelemetry.Metrics.MetricPoint.GetHistogramBuckets() -> OpenTelemetry.Metrics.HistogramBuckets
OpenTelemetry.Metrics.MetricPointsAccessor
OpenTelemetry.Metrics.MetricPointsAccessor.MetricPointsAccessor() -> void
OpenTelemetry.Metrics.MetricPointsAccessor.Dispose() -> void
Expand Down
8 changes: 5 additions & 3 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
aggregation is not implemented yet.
([#2660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2660))

* Renamed `HistogramMeasurements` to `HistogramBuckets` and added an enumerator
of type `HistogramBucket` for enumerating `BucketCounts` and `ExplicitBounds`.
Removed `GetBucketCounts` and `GetExplicitBounds` methods from `MetricPoint`.
* Removed the public property `HistogramMeasurements` and added a public method
`GetHistogramBuckets` instead. Renamed the class `HistogramMeasurements` to
`HistogramBuckets` and added an enumerator of type `HistogramBucket` for
enumerating `BucketCounts` and `ExplicitBounds`. Removed `GetBucketCounts` and
`GetExplicitBounds` methods from `MetricPoint`.
([#2664](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2664))

## 1.2.0-beta2
Expand Down
69 changes: 40 additions & 29 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace OpenTelemetry.Metrics
public struct MetricPoint
{
private readonly AggregationType aggType;
private readonly HistogramBuckets histogramBuckets;
private long longVal;
private long lastLongSum;
private double doubleVal;
Expand Down Expand Up @@ -51,15 +52,15 @@ internal MetricPoint(

if (this.aggType == AggregationType.Histogram)
{
this.HistogramBuckets = new HistogramBuckets(histogramBounds);
this.histogramBuckets = new HistogramBuckets(histogramBounds);
}
else if (this.aggType == AggregationType.HistogramSumCount)
{
this.HistogramBuckets = new HistogramBuckets(null);
this.histogramBuckets = new HistogramBuckets(null);
}
else
{
this.HistogramBuckets = null;
this.histogramBuckets = null;
}
}

Expand All @@ -76,15 +77,13 @@ internal MetricPoint(

public double DoubleValue { get; internal set; }

public readonly HistogramBuckets HistogramBuckets { get; }

internal MetricPointStatus MetricPointStatus { get; private set; }

public long GetHistogramCount()
{
if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount)
{
return this.HistogramBuckets.Count;
return this.histogramBuckets.Count;
}
else
{
Expand All @@ -96,14 +95,26 @@ public double GetHistogramSum()
{
if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount)
{
return this.HistogramBuckets.Sum;
return this.histogramBuckets.Sum;
}
else
{
throw new NotSupportedException($"{nameof(this.GetHistogramSum)} is not supported for this metric type.");
}
}

public HistogramBuckets GetHistogramBuckets()
{
if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount)
{
return this.histogramBuckets;
}
else
{
throw new NotSupportedException($"{nameof(this.GetHistogramBuckets)} is not supported for this metric type.");
}
}

internal void Update(long number)
{
switch (this.aggType)
Expand Down Expand Up @@ -179,31 +190,31 @@ internal void Update(double number)
case AggregationType.Histogram:
{
int i;
for (i = 0; i < this.HistogramBuckets.ExplicitBounds.Length; i++)
for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++)
{
// Upper bound is inclusive
if (number <= this.HistogramBuckets.ExplicitBounds[i])
if (number <= this.histogramBuckets.ExplicitBounds[i])
{
break;
}
}

lock (this.HistogramBuckets.LockObject)
lock (this.histogramBuckets.LockObject)
{
this.HistogramBuckets.CountVal++;
this.HistogramBuckets.SumVal += number;
this.HistogramBuckets.BucketCounts[i]++;
this.histogramBuckets.CountVal++;
this.histogramBuckets.SumVal += number;
this.histogramBuckets.BucketCounts[i]++;
}

break;
}

case AggregationType.HistogramSumCount:
{
lock (this.HistogramBuckets.LockObject)
lock (this.histogramBuckets.LockObject)
{
this.HistogramBuckets.CountVal++;
this.HistogramBuckets.SumVal += number;
this.histogramBuckets.CountVal++;
this.histogramBuckets.SumVal += number;
}

break;
Expand Down Expand Up @@ -325,22 +336,22 @@ internal void TakeSnapshot(bool outputDelta)

case AggregationType.Histogram:
{
lock (this.HistogramBuckets.LockObject)
lock (this.histogramBuckets.LockObject)
{
this.HistogramBuckets.Count = this.HistogramBuckets.CountVal;
this.HistogramBuckets.Sum = this.HistogramBuckets.SumVal;
this.histogramBuckets.Count = this.histogramBuckets.CountVal;
this.histogramBuckets.Sum = this.histogramBuckets.SumVal;
if (outputDelta)
{
this.HistogramBuckets.CountVal = 0;
this.HistogramBuckets.SumVal = 0;
this.histogramBuckets.CountVal = 0;
this.histogramBuckets.SumVal = 0;
}

for (int i = 0; i < this.HistogramBuckets.BucketCounts.Length; i++)
for (int i = 0; i < this.histogramBuckets.BucketCounts.Length; i++)
{
this.HistogramBuckets.AggregatedBucketCounts[i] = this.HistogramBuckets.BucketCounts[i];
this.histogramBuckets.AggregatedBucketCounts[i] = this.histogramBuckets.BucketCounts[i];
if (outputDelta)
{
this.HistogramBuckets.BucketCounts[i] = 0;
this.histogramBuckets.BucketCounts[i] = 0;
}
}

Expand All @@ -352,14 +363,14 @@ internal void TakeSnapshot(bool outputDelta)

case AggregationType.HistogramSumCount:
{
lock (this.HistogramBuckets.LockObject)
lock (this.histogramBuckets.LockObject)
{
this.HistogramBuckets.Count = this.HistogramBuckets.CountVal;
this.HistogramBuckets.Sum = this.HistogramBuckets.SumVal;
this.histogramBuckets.Count = this.histogramBuckets.CountVal;
this.histogramBuckets.Sum = this.histogramBuckets.SumVal;
if (outputDelta)
{
this.HistogramBuckets.CountVal = 0;
this.HistogramBuckets.SumVal = 0;
this.histogramBuckets.CountVal = 0;
this.histogramBuckets.SumVal = 0;
}

this.MetricPointStatus = MetricPointStatus.NoCollectPending;
Expand Down
6 changes: 3 additions & 3 deletions test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void HistogramDistributeToAllBucketsDefault()
Assert.Equal(22, count);

int actualCount = 0;
foreach (var histogramMeasurement in histogramPoint.HistogramBuckets)
foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
{
Assert.Equal(2, histogramMeasurement.BucketCount);
actualCount++;
Expand Down Expand Up @@ -92,7 +92,7 @@ public void HistogramDistributeToAllBucketsCustom()
int index = 0;
int actualCount = 0;
var expectedBucketCounts = new long[] { 5, 2, 0 };
foreach (var histogramMeasurement in histogramPoint.HistogramBuckets)
foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
{
Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount);
index++;
Expand Down Expand Up @@ -128,7 +128,7 @@ public void HistogramWithOnlySumCount()
Assert.Equal(7, count);

// There should be no enumeration of BucketCounts and ExplicitBounds for HistogramSumCount
var enumerator = histogramPoint.HistogramBuckets.GetEnumerator();
var enumerator = histogramPoint.GetHistogramBuckets().GetEnumerator();
Assert.False(enumerator.MoveNext());
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public void ViewToProduceCustomHistogramBound()
int index = 0;
int actualCount = 0;
var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 0 };
foreach (var histogramMeasurement in histogramPoint.HistogramBuckets)
foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
{
Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount);
index++;
Expand All @@ -424,7 +424,7 @@ public void ViewToProduceCustomHistogramBound()
index = 0;
actualCount = 0;
expectedBucketCounts = new long[] { 5, 2, 0 };
foreach (var histogramMeasurement in histogramPoint.HistogramBuckets)
foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
{
Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount);
index++;
Expand Down

0 comments on commit e41e84e

Please sign in to comment.