From dd040d7f8ff90eaafb325482a76d669ed31a6367 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 11:58:52 -0700 Subject: [PATCH 01/13] Add ViewId to MetricStreamConfiguration --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 4 +++- src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 725b9c0acbf..181eb2f997f 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -155,13 +155,15 @@ internal MeterProviderSdk( // There may be excess space wasted, but it'll eligible for // GC right after this method. var metricStreamConfigs = new List(viewConfigCount); - foreach (var viewConfig in this.viewConfigs) + for (var i = 0; i < viewConfigCount; ++i) { + var viewConfig = this.viewConfigs[i]; MetricStreamConfiguration metricStreamConfig = null; try { metricStreamConfig = viewConfig(instrument); + metricStreamConfig.ViewId = i; if (metricStreamConfig is ExplicitBucketHistogramConfiguration && instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>)) diff --git a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs index 4bd7a0e3a52..0bf49cba4f4 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs @@ -107,6 +107,8 @@ public string[] TagKeys internal string[] CopiedTagKeys { get; private set; } + internal int ViewId { get; set; } + // TODO: MetricPoints caps can be configured here on // a per stream basis, when we add such a capability // in the future. From 6497ef3ea6885f7af6296ee0a68e831d745b7be4 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 16:40:01 -0700 Subject: [PATCH 02/13] Refactor MetricStreamIdentity --- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 48 +++++++------------ .../Metrics/MetricStreamIdentity.cs | 21 ++++---- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index cb60edd32b4..d070c5ab839 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -38,11 +38,7 @@ public abstract partial class MetricReader internal Metric AddMetricWithNoViews(Instrument instrument) { - var meterName = instrument.Meter.Name; - var meterVersion = instrument.Meter.Version; - var metricName = instrument.Name; - var metricStreamName = $"{meterName}.{meterVersion}.{metricName}"; - var metricStreamIdentity = new MetricStreamIdentity(instrument.Meter, metricName, instrument.Unit, instrument.Description, instrument.GetType(), null, null); + var metricStreamIdentity = new MetricStreamIdentity(instrument, metricStreamConfiguration: null); lock (this.instrumentCreationLock) { if (this.instrumentIdentityToMetric.TryGetValue(metricStreamIdentity, out var existingMetric)) @@ -50,11 +46,11 @@ internal Metric AddMetricWithNoViews(Instrument instrument) return existingMetric; } - if (this.metricStreamNames.Contains(metricStreamName)) + if (this.metricStreamNames.Contains(metricStreamIdentity.MetricStreamName)) { OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( - metricName, - meterName, + metricStreamIdentity.InstrumentName, + metricStreamIdentity.MeterName, "Metric instrument has the same name as an existing one but differs by description, unit, or instrument type. Measurements from this instrument will still be exported but may result in conflicts.", "Either change the name of the instrument or use MeterProviderBuilder.AddView to resolve the conflict."); } @@ -62,7 +58,7 @@ internal Metric AddMetricWithNoViews(Instrument instrument) var index = ++this.metricIndex; if (index >= this.maxMetricStreams) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); return null; } else @@ -78,13 +74,13 @@ internal Metric AddMetricWithNoViews(Instrument instrument) // Could be improved with separate Event. // Also the message could call out what Instruments // and types (eg: int, long etc) are supported. - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "Unsupported instrument. Details: " + nse.Message, "Switch to a supported instrument type."); + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, "Unsupported instrument. Details: " + nse.Message, "Switch to a supported instrument type."); return null; } this.instrumentIdentityToMetric[metricStreamIdentity] = metric; this.metrics[index] = metric; - this.metricStreamNames.Add(metricStreamName); + this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName); return metric; } } @@ -114,21 +110,13 @@ internal List AddMetricsListWithViews(Instrument instrument, List AddMetricsListWithViews(Instrument instrument, List= this.maxMetricStreams) { - OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, "Maximum allowed Metric streams for the provider exceeded.", "Use MeterProviderBuilder.AddView to drop unused instruments. Or use MeterProviderBuilder.SetMaxMetricStreams to configure MeterProvider to allow higher limit."); } else { Metric metric; - metric = new Metric(metricStreamIdentity, this.Temporality, this.maxMetricPointsPerMetricStream, histogramBucketBounds, tagKeysInteresting); + metric = new Metric(metricStreamIdentity, this.Temporality, this.maxMetricPointsPerMetricStream, metricStreamIdentity.HistogramBucketBounds, metricStreamIdentity.TagKeys); this.instrumentIdentityToMetric[metricStreamIdentity] = metric; this.metrics[index] = metric; metrics.Add(metric); - this.metricStreamNames.Add(metricStreamName); + this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName); } } diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index 7bf9de8e92b..bc639039ac4 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -24,15 +24,17 @@ namespace OpenTelemetry.Metrics private static readonly StringArrayEqualityComparer StringArrayComparer = new StringArrayEqualityComparer(); private readonly int hashCode; - public MetricStreamIdentity(Meter meter, string instrumentName, string unit, string description, Type instrumentType, string[] tagKeys, double[] histogramBucketBounds) + public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration metricStreamConfiguration) { - this.MeterName = meter.Name; - this.MeterVersion = meter.Version ?? string.Empty; - this.InstrumentName = instrumentName; - this.Unit = unit ?? string.Empty; - this.Description = description ?? string.Empty; - this.InstrumentType = instrumentType; - + this.MeterName = instrument.Meter.Name; + this.MeterVersion = instrument.Meter.Version ?? string.Empty; + this.InstrumentName = metricStreamConfiguration?.Name ?? instrument.Name; + this.Unit = instrument.Unit ?? string.Empty; + this.Description = metricStreamConfiguration?.Description ?? instrument.Description ?? string.Empty; + this.InstrumentType = instrument.GetType(); + this.MetricStreamName = $"{this.MeterName}.{this.MeterVersion}.{this.InstrumentName}"; + + var tagKeys = metricStreamConfiguration?.CopiedTagKeys; if (tagKeys != null && tagKeys.Length > 0) { this.TagKeys = new string[tagKeys.Length]; @@ -43,6 +45,7 @@ public MetricStreamIdentity(Meter meter, string instrumentName, string unit, str this.TagKeys = null; } + var histogramBucketBounds = (metricStreamConfiguration as ExplicitBucketHistogramConfiguration)?.CopiedBoundaries; if (histogramBucketBounds != null && histogramBucketBounds.Length > 0) { this.HistogramBucketBounds = new double[histogramBucketBounds.Length]; @@ -88,6 +91,8 @@ public MetricStreamIdentity(Meter meter, string instrumentName, string unit, str public Type InstrumentType { get; } + public string MetricStreamName { get; } + public string[] TagKeys { get; } public double[] HistogramBucketBounds { get; } From 12e16fd432a04cc1b3d5680c8a57e047af7d42e5 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 16:46:40 -0700 Subject: [PATCH 03/13] Add ViewId to MetricStreamIdentity --- src/OpenTelemetry/Metrics/MetricStreamIdentity.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index bc639039ac4..288f495c254 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -32,6 +32,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration met this.Unit = instrument.Unit ?? string.Empty; this.Description = metricStreamConfiguration?.Description ?? instrument.Description ?? string.Empty; this.InstrumentType = instrument.GetType(); + this.ViewId = metricStreamConfiguration?.ViewId; this.MetricStreamName = $"{this.MeterName}.{this.MeterVersion}.{this.InstrumentName}"; var tagKeys = metricStreamConfiguration?.CopiedTagKeys; @@ -65,6 +66,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration met hash = (hash * 31) + this.InstrumentName.GetHashCode(); hash = this.Unit == null ? hash : (hash * 31) + this.Unit.GetHashCode(); hash = this.Description == null ? hash : (hash * 31) + this.Description.GetHashCode(); + hash = !this.ViewId.HasValue ? hash : (hash * 31) + this.ViewId.Value; hash = this.TagKeys == null ? hash : (hash * 31) + StringArrayComparer.GetHashCode(this.TagKeys); if (this.HistogramBucketBounds != null) { @@ -91,6 +93,8 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration met public Type InstrumentType { get; } + public int? ViewId { get; } + public string MetricStreamName { get; } public string[] TagKeys { get; } @@ -114,6 +118,7 @@ public bool Equals(MetricStreamIdentity other) && this.InstrumentName == other.InstrumentName && this.Unit == other.Unit && this.Description == other.Description + && this.ViewId == other.ViewId && StringArrayComparer.Equals(this.TagKeys, other.TagKeys) && HistogramBoundsEqual(this.HistogramBucketBounds, other.HistogramBucketBounds); } From 3042f1c33a23c23498d669d74fb0fd62107d878f Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 17:06:53 -0700 Subject: [PATCH 04/13] Fix tag keys --- src/OpenTelemetry/Metrics/MetricStreamIdentity.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index 288f495c254..f7dcd9eb86e 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -36,7 +36,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration met this.MetricStreamName = $"{this.MeterName}.{this.MeterVersion}.{this.InstrumentName}"; var tagKeys = metricStreamConfiguration?.CopiedTagKeys; - if (tagKeys != null && tagKeys.Length > 0) + if (tagKeys != null) { this.TagKeys = new string[tagKeys.Length]; tagKeys.CopyTo(this.TagKeys, 0); From 0fc2cefdd3363bba65f4af9f7d5799a92ad2256f Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 17:07:15 -0700 Subject: [PATCH 05/13] Modify tests to match new expectations --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 10 ++++++++-- test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs | 10 +++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 42701395cfc..394a4a8d217 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -551,13 +551,19 @@ public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_Tw meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Single(exportedItems); + Assert.Equal(2, exportedItems.Count); + var metric1 = new List() { exportedItems[0] }; var tag1 = new List> { tags[0] }; - Assert.Equal("name", exportedItems[0].Name); Assert.Equal(20, GetLongSum(metric1)); CheckTagsForNthMetricPoint(metric1, tag1, 1); + + var metric2 = new List() { exportedItems[1] }; + var tag2 = new List> { tags[0] }; + Assert.Equal("name", exportedItems[1].Name); + Assert.Equal(20, GetLongSum(metric2)); + CheckTagsForNthMetricPoint(metric2, tag2, 1); } [Fact] diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index ac6e30dd49d..aca1c82d5d4 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -445,16 +445,16 @@ public void ViewToProduceMultipleStreamsWithDuplicatesFromInstrument() .AddInMemoryExporter(exportedItems) .Build(); - // Expecting two metric stream. - // the .AddView("name1", "renamedStream2") - // won't produce new Metric as the name - // conflicts. + // Expecting three metric stream. + // the second .AddView("name1", "renamedStream2") + // produces a conflicting metric stream. var counterLong = meter.CreateCounter("name1"); counterLong.Add(10); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(2, exportedItems.Count); + Assert.Equal(3, exportedItems.Count); Assert.Equal("renamedStream1", exportedItems[0].Name); Assert.Equal("renamedStream2", exportedItems[1].Name); + Assert.Equal("renamedStream2", exportedItems[2].Name); } [Fact] From af66134c86f33e007138fcbb5f74aa4897ca7269 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Apr 2022 17:15:42 -0700 Subject: [PATCH 06/13] Remove gratuitous check. ViewId makes it unnecessary. --- src/OpenTelemetry/Metrics/MetricReaderExt.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/MetricReaderExt.cs index d070c5ab839..83a64d4eee9 100644 --- a/src/OpenTelemetry/Metrics/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/MetricReaderExt.cs @@ -125,13 +125,7 @@ internal List AddMetricsListWithViews(Instrument instrument, List Date: Thu, 7 Apr 2022 09:51:23 -0700 Subject: [PATCH 07/13] Move helper test methods to base class --- .../Metrics/MetricAPITest.cs | 105 +-------------- .../Metrics/MetricTestsBase.cs | 126 ++++++++++++++++++ .../Metrics/MetricViewTests.cs | 2 +- 3 files changed, 128 insertions(+), 105 deletions(-) create mode 100644 test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 394a4a8d217..bc77d82bb92 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -27,7 +27,7 @@ namespace OpenTelemetry.Metrics.Tests { - public class MetricApiTest + public class MetricApiTest : MetricTestsBase { private const int MaxTimeToAllowForFlush = 10000; private static readonly int NumberOfThreads = Environment.ProcessorCount; @@ -1549,109 +1549,6 @@ public void UnsupportedMetricInstrument() Assert.Empty(exportedItems); } - private static void ValidateMetricPointTags(List> expectedTags, ReadOnlyTagCollection actualTags) - { - int tagIndex = 0; - foreach (var tag in actualTags) - { - Assert.Equal(expectedTags[tagIndex].Key, tag.Key); - Assert.Equal(expectedTags[tagIndex].Value, tag.Value); - tagIndex++; - } - - Assert.Equal(expectedTags.Count, tagIndex); - } - - private static long GetLongSum(List metrics) - { - long sum = 0; - foreach (var metric in metrics) - { - foreach (ref readonly var metricPoint in metric.GetMetricPoints()) - { - if (metric.MetricType.IsSum()) - { - sum += metricPoint.GetSumLong(); - } - else - { - sum += metricPoint.GetGaugeLastValueLong(); - } - } - } - - return sum; - } - - private static double GetDoubleSum(List metrics) - { - double sum = 0; - foreach (var metric in metrics) - { - foreach (ref readonly var metricPoint in metric.GetMetricPoints()) - { - if (metric.MetricType.IsSum()) - { - sum += metricPoint.GetSumDouble(); - } - else - { - sum += metricPoint.GetGaugeLastValueDouble(); - } - } - } - - return sum; - } - - private static int GetNumberOfMetricPoints(List metrics) - { - int count = 0; - foreach (var metric in metrics) - { - foreach (ref readonly var metricPoint in metric.GetMetricPoints()) - { - count++; - } - } - - return count; - } - - private static MetricPoint? GetFirstMetricPoint(List 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 metrics, List> tags, int n) - { - var metric = metrics[0]; - var metricPointEnumerator = metric.GetMetricPoints().GetEnumerator(); - - for (int i = 0; i < n; i++) - { - Assert.True(metricPointEnumerator.MoveNext()); - } - - int index = 0; - var metricPoint = metricPointEnumerator.Current; - foreach (var tag in metricPoint.Tags) - { - Assert.Equal(tags[index].Key, tag.Key); - Assert.Equal(tags[index].Value, tag.Value); - index++; - } - } - private static void CounterUpdateThread(object obj) where T : struct, IComparable { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs new file mode 100644 index 00000000000..5f89766a79e --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestsBase.cs @@ -0,0 +1,126 @@ +// +// 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.Collections.Generic; +using Xunit; + +namespace OpenTelemetry.Metrics.Tests; + +public class MetricTestsBase +{ + public static void ValidateMetricPointTags(List> expectedTags, ReadOnlyTagCollection actualTags) + { + int tagIndex = 0; + foreach (var tag in actualTags) + { + Assert.Equal(expectedTags[tagIndex].Key, tag.Key); + Assert.Equal(expectedTags[tagIndex].Value, tag.Value); + tagIndex++; + } + + Assert.Equal(expectedTags.Count, tagIndex); + } + + public static long GetLongSum(List metrics) + { + long sum = 0; + foreach (var metric in metrics) + { + foreach (ref readonly var metricPoint in metric.GetMetricPoints()) + { + if (metric.MetricType.IsSum()) + { + sum += metricPoint.GetSumLong(); + } + else + { + sum += metricPoint.GetGaugeLastValueLong(); + } + } + } + + return sum; + } + + public static double GetDoubleSum(List metrics) + { + double sum = 0; + foreach (var metric in metrics) + { + foreach (ref readonly var metricPoint in metric.GetMetricPoints()) + { + if (metric.MetricType.IsSum()) + { + sum += metricPoint.GetSumDouble(); + } + else + { + sum += metricPoint.GetGaugeLastValueDouble(); + } + } + } + + return sum; + } + + public static int GetNumberOfMetricPoints(List metrics) + { + int count = 0; + foreach (var metric in metrics) + { + foreach (ref readonly var metricPoint in metric.GetMetricPoints()) + { + count++; + } + } + + return count; + } + + public static MetricPoint? GetFirstMetricPoint(List metrics) + { + foreach (var metric in metrics) + { + foreach (ref readonly var metricPoint in metric.GetMetricPoints()) + { + return metricPoint; + } + } + + return null; + } + + // Provide tags input sorted by Key + public static void CheckTagsForNthMetricPoint(List metrics, List> tags, int n) + { + var metric = metrics[0]; + var metricPointEnumerator = metric.GetMetricPoints().GetEnumerator(); + + for (int i = 0; i < n; i++) + { + Assert.True(metricPointEnumerator.MoveNext()); + } + + int index = 0; + var metricPoint = metricPointEnumerator.Current; + foreach (var tag in metricPoint.Tags) + { + Assert.Equal(tags[index].Key, tag.Key); + Assert.Equal(tags[index].Value, tag.Value); + index++; + } + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index aca1c82d5d4..d629bf65086 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -24,7 +24,7 @@ namespace OpenTelemetry.Metrics.Tests { - public class MetricViewTests + public class MetricViewTests : MetricTestsBase { private const int MaxTimeToAllowForFlush = 10000; From 276a360cf12d3b0354213d3ab5c0998dcdd35c1d Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 7 Apr 2022 09:55:41 -0700 Subject: [PATCH 08/13] Move view tests to where all the other view tests are --- .../Metrics/MetricAPITest.cs | 279 ------------------ .../Metrics/MetricViewTests.cs | 279 ++++++++++++++++++ 2 files changed, 279 insertions(+), 279 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index bc77d82bb92..fe264f06edc 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -365,285 +365,6 @@ public void DuplicateInstrumentRegistration_NoViews_DuplicateInstruments_Differe Assert.Equal(20D, metricPoint2.GetHistogramSum()); } - [Fact] - public void DuplicateInstrumentRegistration_WithViews_DuplicateInstruments_DifferentDescription() - { - var exportedItems = new List(); - - using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription1" }) - .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription2" }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); - - var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); - - instrument.Add(10); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(2, exportedItems.Count); - - var metric1 = exportedItems[0]; - var metric2 = exportedItems[1]; - Assert.Equal("newDescription1", metric1.Description); - Assert.Equal("newDescription2", metric2.Description); - - List metric1MetricPoints = new List(); - foreach (ref readonly var mp in metric1.GetMetricPoints()) - { - metric1MetricPoints.Add(mp); - } - - Assert.Single(metric1MetricPoints); - var metricPoint1 = metric1MetricPoints[0]; - Assert.Equal(10, metricPoint1.GetSumLong()); - - List metric2MetricPoints = new List(); - foreach (ref readonly var mp in metric2.GetMetricPoints()) - { - metric2MetricPoints.Add(mp); - } - - Assert.Single(metric2MetricPoints); - var metricPoint2 = metric2MetricPoints[0]; - Assert.Equal(10, metricPoint2.GetSumLong()); - } - - [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoInstruments_ThreeStreams() - { - var exportedItems = new List(); - - using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddView((instrument) => - { - return new MetricStreamConfiguration() { Name = "MetricStreamA", Description = "description" }; - }) - .AddView((instrument) => - { - return instrument.Description == "description1" - ? new MetricStreamConfiguration() { Name = "MetricStreamB" } - : new MetricStreamConfiguration() { Name = "MetricStreamC" }; - }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); - - var instrument1 = meter.CreateCounter("name", "unit", "description1"); - var instrument2 = meter.CreateCounter("name", "unit", "description2"); - - instrument1.Add(10); - instrument2.Add(10); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal(3, exportedItems.Count); - - var metricA = exportedItems[0]; - var metricB = exportedItems[1]; - var metricC = exportedItems[2]; - - Assert.Equal("MetricStreamA", metricA.Name); - Assert.Equal(20, GetAggregatedValue(metricA)); - - Assert.Equal("MetricStreamB", metricB.Name); - Assert.Equal(10, GetAggregatedValue(metricB)); - - Assert.Equal("MetricStreamC", metricC.Name); - Assert.Equal(10, GetAggregatedValue(metricC)); - - long GetAggregatedValue(Metric metric) - { - var metricPoints = new List(); - foreach (ref readonly var mp in metric.GetMetricPoints()) - { - metricPoints.Add(mp); - } - - Assert.Single(metricPoints); - return metricPoints[0].GetSumLong(); - } - } - - [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentTags() - { - var exportedItems = new List(); - - using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddView((instrument) => - { - return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; - }) - .AddView((instrument) => - { - return new MetricStreamConfiguration { TagKeys = new[] { "key2" } }; - }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); - - var instrument1 = meter.CreateCounter("name"); - var instrument2 = meter.CreateCounter("name"); - - var tags = new KeyValuePair[] - { - new("key1", "value"), - new("key2", "value"), - }; - - instrument1.Add(10, tags); - instrument2.Add(10, tags); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - Assert.Equal(2, exportedItems.Count); - var metric1 = new List() { exportedItems[0] }; - var metric2 = new List() { exportedItems[1] }; - var tag1 = new List> { tags[0] }; - var tag2 = new List> { tags[1] }; - - Assert.Equal("name", exportedItems[0].Name); - Assert.Equal("name", exportedItems[1].Name); - Assert.Equal(20, GetLongSum(metric1)); - Assert.Equal(20, GetLongSum(metric2)); - CheckTagsForNthMetricPoint(metric1, tag1, 1); - CheckTagsForNthMetricPoint(metric2, tag2, 1); - } - - [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_SameTags() - { - var exportedItems = new List(); - - using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddView((instrument) => - { - return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; - }) - .AddView((instrument) => - { - return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; - }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); - - var instrument1 = meter.CreateCounter("name"); - var instrument2 = meter.CreateCounter("name"); - - var tags = new KeyValuePair[] - { - new("key1", "value"), - new("key2", "value"), - }; - - instrument1.Add(10, tags); - instrument2.Add(10, tags); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - Assert.Equal(2, exportedItems.Count); - - var metric1 = new List() { exportedItems[0] }; - var tag1 = new List> { tags[0] }; - Assert.Equal("name", exportedItems[0].Name); - Assert.Equal(20, GetLongSum(metric1)); - CheckTagsForNthMetricPoint(metric1, tag1, 1); - - var metric2 = new List() { exportedItems[1] }; - var tag2 = new List> { tags[0] }; - Assert.Equal("name", exportedItems[1].Name); - Assert.Equal(20, GetLongSum(metric2)); - CheckTagsForNthMetricPoint(metric2, tag2, 1); - } - - [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentHistogramBounds() - { - var exportedItems = new List(); - - using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .AddView((instrument) => - { - return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 5.0, 10.0 } }; - }) - .AddView((instrument) => - { - return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 10.0, 20.0 } }; - }) - .AddInMemoryExporter(exportedItems); - - using var meterProvider = meterProviderBuilder.Build(); - - var instrument1 = meter.CreateHistogram("name"); - var instrument2 = meter.CreateHistogram("name"); - - instrument1.Record(15); - instrument2.Record(15); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - Assert.Equal(2, exportedItems.Count); - var metric1 = exportedItems[0]; - var metric2 = exportedItems[1]; - - Assert.Equal("name", exportedItems[0].Name); - Assert.Equal("name", exportedItems[1].Name); - - var metricPoints = new List(); - foreach (ref readonly var mp in metric1.GetMetricPoints()) - { - metricPoints.Add(mp); - } - - Assert.Single(metricPoints); - var metricPoint = metricPoints[0]; - Assert.Equal(2, metricPoint.GetHistogramCount()); - Assert.Equal(30, metricPoint.GetHistogramSum()); - - var index = 0; - var actualCount = 0; - var expectedBucketCounts = new long[] { 0, 0, 2 }; - foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) - { - Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); - index++; - actualCount++; - } - - metricPoints = new List(); - foreach (ref readonly var mp in metric2.GetMetricPoints()) - { - metricPoints.Add(mp); - } - - Assert.Single(metricPoints); - metricPoint = metricPoints[0]; - Assert.Equal(2, metricPoint.GetHistogramCount()); - Assert.Equal(30, metricPoint.GetHistogramSum()); - - index = 0; - actualCount = 0; - expectedBucketCounts = new long[] { 0, 2, 0 }; - foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) - { - Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); - index++; - actualCount++; - } - } - [Fact] public void DuplicateInstrumentNamesFromDifferentMetersWithSameNameDifferentVersion() { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index d629bf65086..4eb7f865093 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -758,5 +758,284 @@ public void ViewToDropAndRetainInstrument() Assert.Single(exportedItems); Assert.Equal("server.request_renamed", exportedItems[0].Name); } + + [Fact] + public void ViewConflict_OneInstrument_DifferentDescription() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription1" }) + .AddView("instrumentName", new MetricStreamConfiguration() { Description = "newDescription2" }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument = meter.CreateCounter("instrumentName", "instrumentUnit", "instrumentDescription"); + + instrument.Add(10); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.Equal(2, exportedItems.Count); + + var metric1 = exportedItems[0]; + var metric2 = exportedItems[1]; + Assert.Equal("newDescription1", metric1.Description); + Assert.Equal("newDescription2", metric2.Description); + + List metric1MetricPoints = new List(); + foreach (ref readonly var mp in metric1.GetMetricPoints()) + { + metric1MetricPoints.Add(mp); + } + + Assert.Single(metric1MetricPoints); + var metricPoint1 = metric1MetricPoints[0]; + Assert.Equal(10, metricPoint1.GetSumLong()); + + List metric2MetricPoints = new List(); + foreach (ref readonly var mp in metric2.GetMetricPoints()) + { + metric2MetricPoints.Add(mp); + } + + Assert.Single(metric2MetricPoints); + var metricPoint2 = metric2MetricPoints[0]; + Assert.Equal(10, metricPoint2.GetSumLong()); + } + + [Fact] + public void ViewConflict_TwoInstruments_ThreeStreams() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new MetricStreamConfiguration() { Name = "MetricStreamA", Description = "description" }; + }) + .AddView((instrument) => + { + return instrument.Description == "description1" + ? new MetricStreamConfiguration() { Name = "MetricStreamB" } + : new MetricStreamConfiguration() { Name = "MetricStreamC" }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name", "unit", "description1"); + var instrument2 = meter.CreateCounter("name", "unit", "description2"); + + instrument1.Add(10); + instrument2.Add(10); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.Equal(3, exportedItems.Count); + + var metricA = exportedItems[0]; + var metricB = exportedItems[1]; + var metricC = exportedItems[2]; + + Assert.Equal("MetricStreamA", metricA.Name); + Assert.Equal(20, GetAggregatedValue(metricA)); + + Assert.Equal("MetricStreamB", metricB.Name); + Assert.Equal(10, GetAggregatedValue(metricB)); + + Assert.Equal("MetricStreamC", metricC.Name); + Assert.Equal(10, GetAggregatedValue(metricC)); + + long GetAggregatedValue(Metric metric) + { + var metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + return metricPoints[0].GetSumLong(); + } + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentTags() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key2" } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("name"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Equal(2, exportedItems.Count); + var metric1 = new List() { exportedItems[0] }; + var metric2 = new List() { exportedItems[1] }; + var tag1 = new List> { tags[0] }; + var tag2 = new List> { tags[1] }; + + Assert.Equal("name", exportedItems[0].Name); + Assert.Equal("name", exportedItems[1].Name); + Assert.Equal(20, GetLongSum(metric1)); + Assert.Equal(20, GetLongSum(metric2)); + CheckTagsForNthMetricPoint(metric1, tag1, 1); + CheckTagsForNthMetricPoint(metric2, tag2, 1); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_SameTags() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddView((instrument) => + { + return new MetricStreamConfiguration { TagKeys = new[] { "key1" } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("name"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Equal(2, exportedItems.Count); + + var metric1 = new List() { exportedItems[0] }; + var tag1 = new List> { tags[0] }; + Assert.Equal("name", exportedItems[0].Name); + Assert.Equal(20, GetLongSum(metric1)); + CheckTagsForNthMetricPoint(metric1, tag1, 1); + + var metric2 = new List() { exportedItems[1] }; + var tag2 = new List> { tags[0] }; + Assert.Equal("name", exportedItems[1].Name); + Assert.Equal(20, GetLongSum(metric2)); + CheckTagsForNthMetricPoint(metric2, tag2, 1); + } + + [Fact] + public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentHistogramBounds() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 5.0, 10.0 } }; + }) + .AddView((instrument) => + { + return new ExplicitBucketHistogramConfiguration { Boundaries = new[] { 10.0, 20.0 } }; + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateHistogram("name"); + var instrument2 = meter.CreateHistogram("name"); + + instrument1.Record(15); + instrument2.Record(15); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Equal(2, exportedItems.Count); + var metric1 = exportedItems[0]; + var metric2 = exportedItems[1]; + + Assert.Equal("name", exportedItems[0].Name); + Assert.Equal("name", exportedItems[1].Name); + + var metricPoints = new List(); + foreach (ref readonly var mp in metric1.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + var metricPoint = metricPoints[0]; + Assert.Equal(2, metricPoint.GetHistogramCount()); + Assert.Equal(30, metricPoint.GetHistogramSum()); + + var index = 0; + var actualCount = 0; + var expectedBucketCounts = new long[] { 0, 0, 2 }; + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + + metricPoints = new List(); + foreach (ref readonly var mp in metric2.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + Assert.Single(metricPoints); + metricPoint = metricPoints[0]; + Assert.Equal(2, metricPoint.GetHistogramCount()); + Assert.Equal(30, metricPoint.GetHistogramSum()); + + index = 0; + actualCount = 0; + expectedBucketCounts = new long[] { 0, 2, 0 }; + foreach (var histogramMeasurement in metricPoint.GetHistogramBuckets()) + { + Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); + index++; + actualCount++; + } + } } } From da2fe608ead475a8dd1f243a66d520e453039325 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 7 Apr 2022 11:50:06 -0700 Subject: [PATCH 09/13] Rename some tests --- test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 4eb7f865093..c8630701e5c 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -807,7 +807,7 @@ public void ViewConflict_OneInstrument_DifferentDescription() } [Fact] - public void ViewConflict_TwoInstruments_ThreeStreams() + public void ViewConflict_TwoDistinctInstruments_ThreeStreams() { var exportedItems = new List(); @@ -864,7 +864,7 @@ long GetAggregatedValue(Metric metric) } [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentTags() + public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentTags() { var exportedItems = new List(); @@ -912,7 +912,7 @@ public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_Tw } [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_SameTags() + public void ViewConflict_TwoIdenticalInstruments_TwoViews_SameTags() { var exportedItems = new List(); @@ -961,7 +961,7 @@ public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_Tw } [Fact] - public void DuplicateInstrumentRegistration_WithViews_TwoIdenticalInstruments_TwoViews_DifferentHistogramBounds() + public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentHistogramBounds() { var exportedItems = new List(); From 8876f20106b415d330c0c71aead58546d8127fb5 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 7 Apr 2022 11:57:25 -0700 Subject: [PATCH 10/13] Add additional tests from #3129 --- .../Metrics/MetricViewTests.cs | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index c8630701e5c..3a12cd4ac05 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -1037,5 +1037,97 @@ public void ViewConflict_TwoIdenticalInstruments_TwoViews_DifferentHistogramBoun actualCount++; } } + + [Fact] + public void ViewConflict_TwoInstruments_OneMatchesView() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + if (instrument.Name == "name") + { + return new MetricStreamConfiguration { Name = "othername", TagKeys = new[] { "key1" } }; + } + else + { + return null; + } + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("othername"); + + var tags = new KeyValuePair[] + { + new("key1", "value"), + new("key2", "value"), + }; + + instrument1.Add(10, tags); + instrument2.Add(10, tags); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Equal(2, exportedItems.Count); + var metric1 = new List() { exportedItems[0] }; + var metric2 = new List() { exportedItems[1] }; + + var tags1 = new List> { tags[0] }; + var tags2 = new List> { tags[0], tags[1] }; + + Assert.Equal("othername", exportedItems[0].Name); + Assert.Equal("othername", exportedItems[1].Name); + + Assert.Equal(10, GetLongSum(metric1)); + Assert.Equal(10, GetLongSum(metric2)); + + CheckTagsForNthMetricPoint(metric1, tags1, 1); + CheckTagsForNthMetricPoint(metric2, tags2, 1); + } + + [Fact] + public void ViewConflict_TwoInstruments_ConflictAvoidedBecauseSecondInstrumentIsDropped() + { + var exportedItems = new List(); + + using var meter = new Meter($"{Utils.GetCurrentMethodName()}"); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView((instrument) => + { + if (instrument.Name == "name") + { + return new MetricStreamConfiguration { Name = "othername" }; + } + else + { + return MetricStreamConfiguration.Drop; + } + }) + .AddInMemoryExporter(exportedItems); + + using var meterProvider = meterProviderBuilder.Build(); + + var instrument1 = meter.CreateCounter("name"); + var instrument2 = meter.CreateCounter("othername"); + + instrument1.Add(10); + instrument2.Add(20); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.Single(exportedItems); + var metric1 = new List() { exportedItems[0] }; + + Assert.Equal("othername", exportedItems[0].Name); + Assert.Equal(10, GetLongSum(metric1)); + } } } From 274a47ff76d755ac33d25d1b567eff1c6eb3e6c6 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Thu, 7 Apr 2022 12:15:23 -0700 Subject: [PATCH 11/13] Fix Drop configuration ViewId from getting overridden --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 9 ++++++++- src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 181eb2f997f..7906e27e532 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -163,7 +163,14 @@ internal MeterProviderSdk( try { metricStreamConfig = viewConfig(instrument); - metricStreamConfig.ViewId = i; + + // The SDK provides some static MetricStreamConfigurations. + // For example, the Drop configuration. The static ViewId + // should not be changed for these configurations. + if (!metricStreamConfig.ViewId.HasValue) + { + metricStreamConfig.ViewId = i; + } if (metricStreamConfig is ExplicitBucketHistogramConfiguration && instrument.GetType().GetGenericTypeDefinition() != typeof(Histogram<>)) diff --git a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs index 0bf49cba4f4..8d4f3e93b4f 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs @@ -32,7 +32,7 @@ public class MetricStreamConfiguration /// Note: All metrics for the given instrument will be dropped (not /// collected). /// - public static MetricStreamConfiguration Drop { get; } = new MetricStreamConfiguration(); + public static MetricStreamConfiguration Drop { get; } = new MetricStreamConfiguration { ViewId = -1 }; /// /// Gets or sets the optional name of the metric stream. @@ -107,7 +107,7 @@ public string[] TagKeys internal string[] CopiedTagKeys { get; private set; } - internal int ViewId { get; set; } + internal int? ViewId { get; set; } // TODO: MetricPoints caps can be configured here on // a per stream basis, when we add such a capability From 3ec7f6ed52403f2efe646a5b5e80ebe79ae21e88 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 12 Apr 2022 09:03:36 -0700 Subject: [PATCH 12/13] Do not copy tag keys and histogram bounds. It's already been done. --- .../Metrics/MetricStreamIdentity.cs | 24 ++----------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index f7dcd9eb86e..1bb9f544491 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -34,28 +34,8 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration met this.InstrumentType = instrument.GetType(); this.ViewId = metricStreamConfiguration?.ViewId; this.MetricStreamName = $"{this.MeterName}.{this.MeterVersion}.{this.InstrumentName}"; - - var tagKeys = metricStreamConfiguration?.CopiedTagKeys; - if (tagKeys != null) - { - this.TagKeys = new string[tagKeys.Length]; - tagKeys.CopyTo(this.TagKeys, 0); - } - else - { - this.TagKeys = null; - } - - var histogramBucketBounds = (metricStreamConfiguration as ExplicitBucketHistogramConfiguration)?.CopiedBoundaries; - if (histogramBucketBounds != null && histogramBucketBounds.Length > 0) - { - this.HistogramBucketBounds = new double[histogramBucketBounds.Length]; - histogramBucketBounds.CopyTo(this.HistogramBucketBounds, 0); - } - else - { - this.HistogramBucketBounds = null; - } + this.TagKeys = metricStreamConfiguration?.CopiedTagKeys; + this.HistogramBucketBounds = (metricStreamConfiguration as ExplicitBucketHistogramConfiguration)?.CopiedBoundaries; unchecked { From a9484655826cb6108e6c5ab0525db5f1535fae44 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 12 Apr 2022 10:30:17 -0700 Subject: [PATCH 13/13] Update changelog --- src/OpenTelemetry/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 05ecd026138..284489a196a 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -8,6 +8,12 @@ may be possible in the future. ([#3126](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3126)) +* Conformed to the specification to ensure that each view that an instrument + matches results in a new metric stream. With this change it is possible for + views to introduce conflicting metric streams. Any conflicts encountered will + result in a diagnostic log. + ([#3148](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3148)) + ## 1.2.0-rc4 Released 2022-Mar-30