dotnet / extensions

This repository contains a suite of libraries that provide facilities commonly needed when creating production-ready applications.
MIT License
2.68k stars 759 forks source link

Inconsistent behavior in extended logging #4683

Closed xakep139 closed 11 months ago

xakep139 commented 1 year ago

Description

Current implementation of ExtendedLogger enables both enrichment and redaction regardless of what was enabled.

Reproduction Steps

Scenario №1:

using var loggerFactory =
    LoggerFactory.Create(builder =>
    {
        builder.EnableRedaction();
        builder.Services.AddLogEnricher<CustomEnricher>();
    });

var logger = loggerFactory.CreateLogger("MyDemoLogger");

Even though I haven't enabled enrichment, the CustomEnricher will be called.

Scenario №2:

using var loggerFactory =
    LoggerFactory.Create(builder =>
    {
        builder.EnableEnrichment();
        builder.Services.AddRedaction();
    });

var logger = loggerFactory.CreateLogger("MyDemoLogger");

Even though I haven't enabled redaction, the ErasingRedactor will be called to redact data annotated with data classification.

Scenario №3: One more confusing thing is that when I enable redaction feature, but don't have any redactors registered, I would get sensitive data being emitted as-is. Consider this example:

using var loggerFactory = LoggerFactory.Create(builder => builder.EnableRedaction());
var logger = loggerFactory.CreateLogger("MyDemoLogger");
Log.LogMethod(logger, "Sensitive user name");

internal static partial class Log
{
    [LoggerMessage(Level = LogLevel.Warning, Message = "User {Name} has now different status")]
    public static partial void LogMethod(ILogger logger, [PrivateData] string name);
}

Expected behavior

Each call to EnableRedaction() and EnableEnrichment() enables the corresponding feature only.

Actual behavior

Any of calls EnableRedaction() and EnableEnrichment() apply both of them.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

geeknoid commented 1 year ago

What's the actual issue with No 3?

geeknoid commented 1 year ago

From an implementation standpoint, calling either EnableRedaction or EnableEnrichment ends up activating the new extended logger, which enables all the new features.

xakep139 commented 1 year ago

From an implementation standpoint, calling either EnableRedaction or EnableEnrichment ends up activating the new extended logger, which enables all the new features.

I understand that, but from a user's perspective: if we have two different APIs that do one thing - then we don't need two APIs. If these two APIs mean different things, then we need to do two different things. We can update the implementation to have an internal mode (or two boolean properties) to adjust its behavior.

xakep139 commented 1 year ago

What's the actual issue with No 3?

That it doesn't redact anything - even though a user feels that redaction is enabled.