Use case-insensitive matching for source names in level overrides

[tillig]

[I'm unclear if this is the right repo for this or not; I'd be happy to move or refile if needed.]

Microsoft.Extensions.Configuration is a case-insensitive construct. For example, configuration.GetSection("a:b:c") will yield the same as configuration.GetSection("A:B:C"). This allows things like environment variables or command line parameters to participate in config but also be named in a contextually correct fashion (e.g., environment variable A__B__C).

In configuring the minimum level and overrides for log sources, I think I've discovered that the JSON configuration values are case-sensitive. For example, this...

{
  "serilog": {
    "minimumLevel": {
      "default": "Information",
      "override": {
        "microsoft": "Warning",
        "system": "Warning"
      }
    }
  }
}

...does not work - it still shows informational messages from Microsoft.* sources.

However, this:

{
  "serilog": {
    "minimumLevel": {
      "default": "Information",
      "override": {
        "Microsoft": "Warning",
        "System": "Warning"
      }
    }
  }
}

does work - only warnings show up for the Microsoft.* sources.

The only difference is the capitalization of the source names, which indicates the source names here are case-sensitive.

Granted, JSON itself is case-sensitive but since this is running through the MS configuration stuff, it can't really be assumed to be case-sensitive, especially if someone configures it through the environment:

SERILOG__MINIMUMLEVEL__OVERRIDE__MICROSOFT="Warning"

Where does that comparison/filtering take place? Is that a core Serilog thing? I could throw a PR in for that if I know where to start tracking it down.

[nblumhardt]

Hi Travis! Yes, this is the place :-)

You're right in that we bend the conventions a bit, here - because namespaces are case-sensitive we're stuck with {"Namespace": "Microsoft", "Level": "Debug"}, otherwise.

Not sure what the MEL provider does in this case - they use the same syntax - but I'd guess it's similar.

What do you think?

[tillig]

In MEL it appears they use a case-insensitive comparison for the category name (namespace).

However, let me drum up a quick unit test to verify this is the case and that I'm not looking at the wrong thing.

[tillig]

Verified, the namespace handling in MEL is case-insensitive. Here is a quick unit test project you can use to play with it. I borrowed some of the test stubs they use to get some in-memory logging going, so you'll see that in there, too.

LoggingTests.zip

Here's the test:

namespace LoggingTests
{
    public class LogFilterTests
    {
        [Theory]
        [InlineData("Logging:LogLevel:LoggingTests")]
        [InlineData("logging:loglevel:loggingtests")]
        [InlineData("logging:loglevel:LOGGINGTESTS")]
        public void CaseSensitivity(string configPath)
        {
            var configData = new Dictionary<string, string>
            {
                { "Logging:LogLevel:Default", "Debug" },
                { configPath, "Information" }
            };
            var config = new ConfigurationBuilder()
                .AddInMemoryCollection(configData)
                .Build();

            var sink = new TestSink();
            var logger = new ServiceCollection()
                .AddLogging(lb =>
                {
                    lb.AddConfiguration(config.GetSection("Logging"));
                    lb.ClearProviders();
                    lb.AddProvider(new TestLoggerProvider(sink, true));
                })
                .BuildServiceProvider()
                .GetRequiredService<ILogger<LogFilterTests>>();

            logger.LogInformation("Should be written.");

            Assert.Equal(1, sink.Writes.Count);

            logger.LogDebug("Will be filtered out.");

            Assert.Equal(1, sink.Writes.Count);
        }
    }
}

LoggingTests is my namespace, so I mocked up configuration that specifies the settings in a few different cases. In all of these, MEL handles it in a case-insensitive fashion.

By way of comparison, the only way we've stayed case-sensitive for type names in Autofac config is because the type is always the value and never the key. Given the syntax here, you're right, you're kinda stuck... but I might argue you're stuck with case insensitive comparisons for namespaces, so you can't have mynamespace and MyNamespace as two separate namespaces in the same project, right? Maybe folks just shouldn't do that? 🙈

Anyway, figured I'd raise it, since I was in the process of cleaning up the RANDOMCASING PeopleHadPut inTheConfigurationFiles in our apps and on going to camelCasing things broke.

[nblumhardt]

Thanks for the analysis. I don't think it'd hurt for us to switch to case-insensitive comparisons in this case; although it would introduce a slight inconsistency with the core Serilog project's Matching.FromSource(string) I think we could live with it. Fair to mark it as an enhancement, though :-D

[tillig]

Is this something that would happen here or in core Serilog? It appears it may be core.

• Specifying an override in configuration ends up calling Override on the LoggerConfiguration.MinimumLevel.
• Calling LoggerConfiguration.MinimumLevel.Override adds override values to an override dictionary inside LoggerConfiguration.
• Overrides for a logger get added to a LevelOverrideMap.
• LevelOverrideMap appears to be where the case-sensitive comparison is happening.

I think the fix would be in LevelOverrideMap unless I'm mistaken. Does that sound right?

[nblumhardt]

Ah, yes of course - I forgot that this is implemented using LevelOverrideMap. Hmmm 🤔 - changing it there would have a bit more impact and would need more careful consideration. I'll move this issue over to that repo and revisit when I can 👍

[tillig]

In the event it helps (or not?) I looked at how log4net does it.

In .NET desktop framework, it's using object equality. In .NET Core/compact it uses the string type's == override. The object equality is because they're interning strings in .NET desktop framework.

Anyway, this boils down to log4net being case-sensitive for its logger names.

That said, log4net is also pretty tightly tied to loggers being based on types - a "logger repository" in there is tied to an assembly and used as a central sort of cache key. You could ostensibly have two different loggers for any given type if there were two different repositories, each based on a different assembly. It's... an interesting and somewhat complex thing.

[nblumhardt]

Thanks for the follow-up. I think log4net was probably the origin of this decision (it was the main influence on conventions in Serilog at the beginning).

This needs some thought - unfortunately it's not a change we can make quickly, but I'll definitely loop back around to this when I have time to think it through properly.

[tillig]

No worries, just trying to help. Let me know if there's anything else I can do.

[zvirja]

There is an exotic issue related to Ubuntu-based docker images. Apparently the convention for Ubuntu is to have all the environment variable keys uppercased and without DOTS in names. If I configure docker image and pass key not in upper case or containing dot, apparently Ubuntu simply ignores that environment variable. Could be related to this. So we are restricted to uppercase characters and underscores in key.

That lead to an issue if I try to override minimum level with env variable as

this is simply omitted by Ubuntu:

SERILOG__MINIMUMLEVEL__OVERRIDE__MyApp.MyService=Verbose

and this does not work due to case-sensitive match and replaced DOT:

SERILOG__MINIMUMLEVEL__OVERRIDE__MYAPP_MYSERVICE=Verbose

We could of course perform case-insensitive match, but that would not help. For Product.My_Service and Product.My.Service configuration will looks the same -> PRODUCT_MY_SERVICE, so normalization will need to be implemented at a moment we test input value, which is of course dirty 😟

I do not see a nice way to overcome that without changing configuration schema (so that type name is passed in value, not key). But wanted to post it here, so you are aware 😉

[nblumhardt]

I don't think this is a change we are likely to make in the near future; closing as stale, but thanks for raising it nonetheless. (Hope all's well with you @tillig 👋)

[tillig]

Somewhat related - serilog/serilog-settings-configuration#386 found that the parser for the log level enum in configuration was inconsistently case-sensitive and case-insensitive, and that got resolved to be case-insensitive everywhere. That makes things just slightly more inconsistent: enum parsing is case-insensitive, but the keys are case-sensitive.