diff --git a/src/OpenTelemetry/Metrics/HistogramBucket.cs b/src/OpenTelemetry/Metrics/HistogramBucket.cs index 646d3a2964b..fc876fa4cf1 100644 --- a/src/OpenTelemetry/Metrics/HistogramBucket.cs +++ b/src/OpenTelemetry/Metrics/HistogramBucket.cs @@ -16,6 +16,9 @@ namespace OpenTelemetry.Metrics { + /// + /// Represents a bucket in the histogram metric type. + /// public readonly struct HistogramBucket { internal HistogramBucket(double explicitBound, long bucketCount) @@ -24,8 +27,15 @@ internal HistogramBucket(double explicitBound, long bucketCount) this.BucketCount = bucketCount; } + /// + /// Gets the configured bounds for the bucket or for the catch-all bucket. + /// public double ExplicitBound { get; } + /// + /// Gets the count of items in the bucket. + /// public long BucketCount { get; } } } diff --git a/src/OpenTelemetry/Metrics/HistogramBuckets.cs b/src/OpenTelemetry/Metrics/HistogramBuckets.cs index 8b4464c2ac4..a68033d4300 100644 --- a/src/OpenTelemetry/Metrics/HistogramBuckets.cs +++ b/src/OpenTelemetry/Metrics/HistogramBuckets.cs @@ -16,26 +16,37 @@ namespace OpenTelemetry.Metrics { + /// + /// A collection of s associated with a histogram metric type. + /// + // Note: Does not implement IEnumerable<> to prevent accidental boxing. public class HistogramBuckets { - internal readonly long[] BucketCounts; + internal readonly double[] ExplicitBounds; - internal readonly long[] AggregatedBucketCounts; + internal readonly long[] RunningBucketCounts; - internal readonly double[] ExplicitBounds; + internal readonly long[] SnapshotBucketCounts; - internal readonly object LockObject; + internal double RunningSum; - internal HistogramBuckets(double[] histogramBounds) + internal double SnapshotSum; + + internal HistogramBuckets(double[] explicitBounds) { - this.ExplicitBounds = histogramBounds; - this.BucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; - this.AggregatedBucketCounts = histogramBounds != null ? new long[histogramBounds.Length + 1] : null; - this.LockObject = new object(); + this.ExplicitBounds = explicitBounds; + this.RunningBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : null; + this.SnapshotBucketCounts = explicitBounds != null ? new long[explicitBounds.Length + 1] : new long[0]; } + internal object LockObject => this.SnapshotBucketCounts; + public Enumerator GetEnumerator() => new(this); + /// + /// Enumerates the elements of a . + /// + // Note: Does not implement IEnumerator<> to prevent accidental boxing. public struct Enumerator { private readonly int numberOfBuckets; @@ -47,11 +58,22 @@ internal Enumerator(HistogramBuckets histogramMeasurements) this.histogramMeasurements = histogramMeasurements; this.index = 0; this.Current = default; - this.numberOfBuckets = histogramMeasurements.AggregatedBucketCounts == null ? 0 : histogramMeasurements.AggregatedBucketCounts.Length; + this.numberOfBuckets = histogramMeasurements.SnapshotBucketCounts.Length; } + /// + /// Gets the at the current position of the enumerator. + /// public HistogramBucket Current { get; private set; } + /// + /// Advances the enumerator to the next element of the . + /// + /// if the enumerator was + /// successfully advanced to the next element; if the enumerator has passed the end of the + /// collection. public bool MoveNext() { if (this.index < this.numberOfBuckets) @@ -59,7 +81,7 @@ public bool MoveNext() double explicitBound = this.index < this.numberOfBuckets - 1 ? this.histogramMeasurements.ExplicitBounds[this.index] : double.PositiveInfinity; - long bucketCount = this.histogramMeasurements.AggregatedBucketCounts[this.index]; + long bucketCount = this.histogramMeasurements.SnapshotBucketCounts[this.index]; this.Current = new HistogramBucket(explicitBound, bucketCount); this.index++; return true; diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index e3981667564..983976d5e9e 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -23,7 +23,8 @@ namespace OpenTelemetry.Metrics public sealed class Metric { internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 }; - private AggregatorStore aggStore; + + private readonly AggregatorStore aggStore; internal Metric( Instrument instrument, @@ -38,7 +39,8 @@ internal Metric( this.Description = metricDescription ?? string.Empty; this.Unit = instrument.Unit ?? string.Empty; this.Meter = instrument.Meter; - AggregationType aggType = default; + + AggregationType aggType; if (instrument.GetType() == typeof(ObservableCounter) || instrument.GetType() == typeof(ObservableCounter) || instrument.GetType() == typeof(ObservableCounter) diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index c8b88755a1e..e91c1253ace 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -21,37 +21,45 @@ namespace OpenTelemetry.Metrics { + /// + /// Stores details about a metric data point. + /// public struct MetricPoint { private readonly AggregationType aggType; + private readonly HistogramBuckets histogramBuckets; + // Represents temporality adjusted "value" for double/long metric types or "count" when histogram + private MetricPointValueStorage runningValue; + // Represents either "value" for double/long metric types or "count" when histogram - private MetricPointValueStorage primaryValue; + private MetricPointValueStorage snapshotValue; - // Represents either "lastValue" for double/long metric types when delta or "sum" when histogram - private MetricPointValueStorage secondaryValue; + private MetricPointValueStorage deltaLastValue; internal MetricPoint( AggregationType aggType, DateTimeOffset startTime, string[] keys, object[] values, - double[] histogramBounds) + double[] histogramExplicitBounds) { Debug.Assert((keys?.Length ?? 0) == (values?.Length ?? 0), "Key and value array lengths did not match."); + Debug.Assert(histogramExplicitBounds != null, "Histogram explicit Bounds was null."); this.aggType = aggType; this.StartTime = startTime; this.Tags = new ReadOnlyTagCollection(keys, values); this.EndTime = default; - this.primaryValue = default; - this.secondaryValue = default; + this.runningValue = default; + this.snapshotValue = default; + this.deltaLastValue = default; this.MetricPointStatus = MetricPointStatus.NoCollectPending; if (this.aggType == AggregationType.Histogram) { - this.histogramBuckets = new HistogramBuckets(histogramBounds); + this.histogramBuckets = new HistogramBuckets(histogramExplicitBounds); } else if (this.aggType == AggregationType.HistogramSumCount) { @@ -66,8 +74,15 @@ internal MetricPoint( /// /// Gets the tags associated with the metric point. /// - public ReadOnlyTagCollection Tags { get; } + public ReadOnlyTagCollection Tags + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; + } + /// + /// Gets the start time associated with the metric point. + /// public DateTimeOffset StartTime { [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -77,6 +92,9 @@ public DateTimeOffset StartTime internal set; } + /// + /// Gets the end time associated with the metric point. + /// public DateTimeOffset EndTime { [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -86,90 +104,139 @@ public DateTimeOffset EndTime internal set; } - internal MetricPointStatus MetricPointStatus { get; private set; } + internal MetricPointStatus MetricPointStatus + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private set; + } + /// + /// Gets the sum long value associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Long sum value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public long GetSumLong() { - if (this.aggType == AggregationType.LongSumIncomingDelta || this.aggType == AggregationType.LongSumIncomingCumulative) - { - return this.primaryValue.SnapshotAsLong; - } - else + if (this.aggType != AggregationType.LongSumIncomingDelta && this.aggType != AggregationType.LongSumIncomingCumulative) { - throw new NotSupportedException($"{nameof(this.GetSumLong)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetSumLong)); } + + return this.snapshotValue.AsLong; } + /// + /// Gets the sum double value associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Double sum value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public double GetSumDouble() { - if (this.aggType == AggregationType.DoubleSumIncomingDelta || this.aggType == AggregationType.DoubleSumIncomingCumulative) + if (this.aggType != AggregationType.DoubleSumIncomingDelta && this.aggType != AggregationType.DoubleSumIncomingCumulative) { - return this.primaryValue.SnapshotAsDouble; - } - else - { - throw new NotSupportedException($"{nameof(this.GetSumDouble)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetSumDouble)); } + + return this.snapshotValue.AsDouble; } + /// + /// Gets the last long value of the gauge associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Long gauge value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public long GetGaugeLastValueLong() { - if (this.aggType == AggregationType.LongGauge) + if (this.aggType != AggregationType.LongGauge) { - return this.primaryValue.SnapshotAsLong; - } - else - { - throw new NotSupportedException($"{nameof(this.GetGaugeLastValueLong)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetGaugeLastValueLong)); } + + return this.snapshotValue.AsLong; } + /// + /// Gets the last double value of the gauge associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Double gauge value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public double GetGaugeLastValueDouble() { - if (this.aggType == AggregationType.DoubleGauge) - { - return this.primaryValue.SnapshotAsDouble; - } - else + if (this.aggType != AggregationType.DoubleGauge) { - throw new NotSupportedException($"{nameof(this.GetGaugeLastValueDouble)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetGaugeLastValueDouble)); } + + return this.snapshotValue.AsDouble; } + /// + /// Gets the count value of the histogram associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Count value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public long GetHistogramCount() { - if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) + if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) { - return this.primaryValue.SnapshotAsLong; - } - else - { - throw new NotSupportedException($"{nameof(this.GetHistogramCount)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramCount)); } + + return this.snapshotValue.AsLong; } + /// + /// Gets the sum value of the histogram associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// Sum value. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public double GetHistogramSum() { - if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) - { - return this.secondaryValue.SnapshotAsDouble; - } - else + if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) { - throw new NotSupportedException($"{nameof(this.GetHistogramSum)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramSum)); } + + return this.histogramBuckets.SnapshotSum; } + /// + /// Gets the buckets of the histogram associated with the metric point. + /// + /// + /// Applies to metric type. + /// + /// . + [MethodImpl(MethodImplOptions.AggressiveInlining)] public HistogramBuckets GetHistogramBuckets() { - if (this.aggType == AggregationType.Histogram || this.aggType == AggregationType.HistogramSumCount) + if (this.aggType != AggregationType.Histogram && this.aggType != AggregationType.HistogramSumCount) { - return this.histogramBuckets; - } - else - { - throw new NotSupportedException($"{nameof(this.GetHistogramBuckets)} is not supported for this metric type."); + this.ThrowNotSupportedMetricTypeException(nameof(this.GetHistogramBuckets)); } + + return this.histogramBuckets; } internal void Update(long number) @@ -178,19 +245,19 @@ internal void Update(long number) { case AggregationType.LongSumIncomingDelta: { - Interlocked.Add(ref this.primaryValue.CurrentAsLong, number); + Interlocked.Add(ref this.runningValue.AsLong, number); break; } case AggregationType.LongSumIncomingCumulative: { - Interlocked.Exchange(ref this.primaryValue.CurrentAsLong, number); + Interlocked.Exchange(ref this.runningValue.AsLong, number); break; } case AggregationType.LongGauge: { - Interlocked.Exchange(ref this.primaryValue.CurrentAsLong, number); + Interlocked.Exchange(ref this.runningValue.AsLong, number); break; } @@ -225,22 +292,22 @@ internal void Update(double number) double initValue, newValue; do { - initValue = this.primaryValue.CurrentAsDouble; + initValue = this.runningValue.AsDouble; newValue = initValue + number; } - while (initValue != Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, newValue, initValue)); + while (initValue != Interlocked.CompareExchange(ref this.runningValue.AsDouble, newValue, initValue)); break; } case AggregationType.DoubleSumIncomingCumulative: { - Interlocked.Exchange(ref this.primaryValue.CurrentAsDouble, number); + Interlocked.Exchange(ref this.runningValue.AsDouble, number); break; } case AggregationType.DoubleGauge: { - Interlocked.Exchange(ref this.primaryValue.CurrentAsDouble, number); + Interlocked.Exchange(ref this.runningValue.AsDouble, number); break; } @@ -258,9 +325,9 @@ internal void Update(double number) lock (this.histogramBuckets.LockObject) { - this.primaryValue.CurrentAsLong++; - this.secondaryValue.CurrentAsDouble += number; - this.histogramBuckets.BucketCounts[i]++; + this.runningValue.AsLong++; + this.histogramBuckets.RunningSum += number; + this.histogramBuckets.RunningBucketCounts[i]++; } break; @@ -270,8 +337,8 @@ internal void Update(double number) { lock (this.histogramBuckets.LockObject) { - this.primaryValue.CurrentAsLong++; - this.secondaryValue.CurrentAsDouble += number; + this.runningValue.AsLong++; + this.histogramBuckets.RunningSum += number; } break; @@ -301,21 +368,21 @@ internal void TakeSnapshot(bool outputDelta) { if (outputDelta) { - long initValue = Interlocked.Read(ref this.primaryValue.CurrentAsLong); - this.primaryValue.SnapshotAsLong = initValue - this.secondaryValue.CurrentAsLong; - this.secondaryValue.CurrentAsLong = initValue; + long initValue = Interlocked.Read(ref this.runningValue.AsLong); + this.snapshotValue.AsLong = initValue - this.deltaLastValue.AsLong; + this.deltaLastValue.AsLong = initValue; this.MetricPointStatus = MetricPointStatus.NoCollectPending; // Check again if value got updated, if yes reset status. // This ensures no Updates get Lost. - if (initValue != Interlocked.Read(ref this.primaryValue.CurrentAsLong)) + if (initValue != Interlocked.Read(ref this.runningValue.AsLong)) { this.MetricPointStatus = MetricPointStatus.CollectPending; } } else { - this.primaryValue.SnapshotAsLong = Interlocked.Read(ref this.primaryValue.CurrentAsLong); + this.snapshotValue.AsLong = Interlocked.Read(ref this.runningValue.AsLong); } break; @@ -331,14 +398,14 @@ internal void TakeSnapshot(bool outputDelta) // As long as the value is not -ve infinity, // the exchange (to 0.0) will never occur, // but we get the original value atomically. - double initValue = Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, 0.0, double.NegativeInfinity); - this.primaryValue.SnapshotAsDouble = initValue - this.secondaryValue.CurrentAsDouble; - this.secondaryValue.CurrentAsDouble = initValue; + double initValue = Interlocked.CompareExchange(ref this.runningValue.AsDouble, 0.0, double.NegativeInfinity); + this.snapshotValue.AsDouble = initValue - this.deltaLastValue.AsDouble; + this.deltaLastValue.AsDouble = initValue; this.MetricPointStatus = MetricPointStatus.NoCollectPending; // Check again if value got updated, if yes reset status. // This ensures no Updates get Lost. - if (initValue != Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, 0.0, double.NegativeInfinity)) + if (initValue != Interlocked.CompareExchange(ref this.runningValue.AsDouble, 0.0, double.NegativeInfinity)) { this.MetricPointStatus = MetricPointStatus.CollectPending; } @@ -350,7 +417,7 @@ internal void TakeSnapshot(bool outputDelta) // As long as the value is not -ve infinity, // the exchange (to 0.0) will never occur, // but we get the original value atomically. - this.primaryValue.SnapshotAsDouble = Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, 0.0, double.NegativeInfinity); + this.snapshotValue.AsDouble = Interlocked.CompareExchange(ref this.runningValue.AsDouble, 0.0, double.NegativeInfinity); } break; @@ -358,12 +425,12 @@ internal void TakeSnapshot(bool outputDelta) case AggregationType.LongGauge: { - this.primaryValue.SnapshotAsLong = Interlocked.Read(ref this.primaryValue.CurrentAsLong); + this.snapshotValue.AsLong = Interlocked.Read(ref this.runningValue.AsLong); this.MetricPointStatus = MetricPointStatus.NoCollectPending; // Check again if value got updated, if yes reset status. // This ensures no Updates get Lost. - if (this.primaryValue.SnapshotAsLong != Interlocked.Read(ref this.primaryValue.CurrentAsLong)) + if (this.snapshotValue.AsLong != Interlocked.Read(ref this.runningValue.AsLong)) { this.MetricPointStatus = MetricPointStatus.CollectPending; } @@ -378,12 +445,12 @@ internal void TakeSnapshot(bool outputDelta) // As long as the value is not -ve infinity, // the exchange (to 0.0) will never occur, // but we get the original value atomically. - this.primaryValue.SnapshotAsDouble = Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, 0.0, double.NegativeInfinity); + this.snapshotValue.AsDouble = Interlocked.CompareExchange(ref this.runningValue.AsDouble, 0.0, double.NegativeInfinity); this.MetricPointStatus = MetricPointStatus.NoCollectPending; // Check again if value got updated, if yes reset status. // This ensures no Updates get Lost. - if (this.primaryValue.SnapshotAsDouble != Interlocked.CompareExchange(ref this.primaryValue.CurrentAsDouble, 0.0, double.NegativeInfinity)) + if (this.snapshotValue.AsDouble != Interlocked.CompareExchange(ref this.runningValue.AsDouble, 0.0, double.NegativeInfinity)) { this.MetricPointStatus = MetricPointStatus.CollectPending; } @@ -395,20 +462,20 @@ internal void TakeSnapshot(bool outputDelta) { lock (this.histogramBuckets.LockObject) { - this.primaryValue.SnapshotAsLong = this.primaryValue.CurrentAsLong; - this.secondaryValue.SnapshotAsDouble = this.secondaryValue.CurrentAsDouble; + this.snapshotValue.AsLong = this.runningValue.AsLong; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { - this.primaryValue.CurrentAsLong = 0; - this.secondaryValue.CurrentAsDouble = 0; + this.runningValue.AsLong = 0; + this.histogramBuckets.RunningSum = 0; } - for (int i = 0; i < this.histogramBuckets.BucketCounts.Length; i++) + for (int i = 0; i < this.histogramBuckets.RunningBucketCounts.Length; i++) { - this.histogramBuckets.AggregatedBucketCounts[i] = this.histogramBuckets.BucketCounts[i]; + this.histogramBuckets.SnapshotBucketCounts[i] = this.histogramBuckets.RunningBucketCounts[i]; if (outputDelta) { - this.histogramBuckets.BucketCounts[i] = 0; + this.histogramBuckets.RunningBucketCounts[i] = 0; } } @@ -422,12 +489,12 @@ internal void TakeSnapshot(bool outputDelta) { lock (this.histogramBuckets.LockObject) { - this.primaryValue.SnapshotAsLong = this.primaryValue.CurrentAsLong; - this.secondaryValue.SnapshotAsDouble = this.secondaryValue.CurrentAsDouble; + this.snapshotValue.AsLong = this.runningValue.AsLong; + this.histogramBuckets.SnapshotSum = this.histogramBuckets.RunningSum; if (outputDelta) { - this.primaryValue.CurrentAsLong = 0; - this.secondaryValue.CurrentAsDouble = 0; + this.runningValue.AsLong = 0; + this.histogramBuckets.RunningSum = 0; } this.MetricPointStatus = MetricPointStatus.NoCollectPending; @@ -437,5 +504,11 @@ internal void TakeSnapshot(bool outputDelta) } } } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void ThrowNotSupportedMetricTypeException(string methodName) + { + throw new NotSupportedException($"{methodName} is not supported for this metric type."); + } } } diff --git a/src/OpenTelemetry/Metrics/MetricPointValueStorage.cs b/src/OpenTelemetry/Metrics/MetricPointValueStorage.cs index bed3a431d51..778e97885bb 100644 --- a/src/OpenTelemetry/Metrics/MetricPointValueStorage.cs +++ b/src/OpenTelemetry/Metrics/MetricPointValueStorage.cs @@ -22,15 +22,9 @@ namespace OpenTelemetry.Metrics internal struct MetricPointValueStorage { [FieldOffset(0)] - public long CurrentAsLong; + public long AsLong; [FieldOffset(0)] - public double CurrentAsDouble; - - [FieldOffset(8)] - public long SnapshotAsLong; - - [FieldOffset(8)] - public double SnapshotAsDouble; + public double AsDouble; } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index d89b748d582..6656c5b267d 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -777,7 +777,7 @@ private void MultithreadedHistogramTest(long[] expected, T[] values) { foreach (var metricPoint in metric.GetMetricPoints()) { - bucketCounts = metricPoint.GetHistogramBuckets().BucketCounts; + bucketCounts = metricPoint.GetHistogramBuckets().RunningBucketCounts; } } }));