From 3fd02dbe74962399902a9fd3c15a562fb2f93258 Mon Sep 17 00:00:00 2001 From: Victor Lu Date: Fri, 18 Jun 2021 15:12:32 -0700 Subject: [PATCH 1/2] Refactor for DataValue --- .../Metrics/DataPoint/DataPoint.cs | 66 ++++------------- .../Metrics/DataPoint/DataPoint{T}.cs | 16 ++++- .../Metrics/DataPoint/DataValue.cs | 48 +++++++++++++ .../Metrics/DataPoint/DataValue{T}.cs | 31 ++++++++ .../Metrics/DataPoint/Exemplar.cs | 70 +++++-------------- .../Metrics/DataPoint/Exemplar{T}.cs | 18 ++++- .../Metrics/DataPoint/IDataPoint.cs | 4 +- .../Metrics/DataPoint/IDataValue.cs | 23 ++++++ .../Metrics/DataPoint/IExemplar.cs | 4 +- .../GaugeMetricAggregator.cs | 44 ++---------- .../Metrics/MetricAggregators/IGaugeMetric.cs | 2 +- .../Metrics/MetricAggregators/ISumMetric.cs | 2 +- .../MetricAggregators/SumMetricAggregator.cs | 20 ++++-- .../Processors/MetricConsoleExporter.cs | 6 +- 14 files changed, 190 insertions(+), 164 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/DataPoint/DataValue.cs create mode 100644 src/OpenTelemetry/Metrics/DataPoint/DataValue{T}.cs create mode 100644 src/OpenTelemetry/Metrics/DataPoint/IDataValue.cs diff --git a/src/OpenTelemetry/Metrics/DataPoint/DataPoint.cs b/src/OpenTelemetry/Metrics/DataPoint/DataPoint.cs index 1b59f5e6d82..83fa90612fc 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/DataPoint.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/DataPoint.cs @@ -23,26 +23,27 @@ namespace OpenTelemetry.Metrics { private static readonly KeyValuePair[] EmptyTag = new KeyValuePair[0]; - private readonly Type valueType; - private readonly long longValue; - private readonly double doubleValue; + private readonly IDataValue value; internal DataPoint(DateTimeOffset timestamp, long value, KeyValuePair[] tags) { this.Timestamp = timestamp; this.Tags = tags; - this.valueType = typeof(long); - this.longValue = value; - this.doubleValue = 0; + this.value = new DataValue(value); } internal DataPoint(DateTimeOffset timestamp, double value, KeyValuePair[] tags) { this.Timestamp = timestamp; this.Tags = tags; - this.valueType = typeof(double); - this.longValue = 0; - this.doubleValue = value; + this.value = new DataValue(value); + } + + internal DataPoint(DateTimeOffset timestamp, IDataValue value, KeyValuePair[] tags) + { + this.Timestamp = timestamp; + this.Tags = tags; + this.value = value; } internal DataPoint(DateTimeOffset timestamp, long value) @@ -55,52 +56,15 @@ internal DataPoint(DateTimeOffset timestamp, double value) { } - public DateTimeOffset Timestamp { get; } - - public readonly KeyValuePair[] Tags { get; } - - public object Value + internal DataPoint(DateTimeOffset timestamp, IDataValue value) + : this(timestamp, value, DataPoint.EmptyTag) { - get - { - if (this.valueType == typeof(long)) - { - return this.longValue; - } - else if (this.valueType == typeof(double)) - { - return this.doubleValue; - } - else - { - throw new Exception("Unsupported Type"); - } - } } - internal static DataPoint CreateDataPoint(DateTimeOffset timestamp, T value, KeyValuePair[] tags) - { - DataPoint dp; + public DateTimeOffset Timestamp { get; } - if (typeof(T) == typeof(int)) - { - // Promoted to Long - dp = new DataPoint(timestamp, (int)(object)value, tags); - } - else if (typeof(T) == typeof(long)) - { - dp = new DataPoint(timestamp, (long)(object)value, tags); - } - else if (typeof(T) == typeof(double)) - { - dp = new DataPoint(timestamp, (double)(object)value, tags); - } - else - { - throw new Exception("Unsupported Type"); - } + public readonly KeyValuePair[] Tags { get; } - return dp; - } + public object Value => this.value.Value; } } diff --git a/src/OpenTelemetry/Metrics/DataPoint/DataPoint{T}.cs b/src/OpenTelemetry/Metrics/DataPoint/DataPoint{T}.cs index 3885283e50d..e7b895d54c6 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/DataPoint{T}.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/DataPoint{T}.cs @@ -24,9 +24,16 @@ namespace OpenTelemetry.Metrics { private static readonly KeyValuePair[] EmptyTag = new KeyValuePair[0]; - private readonly T value; + private readonly IDataValue value; internal DataPoint(DateTimeOffset timestamp, T value, KeyValuePair[] tags) + { + this.Timestamp = timestamp; + this.Tags = tags; + this.value = new DataValue(value); + } + + internal DataPoint(DateTimeOffset timestamp, IDataValue value, KeyValuePair[] tags) { this.Timestamp = timestamp; this.Tags = tags; @@ -38,10 +45,15 @@ internal DataPoint(DateTimeOffset timestamp, T value) { } + internal DataPoint(DateTimeOffset timestamp, IDataValue value) + : this(timestamp, value, DataPoint.EmptyTag) + { + } + public DateTimeOffset Timestamp { get; } public readonly KeyValuePair[] Tags { get; } - public object Value => (object)this.value; + public object Value => this.value.Value; } } diff --git a/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs b/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs new file mode 100644 index 00000000000..33fcafc3f1b --- /dev/null +++ b/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs @@ -0,0 +1,48 @@ +// +// 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; + +namespace OpenTelemetry.Metrics +{ + public readonly struct DataValue : IDataValue + { + private readonly IDataValue value; + + internal DataValue(int value) + { + // Promote to long + this.value = new DataValue(value); + } + + internal DataValue(long value) + { + this.value = new DataValue(value); + } + + internal DataValue(double value) + { + this.value = new DataValue(value); + } + + internal DataValue(IDataValue value) + { + this.value = value; + } + + public object Value => this.value.Value; + } +} diff --git a/src/OpenTelemetry/Metrics/DataPoint/DataValue{T}.cs b/src/OpenTelemetry/Metrics/DataPoint/DataValue{T}.cs new file mode 100644 index 00000000000..d3733e08f82 --- /dev/null +++ b/src/OpenTelemetry/Metrics/DataPoint/DataValue{T}.cs @@ -0,0 +1,31 @@ +// +// 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 +{ + internal readonly struct DataValue : IDataValue + where T : struct + { + private readonly T value; + + internal DataValue(T value) + { + this.value = value; + } + + public object Value => (object)this.value; + } +} diff --git a/src/OpenTelemetry/Metrics/DataPoint/Exemplar.cs b/src/OpenTelemetry/Metrics/DataPoint/Exemplar.cs index 8d9e8590d82..4fba9dd4700 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/Exemplar.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/Exemplar.cs @@ -24,9 +24,7 @@ namespace OpenTelemetry.Metrics private static readonly KeyValuePair[] EmptyTag = new KeyValuePair[0]; private static readonly byte[] EmptyId = new byte[0]; - private readonly Type valueType; - private readonly long longValue; - private readonly double doubleValue; + private readonly IDataValue value; internal Exemplar(DateTimeOffset timestamp, long value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) { @@ -34,9 +32,7 @@ internal Exemplar(DateTimeOffset timestamp, long value, byte[] spanId, byte[] tr this.FilteredTags = filteredTags; this.SpanId = spanId; this.TraceId = traceId; - this.valueType = typeof(long); - this.longValue = value; - this.doubleValue = 0; + this.value = new DataValue(value); } internal Exemplar(DateTimeOffset timestamp, double value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) @@ -45,9 +41,16 @@ internal Exemplar(DateTimeOffset timestamp, double value, byte[] spanId, byte[] this.FilteredTags = filteredTags; this.SpanId = spanId; this.TraceId = traceId; - this.valueType = typeof(double); - this.longValue = 0; - this.doubleValue = value; + this.value = new DataValue(value); + } + + internal Exemplar(DateTimeOffset timestamp, IDataValue value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) + { + this.Timestamp = timestamp; + this.FilteredTags = filteredTags; + this.SpanId = spanId; + this.TraceId = traceId; + this.value = value; } internal Exemplar(DateTimeOffset timestamp, long value) @@ -60,6 +63,11 @@ internal Exemplar(DateTimeOffset timestamp, double value) { } + internal Exemplar(DateTimeOffset timestamp, IDataValue value) + : this(timestamp, value, Exemplar.EmptyId, Exemplar.EmptyId, Exemplar.EmptyTag) + { + } + public DateTimeOffset Timestamp { get; } public readonly KeyValuePair[] FilteredTags { get; } @@ -68,48 +76,6 @@ internal Exemplar(DateTimeOffset timestamp, double value) public readonly byte[] TraceId { get; } - public object Value - { - get - { - if (this.valueType == typeof(long)) - { - return this.longValue; - } - else if (this.valueType == typeof(double)) - { - return this.doubleValue; - } - else - { - throw new Exception("Unsupported Type"); - } - } - } - - internal static Exemplar CreateExemplar(DateTimeOffset timestamp, T value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) - { - Exemplar dp; - - if (typeof(T) == typeof(int)) - { - // Promoted to Long - dp = new Exemplar(timestamp, (int)(object)value, spanId, traceId, filteredTags); - } - else if (typeof(T) == typeof(long)) - { - dp = new Exemplar(timestamp, (long)(object)value, spanId, traceId, filteredTags); - } - else if (typeof(T) == typeof(double)) - { - dp = new Exemplar(timestamp, (double)(object)value, spanId, traceId, filteredTags); - } - else - { - throw new Exception("Unsupported Type"); - } - - return dp; - } + public object Value => this.value.Value; } } diff --git a/src/OpenTelemetry/Metrics/DataPoint/Exemplar{T}.cs b/src/OpenTelemetry/Metrics/DataPoint/Exemplar{T}.cs index 50ab08f4b66..000e7bce7d9 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/Exemplar{T}.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/Exemplar{T}.cs @@ -25,9 +25,18 @@ namespace OpenTelemetry.Metrics private static readonly KeyValuePair[] EmptyTag = new KeyValuePair[0]; private static readonly byte[] EmptyId = new byte[0]; - private readonly T value; + private readonly IDataValue value; internal Exemplar(DateTimeOffset timestamp, T value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) + { + this.Timestamp = timestamp; + this.FilteredTags = filteredTags; + this.SpanId = spanId; + this.TraceId = traceId; + this.value = new DataValue(value); + } + + internal Exemplar(DateTimeOffset timestamp, IDataValue value, byte[] spanId, byte[] traceId, KeyValuePair[] filteredTags) { this.Timestamp = timestamp; this.FilteredTags = filteredTags; @@ -41,6 +50,11 @@ internal Exemplar(DateTimeOffset timestamp, T value) { } + internal Exemplar(DateTimeOffset timestamp, IDataValue value) + : this(timestamp, value, Exemplar.EmptyId, Exemplar.EmptyId, Exemplar.EmptyTag) + { + } + public DateTimeOffset Timestamp { get; } public readonly KeyValuePair[] FilteredTags { get; } @@ -49,6 +63,6 @@ internal Exemplar(DateTimeOffset timestamp, T value) public readonly byte[] TraceId { get; } - public object Value => (object)this.value; + public object Value => this.value.Value; } } diff --git a/src/OpenTelemetry/Metrics/DataPoint/IDataPoint.cs b/src/OpenTelemetry/Metrics/DataPoint/IDataPoint.cs index 943592f7192..4f0892795dc 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/IDataPoint.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/IDataPoint.cs @@ -19,12 +19,10 @@ namespace OpenTelemetry.Metrics { - internal interface IDataPoint + public interface IDataPoint : IDataValue { DateTimeOffset Timestamp { get; } KeyValuePair[] Tags { get; } - - object Value { get; } } } diff --git a/src/OpenTelemetry/Metrics/DataPoint/IDataValue.cs b/src/OpenTelemetry/Metrics/DataPoint/IDataValue.cs new file mode 100644 index 00000000000..62503b58ef7 --- /dev/null +++ b/src/OpenTelemetry/Metrics/DataPoint/IDataValue.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 IDataValue + { + object Value { get; } + } +} diff --git a/src/OpenTelemetry/Metrics/DataPoint/IExemplar.cs b/src/OpenTelemetry/Metrics/DataPoint/IExemplar.cs index 3c0bf730f79..f51f9635706 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/IExemplar.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/IExemplar.cs @@ -19,7 +19,7 @@ namespace OpenTelemetry.Metrics { - internal interface IExemplar + public interface IExemplar : IDataValue { DateTimeOffset Timestamp { get; } @@ -28,7 +28,5 @@ internal interface IExemplar byte[] SpanId { get; } byte[] TraceId { get; } - - object Value { get; } } } diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs index 8bf0fbfb582..b57f9648880 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/GaugeMetricAggregator.cs @@ -22,9 +22,7 @@ namespace OpenTelemetry.Metrics internal class GaugeMetricAggregator : IGaugeMetric, IAggregator { private readonly object lockUpdate = new object(); - private Type valueType; - private long longValue; - private double doubleValue; + private IDataValue value; internal GaugeMetricAggregator(string name, DateTimeOffset startTimeExclusive, KeyValuePair[] attributes) { @@ -44,24 +42,7 @@ internal GaugeMetricAggregator(string name, DateTimeOffset startTimeExclusive, K public IEnumerable Exemplars { get; private set; } = new List(); - public IDataPoint LastValue - { - get - { - if (this.valueType == typeof(long)) - { - return DataPoint.CreateDataPoint(this.EndTimeInclusive, this.longValue, this.Attributes); - } - else if (this.valueType == typeof(double)) - { - return DataPoint.CreateDataPoint(this.EndTimeInclusive, this.doubleValue, this.Attributes); - } - else - { - throw new Exception("Unsupported Type"); - } - } - } + public IDataValue LastValue => this.value; public void Update(DateTimeOffset dt, T value) where T : struct @@ -69,22 +50,7 @@ public void Update(DateTimeOffset dt, T value) lock (this.lockUpdate) { this.EndTimeInclusive = dt; - if (typeof(T) == typeof(int)) - { - // Promote to Long - this.valueType = typeof(long); - this.longValue = (int)(object)value; - } - else if (typeof(T) == typeof(long)) - { - this.valueType = typeof(T); - this.longValue = (long)(object)value; - } - else if (typeof(T) == typeof(double)) - { - this.valueType = typeof(T); - this.doubleValue = (double)(object)value; - } + this.value = new DataValue(value); } } @@ -96,9 +62,7 @@ public IMetric Collect(DateTimeOffset dt) { cloneItem.Exemplars = this.Exemplars; cloneItem.EndTimeInclusive = dt; - cloneItem.valueType = this.valueType; - cloneItem.longValue = this.longValue; - cloneItem.doubleValue = this.doubleValue; + cloneItem.value = this.LastValue; } return cloneItem; diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/IGaugeMetric.cs b/src/OpenTelemetry/Metrics/MetricAggregators/IGaugeMetric.cs index aaaeefaffbc..74ddbb977c1 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/IGaugeMetric.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/IGaugeMetric.cs @@ -22,6 +22,6 @@ internal interface IGaugeMetric : IMetric { IEnumerable Exemplars { get; } - IDataPoint LastValue { get; } + IDataValue LastValue { get; } } } diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs index bf6514f4c5e..71ec27c7ca9 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/ISumMetric.cs @@ -26,6 +26,6 @@ internal interface ISumMetric : IMetric IEnumerable Exemplars { get; } - object Sum { get; } + IDataValue Sum { get; } } } diff --git a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs index de2f0ca24f9..8b34263ea84 100644 --- a/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs +++ b/src/OpenTelemetry/Metrics/MetricAggregators/SumMetricAggregator.cs @@ -54,31 +54,39 @@ internal SumMetricAggregator(string name, DateTimeOffset startTimeExclusive, Key public IEnumerable Exemplars { get; private set; } = new List(); - public object Sum + public IDataValue Sum { get { if (this.valueType == typeof(long)) { + long sum; + if (this.IsMonotonic) { - return this.sumPos + (long)this.dsumPos; + sum = this.sumPos + (long)this.dsumPos; } else { - return this.sumPos + (long)this.dsumPos + this.sumNeg + (long)this.dsumNeg; + sum = this.sumPos + (long)this.dsumPos + this.sumNeg + (long)this.dsumNeg; } + + return new DataValue(sum); } else if (this.valueType == typeof(double)) { + double sum; + if (this.IsMonotonic) { - return this.dsumPos + (double)this.sumPos; + sum = this.dsumPos + (double)this.sumPos; } else { - return this.dsumPos + (double)this.sumPos + this.dsumNeg + (double)this.sumNeg; + sum = this.dsumPos + (double)this.sumPos + this.dsumNeg + (double)this.sumNeg; } + + return new DataValue(sum); } throw new Exception("Unsupported Type"); @@ -181,7 +189,7 @@ public IMetric Collect(DateTimeOffset dt) public string ToDisplayString() { - return $"Delta={this.IsDeltaTemporality},Count={this.countPos},Sum={this.Sum}"; + return $"Delta={this.IsDeltaTemporality},Mon={this.IsMonotonic},Count={this.countPos},Sum={this.Sum.Value}"; } } } diff --git a/src/OpenTelemetry/Metrics/Processors/MetricConsoleExporter.cs b/src/OpenTelemetry/Metrics/Processors/MetricConsoleExporter.cs index f9acfeea4a3..81bb318d203 100644 --- a/src/OpenTelemetry/Metrics/Processors/MetricConsoleExporter.cs +++ b/src/OpenTelemetry/Metrics/Processors/MetricConsoleExporter.cs @@ -38,11 +38,11 @@ public override void OnEnd(MetricItem data) string valueDisplay = string.Empty; if (metric is ISumMetric sumMetric) { - if (sumMetric.Sum is double doubleSum) + if (sumMetric.Sum.Value is double doubleSum) { valueDisplay = ((double)doubleSum).ToString(CultureInfo.InvariantCulture); } - else if (sumMetric.Sum is long longSum) + else if (sumMetric.Sum.Value is long longSum) { valueDisplay = ((long)longSum).ToString(); } @@ -73,7 +73,7 @@ public override void OnEnd(MetricItem data) string time = $"{metric.StartTimeExclusive.ToLocalTime().ToString("HH:mm:ss.fff")} {metric.EndTimeInclusive.ToLocalTime().ToString("HH:mm:ss.fff")}"; - var msg = $"Export[{this.name}] {time} {metric.Name} [{string.Join(";", tags)}] {kind} Value: {valueDisplay}"; + var msg = $"Export[{this.name}] {time} {metric.Name} [{string.Join(";", tags)}] {kind} Value: {valueDisplay}, Details: {metric.ToDisplayString()}"; Console.WriteLine(msg); } } From 060d29d904ae81e00b6fb91226a6d4dadeb54549 Mon Sep 17 00:00:00 2001 From: Victor Lu Date: Fri, 18 Jun 2021 17:00:15 -0700 Subject: [PATCH 2/2] Fix bug from PR feedback --- src/OpenTelemetry/Metrics/DataPoint/DataValue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs b/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs index 33fcafc3f1b..e3db6eef5a0 100644 --- a/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs +++ b/src/OpenTelemetry/Metrics/DataPoint/DataValue.cs @@ -25,7 +25,7 @@ namespace OpenTelemetry.Metrics internal DataValue(int value) { // Promote to long - this.value = new DataValue(value); + this.value = new DataValue(value); } internal DataValue(long value)