Skip to content

Commit 7ce0c55

Browse files
authored
[sdk-metrics] Refactor duplicated update completion code into a helper in MetricPoint (#5398)
1 parent 05b0ca4 commit 7ce0c55

File tree

1 file changed

+26
-68
lines changed

1 file changed

+26
-68
lines changed

src/OpenTelemetry/Metrics/MetricPoint.cs

Lines changed: 26 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -445,23 +445,7 @@ internal void Update(long number)
445445
}
446446
}
447447

448-
// There is a race with Snapshot:
449-
// Update() updates the value
450-
// Snapshot snapshots the value
451-
// Snapshot sets status to NoCollectPending
452-
// Update sets status to CollectPending -- this is not right as the Snapshot
453-
// already included the updated value.
454-
// In the absence of any new Update call until next Snapshot,
455-
// this results in exporting an Update even though
456-
// it had no update.
457-
// TODO: For Delta, this can be mitigated
458-
// by ignoring Zero points
459-
this.MetricPointStatus = MetricPointStatus.CollectPending;
460-
461-
if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
462-
{
463-
Interlocked.Decrement(ref this.ReferenceCount);
464-
}
448+
this.CompleteUpdate();
465449
}
466450

467451
internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool isSampled)
@@ -573,23 +557,7 @@ internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string,
573557
}
574558
}
575559

576-
// There is a race with Snapshot:
577-
// Update() updates the value
578-
// Snapshot snapshots the value
579-
// Snapshot sets status to NoCollectPending
580-
// Update sets status to CollectPending -- this is not right as the Snapshot
581-
// already included the updated value.
582-
// In the absence of any new Update call until next Snapshot,
583-
// this results in exporting an Update even though
584-
// it had no update.
585-
// TODO: For Delta, this can be mitigated
586-
// by ignoring Zero points
587-
this.MetricPointStatus = MetricPointStatus.CollectPending;
588-
589-
if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
590-
{
591-
Interlocked.Decrement(ref this.ReferenceCount);
592-
}
560+
this.CompleteUpdate();
593561
}
594562

595563
internal void Update(double number)
@@ -651,23 +619,7 @@ internal void Update(double number)
651619
}
652620
}
653621

654-
// There is a race with Snapshot:
655-
// Update() updates the value
656-
// Snapshot snapshots the value
657-
// Snapshot sets status to NoCollectPending
658-
// Update sets status to CollectPending -- this is not right as the Snapshot
659-
// already included the updated value.
660-
// In the absence of any new Update call until next Snapshot,
661-
// this results in exporting an Update even though
662-
// it had no update.
663-
// TODO: For Delta, this can be mitigated
664-
// by ignoring Zero points
665-
this.MetricPointStatus = MetricPointStatus.CollectPending;
666-
667-
if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
668-
{
669-
Interlocked.Decrement(ref this.ReferenceCount);
670-
}
622+
this.CompleteUpdate();
671623
}
672624

673625
internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool isSampled)
@@ -785,23 +737,7 @@ internal void UpdateWithExemplar(double number, ReadOnlySpan<KeyValuePair<string
785737
}
786738
}
787739

788-
// There is a race with Snapshot:
789-
// Update() updates the value
790-
// Snapshot snapshots the value
791-
// Snapshot sets status to NoCollectPending
792-
// Update sets status to CollectPending -- this is not right as the Snapshot
793-
// already included the updated value.
794-
// In the absence of any new Update call until next Snapshot,
795-
// this results in exporting an Update even though
796-
// it had no update.
797-
// TODO: For Delta, this can be mitigated
798-
// by ignoring Zero points
799-
this.MetricPointStatus = MetricPointStatus.CollectPending;
800-
801-
if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
802-
{
803-
Interlocked.Decrement(ref this.ReferenceCount);
804-
}
740+
this.CompleteUpdate();
805741
}
806742

807743
internal void TakeSnapshot(bool outputDelta)
@@ -1495,6 +1431,28 @@ private void UpdateBase2ExponentialHistogramWithMinMax(double number, ReadOnlySp
14951431
this.mpComponents.ReleaseLock();
14961432
}
14971433

1434+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
1435+
private void CompleteUpdate()
1436+
{
1437+
// There is a race with Snapshot:
1438+
// Update() updates the value
1439+
// Snapshot snapshots the value
1440+
// Snapshot sets status to NoCollectPending
1441+
// Update sets status to CollectPending -- this is not right as the Snapshot
1442+
// already included the updated value.
1443+
// In the absence of any new Update call until next Snapshot,
1444+
// this results in exporting an Update even though
1445+
// it had no update.
1446+
// TODO: For Delta, this can be mitigated
1447+
// by ignoring Zero points
1448+
this.MetricPointStatus = MetricPointStatus.CollectPending;
1449+
1450+
if (this.aggregatorStore.OutputDeltaWithUnusedMetricPointReclaimEnabled)
1451+
{
1452+
Interlocked.Decrement(ref this.ReferenceCount);
1453+
}
1454+
}
1455+
14981456
[MethodImpl(MethodImplOptions.NoInlining)]
14991457
private readonly void ThrowNotSupportedMetricTypeException(string methodName)
15001458
{

0 commit comments

Comments
 (0)