From eb247833557fd8fdae50b120c00972d8161a7a5a Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Wed, 6 Oct 2021 14:57:28 -0700 Subject: [PATCH 01/16] main logic --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 771412bc428..d8708c5402c 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -19,6 +19,7 @@ using System.Diagnostics; using System.Diagnostics.Metrics; using System.Linq; +using System.Text.RegularExpressions; using OpenTelemetry.Internal; using OpenTelemetry.Resources; @@ -87,10 +88,38 @@ internal MeterProviderSdk( } // Setup Listener - var meterSourcesToSubscribe = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var name in meterSources) + var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); + + if (meterSources.Any()) { - meterSourcesToSubscribe[name] = true; + var wildcardMode = false; + foreach (var meterSource in meterSources) + { + if (meterSource.Contains('*')) + { + wildcardMode = true; + break; + } + } + + if (wildcardMode) + { + Regex regex = GetWildcardRegex(meterSources); + foreach (var meterSource in meterSources) + { + if (regex.IsMatch(meterSource)) + { + meterSourcesToSubscribe.Add(meterSource); + } + } + } + else + { + foreach (var meterSource in meterSources) + { + meterSourcesToSubscribe.Add(meterSource); + } + } } this.listener = new MeterListener(); @@ -99,7 +128,7 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (meterSourcesToSubscribe.ContainsKey(instrument.Meter.Name)) + if (meterSourcesToSubscribe.Contains(instrument.Meter.Name)) { // Creating list with initial capacity as the maximum // possible size, to avoid any array resize/copy internally. @@ -197,7 +226,7 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (meterSourcesToSubscribe.ContainsKey(instrument.Meter.Name)) + if (meterSourcesToSubscribe.Contains(instrument.Meter.Name)) { var metricName = instrument.Name; Metric metric = null; @@ -243,6 +272,12 @@ internal MeterProviderSdk( this.listener.MeasurementsCompleted = (instrument, state) => this.MeasurementsCompleted(instrument, state); this.listener.Start(); + + static Regex GetWildcardRegex(IEnumerable collection) + { + var pattern = '^' + string.Join("|", from name in collection select "(?:" + Regex.Escape(name).Replace("\\*", ".*") + ')') + '$'; + return new Regex(pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); + } } internal Resource Resource { get; } From a3392a2ec1eba8a82a7684563540db1addbacec9 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Wed, 6 Oct 2021 14:58:57 -0700 Subject: [PATCH 02/16] tests to be finished --- .../Metrics/MetricAPITest.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 3bd95500084..ca020670d93 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -165,6 +165,36 @@ void ProcessExport(Batch batch) Assert.Equal(1, metricCount); } + [Fact] + public void WildCardTest() + { + var metricItems = new List(); + int metricCount = 0; + var metricExporter = new TestExporter(ProcessExport); + + void ProcessExport(Batch batch) + { + foreach (var metric in batch) + { + metricCount++; + } + } + + var meterSources = new[] { "*", "MeterSource.*", "MeterSource.*.Namespace" }; + + using var meter1 = new Meter(meterSources[0]); + using var meter2 = new Meter(meterSources[1]); + using var meter3 = new Meter(meterSources[2]); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meterSources[0]) + .AddMeter(meterSources[1]) + .AddMeter(meterSources[2]) + .Build(); + + // Do some tests + } + [Theory] [InlineData(true)] [InlineData(false)] From 3d9396a772b632cccf6ff8e562b94aa666c3def9 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 00:20:59 -0700 Subject: [PATCH 03/16] comment and test --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 42 ++++++++++--------- .../Metrics/MetricAPITest.cs | 34 +++++++-------- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index d8708c5402c..976cd55d2a4 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -89,45 +89,42 @@ internal MeterProviderSdk( // Setup Listener var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); - + var wildcardMode = false; + Regex regex = null; if (meterSources.Any()) { - var wildcardMode = false; foreach (var meterSource in meterSources) { if (meterSource.Contains('*')) { wildcardMode = true; + regex = GetWildcardRegex(meterSources); break; } } + } - if (wildcardMode) + this.listener = new MeterListener(); + var viewConfigCount = this.viewConfigs.Count; + if (viewConfigCount > 0) + { + this.listener.InstrumentPublished = (instrument, listener) => { - Regex regex = GetWildcardRegex(meterSources); - foreach (var meterSource in meterSources) + if (wildcardMode) { - if (regex.IsMatch(meterSource)) + if (ShouldListenTo(instrument.Meter.Name)) { - meterSourcesToSubscribe.Add(meterSource); + meterSourcesToSubscribe.Add(instrument.Meter.Name); } } - } - else - { - foreach (var meterSource in meterSources) + else { - meterSourcesToSubscribe.Add(meterSource); + foreach (var meterSource in meterSources) + { + meterSourcesToSubscribe.Add(meterSource); + } } - } - } - this.listener = new MeterListener(); - var viewConfigCount = this.viewConfigs.Count; - if (viewConfigCount > 0) - { - this.listener.InstrumentPublished = (instrument, listener) => - { if (meterSourcesToSubscribe.Contains(instrument.Meter.Name)) { // Creating list with initial capacity as the maximum @@ -278,6 +275,11 @@ static Regex GetWildcardRegex(IEnumerable collection) var pattern = '^' + string.Join("|", from name in collection select "(?:" + Regex.Escape(name).Replace("\\*", ".*") + ')') + '$'; return new Regex(pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); } + + bool ShouldListenTo(string instrumentName) + { + return regex.IsMatch(instrumentName); + } } internal Resource Resource { get; } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index ca020670d93..b27c3543aa5 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -166,33 +166,27 @@ void ProcessExport(Batch batch) } [Fact] - public void WildCardTest() + public void MeterSourcesWildCardSupportTest() { - var metricItems = new List(); - int metricCount = 0; - var metricExporter = new TestExporter(ProcessExport); - - void ProcessExport(Batch batch) - { - foreach (var metric in batch) - { - metricCount++; - } - } - - var meterSources = new[] { "*", "MeterSource.*", "MeterSource.*.Namespace" }; - + var meterSources = new[] { "MeterSources.*", "MeterSources.*.Subset" }; using var meter1 = new Meter(meterSources[0]); using var meter2 = new Meter(meterSources[1]); - using var meter3 = new Meter(meterSources[2]); + var exportedItems = new List(); using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meterSources[0]) - .AddMeter(meterSources[1]) - .AddMeter(meterSources[2]) + .AddMeter(meter1.Name) + .AddMeter(meter2.Name) + .AddInMemoryExporter(exportedItems) + .AddView("name1", "renamed") .Build(); - // Do some tests + var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); + meter1.CreateObservableGauge("myGauge1", () => measurement); + meter2.CreateObservableGauge("myGauge2", () => measurement); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.Equal("myGauge1", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); } [Theory] From 690970d4fa0ab09b49b3d57c9a85ba154f63c610 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 09:51:32 -0700 Subject: [PATCH 04/16] fix logic --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 976cd55d2a4..0921cbffae6 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -102,6 +102,14 @@ internal MeterProviderSdk( break; } } + + if (!wildcardMode) + { + foreach (var meterSource in meterSources) + { + meterSourcesToSubscribe.Add(meterSource); + } + } } this.listener = new MeterListener(); @@ -110,22 +118,8 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (wildcardMode) - { - if (ShouldListenTo(instrument.Meter.Name)) - { - meterSourcesToSubscribe.Add(instrument.Meter.Name); - } - } - else - { - foreach (var meterSource in meterSources) - { - meterSourcesToSubscribe.Add(meterSource); - } - } - - if (meterSourcesToSubscribe.Contains(instrument.Meter.Name)) + if ((wildcardMode && regex.IsMatch(instrument.Meter.Name)) + || (!wildcardMode && meterSourcesToSubscribe.Contains(instrument.Meter.Name))) { // Creating list with initial capacity as the maximum // possible size, to avoid any array resize/copy internally. @@ -275,11 +269,6 @@ static Regex GetWildcardRegex(IEnumerable collection) var pattern = '^' + string.Join("|", from name in collection select "(?:" + Regex.Escape(name).Replace("\\*", ".*") + ')') + '$'; return new Regex(pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); } - - bool ShouldListenTo(string instrumentName) - { - return regex.IsMatch(instrumentName); - } } internal Resource Resource { get; } From af79097fe528123af2dc83bf351150fe640b4406 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 10:03:58 -0700 Subject: [PATCH 05/16] fix and clean --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 0921cbffae6..4989a18a7f2 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -118,8 +118,9 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if ((wildcardMode && regex.IsMatch(instrument.Meter.Name)) - || (!wildcardMode && meterSourcesToSubscribe.Contains(instrument.Meter.Name))) + var meterName = instrument.Meter.Name; + if ((wildcardMode && regex.IsMatch(meterName)) + || (!wildcardMode && meterSourcesToSubscribe.Contains(meterName))) { // Creating list with initial capacity as the maximum // possible size, to avoid any array resize/copy internally. @@ -217,7 +218,9 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (meterSourcesToSubscribe.Contains(instrument.Meter.Name)) + var meterName = instrument.Meter.Name; + if ((wildcardMode && regex.IsMatch(meterName)) + || (!wildcardMode && meterSourcesToSubscribe.Contains(meterName))) { var metricName = instrument.Name; Metric metric = null; From 490c0aea1b6b220eb38e83e871f90652ba863212 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 15:24:16 -0700 Subject: [PATCH 06/16] comments --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 20 +++++++++++++------ .../Metrics/MetricAPITest.cs | 10 +++++++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 4989a18a7f2..5f79a34db72 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -112,15 +112,25 @@ internal MeterProviderSdk( } } + Func shouldListenTo = instrument => + { + if (wildcardMode) + { + return regex.IsMatch(instrument.Meter.Name); + } + else + { + return meterSourcesToSubscribe.Contains(instrument.Meter.Name); + } + }; + this.listener = new MeterListener(); var viewConfigCount = this.viewConfigs.Count; if (viewConfigCount > 0) { this.listener.InstrumentPublished = (instrument, listener) => { - var meterName = instrument.Meter.Name; - if ((wildcardMode && regex.IsMatch(meterName)) - || (!wildcardMode && meterSourcesToSubscribe.Contains(meterName))) + if (shouldListenTo(instrument)) { // Creating list with initial capacity as the maximum // possible size, to avoid any array resize/copy internally. @@ -218,9 +228,7 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - var meterName = instrument.Meter.Name; - if ((wildcardMode && regex.IsMatch(meterName)) - || (!wildcardMode && meterSourcesToSubscribe.Contains(meterName))) + if (shouldListenTo(instrument)) { var metricName = instrument.Name; Metric metric = null; diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index b27c3543aa5..17afbaea508 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -168,14 +168,14 @@ void ProcessExport(Batch batch) [Fact] public void MeterSourcesWildCardSupportTest() { - var meterSources = new[] { "MeterSources.*", "MeterSources.*.Subset" }; + var meterSources = new[] { "AbcCompany.XyzProduct.ComponentA", "AbcCompany.XyzProduct.ComponentB", "SomeCompany.SomeProduct.SomeComponent" }; using var meter1 = new Meter(meterSources[0]); using var meter2 = new Meter(meterSources[1]); + using var meter3 = new Meter(meterSources[2]); var exportedItems = new List(); using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter1.Name) - .AddMeter(meter2.Name) + .AddMeter("AbcCompany.XyzProduct.*") .AddInMemoryExporter(exportedItems) .AddView("name1", "renamed") .Build(); @@ -183,10 +183,14 @@ public void MeterSourcesWildCardSupportTest() var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter1.CreateObservableGauge("myGauge1", () => measurement); meter2.CreateObservableGauge("myGauge2", () => measurement); + meter3.CreateObservableGauge("myGauge3", () => measurement); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal("myGauge1", exportedItems[0].Name); Assert.Equal("myGauge2", exportedItems[1].Name); + + // the third element "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. + Assert.True(exportedItems.Count == 2); } [Theory] From 9cf83e1d108f72174c142e78e2c71ebb6c974881 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 16:17:54 -0700 Subject: [PATCH 07/16] ver1 --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 5f79a34db72..3323f6c1ad0 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -112,17 +112,21 @@ internal MeterProviderSdk( } } - Func shouldListenTo = instrument => + Func shouldListenTo; + if (wildcardMode) { - if (wildcardMode) + shouldListenTo = instrument => { return regex.IsMatch(instrument.Meter.Name); - } - else + }; + } + else + { + shouldListenTo = instrument => { return meterSourcesToSubscribe.Contains(instrument.Meter.Name); - } - }; + }; + } this.listener = new MeterListener(); var viewConfigCount = this.viewConfigs.Count; From 81be58a4f90fd796f9a9a71a96337315fd6c3f21 Mon Sep 17 00:00:00 2001 From: yunl Date: Thu, 7 Oct 2021 17:10:58 -0700 Subject: [PATCH 08/16] Update test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs Co-authored-by: Cijo Thomas --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 17afbaea508..6d7d102925d 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -168,7 +168,7 @@ void ProcessExport(Batch batch) [Fact] public void MeterSourcesWildCardSupportTest() { - var meterSources = new[] { "AbcCompany.XyzProduct.ComponentA", "AbcCompany.XyzProduct.ComponentB", "SomeCompany.SomeProduct.SomeComponent" }; + var meterNames = new[] { "AbcCompany.XyzProduct.ComponentA", "AbcCompany.XyzProduct.ComponentB", "SomeCompany.SomeProduct.SomeComponent" }; using var meter1 = new Meter(meterSources[0]); using var meter2 = new Meter(meterSources[1]); using var meter3 = new Meter(meterSources[2]); From 18e1b3c9fd96a95182fb6b0798ee332350f498ac Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 17:49:56 -0700 Subject: [PATCH 09/16] comments --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 30 ++-- .../Metrics/MetricAPITest.cs | 164 +++++++++++++++++- 2 files changed, 168 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 3323f6c1ad0..99ba8e8c0fd 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -88,9 +88,9 @@ internal MeterProviderSdk( } // Setup Listener - var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); var wildcardMode = false; Regex regex = null; + Func shouldListenTo = null; if (meterSources.Any()) { foreach (var meterSource in meterSources) @@ -98,6 +98,12 @@ internal MeterProviderSdk( if (meterSource.Contains('*')) { wildcardMode = true; + + shouldListenTo = instrument => + { + return regex.IsMatch(instrument.Meter.Name); + }; + regex = GetWildcardRegex(meterSources); break; } @@ -105,27 +111,17 @@ internal MeterProviderSdk( if (!wildcardMode) { + var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var meterSource in meterSources) { meterSourcesToSubscribe.Add(meterSource); } - } - } - Func shouldListenTo; - if (wildcardMode) - { - shouldListenTo = instrument => - { - return regex.IsMatch(instrument.Meter.Name); - }; - } - else - { - shouldListenTo = instrument => - { - return meterSourcesToSubscribe.Contains(instrument.Meter.Name); - }; + shouldListenTo = instrument => + { + return meterSourcesToSubscribe.Contains(instrument.Meter.Name); + }; + } } this.listener = new MeterListener(); diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 17afbaea508..1c5383e2717 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -165,19 +165,32 @@ void ProcessExport(Batch batch) Assert.Equal(1, metricCount); } - [Fact] - public void MeterSourcesWildCardSupportTest() + public static IEnumerable ViewConfigs => + new List + { + new object[] { "myGauge1", "newName" }, + }; + + [Theory] + [MemberData(nameof(ViewConfigs))] + public void MeterSourcesWildcardSupportMatchTest(string instrumentName, string newName) { - var meterSources = new[] { "AbcCompany.XyzProduct.ComponentA", "AbcCompany.XyzProduct.ComponentB", "SomeCompany.SomeProduct.SomeComponent" }; - using var meter1 = new Meter(meterSources[0]); - using var meter2 = new Meter(meterSources[1]); - using var meter3 = new Meter(meterSources[2]); + var meterNames = new[] + { + "AbcCompany.XyzProduct.ComponentA", + "AbcCompany.XyzProduct.ComponentB", + "SomeCompany.SomeProduct.SomeComponent", + }; + + using var meter1 = new Meter(meterNames[0]); + using var meter2 = new Meter(meterNames[1]); + using var meter3 = new Meter(meterNames[2]); var exportedItems = new List(); using var meterProvider = Sdk.CreateMeterProviderBuilder() .AddMeter("AbcCompany.XyzProduct.*") .AddInMemoryExporter(exportedItems) - .AddView("name1", "renamed") + .AddView(instrumentName, newName) .Build(); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); @@ -186,11 +199,144 @@ public void MeterSourcesWildCardSupportTest() meter3.CreateObservableGauge("myGauge3", () => measurement); meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.Equal("myGauge1", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); // the third element "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. Assert.True(exportedItems.Count == 2); + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + } + + [Theory] + [MemberData(nameof(ViewConfigs))] + public void MeterSourcesWildcardSupportTwoPatterns(string instrumentName, string newName) + { + var meterNames = new[] + { + "AbcCompany.XyzProduct.ComponentA", + "AbcCompany.XyzProduct.ComponentB", + "DefCompany.AbcProduct.ComponentC", + "DefCompany.XyzProduct.ComponentC", + }; + + using var meter1 = new Meter(meterNames[0]); + using var meter2 = new Meter(meterNames[1]); + using var meter3 = new Meter(meterNames[2]); + using var meter4 = new Meter(meterNames[3]); + + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter("AbcCompany.XyzProduct.*") + .AddMeter("DefCompany.*.ComponentC") + .AddInMemoryExporter(exportedItems) + .AddView(instrumentName, newName) + .Build(); + + var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); + meter1.CreateObservableGauge("myGauge1", () => measurement); + meter2.CreateObservableGauge("myGauge2", () => measurement); + meter3.CreateObservableGauge("myGauge3", () => measurement); + meter4.CreateObservableGauge("myGauge4", () => measurement); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.True(exportedItems.Count == 4); + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + Assert.Equal("myGauge3", exportedItems[2].Name); + Assert.Equal("myGauge4", exportedItems[3].Name); + } + + [Theory] + [MemberData(nameof(ViewConfigs))] + public void MeterSourcesWildcardSupportCaseInsensitive(string instrumentName, string newName) + { + var meterNames = new[] + { + "abcCompany.xYZProduct.compoNeNta", + "AbcCompany.XyzProduct.ComponentB", + }; + + using var meter1 = new Meter(meterNames[0]); + using var meter2 = new Meter(meterNames[1]); + + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter("AbcCompany.XyzProduct.*") + .AddInMemoryExporter(exportedItems) + .AddView(instrumentName, newName) + .Build(); + + var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); + meter1.CreateObservableGauge("myGauge1", () => measurement); + meter2.CreateObservableGauge("myGauge2", () => measurement); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.True(exportedItems.Count == 2); + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + } + + [Theory] + [MemberData(nameof(ViewConfigs))] + public void MeterSourcesWildcardSupportNoAddMeter(string instrumentName, string newName) + { + var meterNames = new[] + { + "AbcCompany.XyzProduct.ComponentA", + "AbcCompany.XyzProduct.ComponentB", + }; + + using var meter1 = new Meter(meterNames[0]); + using var meter2 = new Meter(meterNames[1]); + + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddInMemoryExporter(exportedItems) + .AddView(instrumentName, newName) + .Build(); + + var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.True(exportedItems.Count == 0); + } + + [Theory] + [MemberData(nameof(ViewConfigs))] + public void MeterSourcesWildcardSupportMixWithNoWildcardmeter(string instrumentName, string newName) + { + var meterNames = new[] + { + "AbcCompany.XyzProduct.ComponentA", + "AbcCompany.XyzProduct.ComponentB", + "SomeCompany.SomeProduct.SomeComponent", + }; + + using var meter1 = new Meter(meterNames[0]); + using var meter2 = new Meter(meterNames[1]); + using var meter3 = new Meter(meterNames[2]); + + var exportedItems = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter("AbcCompany.XyzProduct.*") + .AddMeter("SomeCompany.SomeProduct.SomeComponent") + .AddInMemoryExporter(exportedItems) + .AddView(instrumentName, newName) + .Build(); + + var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); + meter1.CreateObservableGauge("myGauge1", () => measurement); + meter2.CreateObservableGauge("myGauge2", () => measurement); + meter3.CreateObservableGauge("myGauge3", () => measurement); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + Assert.True(exportedItems.Count == 3); + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + Assert.Equal("myGauge3", exportedItems[2].Name); } [Theory] From 98d87f67b5bded7cfc3d7d3b1502ca88f6c76098 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Thu, 7 Oct 2021 17:58:02 -0700 Subject: [PATCH 10/16] supress test code warning --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 1c5383e2717..a2e6d433d6b 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -165,7 +165,9 @@ void ProcessExport(Batch batch) Assert.Equal(1, metricCount); } +#pragma warning disable SA1201 // Elements should appear in the correct order public static IEnumerable ViewConfigs => +#pragma warning restore SA1201 // Elements should appear in the correct order new List { new object[] { "myGauge1", "newName" }, From 2a71abc871b6a2963f04571fa42af5db5056d1b0 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 11:17:28 -0700 Subject: [PATCH 11/16] refactor tests and resolve comments --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 14 +- .../Metrics/MetricAPITest.cs | 172 ++++++------------ 2 files changed, 56 insertions(+), 130 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 99ba8e8c0fd..4b35e8e7f3a 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -89,21 +89,16 @@ internal MeterProviderSdk( // Setup Listener var wildcardMode = false; - Regex regex = null; Func shouldListenTo = null; if (meterSources.Any()) { foreach (var meterSource in meterSources) { + Regex regex = null; if (meterSource.Contains('*')) { wildcardMode = true; - - shouldListenTo = instrument => - { - return regex.IsMatch(instrument.Meter.Name); - }; - + shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); regex = GetWildcardRegex(meterSources); break; } @@ -117,10 +112,7 @@ internal MeterProviderSdk( meterSourcesToSubscribe.Add(meterSource); } - shouldListenTo = instrument => - { - return meterSourcesToSubscribe.Contains(instrument.Meter.Name); - }; + shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index a2e6d433d6b..50fd5e58bf1 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -165,59 +165,18 @@ void ProcessExport(Batch batch) Assert.Equal(1, metricCount); } -#pragma warning disable SA1201 // Elements should appear in the correct order - public static IEnumerable ViewConfigs => -#pragma warning restore SA1201 // Elements should appear in the correct order - new List - { - new object[] { "myGauge1", "newName" }, - }; - - [Theory] - [MemberData(nameof(ViewConfigs))] - public void MeterSourcesWildcardSupportMatchTest(string instrumentName, string newName) - { - var meterNames = new[] - { - "AbcCompany.XyzProduct.ComponentA", - "AbcCompany.XyzProduct.ComponentB", - "SomeCompany.SomeProduct.SomeComponent", - }; - - using var meter1 = new Meter(meterNames[0]); - using var meter2 = new Meter(meterNames[1]); - using var meter3 = new Meter(meterNames[2]); - - var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter("AbcCompany.XyzProduct.*") - .AddInMemoryExporter(exportedItems) - .AddView(instrumentName, newName) - .Build(); - - var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); - meter1.CreateObservableGauge("myGauge1", () => measurement); - meter2.CreateObservableGauge("myGauge2", () => measurement); - meter3.CreateObservableGauge("myGauge3", () => measurement); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - // the third element "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. - Assert.True(exportedItems.Count == 2); - Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); - } - [Theory] - [MemberData(nameof(ViewConfigs))] - public void MeterSourcesWildcardSupportTwoPatterns(string instrumentName, string newName) + [InlineData(true)] + [InlineData(false)] + public void MeterSourcesWildcardSupportMatchTest(bool hasView) { var meterNames = new[] { "AbcCompany.XyzProduct.ComponentA", - "AbcCompany.XyzProduct.ComponentB", + "abcCompany.xYzProduct.componentC", "DefCompany.AbcProduct.ComponentC", "DefCompany.XyzProduct.ComponentC", + "SomeCompany.SomeProduct.SomeComponent", }; using var meter1 = new Meter(meterNames[0]); @@ -226,12 +185,17 @@ public void MeterSourcesWildcardSupportTwoPatterns(string instrumentName, string using var meter4 = new Meter(meterNames[3]); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() .AddMeter("AbcCompany.XyzProduct.*") .AddMeter("DefCompany.*.ComponentC") - .AddInMemoryExporter(exportedItems) - .AddView(instrumentName, newName) - .Build(); + .AddInMemoryExporter(exportedItems); + + if (hasView) + { + meterProviderBuilder.AddView("myGauge1", "newName"); + } + + using var meterProvider = meterProviderBuilder.Build(); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter1.CreateObservableGauge("myGauge1", () => measurement); @@ -241,104 +205,74 @@ public void MeterSourcesWildcardSupportTwoPatterns(string instrumentName, string meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.True(exportedItems.Count == 4); - Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); - Assert.Equal("myGauge3", exportedItems[2].Name); - Assert.Equal("myGauge4", exportedItems[3].Name); + if (hasView) + { + Assert.True(exportedItems.Count == 4); // the third element "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + Assert.Equal("myGauge3", exportedItems[2].Name); + Assert.Equal("myGauge4", exportedItems[3].Name); + } } [Theory] - [MemberData(nameof(ViewConfigs))] - public void MeterSourcesWildcardSupportCaseInsensitive(string instrumentName, string newName) + [InlineData(true)] + [InlineData(false)] + public void MeterSourcesWildcardSupportMixWithNoWildcardmeter(bool hasView) { var meterNames = new[] { - "abcCompany.xYZProduct.compoNeNta", - "AbcCompany.XyzProduct.ComponentB", + "AbcCompany.XyzProduct.ComponentA", + "SomeCompany.SomeProduct.SomeComponent", }; using var meter1 = new Meter(meterNames[0]); using var meter2 = new Meter(meterNames[1]); var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() .AddMeter("AbcCompany.XyzProduct.*") - .AddInMemoryExporter(exportedItems) - .AddView(instrumentName, newName) - .Build(); + .AddMeter("SomeCompany.SomeProduct.SomeComponent") + .AddInMemoryExporter(exportedItems); + + if (hasView) + { + meterProviderBuilder.AddView("myGauge1", "newName"); + } + + using var meterProvider = meterProviderBuilder.Build(); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); meter1.CreateObservableGauge("myGauge1", () => measurement); meter2.CreateObservableGauge("myGauge2", () => measurement); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.True(exportedItems.Count == 2); - Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); + if (hasView) + { + Assert.True(exportedItems.Count == 2); + Assert.Equal("newName", exportedItems[0].Name); + Assert.Equal("myGauge2", exportedItems[1].Name); + } } [Theory] - [MemberData(nameof(ViewConfigs))] - public void MeterSourcesWildcardSupportNoAddMeter(string instrumentName, string newName) + [InlineData(true)] + [InlineData(false)] + public void MeterSourcesWildcardSupportWithoutMeter(bool hasView) { - var meterNames = new[] - { - "AbcCompany.XyzProduct.ComponentA", - "AbcCompany.XyzProduct.ComponentB", - }; - - using var meter1 = new Meter(meterNames[0]); - using var meter2 = new Meter(meterNames[1]); - var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddInMemoryExporter(exportedItems) - .AddView(instrumentName, newName) - .Build(); - - var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - Assert.True(exportedItems.Count == 0); - } + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() + .AddInMemoryExporter(exportedItems); - [Theory] - [MemberData(nameof(ViewConfigs))] - public void MeterSourcesWildcardSupportMixWithNoWildcardmeter(string instrumentName, string newName) - { - var meterNames = new[] + if (hasView) { - "AbcCompany.XyzProduct.ComponentA", - "AbcCompany.XyzProduct.ComponentB", - "SomeCompany.SomeProduct.SomeComponent", - }; - - using var meter1 = new Meter(meterNames[0]); - using var meter2 = new Meter(meterNames[1]); - using var meter3 = new Meter(meterNames[2]); - - var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddMeter("AbcCompany.XyzProduct.*") - .AddMeter("SomeCompany.SomeProduct.SomeComponent") - .AddInMemoryExporter(exportedItems) - .AddView(instrumentName, newName) - .Build(); + meterProviderBuilder.AddView("gauge1", "renamed"); + } + using var meterProvider = meterProviderBuilder.Build(); var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); - meter1.CreateObservableGauge("myGauge1", () => measurement); - meter2.CreateObservableGauge("myGauge2", () => measurement); - meter3.CreateObservableGauge("myGauge3", () => measurement); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - Assert.True(exportedItems.Count == 3); - Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); - Assert.Equal("myGauge3", exportedItems[2].Name); + Assert.True(exportedItems.Count == 0); } [Theory] From c0954e57f386e645ee4cd7dedc3f0a79a9bc3ab9 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 11:20:09 -0700 Subject: [PATCH 12/16] readability --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 4b35e8e7f3a..ecbfef57c58 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -98,8 +98,8 @@ internal MeterProviderSdk( if (meterSource.Contains('*')) { wildcardMode = true; - shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); regex = GetWildcardRegex(meterSources); + shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); break; } } From e045d125d2c3237147f2f9e08318355f1684e7d4 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 11:23:43 -0700 Subject: [PATCH 13/16] added test comments --- test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 50fd5e58bf1..132809dbdae 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -173,9 +173,9 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) var meterNames = new[] { "AbcCompany.XyzProduct.ComponentA", - "abcCompany.xYzProduct.componentC", + "abcCompany.xYzProduct.componentC", // Wildcard match is case insensitive. "DefCompany.AbcProduct.ComponentC", - "DefCompany.XyzProduct.ComponentC", + "DefCompany.XyzProduct.ComponentC", // Wildcard match supports matching multiple patterns. "SomeCompany.SomeProduct.SomeComponent", }; @@ -207,7 +207,7 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) if (hasView) { - Assert.True(exportedItems.Count == 4); // the third element "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. + Assert.True(exportedItems.Count == 4); // "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. Assert.Equal("newName", exportedItems[0].Name); Assert.Equal("myGauge2", exportedItems[1].Name); Assert.Equal("myGauge3", exportedItems[2].Name); From f886d9a35cfb8acb03dab86bc6249914fd8f6da8 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 14:40:00 -0700 Subject: [PATCH 14/16] resolved comments; added null check for shouldListento --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 33 ++++------- .../Metrics/MetricAPITest.cs | 57 +++++-------------- 2 files changed, 26 insertions(+), 64 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index ecbfef57c58..5a476a937e1 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -88,32 +88,21 @@ internal MeterProviderSdk( } // Setup Listener - var wildcardMode = false; Func shouldListenTo = null; - if (meterSources.Any()) + if (meterSources.Any(s => s.Contains('*'))) { + var regex = GetWildcardRegex(meterSources); + shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); + } + else if (meterSources.Any()) + { + var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var meterSource in meterSources) { - Regex regex = null; - if (meterSource.Contains('*')) - { - wildcardMode = true; - regex = GetWildcardRegex(meterSources); - shouldListenTo = instrument => regex.IsMatch(instrument.Meter.Name); - break; - } + meterSourcesToSubscribe.Add(meterSource); } - if (!wildcardMode) - { - var meterSourcesToSubscribe = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var meterSource in meterSources) - { - meterSourcesToSubscribe.Add(meterSource); - } - - shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); - } + shouldListenTo = instrument => meterSourcesToSubscribe.Contains(instrument.Meter.Name); } this.listener = new MeterListener(); @@ -122,7 +111,7 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (shouldListenTo(instrument)) + if (shouldListenTo != null && shouldListenTo(instrument)) { // Creating list with initial capacity as the maximum // possible size, to avoid any array resize/copy internally. @@ -220,7 +209,7 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (shouldListenTo(instrument)) + if (shouldListenTo != null && shouldListenTo(instrument)) { var metricName = instrument.Name; Metric metric = null; diff --git a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs index 132809dbdae..73dcb3df392 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs @@ -176,6 +176,7 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) "abcCompany.xYzProduct.componentC", // Wildcard match is case insensitive. "DefCompany.AbcProduct.ComponentC", "DefCompany.XyzProduct.ComponentC", // Wildcard match supports matching multiple patterns. + "GhiCompany.qweProduct.ComponentN", "SomeCompany.SomeProduct.SomeComponent", }; @@ -183,11 +184,14 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) using var meter2 = new Meter(meterNames[1]); using var meter3 = new Meter(meterNames[2]); using var meter4 = new Meter(meterNames[3]); + using var meter5 = new Meter(meterNames[4]); + using var meter6 = new Meter(meterNames[5]); var exportedItems = new List(); var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() .AddMeter("AbcCompany.XyzProduct.*") .AddMeter("DefCompany.*.ComponentC") + .AddMeter("GhiCompany.qweProduct.ComponentN") // Mixing of non-wildcard meter name and wildcard meter name. .AddInMemoryExporter(exportedItems); if (hasView) @@ -202,63 +206,32 @@ public void MeterSourcesWildcardSupportMatchTest(bool hasView) meter2.CreateObservableGauge("myGauge2", () => measurement); meter3.CreateObservableGauge("myGauge3", () => measurement); meter4.CreateObservableGauge("myGauge4", () => measurement); + meter5.CreateObservableGauge("myGauge5", () => measurement); + meter6.CreateObservableGauge("myGauge6", () => measurement); meterProvider.ForceFlush(MaxTimeToAllowForFlush); + Assert.True(exportedItems.Count == 5); // "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. + if (hasView) { - Assert.True(exportedItems.Count == 4); // "SomeCompany.SomeProduct.SomeComponent" will not be subscribed. Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); - Assert.Equal("myGauge3", exportedItems[2].Name); - Assert.Equal("myGauge4", exportedItems[3].Name); } - } - - [Theory] - [InlineData(true)] - [InlineData(false)] - public void MeterSourcesWildcardSupportMixWithNoWildcardmeter(bool hasView) - { - var meterNames = new[] - { - "AbcCompany.XyzProduct.ComponentA", - "SomeCompany.SomeProduct.SomeComponent", - }; - - using var meter1 = new Meter(meterNames[0]); - using var meter2 = new Meter(meterNames[1]); - - var exportedItems = new List(); - var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() - .AddMeter("AbcCompany.XyzProduct.*") - .AddMeter("SomeCompany.SomeProduct.SomeComponent") - .AddInMemoryExporter(exportedItems); - - if (hasView) + else { - meterProviderBuilder.AddView("myGauge1", "newName"); + Assert.Equal("myGauge1", exportedItems[0].Name); } - using var meterProvider = meterProviderBuilder.Build(); - - var measurement = new Measurement(100, new("name", "apple"), new("color", "red")); - meter1.CreateObservableGauge("myGauge1", () => measurement); - meter2.CreateObservableGauge("myGauge2", () => measurement); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - if (hasView) - { - Assert.True(exportedItems.Count == 2); - Assert.Equal("newName", exportedItems[0].Name); - Assert.Equal("myGauge2", exportedItems[1].Name); - } + Assert.Equal("myGauge2", exportedItems[1].Name); + Assert.Equal("myGauge3", exportedItems[2].Name); + Assert.Equal("myGauge4", exportedItems[3].Name); + Assert.Equal("myGauge5", exportedItems[4].Name); } [Theory] [InlineData(true)] [InlineData(false)] - public void MeterSourcesWildcardSupportWithoutMeter(bool hasView) + public void MeterSourcesWildcardSupportWithoutAddingMeterToProvider(bool hasView) { var exportedItems = new List(); var meterProviderBuilder = Sdk.CreateMeterProviderBuilder() From bd2aedb316d24d2c33ba56965da7ddb4c50dcd48 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 15:37:15 -0700 Subject: [PATCH 15/16] comment --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 190 +++++++++--------- 1 file changed, 97 insertions(+), 93 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 5a476a937e1..50b8e2268da 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -88,7 +88,7 @@ internal MeterProviderSdk( } // Setup Listener - Func shouldListenTo = null; + Func shouldListenTo = instrument => false; if (meterSources.Any(s => s.Contains('*'))) { var regex = GetWildcardRegex(meterSources); @@ -111,80 +111,82 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (shouldListenTo != null && shouldListenTo(instrument)) + if (!shouldListenTo(instrument)) { - // Creating list with initial capacity as the maximum - // possible size, to avoid any array resize/copy internally. - // 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) - { - var metricStreamConfig = viewConfig(instrument); - if (metricStreamConfig != null) - { - metricStreamConfigs.Add(metricStreamConfig); - } - } + return; + } - if (metricStreamConfigs.Count == 0) + // Creating list with initial capacity as the maximum + // possible size, to avoid any array resize/copy internally. + // 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) + { + var metricStreamConfig = viewConfig(instrument); + if (metricStreamConfig != null) { - // No views matched. Add null - // which will apply defaults. - // Users can turn off this default - // by adding a view like below as the last view. - // .AddView(instrumentName: "*", new MetricStreamConfiguration() { Aggregation = Aggregation.Drop }) - metricStreamConfigs.Add(null); + metricStreamConfigs.Add(metricStreamConfig); } + } - var maxCountMetricsToBeCreated = metricStreamConfigs.Count; + if (metricStreamConfigs.Count == 0) + { + // No views matched. Add null + // which will apply defaults. + // Users can turn off this default + // by adding a view like below as the last view. + // .AddView(instrumentName: "*", new MetricStreamConfiguration() { Aggregation = Aggregation.Drop }) + metricStreamConfigs.Add(null); + } - // Create list with initial capacity as the max metric count. - // Due to duplicate/max limit, we may not end up using them - // all, and that memory is wasted until Meter disposed. - // TODO: Revisit to see if we need to do metrics.TrimExcess() - var metrics = new List(maxCountMetricsToBeCreated); - lock (this.instrumentCreationLock) + var maxCountMetricsToBeCreated = metricStreamConfigs.Count; + + // Create list with initial capacity as the max metric count. + // Due to duplicate/max limit, we may not end up using them + // all, and that memory is wasted until Meter disposed. + // TODO: Revisit to see if we need to do metrics.TrimExcess() + var metrics = new List(maxCountMetricsToBeCreated); + lock (this.instrumentCreationLock) + { + for (int i = 0; i < maxCountMetricsToBeCreated; i++) { - for (int i = 0; i < maxCountMetricsToBeCreated; i++) + var metricStreamConfig = metricStreamConfigs[i]; + var metricStreamName = metricStreamConfig?.Name ?? instrument.Name; + if (this.metricStreamNames.ContainsKey(metricStreamName)) + { + // TODO: Log that instrument is ignored + // as the resulting Metric name is conflicting + // with existing name. + continue; + } + + if (metricStreamConfig?.Aggregation == Aggregation.Drop) + { + // TODO: Log that instrument is ignored + // as user explicitly asked to drop it + // with View. + continue; + } + + var index = ++this.metricIndex; + if (index >= MaxMetrics) + { + // TODO: Log that instrument is ignored + // as max number of Metrics have reached. + } + else { - var metricStreamConfig = metricStreamConfigs[i]; - var metricStreamName = metricStreamConfig?.Name ?? instrument.Name; - if (this.metricStreamNames.ContainsKey(metricStreamName)) - { - // TODO: Log that instrument is ignored - // as the resulting Metric name is conflicting - // with existing name. - continue; - } - - if (metricStreamConfig?.Aggregation == Aggregation.Drop) - { - // TODO: Log that instrument is ignored - // as user explicitly asked to drop it - // with View. - continue; - } - - var index = ++this.metricIndex; - if (index >= MaxMetrics) - { - // TODO: Log that instrument is ignored - // as max number of Metrics have reached. - } - else - { - Metric metric; - var metricDescription = metricStreamConfig?.Description ?? instrument.Description; - string[] tagKeysInteresting = metricStreamConfig?.TagKeys; - double[] histogramBucketBounds = (metricStreamConfig is HistogramConfiguration histogramConfig - && histogramConfig.BucketBounds != null) ? histogramConfig.BucketBounds : null; - metric = new Metric(instrument, temporality, metricStreamName, metricDescription, histogramBucketBounds, tagKeysInteresting); - - this.metrics[index] = metric; - metrics.Add(metric); - this.metricStreamNames.Add(metricStreamName, true); - } + Metric metric; + var metricDescription = metricStreamConfig?.Description ?? instrument.Description; + string[] tagKeysInteresting = metricStreamConfig?.TagKeys; + double[] histogramBucketBounds = (metricStreamConfig is HistogramConfiguration histogramConfig + && histogramConfig.BucketBounds != null) ? histogramConfig.BucketBounds : null; + metric = new Metric(instrument, temporality, metricStreamName, metricDescription, histogramBucketBounds, tagKeysInteresting); + + this.metrics[index] = metric; + metrics.Add(metric); + this.metricStreamNames.Add(metricStreamName, true); } } @@ -209,37 +211,39 @@ internal MeterProviderSdk( { this.listener.InstrumentPublished = (instrument, listener) => { - if (shouldListenTo != null && shouldListenTo(instrument)) + if (!shouldListenTo(instrument)) { - var metricName = instrument.Name; - Metric metric = null; - lock (this.instrumentCreationLock) - { - if (this.metricStreamNames.ContainsKey(metricName)) - { - // TODO: Log that instrument is ignored - // as the resulting Metric name is conflicting - // with existing name. - return; - } + return; + } - var index = ++this.metricIndex; - if (index >= MaxMetrics) - { - // TODO: Log that instrument is ignored - // as max number of Metrics have reached. - return; - } - else - { - metric = new Metric(instrument, temporality, metricName, instrument.Description); - this.metrics[index] = metric; - this.metricStreamNames.Add(metricName, true); - } + var metricName = instrument.Name; + Metric metric = null; + lock (this.instrumentCreationLock) + { + if (this.metricStreamNames.ContainsKey(metricName)) + { + // TODO: Log that instrument is ignored + // as the resulting Metric name is conflicting + // with existing name. + return; } - listener.EnableMeasurementEvents(instrument, metric); + var index = ++this.metricIndex; + if (index >= MaxMetrics) + { + // TODO: Log that instrument is ignored + // as max number of Metrics have reached. + return; + } + else + { + metric = new Metric(instrument, temporality, metricName, instrument.Description); + this.metrics[index] = metric; + this.metricStreamNames.Add(metricName, true); + } } + + listener.EnableMeasurementEvents(instrument, metric); }; // Everything double From dc84c7d7088f95d7727456020bdff6056ffb0ffa Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Fri, 8 Oct 2021 16:14:48 -0700 Subject: [PATCH 16/16] resolve conflicts --- src/OpenTelemetry/Metrics/MeterProviderSdk.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index 49758781d2f..e11a75697e8 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -216,6 +216,7 @@ internal MeterProviderSdk( OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(instrument.Name, instrument.Meter.Name, "Instrument belongs to a Meter not subscribed by the provider.", "Use AddMeter to add the Meter to the provider."); return; } + try { var metricName = instrument.Name; @@ -224,9 +225,7 @@ internal MeterProviderSdk( { if (this.metricStreamNames.ContainsKey(metricName)) { - // TODO: Log that instrument is ignored - // as the resulting Metric name is conflicting - // with existing name. + OpenTelemetrySdkEventSource.Log.MetricInstrumentIgnored(metricName, instrument.Meter.Name, "Metric name conflicting with existing name.", "Either change the name of the instrument or change name using View."); return; }