[API Proposal]: FakeLogger add ability to get unmodified state

[ghelyar]

Background and motivation

We're using Microsoft.Extensions.Logging for structured logging and want to write unit tests for it, so we tried using FakeLogger but it converts every state value to a string, which is quite annoying and makes it difficult to assert these values for more complex types, where you want to preserve the structure in the associated data. Even for bool, int, Guid etc you need to make sure that you assert against the string form instead of the original value.

It would be nice to be able to preserve and assert against the original unmodified state.

API Proposal

namespace Microsoft.Extensions.Logging.Testing;

public class FakeLogRecord
{
    public object? RawState { get; init; }
}

API Usage

    // NUnit example
    [Test]
    public void Test()
    {
        var logger = new FakeLogger<object>();
        var id = Guid.NewGuid();

        logger.LogInformation("The id is {id}", id);

        Assert.That(logger.LatestRecord.RawState, Is.EquivalentTo(new Dictionary<string, object>
        {
            { "{OriginalFormat}", "The id is {id}" },
            { "id", id }, // should not have to ToString() this
        }));
    }

Alternative Designs

• add an option on the FakeLogger that skips the string conversion on FakeLogRecord.State
• only use the string conversion for FakeLogRecord.StructuredState and not FakeLogRecord.State (but this is a breaking change)

Risks

Adding RawState with an init-only setter should be backwards compatible but they are not supported out of the box for all versions of .NET (e.g. standard/framework).

It could use a public setter here but then it's mutable, although that probably doesn't matter too much as it's only used by tests.

Adding a new parameter the existing constructor would be a breaking change, although most people probably aren't constructing this class from their own code. An optional parameter would be source compatible but binary breaking, but that might be acceptable given that it's only used by unit tests and unlikely to be a transitive dependency. Adding a second constructor overload would not be a breaking change.

[dariusclay]

Asserting against the string form allows you to validate the final log output that would be sent to a backend, including redaction, formatting, etc. How does asserting the interim (unmodified) state provide accurate validations?

[ghelyar]

If you're using structured logging with a log exporter that preserves structure then it's helpful to also test this structure is correct when the logging API is called. For example the log exporter we use JSON serialises the state values (the formatted message is just part of the output), and the monitoring provider we use treats strings and numbers differently, so 1 is not the same as "1" (e.g. if they are numbers you can filter with operations like greater than, but if they are strings then you can't)

You can test the string form by asserting the message property is correct, but you can't test that the original structure is correct because that's also converted to strings by the FakeLogger.

I just want to either be able to opt out of that conversion, or be able to access the raw data as well as the converted data.

[dariusclay]

Thanks for the clarifications. A very similar proposal for this was discussed recently in another PR. I will formalize this and get an API review scheduled for it.

[rwoll]

Conincidentally this relates to #5416, specifically the following snippet from the PR description:

Suggested Followup

While the original bug manifested as string formatting discrepancies, a test asserting the string result of the log is insufficient to robustly test. Asserting the generated output is helpful (as we've also added here), but may easily be incorrectly re-baselined.

To help create a more robust test in the future, I recommend:

1. Adding a test against the Type of each value in state.TagArray
2. Adding tests directly for TypeSymbolExtensions.{ImplementsIConvertible,ImplementsIFormattable,ImplementsISpanFormattable and the newly introduced helper, TypeSymbolExtensions.GetPossiblyNullWrappedType.

See also: #5436.