Skip to content

Commit

Permalink
[sdk-metrics] ReadOnlyExemplarCollection tweaks (#5403)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CodeBlanch
CodeBlanch committed Feb 29, 2024
1 parent 77ef123 commit 42ecd73
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ OpenTelemetry.Metrics.ExemplarMeasurement<T>
OpenTelemetry.Metrics.ExemplarMeasurement<T>.ExemplarMeasurement() -> void
OpenTelemetry.Metrics.ExemplarMeasurement<T>.Tags.get -> System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>>
OpenTelemetry.Metrics.ExemplarMeasurement<T>.Value.get -> T
OpenTelemetry.Metrics.MetricPoint.TryGetExemplars(out OpenTelemetry.Metrics.ReadOnlyExemplarCollection? exemplars) -> bool
OpenTelemetry.Metrics.MetricPoint.TryGetExemplars(out OpenTelemetry.Metrics.ReadOnlyExemplarCollection exemplars) -> bool
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
OpenTelemetry.Metrics.ReadOnlyExemplarCollection
Expand Down
28 changes: 21 additions & 7 deletions src/OpenTelemetry/Metrics/Exemplar/ReadOnlyExemplarCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace OpenTelemetry.Metrics;
#endif
readonly struct ReadOnlyExemplarCollection
{
internal static readonly ReadOnlyExemplarCollection Empty = new(Array.Empty<Exemplar>());
private readonly Exemplar[] exemplars;

internal ReadOnlyExemplarCollection(Exemplar[] exemplars)
Expand Down Expand Up @@ -50,24 +51,37 @@ public Enumerator GetEnumerator()

internal ReadOnlyExemplarCollection Copy()
{
var exemplarCopies = new Exemplar[this.exemplars.Length];
var maximumCount = this.MaximumCount;

int i = 0;
foreach (ref readonly var exemplar in this)
if (maximumCount > 0)
{
exemplar.Copy(ref exemplarCopies[i++]);
var exemplarCopies = new Exemplar[maximumCount];

int i = 0;
foreach (ref readonly var exemplar in this)
{
if (exemplar.IsUpdated())
{
exemplar.Copy(ref exemplarCopies[i++]);
}
}

return new ReadOnlyExemplarCollection(exemplarCopies);
}

return new ReadOnlyExemplarCollection(exemplarCopies);
return Empty;
}

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

foreach (var item in this)
foreach (var exemplar in this)
{
list.Add(item);
// Note: If ToReadOnlyList is ever made public it should make sure
// to take copies of exemplars or make sure the instance was first
// copied using the Copy method above.
list.Add(exemplar);
}

return list;
Expand Down
58 changes: 29 additions & 29 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using OpenTelemetry.Internal;

Expand Down Expand Up @@ -373,10 +372,10 @@ public readonly bool TryGetHistogramMinMaxValues(out double min, out double max)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal
#endif
readonly bool TryGetExemplars([NotNullWhen(true)] out ReadOnlyExemplarCollection? exemplars)
readonly bool TryGetExemplars(out ReadOnlyExemplarCollection exemplars)
{
exemplars = this.mpComponents?.Exemplars;
return exemplars.HasValue;
exemplars = this.mpComponents?.Exemplars ?? ReadOnlyExemplarCollection.Empty;
return exemplars.MaximumCount > 0;
}

internal readonly MetricPoint Copy()
Expand Down Expand Up @@ -945,13 +944,14 @@ internal void TakeSnapshot(bool outputDelta)
internal void TakeSnapshotWithExemplar(bool outputDelta)
{
Debug.Assert(this.mpComponents != null, "this.mpComponents was null");
Debug.Assert(this.mpComponents!.ExemplarReservoir != null, "this.mpComponents.ExemplarReservoir was null");

switch (this.aggType)
{
case AggregationType.LongSumIncomingDelta:
case AggregationType.LongSumIncomingCumulative:
{
this.mpComponents!.AcquireLock();
this.mpComponents.AcquireLock();

if (outputDelta)
{
Expand All @@ -965,7 +965,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
this.snapshotValue.AsLong = this.runningValue.AsLong;
}

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();

this.mpComponents.ReleaseLock();

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

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();

this.mpComponents.ReleaseLock();

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

case AggregationType.LongGauge:
{
this.mpComponents!.AcquireLock();
this.mpComponents.AcquireLock();

this.snapshotValue.AsLong = this.runningValue.AsLong;
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();

this.mpComponents.ReleaseLock();

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

case AggregationType.DoubleGauge:
{
this.mpComponents!.AcquireLock();
this.mpComponents.AcquireLock();

this.snapshotValue.AsDouble = this.runningValue.AsDouble;
this.MetricPointStatus = MetricPointStatus.NoCollectPending;
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();

this.mpComponents.ReleaseLock();

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

case AggregationType.HistogramWithBuckets:
{
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");

var histogramBuckets = this.mpComponents!.HistogramBuckets!;
var histogramBuckets = this.mpComponents.HistogramBuckets!;

this.mpComponents.AcquireLock();

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

histogramBuckets.Snapshot(outputDelta);

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();

this.MetricPointStatus = MetricPointStatus.NoCollectPending;

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

case AggregationType.Histogram:
{
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");

var histogramBuckets = this.mpComponents!.HistogramBuckets!;
var histogramBuckets = this.mpComponents.HistogramBuckets!;

this.mpComponents.AcquireLock();

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

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
this.MetricPointStatus = MetricPointStatus.NoCollectPending;

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

case AggregationType.HistogramWithMinMaxBuckets:
{
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");

var histogramBuckets = this.mpComponents!.HistogramBuckets!;
var histogramBuckets = this.mpComponents.HistogramBuckets!;

this.mpComponents.AcquireLock();

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

histogramBuckets.Snapshot(outputDelta);

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
this.MetricPointStatus = MetricPointStatus.NoCollectPending;

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

case AggregationType.HistogramWithMinMax:
{
Debug.Assert(this.mpComponents!.HistogramBuckets != null, "HistogramBuckets was null");
Debug.Assert(this.mpComponents.HistogramBuckets != null, "HistogramBuckets was null");

var histogramBuckets = this.mpComponents!.HistogramBuckets!;
var histogramBuckets = this.mpComponents.HistogramBuckets!;

this.mpComponents.AcquireLock();

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

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
this.MetricPointStatus = MetricPointStatus.NoCollectPending;

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

case AggregationType.Base2ExponentialHistogram:
{
Debug.Assert(this.mpComponents!.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
Debug.Assert(this.mpComponents.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");

var histogram = this.mpComponents!.Base2ExponentialBucketHistogram!;
var histogram = this.mpComponents.Base2ExponentialBucketHistogram!;

this.mpComponents.AcquireLock();

Expand All @@ -1154,7 +1154,7 @@ internal void TakeSnapshotWithExemplar(bool outputDelta)
histogram.Reset();
}

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
this.MetricPointStatus = MetricPointStatus.NoCollectPending;

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

case AggregationType.Base2ExponentialHistogramWithMinMax:
{
Debug.Assert(this.mpComponents!.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");
Debug.Assert(this.mpComponents.Base2ExponentialBucketHistogram != null, "Base2ExponentialBucketHistogram was null");

var histogram = this.mpComponents!.Base2ExponentialBucketHistogram!;
var histogram = this.mpComponents.Base2ExponentialBucketHistogram!;

this.mpComponents.AcquireLock();

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

this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir?.Collect();
this.mpComponents.Exemplars = this.mpComponents.ExemplarReservoir!.Collect();
this.MetricPointStatus = MetricPointStatus.NoCollectPending;

this.mpComponents.ReleaseLock();
Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Metrics/MetricPointOptionalComponents.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ internal sealed class MetricPointOptionalComponents

public ExemplarReservoir? ExemplarReservoir;

public ReadOnlyExemplarCollection? Exemplars;
public ReadOnlyExemplarCollection Exemplars = ReadOnlyExemplarCollection.Empty;

private int isCriticalSectionOccupied = 0;

Expand All @@ -30,7 +30,7 @@ public MetricPointOptionalComponents Copy()
{
HistogramBuckets = this.HistogramBuckets?.Copy(),
Base2ExponentialBucketHistogram = this.Base2ExponentialBucketHistogram?.Copy(),
Exemplars = this.Exemplars?.Copy(),
Exemplars = this.Exemplars.Copy(),
};

return copy;
Expand Down
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpMetricsExporterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ void AssertExemplars<T>(T value, Metric metric)
var result = metricPoint.TryGetExemplars(out var exemplars);
Assert.True(result);

var exemplarEnumerator = exemplars.Value.GetEnumerator();
var exemplarEnumerator = exemplars.GetEnumerator();
Assert.True(exemplarEnumerator.MoveNext());

ref readonly var exemplar = ref exemplarEnumerator.Current;
Expand Down
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ internal static IReadOnlyList<Exemplar> GetExemplars(MetricPoint mp)
{
if (mp.TryGetExemplars(out var exemplars))
{
return exemplars.Value.ToReadOnlyList();
return exemplars.ToReadOnlyList();
}

return Array.Empty<Exemplar>();
Expand Down

0 comments on commit 42ecd73

Please sign in to comment.