Skip to content

[Metrics] Implement cumulative aggregation for async Long and Double counters in MetricPoint #7227

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

Open
nabutabu wants to merge 22 commits into
base: main
Choose a base branch
from
Open

[Metrics] Implement cumulative aggregation for async Long and Double counters in MetricPoint #7227

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
76d6a88
[Metrics] Implement cumulative aggregation for async Long and Double …
nabutabu Apr 30, 2026
16be094
Revert "[Metrics] Implement cumulative aggregation for async Long and…
nabutabu Apr 30, 2026
0a1526f
[Metrics] Correct asynchronous cumulative metric point handling and u…
nabutabu Apr 30, 2026
6c0d378
[Metrics] Simplify logic and hold metric point instead of indexing twice
nabutabu Apr 30, 2026
d48d885
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Apr 30, 2026
b3bc76d
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu May 6, 2026
a398c3b
[Tests] Add spatial aggregation tests for ObservableCounter and Synch…
nabutabu May 8, 2026
e68e457
[Tests] Add spatial aggregation tests for ObservableGauge and Exempla…
nabutabu May 11, 2026
358c5bb
[Metrics] Enhance UpdateWithExemplar to include exemplar value for lo…
nabutabu May 11, 2026
726a498
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu May 11, 2026
0817a6b
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu May 14, 2026
a265e27
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu May 19, 2026
84409b2
[AggregatorStore] Ensure index check for asynchronous aggregation types
nabutabu May 28, 2026
4311da4
lint issues fixed
nabutabu May 28, 2026
4820002
[MetricApiTests] Skip failing tests related to absent stream handling…
nabutabu Jun 1, 2026
8283f11
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 1, 2026
08d77cf
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 9, 2026
e47bcf5
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 9, 2026
e79bf8d
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 11, 2026
2d8f7c5
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 16, 2026
0dcc90d
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jun 24, 2026
371171b
Merge branch 'main' into otel-7215-metricpointaggregation
nabutabu Jul 3, 2026
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
36 changes: 30 additions & 6 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,18 +931,28 @@ private void UpdateLong(long value, ReadOnlySpan<KeyValuePair<string, object?>>
{
var index = this.FindMetricAggregatorsDefault(tags);

this.UpdateLongMetricPoint(index, value, tags);
this.UpdateLongMetricPoint(index, value, value, tags);
}

private void UpdateLongCustomTags(long value, ReadOnlySpan<KeyValuePair<string, object?>> tags)
{
var index = this.FindMetricAggregatorsCustomTag(tags);
var exemplarValue = value;

this.UpdateLongMetricPoint(index, value, tags);
if (this.IsAsynchronous && this.aggType == AggregationType.LongSumIncomingCumulative)
{
ref var metricPoint = ref this.metricPoints[index];
if (metricPoint.MetricPointStatus == MetricPointStatus.CollectPending)
{
value += metricPoint.GetRunningValueLong();

@cijothomas cijothomas May 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of modifying the incoming value to achieve "spatial aggregation" is good - but it does create incorrect Exemplar values.

It is not that bad - as Exemplars have limited utility in Observable instruments anyway and Exemplars are disabled also by-default for Observable. We should see if the fix can keep Exemplars correct as well. (Exemplar should get the original unmodified value, but Metricpoint should get the updated one..)

@nabutabu nabutabu May 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read more about Exemplars and this does complicate this issue a bit. Blurting out my understanding of what's happening for future me:

Exemplars are defined as: "access to the raw measurement value, time stamp when measurement was made, and trace context", which means each exemplar recording should be the raw reading instead of the cumulative value that the (non-exemplar) async cumulative instrument uses.

This is important because each exemplar metric recording is also associated with a trace. If exemplars stored aggregated values, we'd lose the connection to individual traces for each recording. The exemplar's purpose is to preserve the context of specific measurements that contribute to the aggregate, which this current change loses.

I'll come back when I have more on how we could fix this.

  • First thought is to add a parameter to UpdateWithExemplar in MetricPoint -> "number" would be the cumulative value and the new parameter is the measured value.
  • MetricPoint does already have access to its own running value, maybe we could just subtract that when calling UpdateExemplar, this would need to happen before Interlocked.Add is called on the running value. Also would this work for every kind of instrument?

}
}
Comment thread
nabutabu marked this conversation as resolved.
Outdated

this.UpdateLongMetricPoint(index, value, exemplarValue, tags);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void UpdateLongMetricPoint(int metricPointIndex, long value, ReadOnlySpan<KeyValuePair<string, object?>> tags)
private void UpdateLongMetricPoint(int metricPointIndex, long value, long exemplarValue, ReadOnlySpan<KeyValuePair<string, object?>> tags)
{
if (metricPointIndex < 0)
{
Expand All @@ -962,13 +972,15 @@ private void UpdateLongMetricPoint(int metricPointIndex, long value, ReadOnlySpa
{
this.metricPoints[metricPointIndex].UpdateWithExemplar(
value,
exemplarValue,
tags,
offerExemplar: true);
}
else
{
this.metricPoints[metricPointIndex].UpdateWithExemplar(
value,
exemplarValue,
tags,
offerExemplar: Activity.Current?.Recorded ?? false);
}
Expand All @@ -978,18 +990,28 @@ private void UpdateDouble(double value, ReadOnlySpan<KeyValuePair<string, object
{
var index = this.FindMetricAggregatorsDefault(tags);

this.UpdateDoubleMetricPoint(index, value, tags);
this.UpdateDoubleMetricPoint(index, value, value, tags);
}

private void UpdateDoubleCustomTags(double value, ReadOnlySpan<KeyValuePair<string, object?>> tags)
{
var index = this.FindMetricAggregatorsCustomTag(tags);
var exemplarValue = value;

if (this.IsAsynchronous && this.aggType == AggregationType.DoubleSumIncomingCumulative)
{
ref var metricPoint = ref this.metricPoints[index];
if (metricPoint.MetricPointStatus == MetricPointStatus.CollectPending)
{
value += metricPoint.GetRunningValueDouble();
}
}
Comment thread
nabutabu marked this conversation as resolved.
Outdated

this.UpdateDoubleMetricPoint(index, value, tags);
this.UpdateDoubleMetricPoint(index, value, exemplarValue, tags);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void UpdateDoubleMetricPoint(int metricPointIndex, double value, ReadOnlySpan<KeyValuePair<string, object?>> tags)
private void UpdateDoubleMetricPoint(int metricPointIndex, double value, double exemplarValue, ReadOnlySpan<KeyValuePair<string, object?>> tags)
{
if (metricPointIndex < 0)
{
Expand All @@ -1009,13 +1031,15 @@ private void UpdateDoubleMetricPoint(int metricPointIndex, double value, ReadOnl
{
this.metricPoints[metricPointIndex].UpdateWithExemplar(
value,
exemplarValue,
tags,
offerExemplar: true);
}
else
{
this.metricPoints[metricPointIndex].UpdateWithExemplar(
value,
exemplarValue,
tags,
offerExemplar: Activity.Current?.Recorded ?? false);
}
Expand Down
12 changes: 8 additions & 4 deletions src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs
View file
Open in desktop
Comment thread
martincostello marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ internal void Update(long number)
this.CompleteUpdate();
}

internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool offerExemplar)
internal void UpdateWithExemplar(long number, long exemplarValue, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool offerExemplar)
{
switch (this.aggType)
{
Expand Down Expand Up @@ -489,7 +489,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string,
break;
}

this.UpdateExemplar(number, tags, offerExemplar);
this.UpdateExemplar(exemplarValue, tags, offerExemplar);

this.CompleteUpdate();
}
Expand Down Expand Up @@ -558,7 +558,7 @@ internal void Update(double number)
this.CompleteUpdate();
}

internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool offerExemplar)
internal void UpdateWithExemplar(double number, double exemplarValue, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool offerExemplar)
{
switch (this.aggType)
{
Expand Down Expand Up @@ -619,7 +619,7 @@ internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string
break;
}

this.UpdateExemplar(number, tags, offerExemplar);
this.UpdateExemplar(exemplarValue, tags, offerExemplar);

this.CompleteUpdate();
}
Expand Down Expand Up @@ -893,6 +893,10 @@ internal void NullifyMetricPointState()
this.mpComponents = null;
}

internal long GetRunningValueLong() => Interlocked.Read(ref this.runningValue.AsLong);

internal double GetRunningValueDouble() => InterlockedHelper.Read(ref this.runningValue.AsDouble);

private void UpdateHistogram(double number, ReadOnlySpan<KeyValuePair<string, object?>> tags = default, bool offerExemplar = false)
{
var histogramBuckets = this.mpComponents!.HistogramBuckets!;
Expand Down
Loading
Loading