Skip to content

Commit 42ecd73

Browse files
CodeBlanchCodeBlanch
authored
[sdk-metrics] ReadOnlyExemplarCollection tweaks (#5403)
1 parent 77ef123 commit 42ecd73

File tree

6 files changed

+55
-41
lines changed

6 files changed

+55
-41
lines changed

‎src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ OpenTelemetry.Metrics.ExemplarMeasurement<T>
2323
OpenTelemetry.Metrics.ExemplarMeasurement<T>.ExemplarMeasurement() -> void
2424
OpenTelemetry.Metrics.ExemplarMeasurement<T>.Tags.get -> System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>>
2525
OpenTelemetry.Metrics.ExemplarMeasurement<T>.Value.get -> T
26-
OpenTelemetry.Metrics.MetricPoint.TryGetExemplars(out OpenTelemetry.Metrics.ReadOnlyExemplarCollection? exemplars) -> bool
26+
OpenTelemetry.Metrics.MetricPoint.TryGetExemplars(out OpenTelemetry.Metrics.ReadOnlyExemplarCollection exemplars) -> bool
2727
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
2828
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
2929
OpenTelemetry.Metrics.ReadOnlyExemplarCollection

‎src/OpenTelemetry/Metrics/Exemplar/ReadOnlyExemplarCollection.cs‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace OpenTelemetry.Metrics;
2323
#endif
2424
readonly struct ReadOnlyExemplarCollection
2525
{
26+
internal static readonly ReadOnlyExemplarCollection Empty = new(Array.Empty<Exemplar>());
2627
private readonly Exemplar[] exemplars;
2728

2829
internal ReadOnlyExemplarCollection(Exemplar[] exemplars)
@@ -50,24 +51,37 @@ public Enumerator GetEnumerator()
5051

5152
internal ReadOnlyExemplarCollection Copy()
5253
{
53-
var exemplarCopies = new Exemplar[this.exemplars.Length];
54+
var maximumCount = this.MaximumCount;
5455

55-
int i = 0;
56-
foreach (ref readonly var exemplar in this)
56+
if (maximumCount > 0)
5757
{
58-
exemplar.Copy(ref exemplarCopies[i++]);
58+
var exemplarCopies = new Exemplar[maximumCount];
59+
60+
int i = 0;
61+
foreach (ref readonly var exemplar in this)
62+
{
63+
if (exemplar.IsUpdated())
64+
{
65+
exemplar.Copy(ref exemplarCopies[i++]);
66+
}
67+
}
68+
69+
return new ReadOnlyExemplarCollection(exemplarCopies);
5970
}
6071

61-
return new ReadOnlyExemplarCollection(exemplarCopies);
72+
return Empty;
6273
}
6374

6475
internal IReadOnlyList<Exemplar> ToReadOnlyList()
6576
{
6677
var list = new List<Exemplar>(this.MaximumCount);
6778

68-
foreach (var item in this)
79+
foreach (var exemplar in this)
6980
{
70-
list.Add(item);
81+
// Note: If ToReadOnlyList is ever made public it should make sure
82+
// to take copies of exemplars or make sure the instance was first
83+
// copied using the Copy method above.
84+
list.Add(exemplar);
7185
}
7286

7387
return list;

‎src/OpenTelemetry/Metrics/MetricPoint.cs‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System.Diagnostics;
5-
using System.Diagnostics.CodeAnalysis;
65
using System.Runtime.CompilerServices;
76
using OpenTelemetry.Internal;
87

@@ -373,10 +372,10 @@ public readonly bool TryGetHistogramMinMaxValues(out double min, out double max)
373372
[MethodImpl(MethodImplOptions.AggressiveInlining)]
374373
internal
375374
#endif
376-
readonly bool TryGetExemplars([NotNullWhen(true)] out ReadOnlyExemplarCollection? exemplars)
375+
readonly bool TryGetExemplars(out ReadOnlyExemplarCollection exemplars)
377376
{
378-
exemplars = this.mpComponents?.Exemplars;
379-
return exemplars.HasValue;
377+
exemplars = this.mpComponents?.Exemplars ?? ReadOnlyExemplarCollection.Empty;
378+
return exemplars.MaximumCount > 0;
380379
}
381380

382381
internal readonly MetricPoint Copy()
@@ -945,13 +944,14 @@ internal void TakeSnapshot(bool outputDelta)
945944
internal void TakeSnapshotWithExemplar(bool outputDelta)
946945
{
947946
Debug.Assert(this.mpComponents != null, "this.mpComponents was null");
947+
Debug.Assert(this.mpComponents!.ExemplarReservoir != null, "this.mpComponents.ExemplarReservoir was null");
948948

949949
switch (this.aggType)
950950
{
951951
case AggregationType.LongSumIncomingDelta:
952952
case AggregationType.LongSumIncomingCumulative:
953953
{
954-
this.mpComponents!.AcquireLock();
954+
this.mpComponents.AcquireLock();
955955

956956
if (outputDelta)
957957
{
@@ -965,7 +965,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
965965
this.snapshotValue.AsLong = this.runningValue.AsLong;
966966
}
967967

968-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
968+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
969969

970970
this.mpComponents.ReleaseLock();
971971

@@ -989,7 +989,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
989989
this.snapshotValue.AsDouble = this.runningValue.AsDouble;
990990
}
991991

992-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
992+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
993993

994994
this.mpComponents.ReleaseLock();
995995

@@ -998,11 +998,11 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
998998

999999
case AggregationType.LongGauge:
10001000
{
1001-
this.mpComponents!.AcquireLock();
1001+
this.mpComponents.AcquireLock();
10021002

10031003
this.snapshotValue.AsLong = this.runningValue.AsLong;
10041004
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
1005-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1005+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
10061006

10071007
this.mpComponents.ReleaseLock();
10081008

@@ -1011,11 +1011,11 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10111011

10121012
case AggregationType.DoubleGauge:
10131013
{
1014-
this.mpComponents!.AcquireLock();
1014+
this.mpComponents.AcquireLock();
10151015

10161016
this.snapshotValue.AsDouble = this.runningValue.AsDouble;
10171017
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
1018-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1018+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
10191019

10201020
this.mpComponents.ReleaseLock();
10211021

@@ -1024,9 +1024,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10241024

10251025
case AggregationType.HistogramWithBuckets:
10261026
{
1027-
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
1027+
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");
10281028

1029-
var histogramBuckets = this.mpComponents!.HistogramBuckets!;
1029+
var histogramBuckets = this.mpComponents.HistogramBuckets!;
10301030

10311031
this.mpComponents.AcquireLock();
10321032

@@ -1041,7 +1041,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10411041

10421042
histogramBuckets.Snapshot(outputDelta);
10431043

1044-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1044+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
10451045

10461046
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
10471047

@@ -1052,9 +1052,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10521052

10531053
case AggregationType.Histogram:
10541054
{
1055-
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
1055+
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");
10561056

1057-
var histogramBuckets = this.mpComponents!.HistogramBuckets!;
1057+
var histogramBuckets = this.mpComponents.HistogramBuckets!;
10581058

10591059
this.mpComponents.AcquireLock();
10601060

@@ -1067,7 +1067,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10671067
histogramBuckets.RunningSum = 0;
10681068
}
10691069

1070-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1070+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
10711071
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
10721072

10731073
this.mpComponents.ReleaseLock();
@@ -1077,9 +1077,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10771077

10781078
case AggregationType.HistogramWithMinMaxBuckets:
10791079
{
1080-
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
1080+
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");
10811081

1082-
var histogramBuckets = this.mpComponents!.HistogramBuckets!;
1082+
var histogramBuckets = this.mpComponents.HistogramBuckets!;
10831083

10841084
this.mpComponents.AcquireLock();
10851085

@@ -1098,7 +1098,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
10981098

10991099
histogramBuckets.Snapshot(outputDelta);
11001100

1101-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1101+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
11021102
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
11031103

11041104
this.mpComponents.ReleaseLock();
@@ -1108,9 +1108,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11081108

11091109
case AggregationType.HistogramWithMinMax:
11101110
{
1111-
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
1111+
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");
11121112

1113-
var histogramBuckets = this.mpComponents!.HistogramBuckets!;
1113+
var histogramBuckets = this.mpComponents.HistogramBuckets!;
11141114

11151115
this.mpComponents.AcquireLock();
11161116

@@ -1127,7 +1127,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11271127
histogramBuckets.RunningMax = double.NegativeInfinity;
11281128
}
11291129

1130-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1130+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
11311131
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
11321132

11331133
this.mpComponents.ReleaseLock();
@@ -1137,9 +1137,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11371137

11381138
case AggregationType.Base2ExponentialHistogram:
11391139
{
1140-
Debug.Assert(this.mpComponents!.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
1140+
Debug.Assert(this.mpComponents.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
11411141

1142-
var histogram = this.mpComponents!.Base2ExponentialBucketHistogram!;
1142+
var histogram = this.mpComponents.Base2ExponentialBucketHistogram!;
11431143

11441144
this.mpComponents.AcquireLock();
11451145

@@ -1154,7 +1154,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11541154
histogram.Reset();
11551155
}
11561156

1157-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1157+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
11581158
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
11591159

11601160
this.mpComponents.ReleaseLock();
@@ -1164,9 +1164,9 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11641164

11651165
case AggregationType.Base2ExponentialHistogramWithMinMax:
11661166
{
1167-
Debug.Assert(this.mpComponents!.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
1167+
Debug.Assert(this.mpComponents.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
11681168

1169-
var histogram = this.mpComponents!.Base2ExponentialBucketHistogram!;
1169+
var histogram = this.mpComponents.Base2ExponentialBucketHistogram!;
11701170

11711171
this.mpComponents.AcquireLock();
11721172

@@ -1185,7 +1185,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
11851185
histogram.RunningMax = double.NegativeInfinity;
11861186
}
11871187

1188-
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
1188+
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
11891189
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
11901190

11911191
this.mpComponents.ReleaseLock();

‎src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ internal sealed class MetricPointOptionalComponents
2020

2121
public ExemplarReservoir? ExemplarReservoir;
2222

23-
public ReadOnlyExemplarCollection? Exemplars;
23+
public ReadOnlyExemplarCollection Exemplars = ReadOnlyExemplarCollection.Empty;
2424

2525
private int isCriticalSectionOccupied = 0;
2626

@@ -30,7 +30,7 @@ public MetricPointOptionalComponents Copy()
3030
{
3131
HistogramBuckets = this.HistogramBuckets?.Copy(),
3232
Base2ExponentialBucketHistogram = this.Base2ExponentialBucketHistogram?.Copy(),
33-
Exemplars = this.Exemplars?.Copy(),
33+
Exemplars = this.Exemplars.Copy(),
3434
};
3535

3636
return copy;

‎test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ void AssertExemplars<T>(T value, Metric metric)
849849
var result = metricPoint.TryGetExemplars(out var exemplars);
850850
Assert.True(result);
851851

852-
var exemplarEnumerator = exemplars.Value.GetEnumerator();
852+
var exemplarEnumerator = exemplars.GetEnumerator();
853853
Assert.True(exemplarEnumerator.MoveNext());
854854

855855
ref readonly var exemplar = ref exemplarEnumerator.Current;

‎test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ internal static IReadOnlyList<Exemplar> GetExemplars(MetricPoint mp)
237237
{
238238
if (mp.TryGetExemplars(out var exemplars))
239239
{
240-
return exemplars.Value.ToReadOnlyList();
240+
return exemplars.ToReadOnlyList();
241241
}
242242

243243
return Array.Empty<Exemplar>();

0 commit comments

Comments
 (0)