Skip to content

Commit

Permalink
Metrics: Improve perf by moving timestamps off of MetricPoint (#3082)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CodeBlanch
CodeBlanch authored Mar 31, 2022
1 parent 3e65569 commit 213fc03
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 70 deletions.
29 changes: 13 additions & 16 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ internal sealed class AggregatorStore
private int batchSize = 0;
private int metricCapHitMessageLogged;
private bool zeroTagMetricPointInitialized;
private DateTimeOffset startTimeExclusive;
private DateTimeOffset endTimeInclusive;

internal AggregatorStore(
string name,
Expand All @@ -66,7 +64,7 @@ internal AggregatorStore(
this.aggType = aggType;
this.outputDelta = temporality == AggregationTemporality.Delta;
this.histogramBounds = histogramBounds;
this.startTimeExclusive = DateTimeOffset.UtcNow;
this.StartTimeExclusive = DateTimeOffset.UtcNow;
if (tagKeysInteresting == null)
{
this.updateLongCallback = this.UpdateLong;
Expand All @@ -86,6 +84,10 @@ internal AggregatorStore(

private delegate void UpdateDoubleDelegate(double value, ReadOnlySpan<KeyValuePair<string, object>> tags);

internal DateTimeOffset StartTimeExclusive { get; private set; }

internal DateTimeOffset EndTimeInclusive { get; private set; }

internal void Update(long value, ReadOnlySpan<KeyValuePair<string, object>> tags)
{
this.updateLongCallback(value, tags);
Expand All @@ -109,7 +111,7 @@ internal int Snapshot()
this.SnapshotCumulative(indexSnapshot);
}

this.endTimeInclusive = DateTimeOffset.UtcNow;
this.EndTimeInclusive = DateTimeOffset.UtcNow;
return this.batchSize;
}

Expand All @@ -128,9 +130,9 @@ internal void SnapshotDelta(int indexSnapshot)
this.batchSize++;
}

if (this.endTimeInclusive != default)
if (this.EndTimeInclusive != default)
{
this.startTimeExclusive = this.endTimeInclusive;
this.StartTimeExclusive = this.EndTimeInclusive;
}
}

Expand All @@ -139,7 +141,7 @@ internal void SnapshotCumulative(int indexSnapshot)
for (int i = 0; i <= indexSnapshot; i++)
{
ref var metricPoint = ref this.metricPoints[i];
if (metricPoint.StartTime == default)
if (!metricPoint.IsInitialized)
{
continue;
}
Expand All @@ -151,9 +153,7 @@ internal void SnapshotCumulative(int indexSnapshot)
}

internal MetricPointsAccessor GetMetricPoints()
{
return new MetricPointsAccessor(this.metricPoints, this.currentMetricPointBatch, this.batchSize, this.startTimeExclusive, this.endTimeInclusive);
}
=> new(this.metricPoints, this.currentMetricPointBatch, this.batchSize);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void InitializeZeroTagPointIfNotInitialized()
Expand All @@ -164,8 +164,7 @@ private void InitializeZeroTagPointIfNotInitialized()
{
if (!this.zeroTagMetricPointInitialized)
{
var dt = DateTimeOffset.UtcNow;
this.metricPoints[0] = new MetricPoint(this.aggType, dt, null, null, this.histogramBounds);
this.metricPoints[0] = new MetricPoint(this, this.aggType, null, null, this.histogramBounds);
this.zeroTagMetricPointInitialized = true;
}
}
Expand Down Expand Up @@ -231,8 +230,7 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
}

ref var metricPoint = ref this.metricPoints[aggregatorIndex];
var dt = DateTimeOffset.UtcNow;
metricPoint = new MetricPoint(this.aggType, dt, sortedTags.Keys, sortedTags.Values, this.histogramBounds);
metricPoint = new MetricPoint(this, this.aggType, sortedTags.Keys, sortedTags.Values, this.histogramBounds);

// Add to dictionary *after* initializing MetricPoint
// as other threads can start writing to the
Expand Down Expand Up @@ -282,8 +280,7 @@ private int LookupAggregatorStore(string[] tagKeys, object[] tagValues, int leng
}

ref var metricPoint = ref this.metricPoints[aggregatorIndex];
var dt = DateTimeOffset.UtcNow;
metricPoint = new MetricPoint(this.aggType, dt, givenTags.Keys, givenTags.Values, this.histogramBounds);
metricPoint = new MetricPoint(this, this.aggType, givenTags.Keys, givenTags.Values, this.histogramBounds);

// Add to dictionary *after* initializing MetricPoint
// as other threads can start writing to the
Expand Down
30 changes: 11 additions & 19 deletions src/OpenTelemetry/Metrics/MetricPoint.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ namespace OpenTelemetry.Metrics
/// </summary>
public struct MetricPoint
{
private readonly AggregatorStore aggregatorStore;

private readonly AggregationType aggType;

private readonly HistogramBuckets histogramBuckets;
Expand All @@ -39,19 +41,18 @@ public struct MetricPoint
private MetricPointValueStorage deltaLastValue;

internal MetricPoint(
AggregatorStore aggregatorStore,
AggregationType aggType,
DateTimeOffset startTime,
string[] keys,
object[] values,
double[] histogramExplicitBounds)
{
Debug.Assert(aggregatorStore != null, "AggregatorStore was null.");
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.runningValue = default;
this.snapshotValue = default;
this.deltaLastValue = default;
Expand All @@ -69,6 +70,9 @@ internal MetricPoint(
{
this.histogramBuckets = null;
}

// Note: Intentionally set last because this is used to detect valid MetricPoints.
this.aggregatorStore = aggregatorStore;
}

/// <summary>
Expand All @@ -83,26 +87,12 @@ public readonly ReadOnlyTagCollection Tags
/// <summary>
/// Gets the start time associated with the metric point.
/// </summary>
public DateTimeOffset StartTime
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
readonly get;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal set;
}
public readonly DateTimeOffset StartTime => this.aggregatorStore.StartTimeExclusive;

/// <summary>
/// Gets the end time associated with the metric point.
/// </summary>
public DateTimeOffset EndTime
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
readonly get;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal set;
}
public readonly DateTimeOffset EndTime => this.aggregatorStore.EndTimeInclusive;

internal MetricPointStatus MetricPointStatus
{
Expand All @@ -113,6 +103,8 @@ internal MetricPointStatus MetricPointStatus
private set;
}

internal readonly bool IsInitialized => this.aggregatorStore != null;

/// <summary>
/// Gets the sum long value associated with the metric point.
/// </summary>
Expand Down
36 changes: 5 additions & 31 deletions src/OpenTelemetry/Metrics/MetricPointsAccessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// limitations under the License.
// </copyright>

using System;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Metrics
Expand All @@ -28,28 +27,22 @@ public readonly struct MetricPointsAccessor
private readonly MetricPoint[] metricsPoints;
private readonly int[] metricPointsToProcess;
private readonly long targetCount;
private readonly DateTimeOffset start;
private readonly DateTimeOffset end;

internal MetricPointsAccessor(MetricPoint[] metricsPoints, int[] metricPointsToProcess, long targetCount, DateTimeOffset start, DateTimeOffset end)
internal MetricPointsAccessor(MetricPoint[] metricsPoints, int[] metricPointsToProcess, long targetCount)
{
Guard.ThrowIfNull(metricsPoints);

this.metricsPoints = metricsPoints;
this.metricPointsToProcess = metricPointsToProcess;
this.targetCount = targetCount;
this.start = start;
this.end = end;
}

/// <summary>
/// Returns an enumerator that iterates through the <see cref="MetricPointsAccessor"/>.
/// </summary>
/// <returns><see cref="Enumerator"/>.</returns>
public Enumerator GetEnumerator()
{
return new Enumerator(this.metricsPoints, this.metricPointsToProcess, this.targetCount, this.start, this.end);
}
=> new(this.metricsPoints, this.metricPointsToProcess, this.targetCount);

/// <summary>
/// Enumerates the elements of a <see cref="MetricPointsAccessor"/>.
Expand All @@ -58,31 +51,22 @@ public struct Enumerator
{
private readonly MetricPoint[] metricsPoints;
private readonly int[] metricPointsToProcess;
private readonly DateTimeOffset start;
private readonly DateTimeOffset end;
private readonly long targetCount;
private long index;

internal Enumerator(MetricPoint[] metricsPoints, int[] metricPointsToProcess, long targetCount, DateTimeOffset start, DateTimeOffset end)
internal Enumerator(MetricPoint[] metricsPoints, int[] metricPointsToProcess, long targetCount)
{
this.metricsPoints = metricsPoints;
this.metricPointsToProcess = metricPointsToProcess;
this.targetCount = targetCount;
this.index = -1;
this.start = start;
this.end = end;
}

/// <summary>
/// Gets the <see cref="MetricPoint"/> at the current position of the enumerator.
/// </summary>
public ref readonly MetricPoint Current
{
get
{
return ref this.metricsPoints[this.metricPointsToProcess[this.index]];
}
}
=> ref this.metricsPoints[this.metricPointsToProcess[this.index]];

/// <summary>
/// Advances the enumerator to the next element of the <see
Expand All @@ -93,17 +77,7 @@ public ref readonly MetricPoint Current
/// langword="false"/> if the enumerator has passed the end of the
/// collection.</returns>
public bool MoveNext()
{
while (++this.index < this.targetCount)
{
ref var metricPoint = ref this.metricsPoints[this.metricPointsToProcess[this.index]];
metricPoint.StartTime = this.start;
metricPoint.EndTime = this.end;
return true;
}

return false;
}
=> ++this.index < this.targetCount;
}
}
}
10 changes: 6 additions & 4 deletions test/OpenTelemetry.Tests/Metrics/AggregatorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ namespace OpenTelemetry.Metrics.Tests
{
public class AggregatorTest
{
private readonly AggregatorStore aggregatorStore = new("test", AggregationType.Histogram, AggregationTemporality.Cumulative, 1024, Metric.DefaultHistogramBounds);

[Fact]
public void HistogramDistributeToAllBucketsDefault()
{
var histogramPoint = new MetricPoint(AggregationType.Histogram, DateTimeOffset.Now, null, null, Metric.DefaultHistogramBounds);
var histogramPoint = new MetricPoint(this.aggregatorStore, AggregationType.Histogram, null, null, Metric.DefaultHistogramBounds);
histogramPoint.Update(-1);
histogramPoint.Update(0);
histogramPoint.Update(2);
Expand Down Expand Up @@ -65,7 +67,7 @@ public void HistogramDistributeToAllBucketsDefault()
public void HistogramDistributeToAllBucketsCustom()
{
var boundaries = new double[] { 10, 20 };
var histogramPoint = new MetricPoint(AggregationType.Histogram, DateTimeOffset.Now, null, null, boundaries);
var histogramPoint = new MetricPoint(this.aggregatorStore, AggregationType.Histogram, null, null, boundaries);

// 5 recordings <=10
histogramPoint.Update(-10);
Expand Down Expand Up @@ -105,8 +107,8 @@ public void HistogramDistributeToAllBucketsCustom()
[Fact]
public void HistogramWithOnlySumCount()
{
var boundaries = new double[] { };
var histogramPoint = new MetricPoint(AggregationType.HistogramSumCount, DateTimeOffset.Now, null, null, boundaries);
var boundaries = Array.Empty<double>();
var histogramPoint = new MetricPoint(this.aggregatorStore, AggregationType.HistogramSumCount, null, null, boundaries);

histogramPoint.Update(-10);
histogramPoint.Update(0);
Expand Down
43 changes: 43 additions & 0 deletions test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,8 @@ public void MeterSourcesWildcardSupportNegativeTestNoMeterAdded(bool hasView)
[InlineData(false)]
public void CounterAggregationTest(bool exportDelta)
{
DateTime testStartTime = DateTime.UtcNow;

var exportedItems = new List<Metric>();

using var meter = new Meter($"{Utils.GetCurrentMethodName()}.{exportDelta}");
Expand All @@ -643,7 +645,20 @@ public void CounterAggregationTest(bool exportDelta)
long sumReceived = GetLongSum(exportedItems);
Assert.Equal(20, sumReceived);

var metricPoint = GetFirstMetricPoint(exportedItems);
Assert.NotNull(metricPoint);
Assert.True(metricPoint.Value.StartTime >= testStartTime);
Assert.True(metricPoint.Value.EndTime != default);

DateTimeOffset firstRunStartTime = metricPoint.Value.StartTime;
DateTimeOffset firstRunEndTime = metricPoint.Value.EndTime;

exportedItems.Clear();

#if NETFRAMEWORK
Thread.Sleep(5000); // Compensates for low resolution timing in netfx.
#endif

counterLong.Add(10);
counterLong.Add(10);
meterProvider.ForceFlush(MaxTimeToAllowForFlush);
Expand All @@ -657,6 +672,21 @@ public void CounterAggregationTest(bool exportDelta)
Assert.Equal(40, sumReceived);
}

metricPoint = GetFirstMetricPoint(exportedItems);
Assert.NotNull(metricPoint);
Assert.True(metricPoint.Value.StartTime >= testStartTime);
Assert.True(metricPoint.Value.EndTime != default);
if (exportDelta)
{
Assert.True(metricPoint.Value.StartTime == firstRunEndTime);
}
else
{
Assert.Equal(firstRunStartTime, metricPoint.Value.StartTime);
}

Assert.True(metricPoint.Value.EndTime > firstRunEndTime);

exportedItems.Clear();
meterProvider.ForceFlush(MaxTimeToAllowForFlush);
sumReceived = GetLongSum(exportedItems);
Expand Down Expand Up @@ -1401,6 +1431,19 @@ private static int GetNumberOfMetricPoints(List<Metric> metrics)
return count;
}

private static MetricPoint? GetFirstMetricPoint(List<Metric> metrics)
{
foreach (var metric in metrics)
{
foreach (ref readonly var metricPoint in metric.GetMetricPoints())
{
return metricPoint;
}
}

return null;
}

// Provide tags input sorted by Key
private static void CheckTagsForNthMetricPoint(List<Metric> metrics, List<KeyValuePair<string, object>> tags, int n)
{
Expand Down

0 comments on commit 213fc03

Please sign in to comment.