Skip to content

Commit

Permalink
Fix SimpleExemplarReservoir for cumulative (#5230)
Browse files Browse the repository at this point in the history
Co-authored-by: Alan West <[email protected]>
  • Loading branch information
cijothomas and alanwest authored Jan 18, 2024
1 parent 89e5382 commit 45cf7c3
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
state compared to the current activity. For details see:
[#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136)

* Fixed an issue where `SimpleExemplarReservoir` was not resetting internal
state for cumulative temporality.
[#5230](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5230)

## 1.7.0

Released 2023-Dec-08
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,14 @@ public override Exemplar[] Collect(ReadOnlyTagCollection actualTags, bool reset)
if (reset)
{
this.runningExemplars[i].Timestamp = default;
this.measurementsSeen = 0;
}
}

// Reset internal state irrespective of temporality.
// This ensures incoming measurements have fair chance
// of making it to the reservoir.
this.measurementsSeen = 0;

return this.tempExemplars;
}

Expand Down
14 changes: 11 additions & 3 deletions test/OpenTelemetry.Tests/Metrics/MetricExemplarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ public MetricExemplarTests(ITestOutputHelper output)
this.output = output;
}

[Fact]
public void TestExemplarsCounter()
[Theory]
[InlineData(MetricReaderTemporalityPreference.Cumulative)]
[InlineData(MetricReaderTemporalityPreference.Delta)]
public void TestExemplarsCounter(MetricReaderTemporalityPreference temporality)
{
DateTime testStartTime = DateTime.UtcNow;
var exportedItems = new List<Metric>();
Expand All @@ -33,7 +35,7 @@ public void TestExemplarsCounter()
.SetExemplarFilter(new AlwaysOnExemplarFilter())
.AddInMemoryExporter(exportedItems, metricReaderOptions =>
{
metricReaderOptions.TemporalityPreference = MetricReaderTemporalityPreference.Delta;
metricReaderOptions.TemporalityPreference = temporality;
}));

var measurementValues = GenerateRandomValues(10);
Expand All @@ -48,6 +50,12 @@ public void TestExemplarsCounter()
Assert.True(metricPoint.Value.StartTime >= testStartTime);
Assert.True(metricPoint.Value.EndTime != default);
var exemplars = GetExemplars(metricPoint.Value);

// TODO: Modify the test to better test cumulative.
// In cumulative where SimpleExemplarReservoir's size is
// more than the count of new measurements, it is possible
// that the exemplar value is for a measurement that was recorded in the prior
// cycle. The current ValidateExemplars() does not handle this case.
ValidateExemplars(exemplars, metricPoint.Value.StartTime, metricPoint.Value.EndTime, measurementValues, false);

exportedItems.Clear();
Expand Down

0 comments on commit 45cf7c3

Please sign in to comment.