diff --git a/src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs b/src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs index 40dbf479b11..ab3ea252d8b 100644 --- a/src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs +++ b/src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -28,11 +29,16 @@ internal abstract class CounterMetricSdkBase : CounterMetric private readonly ConcurrentDictionary> counterBoundInstruments = new ConcurrentDictionary>(); + private readonly Func> createBoundMetricFunc; + private readonly Func> 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> GetAllBoundInstruments() @@ -58,8 +64,9 @@ internal BoundCounterMetric 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) @@ -97,7 +104,7 @@ internal BoundCounterMetric Bind(LabelSet labelset, bool isShortLived) { boundInstrument.Status = RecordStatus.UpdatePending; - this.counterBoundInstruments.GetOrAdd(labelset, boundInstrument); + boundInstrument = this.counterBoundInstruments.GetOrAdd(labelset, boundInstrument); } break; diff --git a/src/OpenTelemetry/Metrics/DoubleObserverMetricSdk.cs b/src/OpenTelemetry/Metrics/DoubleObserverMetricSdk.cs index c09b8c1d4f3..a00b9c880cd 100644 --- a/src/OpenTelemetry/Metrics/DoubleObserverMetricSdk.cs +++ b/src/OpenTelemetry/Metrics/DoubleObserverMetricSdk.cs @@ -22,6 +22,8 @@ namespace OpenTelemetry.Metrics { internal class DoubleObserverMetricSdk : DoubleObserverMetric { + private static readonly Func NewDoubleObserverMetricHandleSdkFunc = (_) => new DoubleObserverMetricHandleSdk(); + private readonly ConcurrentDictionary observerHandles = new ConcurrentDictionary(); private readonly string metricName; private readonly Action callback; @@ -35,9 +37,7 @@ public DoubleObserverMetricSdk(string name, Action 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); } diff --git a/src/OpenTelemetry/Metrics/Int64ObserverMetricSdk.cs b/src/OpenTelemetry/Metrics/Int64ObserverMetricSdk.cs index 1b5ee3bce82..1cfb5a7cde1 100644 --- a/src/OpenTelemetry/Metrics/Int64ObserverMetricSdk.cs +++ b/src/OpenTelemetry/Metrics/Int64ObserverMetricSdk.cs @@ -22,6 +22,8 @@ namespace OpenTelemetry.Metrics { internal class Int64ObserverMetricSdk : Int64ObserverMetric { + private static readonly Func NewInt64ObserverMetricHandleSdkFunc = (_) => new Int64ObserverMetricHandleSdk(); + private readonly ConcurrentDictionary observerHandles = new ConcurrentDictionary(); private readonly string metricName; private readonly Action callback; @@ -35,9 +37,7 @@ public Int64ObserverMetricSdk(string name, Action 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); } diff --git a/src/OpenTelemetry/Metrics/LabelSetSdk.cs b/src/OpenTelemetry/Metrics/LabelSetSdk.cs index 1606257a438..59b70eafdee 100644 --- a/src/OpenTelemetry/Metrics/LabelSetSdk.cs +++ b/src/OpenTelemetry/Metrics/LabelSetSdk.cs @@ -62,31 +62,13 @@ public override bool Equals(object obj) private static IEnumerable> SortAndDedup(IEnumerable> 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>(); - - int dedupedListIndex = 0; - dedupedList.Add(orderedList[dedupedListIndex]); - for (int i = 1; i < orderedList.Count; i++) + var dedupedList = new SortedDictionary>(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> labels) diff --git a/src/OpenTelemetry/Metrics/MeasureMetricSdk.cs b/src/OpenTelemetry/Metrics/MeasureMetricSdk.cs index 7ddd4c5c3a6..0afeef76476 100644 --- a/src/OpenTelemetry/Metrics/MeasureMetricSdk.cs +++ b/src/OpenTelemetry/Metrics/MeasureMetricSdk.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -23,16 +24,18 @@ internal abstract class MeasureMetricSdk : MeasureMetric where T : struct { private readonly ConcurrentDictionary> measureBoundInstruments = new ConcurrentDictionary>(); + private readonly Func> createMetricFunc; private string metricName; public MeasureMetricSdk(string name) { this.metricName = name; + this.createMetricFunc = (_) => this.CreateMetric(); } public override BoundMeasureMetric Bind(LabelSet labelset) { - return this.measureBoundInstruments.GetOrAdd(labelset, this.CreateMetric()); + return this.measureBoundInstruments.GetOrAdd(labelset, this.createMetricFunc); } public override BoundMeasureMetric Bind(IEnumerable> labels) diff --git a/src/OpenTelemetry/Metrics/MeterSdk.cs b/src/OpenTelemetry/Metrics/MeterSdk.cs index e30f68ca37b..352ed47a52c 100644 --- a/src/OpenTelemetry/Metrics/MeterSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterSdk.cs @@ -24,6 +24,11 @@ namespace OpenTelemetry.Metrics { internal class MeterSdk : Meter { + private static readonly Func NewInt64CounterMetricSdkFunc = (name) => new Int64CounterMetricSdk(name); + private static readonly Func NewDoubleCounterMetricSdkFunc = (name) => new DoubleCounterMetricSdk(name); + private static readonly Func NewInt64MeasureMetricSdkFunc = (name) => new Int64MeasureMetricSdk(name); + private static readonly Func NewDoubleMeasureMetricSdkFunc = (name) => new DoubleMeasureMetricSdk(name); + private readonly string meterName; private readonly MetricProcessor metricProcessor; private readonly ConcurrentDictionary longCounters = new ConcurrentDictionary(); @@ -262,34 +267,46 @@ public virtual void Collect() public override CounterMetric CreateInt64Counter(string name, bool monotonic = true) { - return this.longCounters.GetOrAdd(name, new Int64CounterMetricSdk(name)); + return this.longCounters.GetOrAdd(name, NewInt64CounterMetricSdkFunc); } public override CounterMetric CreateDoubleCounter(string name, bool monotonic = true) { - return this.doubleCounters.GetOrAdd(name, new DoubleCounterMetricSdk(name)); + return this.doubleCounters.GetOrAdd(name, NewDoubleCounterMetricSdkFunc); } public override MeasureMetric CreateDoubleMeasure(string name, bool absolute = true) { - return this.doubleMeasures.GetOrAdd(name, new DoubleMeasureMetricSdk(name)); + return this.doubleMeasures.GetOrAdd(name, NewDoubleMeasureMetricSdkFunc); } public override MeasureMetric CreateInt64Measure(string name, bool absolute = true) { - return this.longMeasures.GetOrAdd(name, new Int64MeasureMetricSdk(name)); + return this.longMeasures.GetOrAdd(name, NewInt64MeasureMetricSdkFunc); } /// public override Int64ObserverMetric CreateInt64Observer(string name, Action 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; } /// public override DoubleObserverMetric CreateDoubleObserver(string name, Action 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; } } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs b/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs index ad394c9ae12..f65c91c848a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricsTest.cs @@ -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() @@ -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>(); + labels1.Add(new KeyValuePair("dim1", "value1")); + + var labels2 = new List>(); + labels2.Add(new KeyValuePair("dim1", "value2")); + + var labels3 = new List>(); + labels3.Add(new KeyValuePair("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() @@ -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>(); + labels1.Add(new KeyValuePair("dim1", "value1")); + + var labels2 = new List>(); + labels2.Add(new KeyValuePair("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() {