From d39a1c7d25f24bdced7e96eb00e1a69029dc1a76 Mon Sep 17 00:00:00 2001 From: Chris R Date: Mon, 14 Aug 2023 17:22:36 -0700 Subject: [PATCH 1/3] Unify with logging category matching --- .../src/Metrics/ListenerSubscription.cs | 55 ++++++++------- .../src/Resources/Strings.resx | 67 ++++++++++++++++++- .../tests/ListenerSubscriptionTests.cs | 32 +++++++-- 3 files changed, 120 insertions(+), 34 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs index 24c6d0b7b7f29..7530ba64e150b 100644 --- a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs +++ b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs @@ -165,36 +165,39 @@ internal static bool RuleMatches(InstrumentRule rule, Instrument instrument, str // Meter - var ruleMeterName = rule.MeterName.AsSpan(); - // Don't allow "*" anywhere except at the end. - var starIndex = ruleMeterName.IndexOf('*'); - if (starIndex != -1 && starIndex != ruleMeterName.Length - 1) + // The same logic as Microsoft.Extensions.Logging.LoggerRuleSelector.IsBetter for category names + var meterName = rule.MeterName; + if (meterName != null) { - return false; - } - // Rule "System.Net.*" matches meter "System.Net" and "System.Net.Http" - if (ruleMeterName.EndsWith(".*".AsSpan(), StringComparison.Ordinal)) - { - ruleMeterName = ruleMeterName.Slice(0, ruleMeterName.Length - 2); - } - // System.Net* matches System.Net and System.Net.Http - else if (starIndex != -1) - { - ruleMeterName = ruleMeterName.Slice(0, ruleMeterName.Length - 1); - } + const char WildcardChar = '*'; - // Rule "" matches everything - if (ruleMeterName.IsEmpty) - { - return true; + int wildcardIndex = meterName.IndexOf(WildcardChar); + if (wildcardIndex != -1 && + meterName.IndexOf(WildcardChar, wildcardIndex + 1) != -1) + { + throw new InvalidOperationException(SR.MoreThanOneWildcard); + } + + ReadOnlySpan prefix, suffix; + if (wildcardIndex == -1) + { + prefix = meterName.AsSpan(); + suffix = default; + } + else + { + prefix = meterName.AsSpan(0, wildcardIndex); + suffix = meterName.AsSpan(wildcardIndex + 1); + } + + if (!instrument.Meter.Name.AsSpan().StartsWith(prefix, StringComparison.OrdinalIgnoreCase) || + !instrument.Meter.Name.AsSpan().EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) + { + return false; + } } - // "System.Net" matches "System.Net" and "System.Net.Http" - return instrument.Meter.Name.AsSpan().StartsWith(ruleMeterName, StringComparison.OrdinalIgnoreCase) - // Exact match +/- ".*" - && (ruleMeterName.Length == instrument.Meter.Name.Length - // Only allow StartsWith on segment boundaries - || instrument.Meter.Name[ruleMeterName.Length] == '.'); + return true; } // Everything must already match the Instrument and listener, or be blank. diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx index ab0d5ca5a461b..0589f84ba89e7 100644 --- a/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx @@ -1,4 +1,64 @@ - + + + @@ -60,4 +120,7 @@ The meter factory does not allow a custom scope value when creating a meter. - + + Only one wildcard character is allowed in category name. + + \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs b/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs index da59cffc28d6a..2bf0e0d95a24f 100644 --- a/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs +++ b/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs @@ -214,13 +214,19 @@ public void RuleCanBeTurnedOffAndOnAgain() [InlineData("", "", "")] [InlineData("*", "", "")] [InlineData("lonG", "", "")] + [InlineData("lonG.", "", "")] [InlineData("lonG*", "", "")] [InlineData("lonG.*", "", "")] + [InlineData("lonG.sil", "", "")] + [InlineData("lonG.sil*", "", "")] [InlineData("lonG.sillY.meteR", "", "")] [InlineData("lonG.sillY.meteR*", "", "")] [InlineData("lonG.sillY.meteR.*", "", "")] + [InlineData("*namE", "", "")] + [InlineData("*.namE", "", "")] + [InlineData("*.sillY.meteR.Name", "", "")] + [InlineData("long*Name", "", "")] [InlineData("lonG.sillY.meteR.namE", "", "")] - [InlineData("lonG.sillY.meteR.namE.*", "", "")] [InlineData("", "instrumenTnamE", "")] [InlineData("lonG.sillY.meteR.namE", "instrumenTnamE", "")] [InlineData("", "", "listeneRnamE")] @@ -237,15 +243,13 @@ public void RuleMatchesTest(string meterName, string instrumentName, string list } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData("*.*", "", "")] [InlineData("", "*", "")] [InlineData("", "", "*")] - [InlineData("lonG.", "", "")] - [InlineData("lonG.sil", "", "")] - [InlineData("lonG.sil*", "", "")] [InlineData("sillY.meteR.namE", "", "")] + [InlineData(".*", "", "")] + [InlineData("*.", "", "")] + [InlineData("lonG.sillY.meteR.namE.*", "", "")] [InlineData("namE", "", "")] - [InlineData("*.namE", "", "")] [InlineData("wrongMeter", "", "")] [InlineData("wrongMeter", "InstrumentName", "")] [InlineData("wrongMeter", "", "ListenerName")] @@ -261,6 +265,17 @@ public void RuleMatchesNegativeTest(string meterName, string instrumentName, str }, meterName, instrumentName, listenerName).Dispose(); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void MultipleWildcardsThrows() + { + RemoteExecutor.Invoke(() => { + var rule = new InstrumentRule("*.*", null, null, MeterScope.Global, enable: true); + var meter = new Meter("Long.Silly.Meter.Name"); + var instrument = meter.CreateCounter("InstrumentName"); + Assert.Throws< InvalidOperationException>(() => ListenerSubscription.RuleMatches(rule, instrument, "ListenerName", new FakeMeterFactory())); + }).Dispose(); + } + [Theory] [MemberData(nameof(IsMoreSpecificTestData))] public void IsMoreSpecificTest(InstrumentRule rule, InstrumentRule? best, bool isLocalScope) @@ -388,3 +403,8 @@ private class FakeMeterFactory : IMeterFactory } } } + +internal class SR +{ + public static string MoreThanOneWildcard => "More than one wildcard is not allowed in a rule."; +} From 2e6a846af97552f04a2ecb8bce4aa9246e43329c Mon Sep 17 00:00:00 2001 From: Chris R Date: Tue, 15 Aug 2023 13:53:28 -0700 Subject: [PATCH 2/3] -1 => 0 --- .../src/Metrics/ListenerSubscription.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs index 7530ba64e150b..c6eb978af5441 100644 --- a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs +++ b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/ListenerSubscription.cs @@ -172,14 +172,14 @@ internal static bool RuleMatches(InstrumentRule rule, Instrument instrument, str const char WildcardChar = '*'; int wildcardIndex = meterName.IndexOf(WildcardChar); - if (wildcardIndex != -1 && - meterName.IndexOf(WildcardChar, wildcardIndex + 1) != -1) + if (wildcardIndex >= 0 && + meterName.IndexOf(WildcardChar, wildcardIndex + 1) >= 0) { throw new InvalidOperationException(SR.MoreThanOneWildcard); } ReadOnlySpan prefix, suffix; - if (wildcardIndex == -1) + if (wildcardIndex < 0) { prefix = meterName.AsSpan(); suffix = default; From 2554d8e2dab02af3f0eec2893cf2ff19aa17ffe0 Mon Sep 17 00:00:00 2001 From: Chris R Date: Tue, 15 Aug 2023 14:01:30 -0700 Subject: [PATCH 3/3] Weird overlapping match --- .../tests/ListenerSubscriptionTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs b/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs index 2bf0e0d95a24f..a0c5d281bfaa0 100644 --- a/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs +++ b/src/libraries/Microsoft.Extensions.Diagnostics/tests/ListenerSubscriptionTests.cs @@ -226,6 +226,7 @@ public void RuleCanBeTurnedOffAndOnAgain() [InlineData("*.namE", "", "")] [InlineData("*.sillY.meteR.Name", "", "")] [InlineData("long*Name", "", "")] + [InlineData("lonG.sillY.meter*MeteR.namE", "", "")] // Shouldn't match, but does, left for compatibility with Logging. [InlineData("lonG.sillY.meteR.namE", "", "")] [InlineData("", "instrumenTnamE", "")] [InlineData("lonG.sillY.meteR.namE", "instrumenTnamE", "")]