Runtime: Enhance user-facing API for strongly-typed ILogger messages

[lodejard]

Is your feature request related to a problem? Please describe.

I'm trying to declare a strongly-typed code path for ILogger messages, but it is difficult to use the Action-returning static method LoggerMessage.Define<T1,T2,...,Tn> in a way that's approachable and maintainable by an entire team.

Describe the solution you'd like

It would be great if there was a strongly-typed struct which could wrap the underlying LoggerMessage.Define call and Action<ILoggerFactory, ...., Exception> field - even completely hide the actual Action signature. Ideally it would also provide more appropriate intellisense on the struct method which calls the action. If would be even cleaner if it could be initialized without repeating the generic argument types, and in most end-user applications if the EventId number was optional it could be derived from the EventId name instead.

internal static class MyAppLoggerExtensions
{
  private readonly static LogMessage<string, int> _logSayHello = (
    LogLevel.Debug, 
    nameof(SayHello), 
    "The program named {ProgramName} is saying hello {HelloCount} times");

  /// <summary>
  /// The program named {ProgramName} is saying hello {HelloCount} times
  /// </summary>
  /// <param name="logger">The logger to write to</param>
  /// <param name="programName">The {ProgramName} message property</param>
  /// <param name="helloCount">The {HelloCount} message property</param>
  public static void SayHello(this ILogger<MyApp> logger, string programName, int helloCount)
  { 
    _logSayHello.Log(logger, programName, helloCount);
  }
}

A LogMessage struct would be a allocation-free. Having implicit operators for tuples which match the constructor parameters enables field initialization without needing to repeat the generic argument types.

public struct LogMessage<T1, ..., Tn>
{
  public LogMessage(LogLevel logLevel, EventId eventId, string formatString);
  public LogMessage(LogLevel logLevel, int eventId, string formatString);
  public LogMessage(LogLevel logLevel, string eventName, string formatString);

  public LogLevel LogLevel {get;}
  public EventId EventId {get;}
  public string FormatString {get;}

  public void Log(ILogger logger, T1 value1, ..., Tn valuen);
  public void Log(ILogger logger, Exception exception, T1 value1, ..., Tn valuen);

  public static implicit operator LogMessage<T1, ..., Tn>((LogLevel logLevel, EventId eventId, string formatString) parameters);
  public static implicit operator LogMessage<T1, ..., Tn>((LogLevel logLevel, int eventId, string formatString) parameters);
  public static implicit operator LogMessage<T1, ..., Tn>((LogLevel logLevel, string eventName, string formatString) parameters);
}

Describe alternatives you've considered

Having one struct per LogLevel could also remove the need to have the first "LogLevel" argument in the initializer - but that would be a larger number of classes and intellisense wouldn't list the level names for you like typing LogLevel. does

[davidfowl]

@lodejard After giving this a second look I wonder how much this matters given that the logger message is mostly used as an internal detail of these static methods. Would it be better to use a type as IDL for code generating one of these objects.

```C#
public partial class Log
{
[LoggerMessage(LogLevel.Debug, 1, nameof(SayHello), "The program named {ProgramName} is saying hello {HelloCount} times")]
public static partial void SayHello(ILogger logger, string name, int helloCount);
}

Then it would generate this:

```C#
public partial class Log
{
    private static readonly Action<ILogger, string, int, Exception?> _sayHello = 
        LoggerMessage.Define<string, int>(LogLevel.Information, new EventId(1, "SayHello"), "The program named {ProgramName} is saying hello {HelloCount} times");

    public static partial void SayHello(ILogger logger, string programName, int helloCount)
    {
        _sayHello(logger, programName, helloCount, null);
    }
}

Usage would look like:

```C#
// Without extension methods
Log.SayHello(logger, "Louis", 10);

// With extension methods
logger.SayHello("Louis", 10);
```

We need partial extension methods @MadsTorgersen @jaredpar. I keep running into cases where small language limitations make the user experience slightly worse for source generators.

[davidfowl]

cc @noahfalk @shirhatti

[geeknoid]

I've been working on a model that uses C# 9.0 source generators. The developer writes:

    [LogInterface(generatedTypeName: "DiagExtensions", generateExtensionMethods: true)]
    interface IDiagExtensions
    {
        [LogMethod("Could not open socket to `{hostName}`", LogLevel.Critical)]
        void CouldNotOpenSocket(string hostName);
    }

And the following is generated automatically:

namespace LoggingExample
{

    static class DiagExtensions
    {

        private readonly struct __CouldNotOpenSocketStruct__ : IReadOnlyList<KeyValuePair<string, object>>
        {
            private readonly string hostName;

            public __CouldNotOpenSocketStruct__(string hostName)
            {
                this.hostName = hostName;

            }

            public string Format()
            {
                return $"Could not open socket to `{hostName}`";
            }

            public KeyValuePair<string, object> this[int index]
            {
                get
                {
                    switch (index)
                    {
                        case 0:
                            return new KeyValuePair<string, object>(nameof(hostName), hostName);

                        default:
                            throw new ArgumentOutOfRangeException(nameof(index));
                    }
                }
            }

            public int Count => 1;

            public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
            {
                yield return new KeyValuePair<string, object>(nameof(hostName), hostName);

            }

            IEnumerator IEnumerable.GetEnumerator()
            {
                return GetEnumerator();
            }
        }
        private static readonly EventId __CouldNotOpenSocketEventId__ = new(0, nameof(CouldNotOpenSocket));

        public static void CouldNotOpenSocket(this ILogger logger, string hostName)
        {
            if (logger.IsEnabled((LogLevel)5))
            {
                var s = new __CouldNotOpenSocketStruct__(hostName);
                logger.Log((LogLevel)5, __CouldNotOpenSocketEventId__, s, null, (s, ex) => s.Format());
            }
        }

    }
}

There is also an option to generate a wrapper type for the ILogger, rather than extension methods:

    [LogInterface(generatedTypeName: "DiagWrapper")]
    interface IDiagWrapper
    {
        [LogMethod("Could not open socket to `{hostName}`", LogLevel.Critical)]
        void CouldNotOpenSocket(string hostName);
    }

which comes out as:

namespace LoggingExample
{

    sealed class DiagWrapper : IDiagWrapper
    {
        private readonly ILogger __logger;

        public DiagWrapper(ILogger logger) => __logger = logger;


        private readonly struct __CouldNotOpenSocketStruct__ : IReadOnlyList<KeyValuePair<string, object>>
        {
            private readonly string hostName;

            public __CouldNotOpenSocketStruct__(string hostName)
            {
                this.hostName = hostName;

            }

            public string Format()
            {
                return $"Could not open socket to `{hostName}`";
            }

            public KeyValuePair<string, object> this[int index]
            {
                get
                {
                    switch (index)
                    {
                        case 0:
                            return new KeyValuePair<string, object>(nameof(hostName), hostName);

                        default:
                            throw new ArgumentOutOfRangeException(nameof(index));
                    }
                }
            }

            public int Count => 1;

            public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
            {
                yield return new KeyValuePair<string, object>(nameof(hostName), hostName);

            }

            IEnumerator IEnumerable.GetEnumerator()
            {
                return GetEnumerator();
            }
        }
        private static readonly EventId __CouldNotOpenSocketEventId__ = new(0, nameof(CouldNotOpenSocket));

        public void CouldNotOpenSocket(string hostName)
        {
            if (__logger.IsEnabled((LogLevel)5))
            {
                var s = new __CouldNotOpenSocketStruct__(hostName);
                __logger.Log((LogLevel)5, __CouldNotOpenSocketEventId__, s, null, (s, ex) => s.Format());
            }
        }

    }
}

I believe this code is both more convenient and more efficient than existing approaches used in the wild.

[geeknoid]

Based on the example I provided above, I propose that we extend the framework with support to automatically generate strongly-type logging methods, based on a developer-authored interface type. The developer provides the minimum amount of information necessary, and the source generator produces a highly efficient set of logging methods. The generated logging methods can be produced as extension methods on ILogger, or can be produced as a type that wraps ILogger and exposes instance logging methods.

More specifically:

• Introduce the types LogInterfaceAttribute and LogMethodAttribute to the Microsoft.Extensions.Logging namespace in the Microsoft.Extensions.Logging.Abstraction assembly.

    /// <summary>
    /// Indicates an interface defines strongly-typed logging methods
    /// </summary>
    /// <remarks>
    /// This interface is a trigger to cause code generation of strongly-typed methods
    /// that provide a convenient and highly efficient way to log through an existing
    /// ILogger interface.
    /// 
    /// The declaration of the interface to which this attribute is applied cannot be
    /// nested in another type.
    /// </remarks>
    [System.AttributeUsage(System.AttributeTargets.Interface, AllowMultiple = false)]
    public sealed class LogInterfaceAttribute : Attribute
    {
        /// <summary>
        /// Creates an attribute to annotate an interface as triggering the generation of strongly-typed logging methods.
        /// </summary>
        /// <param name="generatedTypeName">Determines the name of the generated type.</param>
        /// <param name="generateExtensionMethods">Determines whether the generated type uses extension methods or instance methods.</param>
        public LogInterfaceAttribute(string generatedTypeName, bool generateExtensionMethods = false) => (GeneratedTypeName, GenerateExtensionMethods) = (generatedTypeName, generateExtensionMethods);

        /// <summary>
        /// Gets the name of the generated type.. 
        /// </summary>
        public string GeneratedTypeName { get; }

        /// <summary>
        /// Gets whether the generated type exposes the strongly-typed methods as extension methods or instance methods.
        /// </summary>
        public bool GenerateExtensionMethods { get; }
    }

    /// <summary>
    /// Provides information to guide the production of a strongly-typed logging method.
    /// </summary>
    /// <remarks>
    /// This attribute is applied to individual methods in an interface type annotated with [LogInterface].
    /// </remarks>
    [System.AttributeUsage(System.AttributeTargets.Method, AllowMultiple = false)]
    public sealed class LogMethodAttribute : Attribute
    {
        /// <summary>
        /// Creates an attribute to guide the production of a strongly-typed logging method.
        /// </summary>
        /// <param name="message">The message text output by the logging method. This string is a template that can contain any of the method's parameters.</param>
        /// <param name="level">THe logging level produced when invoking the strongly-typed logging method.</param>
        /// <example>
        /// [LogInterface("Diag")]
        /// interface IDiag
        /// {
        ///     [LogMethod("Could not open socket for {hostName}", LogLevel.Critical)]
        ///     void CouldNotOpenSocket(string hostName);
        /// }
        /// </example>
        public LogMethodAttribute(string message, LogLevel level) => (Message, Level) = (message, level);

        /// <summary>
        /// Gets the message text for the logging method.
        /// </summary>
        public string Message { get; }

        /// <summary>
        /// Gets the logging level for the logging method.
        /// </summary>
        public LogLevel Level { get; }
    }

• Introduce a source generator in the Microsoft.Extensions.Logging.Generators namespace in a new Microsoft.Extensions.Logging.Generators.dll assembly. This source generator would interpret the [LogInterface] and [LogMethods] attributes to generate strongly-typed logging methods for every logging method defined by the user. The motivation for putting the generator logic into a separate assembly is to avoid pulling in this build-time assembly into the application distribution.

I haven't contributed to this project before, but I'd be happy to learn the ropes and get something like the above checked in.

[davidfowl]

A couple of pieces of feedback:

• I think we should ditch the interface and instead should directly use the static partial class with static methods. It feels a bit strange to use an interface as IDL if we don't generate an implementation (the extension method case).
• We need to define how event ids and event names are generated based on the methods. They can't change if new methods are added or removed. These attributes should optionally let you specify an EventId and EventName as an override.
• We need to clearly define what happens when the type of method is unsupported:
  
  
  
  • Do we support generic methods? What does the event name become then?
  
  
  • Do we support overloads? How does it fail? Does it throw? I propose a diagnostic error.
  
  
• EventSource has the EventAttribute as the attribute, I'd prefer a LoggerMessage attribute since that mathes the type we have today (LoggerMessage). We might also need a [Logger] attribute since we don't have a base class but it would be a way to identify the static class to generate methods into.
• We need to discuss how methods that pass exceptions get handled. They should automagically get passed into the exception part of the Log method.
• We should error when the message template references a property (case insensitive) that isn't a declared parameter.

```C#
[AttributeUsage(AttributeTargets.Class, Inherited = false, AllowMultiple = false)]
public sealed class LoggerAttribute : Attribute
{
}

[AttributeUsage(System.AttributeTargets.Method, Inherited = false, AllowMultiple = false)]
public sealed class LoggerMessageAttribute : Attribute
{
public LoggerMessageAttribute(LogLevel logLevel, string formatString)
{
LogLevel = logLevel;
FormatString = formatString;
}

public LogLevel LogLevel { get; }

public string FormatString { get; }

public int? EventId { get; set; }

public string EventName { get; set; }

}

In the generated code:

- If the event id is null, it will generate one based on the name.
- If the event name if null, it will use the method name as the event name.

```C#
[Logger]
public partial class Log
{
    [LogMessage(LogLevel.Debug, "The program named {ProgramName} is saying hello {HelloCount} times")]
    public static partial void SayHello(ILogger logger, string name, int helloCount);
}

Would generate in the same class (note that it's partial):

```C#
public partial class Log
{
private static readonly Action _sayHello =
LoggerMessage.Define(LogLevel.Information, new EventId(1, "SayHello"), "The program named {ProgramName} is saying hello {HelloCount} times");

public static partial void SayHello(ILogger logger, string programName, int helloCount)
{
    _sayHello(logger, programName, helloCount, null);
}

}

Alternative generated code (based on the above):

```C#
public partial class Log
{
    private readonly struct __SayHelloStruct__ : IReadOnlyList<KeyValuePair<string, object>>
    {
        private readonly string programName;
        private readonly int helloCount;

        public __SayHelloStruct__(string programName, int helloCount)
        {
            this.programName = programName;
            this.helloCount = helloCount;

        }

        public string Format()
        {
            return $"The program named {programName} is saying hello {helloCount} times";
        }

        public KeyValuePair<string, object> this[int index]
        {
            get
            {
                switch (index)
                {
                    case 0:
                        return new KeyValuePair<string, object>(nameof(programName), programName);

                    case 1:
                        return new KeyValuePair<string, object>(nameof(helloCount), helloCount);

                    default:
                        throw new ArgumentOutOfRangeException(nameof(index));
                }
            }
        }

        public int Count => 1;

        public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
        {
            yield return new KeyValuePair<string, object>(nameof(programName), programName);
            yield return new KeyValuePair<string, object>(nameof(helloCount), helloCount);
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }

    // Generated EventId from the event name
    private static readonly EventId __SayHelloEventId__ = new(1, nameof(SayHello));

    public static partial void SayHello(string programName, int helloCount)
    {
        if (__logger.IsEnabled((LogLevel)2))
        {
            var message = new __SayHelloStruct__(programName, helloCount);
            __logger.Log((LogLevel)2, __SayHelloEventId__, message, null, (s, ex) => s.Format());
        }
    }
}

This is more efficient than LoggerMessage.Define (we can move all of the format string parsing to compile time) but bloats the codegen a little bit as there's a unique struct per method. There's an in between here where we can use a predefined generic struct like this (https://github.com/dotnet/runtime/blob/583a5367d853e5a15f23b39fae3461aef92af1d4/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LoggerMessage.cs#L636), but it would have a bigger runtime representation as it would need to store reference to something that held onto the names of the values.

I'd recommend we go with the more efficient codegen sacrificing code size but am open to suggestions.

[davidfowl]

Another benefit would be that we can improve startup time by shifting to this model completely in ASP.NET Core itself (see https://github.com/dotnet/runtime/pull/44746)

[7sharp9]

How would this be consumed by non C# languages?

[geeknoid]

@davidfowl Thanks for the excellent feedback.

• About using a partial class to as IDL, I think that's a reasonable pattern and is where I started originally. But I also really like the idea of a wrapping type rather than interface methods. The IntelliSense experience is so much nicer since you don't have the pollution (of temptation) of things like Logger.LogInformation available. It'd be easy to make the generator flexible here, let me experiment a bit.

• EventId handling you suggest makes total sense.

• I'll add error checking to deal with oddball method declaration types.

• I don't like the attribute [Logger] as that conflates things with ILogger/Logger. This stuff is not itself a logger, its all about wrapping a logger. Any other ideas?

• The model I implemented for exceptions is that it takes the first parameter that is an exception type and passes that as the exception parameter to the logger. To reference the exception in the message, you just use the parameter name in a template like any other parameter.

• You'll already get appropriate errors if you specify invalid symbols in the format string. That happens naturally since the format string is emitted as-is in the generated code and the C# compiler will complain of any problems.

[cgillum]

I love the idea of using source generators to make structured logging easier. I had to implement my own mini-framework for this recently and it’s still much more work to maintain than I’d like. One blocker I ran into though is the limitation of 6 parameters in LoggerMessage.Define. I would hope a source generator solution would not be as limited in terms of structured log parameters.

[geeknoid]

How about three attributes:

[LoggerExtensions] - applied to a partial class, code generation will produce static extension methods in that class
[LoggerWrapper] - applied to an interface, code generation will produce a wrapper class around ILogger
[LoggerMessage] - applied to methods to declare a logging message.

[lodejard]

I like this a lot!! It's very close to something I prototyped earlier (with Castle dynamic proxy) to pitch to our team to evolve our strongly-typed logging story.

I would suggest this is more like a specialization of a logger interface rather than a wrapper - so optionally inheriting ILogger<T> would be completely appropriate. There are some use cases to talk about later but there are a handful of reasons I'd strongly recommend public interface ISomethingLogger : ILogger<Something> as a starting point.

Curious how much overhead there is for the message format parsing. Is that something that can be quantified or profiled? Otherwise I'd suggest LoggerMessage.Define is a fine starting point and could be optimized later as a private implementation detail later on if we need.

Regarding EventId - totally agree! That is are very undervalued piece of data most user code leaves out. I'd suggest the method name like "SayHello" makes a perfect EventId string. In the absence of a user provided number having a stable hash of the name would be a fine EventId int.

I'd suggest having the Exception parameter be either first or last rather than anywhere in the list. TBH, last would probably be most natural. In the existing LogWarning(...) overloads the Exception is first only because of how overload resolution works with variable params...

(More thoughts --- need to run to meeting for now though)

[davidfowl]

I would suggest this is more like a specialization of a logger interface rather than a wrapper - so optionally inheriting ILogger would be completely appropriate. There are some use cases to talk about later but there are a handful of reasons I'd strongly recommend public interface ISomethingLogger : ILogger as a starting point.

I'm not a fan of having to inject something different that I have to today. That's why I like the extension/static method approach.

If we have this extension approach then I am suggest that it's optional.

Curious how much overhead there is for the message format parsing. Is that something that can be quantified or profiled? Otherwise I'd suggest LoggerMessage.Define is a fine starting point and could be optimized later as a private implementation detail later on if we need.

Enough to affect the startup time (which we now care about more now). I'm fine with simple codegen to start but @geeknoid already it working and we can compare them.

[lodejard]

I'm not a fan of having to inject something different that I have to today. That's why I like the extension/static method approach.

If we have this extension approach then I am suggest that it's optional.

Makes sense - also I found enough of the old prototype code to kit together as an example.

https://github.com/lodejard/logger-interfaces/blob/main/LoggerCodeGen/Program.cs

TBH having analyzer codefix to turn logger.LogInformation(...) lines into extension methods was my first thought also. There were some folks who pushed back on the idea --- there was just enough code in the extension methods cs file (and redundancy with parameter types repeated in several places) that ended up being a nudge towards using an interface as a logging DSL.

In both cases (interface and/or extensions) I'd suggest an analyzer codefix acting on logger.LogXxxx calls could be a key part of the experience.

[geeknoid]

I created a repo for my current work on this front: https://github.com/geeknoid/LoggingGenerator. Comments welcome.

[geeknoid]

@lodejard What kind of heuristic are you thinking of to convert LogXXX calls to extension methods? New names need to be invented for the strongly-typed methods, so I'm not sure where those should come from.

[lodejard]

@geeknoid good question! At the time was thinking if the overload involved a new EventId(...) or reference to a static EventId field with an initializer - then an initial event name (and number) could be picked up that way.

Failing that, if it's a single Logger.LogXXX(...) being hit by fixup, would try to do the same thing as "extract method" --- where the line becomes Logger.Message1(...) where Message1 remains selected and in rename mode.

In the case that you are "entire project | solution" a code-base that has only format strings the options are limited. Some possibilities as thought exercise:

1. You could try to synthesize a short name from the message, though I suspect that will would never give you great results (especially across different languages).
2. That said at the time I recall thinking, instead of moving to strong method name in one step, have a different fixup for those Log overloads for "add EventId". That would result in Logger.LogXXX(new EventId(42, "EventName"), ...); and you would also have an analyzer error squiggle when the constant "EventName" appears in that constructor "Please provide a meaningful event name"
3. Instead of an intermediate state for multi fixup go right to creating all of the Message1, Message2, etc. with intentionally bad names. An analyzer warning on those logger method names could also prompt the user to ctrl-R-R rename something meaningful.

Typing that out loud - I think I'd lean towards #2 actually. Even if a user didn't want to go all of the way to strongly-typed log messages, it would be a genuinely useful fixup IMO to be able to add new EventId in every log line in a solution. Especially if the analyzer could figure out the "highest visible plus one" EventId number to put in spot-by-spot.

1. Was also kind of kicking around the idea of a user-assisted hint in the format string. For example, in Logger.LogInformation("SayHello: The user {AccountName} has said {Greeting}", username, hellotext); if you match the pattern "alphanum+colon+1ormorespace+message" break it into name "SayHello" and message "The user {AccountName} has said {Greeting}".

4 might be a stretch --- it's not a format that you expect naturally. I was thinking of it because it's the flow I would like to use myself personally. e.g. Declare any new messages inline as I'm working with minimal effort (and without switching to another file) then when I'm ready to turn the change into a PR (and done fussing with message text) I'd ctrl-dot to strongly-type all of the messages in the solution.

[lodejard]

@geeknoid @davidfowl to attempt a summary of strawman :) this is based on just #2 from above

Step one, solution-wide fixup to eventid names where missing:
Logger.LogInformation("{User} says hi.", user);
fixup -> Logger.LogInformation(new EventId(73, "EventName"), "{User} says hi.", user);

Step two, every "EventName" is an error
edit -> Logger.LogInformation(new EventId(73, "SayHello"), "{User} says hi.", user);

Step three, solution-wide fixup to strongly-type events
Logger.LogInformation(new EventId(42, "SayHello"), "{User} says hi.", user);
fixup -> Logger.SayHello(user);

[davidfowl]

I think the code fix and source generator complement each other but aren't directly coupled. We should focus on fleshing out the design for the source generator and then follow up with how we move existing calls to it (if possible)

[noahfalk]

I like this idea : ) I assume we'll need to iterate a bit to find what works best but various thoughts...

Static vs. Instance vs. Interface

I think we should ditch the interface and instead should directly use the static partial class with static methods. It feels a bit strange to use an interface as IDL if we don't generate an implementation (the extension method case).

I might be neglecting some source generator limitation but I don't think anything stops us from generating the interface implementation? I could imagine all of these being possible (omiting attributes for brevity):

interface
```C#
interface IMyLog { SayHello(string arg1, int arg2); }
// generated code
class MyLog : IMyLog { IMyLog.SayHello(string arg1, int arg2) { ... //impl here } }
````

partial instance
```C#
partial struct MyLog { partial SayHello(string arg1, int arg2); }
// generated code
partial struct MyLog { partial SayHello(string arg1, int arg2) { ... //impl here } }
````

partial static
```C#
partial static class MyLog { partial static SayHello(ILogger logger, string arg1, int arg2); }
// generated code
partial static class MyLog { partial static SayHello(ILogger logger, string arg1, int arg2) { ... //impl here } }
````

In terms of usage calling the instance forms seem preferable and discovering the instance field via is probably a hair easier than discovering the static type if you are working in new code.
C# myLog.SayHello("artichoke", 19); // interface or partial instance MyLog.SayHello(_logger, "artichoke", 19); // static

I would expect the interface option to have more perf overhead than the partial value-typed instance or static approaches because we pay for an extra GC heap allocation and interface dispatch at the callsite won't be inlinable. Nothing huge but if you care about nanoseconds for TechEmpower-like speeds then it could show up.

Personally I like the partial struct best at the moment but it isn't a strong preference. We also don't have to pick winners and losers necessarily, a source generator could support all of them simultaneously if we had strong reasons to want multiple options.

DI injection

I could also imagine that interface or partial instance forms give an option (but not a requirement) to inject directly with some appropriate build or runtime discovery mechanism (I'm potentially handwaving something tricky here):
````C#
class HomeController
{
readonly MyLog _log;

public HomeController(MyLog log) => _log = log; // option 1
public HomeController(ILogger<HomeController> logger) => _log = new MyLog(logger); // option 2
public IActionResult Index()
{
    _log.SayHello("Tim", 9);
    return View();
}

}
````

One question the injectable instance raises is where does the category come from. Two options would be make the type generic like ILogger or specify the category in an attribute + use ILoggerFactory as the argument. The latter would be similar to EventSource which takes a name argument in its attribute.

CodeGen

I'd recommend we go with the more efficient codegen sacrificing code size but am open to suggestions.

I think a measurement would be worthwhile. I wouldn't be surprised if the binary size increase is substantial and improved logging performance is slight. As a quick example I added one instance of the strongly typed struct to an existing project and the size grew by 3KB. Direct extrapolation probably has high error, but even 1/4th of that feels like a substantial cost to pay.

it would have a bigger runtime representation as it would need to store reference to something that held onto the names of the values.

There are also sneaky stunts that can use the generic type information to carry strings without increasing the instance size:
````C#
struct LogValues where T : IFormatData2, new()
{
string Format()
{
T f = new T();
return string.Format(f.Format, _arg1, arg2);
// the JIT can devirtualize this and inline so it is as efficient as string.Format("Hello {0} {1}", _arg1, _arg2);
}
}

interface IFormatData2
{
String Format { get; }
String Arg1Name { get; }
String Arg2Name { get; }
}

struct SayHelloNames : IFormatData2
{
public string Format => "Hello {0} {1}";
public string Arg1Name => "Name";
public string Arg2Name => "Count";
}
``` Effectively we took all the entropy that was in the _formatters field and packed it into the hidden generic dictionary. Binary size (and runtime type-system memory size) is probably smaller than the__SayHelloStruct__` option but bigger than the fully generic LogValues struct.

[geeknoid]

My prototype is continuing to evolve. See https://github.com/geeknoid/LoggingGenerator.

There is now an analyzer component that flags use of legacy logging methods, and a fixer component that provides one-click support to turn a legacy logging method call into a new strongly-type call, and generate the appropriate logger message definition which will then trigger the main code generator.

See the README.md file for the example generated code.

[JamesNK]

Feature requests:

1. Validate that log names and IDs are unique. Bugs are caused by copy & paste and devs forgetting to update the ID
2. Provide a way to mark an ID shouldn't be used. A log message might be removed and its ID should be reserved. Example of this feature in protobuf - https://developers.google.com/protocol-buffers/docs/proto3#reserved

For 2, you could add a ReservedIds property to the Logger attribute. If a reserved ID is used then compilation fails.

[Logger(ReservedIds = new[] { 5, 7, 10 })]
public partial class Log
{
    [LogMessage(LogLevel.Debug, "The program named {ProgramName} is saying hello {HelloCount} times")]
    public static partial void SayHello(ILogger logger, string name, int helloCount);
}

We noticed some accidental duplicate IDs in Kestrel logging. Features like these would prevent them.

https://github.com/dotnet/aspnetcore/issues/28261

[geeknoid]

@JamesNK So what's the expected scope of uniqueness for the id values?

Right now, the generator will complain if ids are overlapping within the context of a single logging type. I expect the common case will be there will be one logging type defined either per assembly or per folder/namespace in each assembly. So I'm ensuring that all the methods in a single interface have unique ids. But is that enough? Should the Ids be unique throughout the whole assembly? Or perhaps the ids should be unique per TCategory of an ILogger? My generator supports defining your logging methods relative to either ILogger or ILogger, so maybe that's enough? Specifically, if you have:

namespace Foo
{
    partial class Log
    {
        [LoggerMessage(0, LogLevel.Debug, "Hello!")]
        public static void SayHello(ILogger<HelloSayer> logger);
    }
}

namespace Bar
{
    partial class Log
    {
        [LoggerMessage(0, LogLevel.Debug, "Goodbye!")]
        public static void SayGoodbye(ILogger<GoodbyeSayer> logger);
    }
}

With the above, my code will ensure that the ids aren't reused within each Log classes, but will allow them to be reused between the two classes.

I'll have to think about the reserved id thing. I no longer have an attribute on the type, the attribute is only present on the methods.

[JamesNK]

@JamesNK So what's the expected scope of uniqueness for the id values?

Unique per logging file.

I'm not a logging expert but I believe the idea is that logger name + logger ID should be unique. Using ILogger<T> can be used to ensure that differently named loggers are used with different files (and each file has unique ids).

I'll have to think about the reserved id thing. I no longer have an attribute on the type, the attribute is only present on the methods.

The top-level attribute could be optional, and you only add it if there is a reason to (like adding IDs that have been disallowed)

[davidfowl]

Per logging file isn't a thing. EventIds are unique within a single category (did you mean category?)

[JamesNK]

Per logging file isn't a thing

To be more specific, I meant unique per generated log type (which could be made up of multiple partial classes across files)

There is nothing stopping someone from taking an ILogger with category "Server" and using it with two generated Log types, therefore duplicating IDs. (apart from what I mentioned with ILogger<T>, where usually the category == typeof(T).FullName

[geeknoid]

Within the scope of a single assembly being compiled, I can ensure that no two generated log messages have the same ID. This would be independent of categories, independent of the source file or types the log messages are declared in. How's that?

And in that vein, I can have an assembly-level attribute to record reserved ids. I currently fetch an option controlling pascal casing of arguments from the project file, I can move this from fetching it from this same global attribute too.