Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LoggerFilterRule.CategoryName matches overlapping prefix and suffix #110890

Open
KalleOlaviNiemitalo opened this issue Dec 22, 2024 · 2 comments
Open

Comments

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Dec 22, 2024

Description

In Microsoft.Extensions.Logging, if LoggerFilterRule.CategoryName includes the wildcard *, then it matches categories that have the correct prefix and the correct suffix. However, it should not match categories in which the prefix overlaps the suffix. For example, LoggerFilterRule.CategoryName = "Environment*Mentalist" matches "Environmentalist" but it should not.

Reproduction Steps

LogCategoryWildcardDemo.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.1" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.1" />
  </ItemGroup>

</Project>

Program.cs:

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace LogCategoryWildcardDemo
{
    internal static class Program
    {
        private static void Main()
        {
            var serviceCollection = new ServiceCollection();
            serviceCollection.AddLogging(
                (ILoggingBuilder logging) =>
                {
                    logging.SetMinimumLevel(LogLevel.Critical);
                    logging.AddFilter(category: "Environment*Mentalist", LogLevel.Information);
                    logging.AddSimpleConsole();
                });

            using (ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider())
            {
                ILoggerFactory loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();

                LogAtEachLevel("Environmentalist");
                LogAtEachLevel("Environment.Mentalist");
                LogAtEachLevel("Spectacle");

                void LogAtEachLevel(string categoryName)
                {
                    ILogger logger = loggerFactory.CreateLogger(categoryName);
                    for (LogLevel level = 0; level < LogLevel.None; level++)
                    {
                        logger.Log(level, $"{level} message from {categoryName}");
                    }
                }

                // Close the service provider so that all logs are flushed.
            }
        }
    }
}

dotnet run

Expected behavior

The "Environment*Mentalist" filter rule should not apply to the "Environmentalist" category, whose log level should then be LogLevel.Critical, as configured using SetMinimumLevel.

crit: Environmentalist[0]
      Critical message from Environmentalist
info: Environment.Mentalist[0]
      Information message from Environment.Mentalist
warn: Environment.Mentalist[0]
      Warning message from Environment.Mentalist
fail: Environment.Mentalist[0]
      Error message from Environment.Mentalist
crit: Environment.Mentalist[0]
      Critical message from Environment.Mentalist
crit: Spectacle[0]
      Critical message from Spectacle

Actual behavior

The "Environment*Mentalist" filter rule incorrectly applies to the "Environmentalist" category and changes its log level to LogLevel.Information.

info: Environmentalist[0]
      Information message from Environmentalist
warn: Environmentalist[0]
      Warning message from Environmentalist
fail: Environmentalist[0]
      Error message from Environmentalist
crit: Environmentalist[0]
      Critical message from Environmentalist
info: Environment.Mentalist[0]
      Information message from Environment.Mentalist
warn: Environment.Mentalist[0]
      Warning message from Environment.Mentalist
fail: Environment.Mentalist[0]
      Error message from Environment.Mentalist
crit: Environment.Mentalist[0]
      Critical message from Environment.Mentalist
crit: Spectacle[0]
      Critical message from Spectacle

Regression?

Not a regression; it worked like this ever since support for wildcards was added in aspnet/Logging#924, between .NET Core 2.2 and 3.0.

Known Workarounds

No response

Configuration

.NET SDK 9.0.101 on Windows 10 x64.
.NET Runtime 8.0.11.
NuGet package versions are shown in Reproduction Steps.

Other information

The bug is around here:

if (!category.AsSpan().StartsWith(prefix, StringComparison.OrdinalIgnoreCase) ||
!category.AsSpan().EndsWith(suffix, StringComparison.OrdinalIgnoreCase))
{
return false;
}

Could add a separate category.Length < prefix.Length + suffix.Length check, or perhaps use category.AsSpan(prefix.Length).EndsWith(suffix, StringComparison.OrdinalIgnoreCase).

Also noted in dotnet/extensions#5635 (comment), as the pull request was introducing the same bug to that repository.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 22, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

@tarekgh tarekgh added this to the Future milestone Dec 24, 2024
@tarekgh tarekgh removed the untriaged New issue has not been triaged by the area owner label Dec 24, 2024
@tarekgh
Copy link
Member

tarekgh commented Dec 24, 2024

IIRC, this was discussed long ago and kept the behavior for app compatibility reason. Even I recall the same done in the metric configuration too.

#90559 (review)
CC @noahfalk @Tratcher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants