Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize unnecessarily allocation when initializing MetricProvider #1685

Merged
merged 11 commits into from
Jan 29, 2021
13 changes: 10 additions & 3 deletions src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;

Expand All @@ -28,11 +29,16 @@ internal abstract class CounterMetricSdkBase<T> : CounterMetric<T>
private readonly ConcurrentDictionary<LabelSet, BoundCounterMetricSdkBase<T>> counterBoundInstruments =
new ConcurrentDictionary<LabelSet, BoundCounterMetricSdkBase<T>>();

private readonly Func<LabelSet, BoundCounterMetricSdkBase<T>> createBoundMetricFunc;
private readonly Func<LabelSet, BoundCounterMetricSdkBase<T>> createShortLivedMetricFunc;

private string metricName;

protected CounterMetricSdkBase(string name)
{
this.metricName = name;
this.createBoundMetricFunc = (_) => this.CreateMetric(RecordStatus.Bound);
this.createShortLivedMetricFunc = (_) => this.CreateMetric(RecordStatus.UpdatePending);
}

public ConcurrentDictionary<LabelSet, BoundCounterMetricSdkBase<T>> GetAllBoundInstruments()
Expand All @@ -58,8 +64,9 @@ internal BoundCounterMetric<T> Bind(LabelSet labelset, bool isShortLived)

lock (this.bindUnbindLock)
{
var recStatus = isShortLived ? RecordStatus.UpdatePending : RecordStatus.Bound;
boundInstrument = this.counterBoundInstruments.GetOrAdd(labelset, this.CreateMetric(recStatus));
boundInstrument = this.counterBoundInstruments.GetOrAdd(
labelset,
isShortLived ? this.createShortLivedMetricFunc : this.createBoundMetricFunc);
}

switch (boundInstrument.Status)
Expand Down Expand Up @@ -97,7 +104,7 @@ internal BoundCounterMetric<T> Bind(LabelSet labelset, bool isShortLived)
{
boundInstrument.Status = RecordStatus.UpdatePending;

this.counterBoundInstruments.GetOrAdd(labelset, boundInstrument);
boundInstrument = this.counterBoundInstruments.GetOrAdd(labelset, boundInstrument);
}

break;
Expand Down
6 changes: 3 additions & 3 deletions src/OpenTelemetry/Metrics/DoubleObserverMetricSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace OpenTelemetry.Metrics
{
internal class DoubleObserverMetricSdk : DoubleObserverMetric
{
private static readonly Func<LabelSet, DoubleObserverMetricHandleSdk> NewDoubleObserverMetricHandleSdkFunc = (_) => new DoubleObserverMetricHandleSdk();

private readonly ConcurrentDictionary<LabelSet, DoubleObserverMetricHandleSdk> observerHandles = new ConcurrentDictionary<LabelSet, DoubleObserverMetricHandleSdk>();
private readonly string metricName;
private readonly Action<DoubleObserverMetric> callback;
Expand All @@ -35,9 +37,7 @@ public DoubleObserverMetricSdk(string name, Action<DoubleObserverMetric> callbac
public override void Observe(double value, LabelSet labelset)
{
// TODO cleanup of handle/aggregator. Issue #530
var boundInstrument =
this.observerHandles.GetOrAdd(labelset, new DoubleObserverMetricHandleSdk());

var boundInstrument = this.observerHandles.GetOrAdd(labelset, NewDoubleObserverMetricHandleSdkFunc);
boundInstrument.Observe(value);
}

Expand Down
6 changes: 3 additions & 3 deletions src/OpenTelemetry/Metrics/Int64ObserverMetricSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace OpenTelemetry.Metrics
{
internal class Int64ObserverMetricSdk : Int64ObserverMetric
{
private static readonly Func<LabelSet, Int64ObserverMetricHandleSdk> NewInt64ObserverMetricHandleSdkFunc = (_) => new Int64ObserverMetricHandleSdk();

private readonly ConcurrentDictionary<LabelSet, Int64ObserverMetricHandleSdk> observerHandles = new ConcurrentDictionary<LabelSet, Int64ObserverMetricHandleSdk>();
private readonly string metricName;
private readonly Action<Int64ObserverMetric> callback;
Expand All @@ -35,9 +37,7 @@ public Int64ObserverMetricSdk(string name, Action<Int64ObserverMetric> callback)
public override void Observe(long value, LabelSet labelset)
{
// TODO cleanup of handle/aggregator. Issue #530
var boundInstrument =
this.observerHandles.GetOrAdd(labelset, new Int64ObserverMetricHandleSdk());

var boundInstrument = this.observerHandles.GetOrAdd(labelset, NewInt64ObserverMetricHandleSdkFunc);
boundInstrument.Observe(value);
}

Expand Down
26 changes: 4 additions & 22 deletions src/OpenTelemetry/Metrics/LabelSetSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,31 +62,13 @@ public override bool Equals(object obj)

private static IEnumerable<KeyValuePair<string, string>> SortAndDedup(IEnumerable<KeyValuePair<string, string>> labels)
{
// TODO - could be optimized to avoid creating List twice.
var orderedList = labels.OrderBy(x => x.Key).ToList();
if (orderedList.Count == 1)
{
return orderedList;
}

var dedupedList = new List<KeyValuePair<string, string>>();

int dedupedListIndex = 0;
dedupedList.Add(orderedList[dedupedListIndex]);
for (int i = 1; i < orderedList.Count; i++)
var dedupedList = new SortedDictionary<string, KeyValuePair<string, string>>(StringComparer.Ordinal);
foreach (var label in labels)
{
if (orderedList[i].Key.Equals(orderedList[i - 1].Key, StringComparison.Ordinal))
{
dedupedList[dedupedListIndex] = orderedList[i];
}
else
{
dedupedList.Add(orderedList[i]);
dedupedListIndex++;
}
dedupedList[label.Key] = label;
}

return dedupedList;
return dedupedList.Values;
}

private static string GetLabelSetEncoded(IEnumerable<KeyValuePair<string, string>> labels)
Expand Down
5 changes: 4 additions & 1 deletion src/OpenTelemetry/Metrics/MeasureMetricSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;

Expand All @@ -23,16 +24,18 @@ internal abstract class MeasureMetricSdk<T> : MeasureMetric<T>
where T : struct
{
private readonly ConcurrentDictionary<LabelSet, BoundMeasureMetricSdkBase<T>> measureBoundInstruments = new ConcurrentDictionary<LabelSet, BoundMeasureMetricSdkBase<T>>();
private readonly Func<LabelSet, BoundMeasureMetricSdkBase<T>> createMetricFunc;
private string metricName;

public MeasureMetricSdk(string name)
{
this.metricName = name;
this.createMetricFunc = (_) => this.CreateMetric();
}

public override BoundMeasureMetric<T> Bind(LabelSet labelset)
{
return this.measureBoundInstruments.GetOrAdd(labelset, this.CreateMetric());
return this.measureBoundInstruments.GetOrAdd(labelset, this.createMetricFunc);
}

public override BoundMeasureMetric<T> Bind(IEnumerable<KeyValuePair<string, string>> labels)
Expand Down
29 changes: 23 additions & 6 deletions src/OpenTelemetry/Metrics/MeterSdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ namespace OpenTelemetry.Metrics
{
internal class MeterSdk : Meter
{
private static readonly Func<string, Int64CounterMetricSdk> NewInt64CounterMetricSdkFunc = (name) => new Int64CounterMetricSdk(name);
private static readonly Func<string, DoubleCounterMetricSdk> NewDoubleCounterMetricSdkFunc = (name) => new DoubleCounterMetricSdk(name);
private static readonly Func<string, Int64MeasureMetricSdk> NewInt64MeasureMetricSdkFunc = (name) => new Int64MeasureMetricSdk(name);
private static readonly Func<string, DoubleMeasureMetricSdk> NewDoubleMeasureMetricSdkFunc = (name) => new DoubleMeasureMetricSdk(name);

private readonly string meterName;
private readonly MetricProcessor metricProcessor;
private readonly ConcurrentDictionary<string, Int64CounterMetricSdk> longCounters = new ConcurrentDictionary<string, Int64CounterMetricSdk>();
Expand Down Expand Up @@ -262,34 +267,46 @@ public virtual void Collect()

public override CounterMetric<long> CreateInt64Counter(string name, bool monotonic = true)
{
return this.longCounters.GetOrAdd(name, new Int64CounterMetricSdk(name));
return this.longCounters.GetOrAdd(name, NewInt64CounterMetricSdkFunc);
}

public override CounterMetric<double> CreateDoubleCounter(string name, bool monotonic = true)
{
return this.doubleCounters.GetOrAdd(name, new DoubleCounterMetricSdk(name));
return this.doubleCounters.GetOrAdd(name, NewDoubleCounterMetricSdkFunc);
}

public override MeasureMetric<double> CreateDoubleMeasure(string name, bool absolute = true)
{
return this.doubleMeasures.GetOrAdd(name, new DoubleMeasureMetricSdk(name));
return this.doubleMeasures.GetOrAdd(name, NewDoubleMeasureMetricSdkFunc);
}

public override MeasureMetric<long> CreateInt64Measure(string name, bool absolute = true)
{
return this.longMeasures.GetOrAdd(name, new Int64MeasureMetricSdk(name));
return this.longMeasures.GetOrAdd(name, NewInt64MeasureMetricSdkFunc);
}

/// <inheritdoc/>
public override Int64ObserverMetric CreateInt64Observer(string name, Action<Int64ObserverMetric> callback, bool absolute = true)
{
return this.longObservers.GetOrAdd(name, new Int64ObserverMetricSdk(name, callback));
Int64ObserverMetricSdk metric;
if (!this.longObservers.TryGetValue(name, out metric))
{
metric = this.longObservers.GetOrAdd(name, new Int64ObserverMetricSdk(name, callback));
}

return metric;
}

/// <inheritdoc/>
public override DoubleObserverMetric CreateDoubleObserver(string name, Action<DoubleObserverMetric> callback, bool absolute = true)
{
return this.doubleObservers.GetOrAdd(name, new DoubleObserverMetricSdk(name, callback));
DoubleObserverMetricSdk metric;
if (!this.doubleObservers.TryGetValue(name, out metric))
{
metric = this.doubleObservers.GetOrAdd(name, new DoubleObserverMetricSdk(name, callback));
}

return metric;
}
}
}
110 changes: 108 additions & 2 deletions test/OpenTelemetry.Tests/Metrics/MetricsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace OpenTelemetry.Metrics.Tests
public class MetricsTest
{
[Fact]
public void CounterSendsAggregateToRegisteredProcessor()
public void LongCounterSendsAggregateToRegisteredProcessor()
{
var testProcessor = new TestMetricProcessor();
var meter = Sdk.CreateMeterProviderBuilder()
Expand Down Expand Up @@ -79,7 +79,63 @@ public void CounterSendsAggregateToRegisteredProcessor()
}

[Fact]
public void MeasureSendsAggregateToRegisteredProcessor()
public void DoubleCounterSendsAggregateToRegisteredProcessor()
{
var testProcessor = new TestMetricProcessor();
var meter = Sdk.CreateMeterProviderBuilder()
.SetProcessor(testProcessor)
.Build()
.GetMeter("library1") as MeterSdk;

var testCounter = meter.CreateDoubleCounter("testCounter");

var labels1 = new List<KeyValuePair<string, string>>();
labels1.Add(new KeyValuePair<string, string>("dim1", "value1"));

var labels2 = new List<KeyValuePair<string, string>>();
labels2.Add(new KeyValuePair<string, string>("dim1", "value2"));

var labels3 = new List<KeyValuePair<string, string>>();
labels3.Add(new KeyValuePair<string, string>("dim1", "value3"));

var context = default(SpanContext);
testCounter.Add(context, 100.2, meter.GetLabelSet(labels1));
testCounter.Add(context, 10.2, meter.GetLabelSet(labels1));

var boundCounterLabel2 = testCounter.Bind(labels2);
boundCounterLabel2.Add(context, 200.2);

testCounter.Add(context, 200.2, meter.GetLabelSet(labels3));
testCounter.Add(context, 10.2, meter.GetLabelSet(labels3));

meter.Collect();

Assert.Single(testProcessor.Metrics);
var metric = testProcessor.Metrics[0];

Assert.Equal("testCounter", metric.MetricName);
Assert.Equal("library1", metric.MetricNamespace);

// 3 time series, as 3 unique label sets.
Assert.Equal(3, metric.Data.Count);
var expectedSum = 100.2 + 10.2;
var metricSeries = metric.Data.Single(data => data.Labels.Any(l => l.Key == "dim1" && l.Value == "value1"));
var metricDouble = metricSeries as DoubleSumData;
Assert.Equal(expectedSum, metricDouble.Sum);

expectedSum = 200.2;
metricSeries = metric.Data.Single(data => data.Labels.Any(l => l.Key == "dim1" && l.Value == "value2"));
metricDouble = metricSeries as DoubleSumData;
Assert.Equal(expectedSum, metricDouble.Sum);

expectedSum = 200.2 + 10.2;
metricSeries = metric.Data.Single(data => data.Labels.Any(l => l.Key == "dim1" && l.Value == "value3"));
metricDouble = metricSeries as DoubleSumData;
Assert.Equal(expectedSum, metricDouble.Sum);
}

[Fact]
public void LongMeasureSendsAggregateToRegisteredProcessor()
{
var testProcessor = new TestMetricProcessor();
var meter = Sdk.CreateMeterProviderBuilder()
Expand Down Expand Up @@ -126,6 +182,56 @@ public void MeasureSendsAggregateToRegisteredProcessor()
Assert.Equal(200, metricSummary.Max);
}

[Fact]
public void DoubleMeasureSendsAggregateToRegisteredProcessor()
{
var testProcessor = new TestMetricProcessor();
var meter = Sdk.CreateMeterProviderBuilder()
.SetProcessor(testProcessor)
.Build()
.GetMeter("library1") as MeterSdk;
var testMeasure = meter.CreateDoubleMeasure("testMeasure");

var labels1 = new List<KeyValuePair<string, string>>();
labels1.Add(new KeyValuePair<string, string>("dim1", "value1"));

var labels2 = new List<KeyValuePair<string, string>>();
labels2.Add(new KeyValuePair<string, string>("dim1", "value2"));

var context = default(SpanContext);
testMeasure.Record(context, 100.2, meter.GetLabelSet(labels1));
testMeasure.Record(context, 10.2, meter.GetLabelSet(labels1));
testMeasure.Record(context, 1.2, meter.GetLabelSet(labels1));
testMeasure.Record(context, 200.2, meter.GetLabelSet(labels2));
testMeasure.Record(context, 20.2, meter.GetLabelSet(labels2));

meter.Collect();

Assert.Single(testProcessor.Metrics);
var metric = testProcessor.Metrics[0];
Assert.Equal("testMeasure", metric.MetricName);
Assert.Equal("library1", metric.MetricNamespace);

// 2 time series, as 2 unique label sets.
Assert.Equal(2, metric.Data.Count);

var expectedSum = 100.2 + 10.2 + 1.2;
var metricSeries = metric.Data.Single(data => data.Labels.Any(l => l.Key == "dim1" && l.Value == "value1"));
var metricSummary = metricSeries as DoubleSummaryData;
Assert.Equal(expectedSum, metricSummary.Sum);
Assert.Equal(3, metricSummary.Count);
Assert.Equal(1.2, metricSummary.Min);
Assert.Equal(100.2, metricSummary.Max);

expectedSum = 200.2 + 20.2;
metricSeries = metric.Data.Single(data => data.Labels.Any(l => l.Key == "dim1" && l.Value == "value2"));
metricSummary = metricSeries as DoubleSummaryData;
Assert.Equal(expectedSum, metricSummary.Sum);
Assert.Equal(2, metricSummary.Count);
Assert.Equal(20.2, metricSummary.Min);
Assert.Equal(200.2, metricSummary.Max);
}

[Fact]
public void LongObserverSendsAggregateToRegisteredProcessor()
{
Expand Down