From bd69bd6d29764434bbe08cede8d8003e2f2dccb8 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Mon, 2 Aug 2021 09:15:18 -0700 Subject: [PATCH] Refactor Aggregator and Export data structure (#2214) --- .../ConsoleMetricExporter.cs | 73 ++++--- .../Implementation/MetricItemExtensions.cs | 196 +++++++++++------- .../PrometheusExporterExtensions.cs | 16 +- src/OpenTelemetry/Metrics/AggregatorStore.cs | 15 +- .../GaugeMetricAggregator.cs | 5 + .../HistogramMetricAggregator.cs | 3 + .../Metrics/MetricAggregators/IMetric.cs | 2 + .../Metrics/MetricAggregators/ISumMetric.cs | 2 - .../MetricAggregators/ISumMetricDouble.cs | 23 ++ .../MetricAggregators/ISumMetricLong.cs | 23 ++ .../Metrics/MetricAggregators/MetricType.cs | 51 +++++ .../MetricAggregators/SumMetricAggregator.cs | 147 ------------- .../SumMetricAggregatorDouble.cs | 93 +++++++++ .../SumMetricAggregatorLong.cs | 92 ++++++++ .../MetricAggregators/SumMetricDouble.cs | 66 ++++++ .../MetricAggregators/SumMetricLong.cs | 66 ++++++ .../SummaryMetricAggregator.cs | 3 + .../MetricTests.cs | 22 +- .../Metrics/MetricAPITest.cs | 29 ++- 19 files changed, 634 insertions(+), 293 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricDouble.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricLong.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/MetricType.cs delete mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorDouble.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorLong.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/SumMetricDouble.cs create mode 100644 src/OpenTelemetry/Metrics/MetricAggregators/SumMetricLong.cs diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs index a2904f886d..e641db63d8 100644 --- a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs @@ -56,44 +56,53 @@ public override ExportResult Export(in Batch batch) var tags = metric.Attributes.ToArray().Select(k => $"{k.Key}={k.Value?.ToString()}"); string valueDisplay = string.Empty; - if (metric is ISumMetric sumMetric) - { - if (sumMetric.Sum.Value is double doubleSum) - { - valueDisplay = ((double)doubleSum).ToString(CultureInfo.InvariantCulture); - } - else if (sumMetric.Sum.Value is long longSum) - { - valueDisplay = ((long)longSum).ToString(); - } - } - else if (metric is IGaugeMetric gaugeMetric) - { - if (gaugeMetric.LastValue.Value is double doubleValue) - { - valueDisplay = ((double)doubleValue).ToString(); - } - else if (gaugeMetric.LastValue.Value is long longValue) - { - valueDisplay = ((long)longValue).ToString(); - } - // Qn: tags again ? gaugeMetric.LastValue.Tags - } - else if (metric is ISummaryMetric summaryMetric) + // Switch would be faster than the if.else ladder + // of try and cast. + switch (metric.MetricType) { - valueDisplay = string.Format("Sum: {0} Count: {1}", summaryMetric.PopulationSum, summaryMetric.PopulationCount); + case MetricType.LongSum: + { + valueDisplay = (metric as ISumMetricLong).LongSum.ToString(CultureInfo.InvariantCulture); + break; + } + + case MetricType.DoubleSum: + { + valueDisplay = (metric as ISumMetricDouble).DoubleSum.ToString(CultureInfo.InvariantCulture); + break; + } + + case MetricType.LongGauge: + { + // TODOs + break; + } + + case MetricType.DoubleGauge: + { + // TODOs + break; + } + + case MetricType.Histogram: + { + var histogramMetric = metric as IHistogramMetric; + valueDisplay = string.Format("Sum: {0} Count: {1}", histogramMetric.PopulationSum, histogramMetric.PopulationCount); + break; + } + + case MetricType.Summary: + { + var summaryMetric = metric as ISummaryMetric; + valueDisplay = string.Format("Sum: {0} Count: {1}", summaryMetric.PopulationSum, summaryMetric.PopulationCount); + break; + } } - else if (metric is IHistogramMetric histogramMetric) - { - valueDisplay = string.Format("Sum: {0} Count: {1}", histogramMetric.PopulationSum, histogramMetric.PopulationCount); - } - - var kind = metric.GetType().Name; string time = $"{metric.StartTimeExclusive.ToLocalTime().ToString("HH:mm:ss.fff")} {metric.EndTimeInclusive.ToLocalTime().ToString("HH:mm:ss.fff")}"; - var msg = new StringBuilder($"Export {time} {metric.Name} [{string.Join(";", tags)}] {kind} Value: {valueDisplay}"); + var msg = new StringBuilder($"Export {time} {metric.Name} [{string.Join(";", tags)}] {metric.MetricType} Value: {valueDisplay}"); if (!string.IsNullOrEmpty(metric.Description)) { diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 3fa9c2a76f..f2bd7010b1 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -132,95 +132,135 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this IMetric metric) otlpMetric.Unit = metric.Unit; } - if (metric is ISumMetric sumMetric) + switch (metric.MetricType) { - var sum = new OtlpMetrics.Sum - { - IsMonotonic = sumMetric.IsMonotonic, - AggregationTemporality = sumMetric.IsDeltaTemporality + case MetricType.LongSum: + { + var sumMetric = metric as ISumMetricLong; + var sum = new OtlpMetrics.Sum + { + IsMonotonic = sumMetric.IsMonotonic, + AggregationTemporality = sumMetric.IsDeltaTemporality ? OtlpMetrics.AggregationTemporality.Delta : OtlpMetrics.AggregationTemporality.Cumulative, - }; - var dataPoint = metric.ToNumberDataPoint(sumMetric.Sum.Value, sumMetric.Exemplars); - sum.DataPoints.Add(dataPoint); - otlpMetric.Sum = sum; - } - else if (metric is IGaugeMetric gaugeMetric) - { - var gauge = new OtlpMetrics.Gauge(); - var dataPoint = metric.ToNumberDataPoint(gaugeMetric.LastValue.Value, gaugeMetric.Exemplars); - gauge.DataPoints.Add(dataPoint); - otlpMetric.Gauge = gauge; - } - else if (metric is ISummaryMetric summaryMetric) - { - var summary = new OtlpMetrics.Summary(); - - var dataPoint = new OtlpMetrics.SummaryDataPoint - { - StartTimeUnixNano = (ulong)metric.StartTimeExclusive.ToUnixTimeNanoseconds(), - TimeUnixNano = (ulong)metric.EndTimeInclusive.ToUnixTimeNanoseconds(), - Count = (ulong)summaryMetric.PopulationCount, - Sum = summaryMetric.PopulationSum, - }; - - // TODO: Do TagEnumerationState thing. - foreach (var attribute in metric.Attributes) - { - dataPoint.Attributes.Add(attribute.ToOtlpAttribute()); - } + }; + var dataPoint = metric.ToNumberDataPoint(sumMetric.LongSum, sumMetric.Exemplars); + sum.DataPoints.Add(dataPoint); + otlpMetric.Sum = sum; + break; + } - foreach (var quantile in summaryMetric.Quantiles) - { - var quantileValue = new OtlpMetrics.SummaryDataPoint.Types.ValueAtQuantile + case MetricType.DoubleSum: { - Quantile = quantile.Quantile, - Value = quantile.Value, - }; - dataPoint.QuantileValues.Add(quantileValue); - } - - otlpMetric.Summary = summary; - } - else if (metric is IHistogramMetric histogramMetric) - { - var histogram = new OtlpMetrics.Histogram - { - AggregationTemporality = histogramMetric.IsDeltaTemporality + var sumMetric = metric as ISumMetricDouble; + var sum = new OtlpMetrics.Sum + { + IsMonotonic = sumMetric.IsMonotonic, + AggregationTemporality = sumMetric.IsDeltaTemporality ? OtlpMetrics.AggregationTemporality.Delta : OtlpMetrics.AggregationTemporality.Cumulative, - }; - - var dataPoint = new OtlpMetrics.HistogramDataPoint - { - StartTimeUnixNano = (ulong)metric.StartTimeExclusive.ToUnixTimeNanoseconds(), - TimeUnixNano = (ulong)metric.EndTimeInclusive.ToUnixTimeNanoseconds(), - Count = (ulong)histogramMetric.PopulationCount, - Sum = histogramMetric.PopulationSum, - }; - - foreach (var bucket in histogramMetric.Buckets) - { - dataPoint.BucketCounts.Add((ulong)bucket.Count); + }; + var dataPoint = metric.ToNumberDataPoint(sumMetric.DoubleSum, sumMetric.Exemplars); + sum.DataPoints.Add(dataPoint); + otlpMetric.Sum = sum; + break; + } - // TODO: Verify how to handle the bounds. We've modeled things with - // a LowBoundary and HighBoundary. OTLP data model has modeled this - // differently: https://github.com/open-telemetry/opentelemetry-proto/blob/bacfe08d84e21fb2a779e302d12e8dfeb67e7b86/opentelemetry/proto/metrics/v1/metrics.proto#L554-L568 - dataPoint.ExplicitBounds.Add(bucket.HighBoundary); - } + case MetricType.LongGauge: + { + var gaugeMetric = metric as IGaugeMetric; + var gauge = new OtlpMetrics.Gauge(); + var dataPoint = metric.ToNumberDataPoint(gaugeMetric.LastValue.Value, gaugeMetric.Exemplars); + gauge.DataPoints.Add(dataPoint); + otlpMetric.Gauge = gauge; + break; + } - // TODO: Do TagEnumerationState thing. - foreach (var attribute in metric.Attributes) - { - dataPoint.Attributes.Add(attribute.ToOtlpAttribute()); - } + case MetricType.DoubleGauge: + { + var gaugeMetric = metric as IGaugeMetric; + var gauge = new OtlpMetrics.Gauge(); + var dataPoint = metric.ToNumberDataPoint(gaugeMetric.LastValue.Value, gaugeMetric.Exemplars); + gauge.DataPoints.Add(dataPoint); + otlpMetric.Gauge = gauge; + break; + } - foreach (var exemplar in histogramMetric.Exemplars) - { - dataPoint.Exemplars.Add(exemplar.ToOtlpExemplar()); - } + case MetricType.Histogram: + { + var histogramMetric = metric as IHistogramMetric; + var histogram = new OtlpMetrics.Histogram + { + AggregationTemporality = histogramMetric.IsDeltaTemporality + ? OtlpMetrics.AggregationTemporality.Delta + : OtlpMetrics.AggregationTemporality.Cumulative, + }; + + var dataPoint = new OtlpMetrics.HistogramDataPoint + { + StartTimeUnixNano = (ulong)metric.StartTimeExclusive.ToUnixTimeNanoseconds(), + TimeUnixNano = (ulong)metric.EndTimeInclusive.ToUnixTimeNanoseconds(), + Count = (ulong)histogramMetric.PopulationCount, + Sum = histogramMetric.PopulationSum, + }; + + foreach (var bucket in histogramMetric.Buckets) + { + dataPoint.BucketCounts.Add((ulong)bucket.Count); + + // TODO: Verify how to handle the bounds. We've modeled things with + // a LowBoundary and HighBoundary. OTLP data model has modeled this + // differently: https://github.com/open-telemetry/opentelemetry-proto/blob/bacfe08d84e21fb2a779e302d12e8dfeb67e7b86/opentelemetry/proto/metrics/v1/metrics.proto#L554-L568 + dataPoint.ExplicitBounds.Add(bucket.HighBoundary); + } + + // TODO: Do TagEnumerationState thing. + foreach (var attribute in metric.Attributes) + { + dataPoint.Attributes.Add(attribute.ToOtlpAttribute()); + } + + foreach (var exemplar in histogramMetric.Exemplars) + { + dataPoint.Exemplars.Add(exemplar.ToOtlpExemplar()); + } + + otlpMetric.Histogram = histogram; + break; + } - otlpMetric.Histogram = histogram; + case MetricType.Summary: + { + var summaryMetric = metric as ISummaryMetric; + var summary = new OtlpMetrics.Summary(); + + var dataPoint = new OtlpMetrics.SummaryDataPoint + { + StartTimeUnixNano = (ulong)metric.StartTimeExclusive.ToUnixTimeNanoseconds(), + TimeUnixNano = (ulong)metric.EndTimeInclusive.ToUnixTimeNanoseconds(), + Count = (ulong)summaryMetric.PopulationCount, + Sum = summaryMetric.PopulationSum, + }; + + // TODO: Do TagEnumerationState thing. + foreach (var attribute in metric.Attributes) + { + dataPoint.Attributes.Add(attribute.ToOtlpAttribute()); + } + + foreach (var quantile in summaryMetric.Quantiles) + { + var quantileValue = new OtlpMetrics.SummaryDataPoint.Types.ValueAtQuantile + { + Quantile = quantile.Quantile, + Value = quantile.Value, + }; + dataPoint.QuantileValues.Add(quantileValue); + } + + otlpMetric.Summary = summary; + break; + } } return otlpMetric; diff --git a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterExtensions.cs b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterExtensions.cs index 675acb6c55..c2ddc69fd0 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterExtensions.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterExtensions.cs @@ -50,16 +50,14 @@ public static void WriteMetricsCollection(this PrometheusExporter exporter, Stre .WithName(metric.Name) .WithDescription(metric.Name); - if (metric is ISumMetric sumMetric) + // TODO: Use switch case for higher perf. + if (metric.MetricType == MetricType.LongSum) { - if (sumMetric.Sum.Value is double doubleSum) - { - WriteSum(writer, builder, metric.Attributes, doubleSum); - } - else if (sumMetric.Sum.Value is long longSum) - { - WriteSum(writer, builder, metric.Attributes, longSum); - } + WriteSum(writer, builder, metric.Attributes, (metric as ISumMetricLong).LongSum); + } + else if (metric.MetricType == MetricType.DoubleSum) + { + WriteSum(writer, builder, metric.Attributes, (metric as ISumMetricDouble).DoubleSum); } } } diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 20aa4c8760..e749e13fdc 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -51,9 +51,20 @@ internal IAggregator[] MapToMetrics(string[] seqKey, object[] seqVal) var dt = DateTimeOffset.UtcNow; // TODO: Need to map each instrument to metrics (based on View API) - if (this.instrument.GetType().Name.Contains("Counter")) + // TODO: move most of this logic out of hotpath, and to MeterProvider's + // InstrumentPublished event, which is once per instrument creation. + + if (this.instrument.GetType() == typeof(Counter) + || this.instrument.GetType() == typeof(Counter) + || this.instrument.GetType() == typeof(Counter) + || this.instrument.GetType() == typeof(Counter)) + { + aggregators.Add(new SumMetricAggregatorLong(this.instrument.Name, this.instrument.Description, this.instrument.Unit, this.instrument.Meter, dt, tags)); + } + else if (this.instrument.GetType() == typeof(Counter) + || this.instrument.GetType() == typeof(Counter)) { - aggregators.Add(new SumMetricAggregator(this.instrument.Name, this.instrument.Description, this.instrument.Unit, this.instrument.Meter, dt, tags)); + aggregators.Add(new SumMetricAggregatorDouble(this.instrument.Name, this.instrument.Description, this.instrument.Unit, this.instrument.Meter, dt, tags)); } else if (this.instrument.GetType().Name.Contains("Gauge")) { diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs index 67eadd8286..2a7848a2b5 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs @@ -33,6 +33,9 @@ internal GaugeMetricAggregator(string name, string description, string unit, Met this.Meter = meter; this.StartTimeExclusive = startTimeExclusive; this.Attributes = attributes; + + // TODO: Split this class into two or leverage generic + this.MetricType = MetricType.LongGauge; } public string Name { get; private set; } @@ -53,6 +56,8 @@ internal GaugeMetricAggregator(string name, string description, string unit, Met public IDataValue LastValue => this.value; + public MetricType MetricType { get; private set; } + public void Update(T value) where T : struct { diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/HistogramMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/HistogramMetricAggregator.cs index 6fc3b55675..0f1c79aec4 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/HistogramMetricAggregator.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/HistogramMetricAggregator.cs @@ -50,6 +50,7 @@ internal HistogramMetricAggregator(string name, string description, string unit, this.boundaries = boundaries; this.buckets = this.InitializeBucket(boundaries); + this.MetricType = MetricType.Summary; } public string Name { get; private set; } @@ -74,6 +75,8 @@ internal HistogramMetricAggregator(string name, string description, string unit, public double PopulationSum { get; private set; } + public MetricType MetricType { get; private set; } + public IEnumerable Buckets => this.buckets; public void Update(T value) diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/IMetric.cs b/src/OpenTelemetry/Metrics/MetricAggregators/IMetric.cs index f77a159985..079172dd6c 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/IMetric.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/IMetric.cs @@ -36,6 +36,8 @@ public interface IMetric KeyValuePair[] Attributes { get; } + MetricType MetricType { get; } + string ToDisplayString(); } } diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs index 4fc7c55fff..36d2f4f648 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs @@ -25,7 +25,5 @@ public interface ISumMetric : IMetric bool IsMonotonic { get; } IEnumerable Exemplars { get; } - - IDataValue Sum { get; } } } diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricDouble.cs b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricDouble.cs new file mode 100644 index 0000000000..5b04e1d9db --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricDouble.cs @@ -0,0 +1,23 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +namespace OpenTelemetry.Metrics +{ + public interface ISumMetricDouble : ISumMetric + { + double DoubleSum { get; } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricLong.cs b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricLong.cs new file mode 100644 index 0000000000..033956fdf8 --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetricLong.cs @@ -0,0 +1,23 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +namespace OpenTelemetry.Metrics +{ + public interface ISumMetricLong : ISumMetric + { + long LongSum { get; } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/MetricType.cs b/src/OpenTelemetry/Metrics/MetricAggregators/MetricType.cs new file mode 100644 index 0000000000..c5af3b5d6c --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/MetricType.cs @@ -0,0 +1,51 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +namespace OpenTelemetry.Metrics +{ + public enum MetricType + { + /// + /// Sum of Long type. + /// + LongSum = 0, + + /// + /// Sum of Double type. + /// + DoubleSum = 1, + + /// + /// Gauge of Long type. + /// + LongGauge = 2, + + /// + /// Gauge of Double type. + /// + DoubleGauge = 3, + + /// + /// Histogram. + /// + Histogram = 4, + + /// + /// Summary. + /// + Summary = 5, + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs deleted file mode 100644 index eea048b519..0000000000 --- a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs +++ /dev/null @@ -1,147 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System; -using System.Collections.Generic; -using System.Diagnostics.Metrics; - -namespace OpenTelemetry.Metrics -{ - internal class SumMetricAggregator : ISumMetric, IAggregator - { - private readonly object lockUpdate = new object(); - private Type valueType; - private long sumLong = 0; - private double sumDouble = 0; - - internal SumMetricAggregator(string name, string description, string unit, Meter meter, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) - { - this.Name = name; - this.Description = description; - this.Unit = unit; - this.Meter = meter; - this.StartTimeExclusive = startTimeExclusive; - this.Attributes = attributes; - this.IsMonotonic = true; - } - - public string Name { get; private set; } - - public string Description { get; private set; } - - public string Unit { get; private set; } - - public Meter Meter { get; private set; } - - public DateTimeOffset StartTimeExclusive { get; private set; } - - public DateTimeOffset EndTimeInclusive { get; private set; } - - public KeyValuePair[] Attributes { get; private set; } - - public bool IsDeltaTemporality { get; private set; } - - public bool IsMonotonic { get; } - - public IEnumerable Exemplars { get; private set; } = new List(); - - public IDataValue Sum - { - get - { - if (this.valueType == typeof(long)) - { - return new DataValue(this.sumLong); - } - else if (this.valueType == typeof(double)) - { - return new DataValue(this.sumDouble); - } - - throw new Exception("Unsupported Type"); - } - } - - public void Update(T value) - where T : struct - { - lock (this.lockUpdate) - { - if (typeof(T) == typeof(long)) - { - this.valueType = typeof(T); - var val = (long)(object)value; - if (val < 0) - { - // TODO: log? - // Also, this validation can be done in earlier stage. - } - else - { - this.sumLong += val; - } - } - else if (typeof(T) == typeof(double)) - { - this.valueType = typeof(T); - var val = (double)(object)value; - if (val < 0) - { - // TODO: log? - // Also, this validation can be done in earlier stage. - } - else - { - this.sumDouble += val; - } - } - else - { - throw new Exception("Unsupported Type"); - } - } - } - - public IMetric Collect(DateTimeOffset dt, bool isDelta) - { - var cloneItem = new SumMetricAggregator(this.Name, this.Description, this.Unit, this.Meter, this.StartTimeExclusive, this.Attributes); - - lock (this.lockUpdate) - { - cloneItem.Exemplars = this.Exemplars; - cloneItem.EndTimeInclusive = dt; - cloneItem.valueType = this.valueType; - cloneItem.sumLong = this.sumLong; - cloneItem.sumDouble = this.sumDouble; - cloneItem.IsDeltaTemporality = isDelta; - - if (isDelta) - { - this.StartTimeExclusive = dt; - this.sumLong = 0; - this.sumDouble = 0; - } - } - - return cloneItem; - } - - public string ToDisplayString() - { - return $"Sum={this.Sum.Value}"; - } - } -} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorDouble.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorDouble.cs new file mode 100644 index 0000000000..d266973c3a --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorDouble.cs @@ -0,0 +1,93 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics.Metrics; + +namespace OpenTelemetry.Metrics +{ + internal class SumMetricAggregatorDouble : IAggregator + { + private readonly object lockUpdate = new object(); + private double sumDouble = 0; + private SumMetricDouble sumMetricDouble; + private DateTimeOffset startTimeExclusive; + + internal SumMetricAggregatorDouble(string name, string description, string unit, Meter meter, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) + { + this.startTimeExclusive = startTimeExclusive; + this.sumMetricDouble = new SumMetricDouble(name, description, unit, meter, startTimeExclusive, attributes); + } + + public void Update(T value) + where T : struct + { + // TODO: Replace Lock with + // TryAdd..{Spin..TryAdd..Repeat} if "lost race to another thread" + lock (this.lockUpdate) + { + if (typeof(T) == typeof(double)) + { + // TODO: Confirm this doesn't cause boxing. + var val = (double)(object)value; + if (val < 0) + { + // TODO: log? + // Also, this validation can be done in earlier stage. + } + else + { + this.sumDouble += val; + } + } + else + { + throw new Exception("Unsupported Type"); + } + } + } + + public IMetric Collect(DateTimeOffset dt, bool isDelta) + { + lock (this.lockUpdate) + { + this.sumMetricDouble.StartTimeExclusive = this.startTimeExclusive; + this.sumMetricDouble.EndTimeInclusive = dt; + this.sumMetricDouble.DoubleSum = this.sumDouble; + this.sumMetricDouble.IsDeltaTemporality = isDelta; + if (isDelta) + { + this.startTimeExclusive = dt; + this.sumDouble = 0; + } + } + + // TODO: Confirm that this approach of + // re-using the same instance is correct. + // This avoids allocating a new instance. + // It is read only for Exporters, + // and also there is no parallel + // Collect allowed. + return this.sumMetricDouble; + } + + public string ToDisplayString() + { + return $"Sum={this.sumDouble}"; + } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorLong.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorLong.cs new file mode 100644 index 0000000000..048cbc93a2 --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregatorLong.cs @@ -0,0 +1,92 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics.Metrics; + +namespace OpenTelemetry.Metrics +{ + internal class SumMetricAggregatorLong : IAggregator + { + private readonly object lockUpdate = new object(); + private long sumLong = 0; + private SumMetricLong sumMetricLong; + private DateTimeOffset startTimeExclusive; + + internal SumMetricAggregatorLong(string name, string description, string unit, Meter meter, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) + { + this.startTimeExclusive = startTimeExclusive; + this.sumMetricLong = new SumMetricLong(name, description, unit, meter, startTimeExclusive, attributes); + } + + public void Update(T value) + where T : struct + { + // TODO: Replace Lock with Interlocked.Add + lock (this.lockUpdate) + { + if (typeof(T) == typeof(long)) + { + // TODO: Confirm this doesn't cause boxing. + var val = (long)(object)value; + if (val < 0) + { + // TODO: log? + // Also, this validation can be done in earlier stage. + } + else + { + this.sumLong += val; + } + } + else + { + throw new Exception("Unsupported Type"); + } + } + } + + public IMetric Collect(DateTimeOffset dt, bool isDelta) + { + lock (this.lockUpdate) + { + this.sumMetricLong.StartTimeExclusive = this.startTimeExclusive; + this.sumMetricLong.EndTimeInclusive = dt; + this.sumMetricLong.LongSum = this.sumLong; + this.sumMetricLong.IsDeltaTemporality = isDelta; + if (isDelta) + { + this.startTimeExclusive = dt; + this.sumLong = 0; + } + } + + // TODO: Confirm that this approach of + // re-using the same instance is correct. + // This avoids allocating a new instance. + // It is read only for Exporters, + // and also there is no parallel + // Collect allowed. + return this.sumMetricLong; + } + + public string ToDisplayString() + { + return $"Sum={this.sumLong}"; + } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricDouble.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricDouble.cs new file mode 100644 index 0000000000..3b5486ad6d --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricDouble.cs @@ -0,0 +1,66 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics.Metrics; + +namespace OpenTelemetry.Metrics +{ + internal class SumMetricDouble : ISumMetricDouble + { + internal SumMetricDouble(string name, string description, string unit, Meter meter, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) + { + this.Name = name; + this.Description = description; + this.Unit = unit; + this.Meter = meter; + this.StartTimeExclusive = startTimeExclusive; + this.Attributes = attributes; + this.IsMonotonic = true; + this.MetricType = MetricType.DoubleSum; + } + + public string Name { get; private set; } + + public string Description { get; private set; } + + public string Unit { get; private set; } + + public Meter Meter { get; private set; } + + public DateTimeOffset StartTimeExclusive { get; internal set; } + + public DateTimeOffset EndTimeInclusive { get; internal set; } + + public KeyValuePair[] Attributes { get; private set; } + + public bool IsDeltaTemporality { get; internal set; } + + public bool IsMonotonic { get; } + + public IEnumerable Exemplars { get; private set; } = new List(); + + public double DoubleSum { get; internal set; } + + public MetricType MetricType { get; private set; } + + public string ToDisplayString() + { + return $"Sum={this.DoubleSum}"; + } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricLong.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricLong.cs new file mode 100644 index 0000000000..1398cb694a --- /dev/null +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricLong.cs @@ -0,0 +1,66 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics.Metrics; + +namespace OpenTelemetry.Metrics +{ + internal class SumMetricLong : ISumMetricLong + { + internal SumMetricLong(string name, string description, string unit, Meter meter, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) + { + this.Name = name; + this.Description = description; + this.Unit = unit; + this.Meter = meter; + this.StartTimeExclusive = startTimeExclusive; + this.Attributes = attributes; + this.IsMonotonic = true; + this.MetricType = MetricType.LongSum; + } + + public string Name { get; private set; } + + public string Description { get; private set; } + + public string Unit { get; private set; } + + public Meter Meter { get; private set; } + + public DateTimeOffset StartTimeExclusive { get; internal set; } + + public DateTimeOffset EndTimeInclusive { get; internal set; } + + public KeyValuePair[] Attributes { get; private set; } + + public bool IsDeltaTemporality { get; internal set; } + + public bool IsMonotonic { get; } + + public IEnumerable Exemplars { get; private set; } = new List(); + + public long LongSum { get; internal set; } + + public MetricType MetricType { get; private set; } + + public string ToDisplayString() + { + return $"Sum={this.LongSum}"; + } + } +} diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SummaryMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SummaryMetricAggregator.cs index c06874a3c1..0e7fe40063 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/SummaryMetricAggregator.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SummaryMetricAggregator.cs @@ -35,6 +35,7 @@ internal SummaryMetricAggregator(string name, string description, string unit, M this.StartTimeExclusive = startTimeExclusive; this.Attributes = attributes; this.IsMonotonic = isMonotonic; + this.MetricType = MetricType.Summary; } public string Name { get; private set; } @@ -59,6 +60,8 @@ internal SummaryMetricAggregator(string name, string description, string unit, M public IEnumerable Quantiles => this.quantiles; + public MetricType MetricType { get; private set; } + public void Update(T value) where T : struct { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 8dab7611f6..10f0e6b018 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -64,9 +64,10 @@ void ProcessExport(Batch batch) } } + var processor = new PullMetricProcessor(metricExporter, true); this.meterProvider = Sdk.CreateMeterProviderBuilder() .AddAspNetCoreInstrumentation() - .AddMetricProcessor(new PushMetricProcessor(exporter: metricExporter, exportIntervalMs: 10, isDelta: true)) + .AddMetricProcessor(processor) .Build(); using (var client = this.factory.CreateClient()) @@ -75,22 +76,20 @@ void ProcessExport(Batch batch) response.EnsureSuccessStatusCode(); } - // Wait for at least two exporter invocations - WaitForMetricItems(metricItems, 2); + // Invokes the TestExporter which will invoke ProcessExport + processor.PullRequest(); this.meterProvider.Dispose(); - // There should be more than one result here since we waited for at least two exporter invocations. - // The exporter continues to receive a metric even if it has not changed since the last export. var requestMetrics = metricItems .SelectMany(item => item.Metrics.Where(metric => metric.Name == "http.server.request_count")) .ToArray(); - Assert.True(requestMetrics.Length > 1); + Assert.True(requestMetrics.Length == 1); - var metric = requestMetrics[0] as ISumMetric; + var metric = requestMetrics[0] as ISumMetricLong; Assert.NotNull(metric); - Assert.Equal(1L, metric.Sum.Value); + Assert.Equal(1L, metric.LongSum); var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, "GET"); var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); @@ -101,13 +100,6 @@ void ProcessExport(Batch batch) Assert.Contains(statusCode, metric.Attributes); Assert.Contains(flavor, metric.Attributes); Assert.Equal(4, metric.Attributes.Length); - - for (var i = 1; i < requestMetrics.Length; ++i) - { - metric = requestMetrics[i] as ISumMetric; - Assert.NotNull(metric); - Assert.Equal(0L, metric.Sum.Value); - } } public void Dispose() diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 1f0d8424d6..425576bd40 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -16,18 +16,26 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.Metrics; using System.Threading; using OpenTelemetry.Tests; using Xunit; +using Xunit.Abstractions; namespace OpenTelemetry.Metrics.Tests { public class MetricApiTest { - private static int numberOfThreads = 10; + private static int numberOfThreads = Environment.ProcessorCount; private static long deltaValueUpdatedByEachCall = 10; - private static int numberOfMetricUpdateByEachThread = 1000000; + private static int numberOfMetricUpdateByEachThread = 100000; + private readonly ITestOutputHelper output; + + public MetricApiTest(ITestOutputHelper output) + { + this.output = output; + } [Fact] public void SimpleTest() @@ -42,11 +50,13 @@ void ProcessExport(Batch batch) } } + var pullProcessor = new PullMetricProcessor(metricExporter, true); + var meter = new Meter("TestMeter"); var counterLong = meter.CreateCounter("mycounter"); var meterProvider = Sdk.CreateMeterProviderBuilder() .AddSource("TestMeter") - .AddMetricProcessor(new PushMetricProcessor(metricExporter, 100, isDelta: true)) + .AddMetricProcessor(pullProcessor) .Build(); // setup args to threads. @@ -69,6 +79,9 @@ void ProcessExport(Batch batch) // Block until all threads started. mreToEnsureAllThreadsStarted.WaitOne(); + Stopwatch sw = new Stopwatch(); + sw.Start(); + // unblock all the threads. // (i.e let them start counter.Add) mreToBlockUpdateThreads.Set(); @@ -79,11 +92,11 @@ void ProcessExport(Batch batch) t[i].Join(); } - meterProvider.Dispose(); + var timeTakenInMilliseconds = sw.ElapsedMilliseconds; + this.output.WriteLine($"Took {timeTakenInMilliseconds} msecs. Total threads: {numberOfThreads}, each thread doing {numberOfMetricUpdateByEachThread} recordings."); - // TODO: Once Dispose does flush, we may not need this - // unknown sleep below. - Thread.Sleep(1000); + meterProvider.Dispose(); + pullProcessor.PullRequest(); long sumReceived = 0; foreach (var metricItem in metricItems) @@ -91,7 +104,7 @@ void ProcessExport(Batch batch) var metrics = metricItem.Metrics; foreach (var metric in metrics) { - sumReceived += (long)(metric as ISumMetric).Sum.Value; + sumReceived += (metric as ISumMetricLong).LongSum; } }