[API Proposal]: Extend LoggingGenerator to support scopes

[kimsey0]

Background and motivation

51064 introduced a new LoggingGenerator source generator which allows writing code like

public partial class LoggingSample3
{
    private readonly ILogger _logger;

    public LoggingSample3(ILogger logger)
    {
        _logger = logger;
    }

    [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")]
    public partial void LogName(string name);
}

and having an implementation using LoggerMessage.Define generated like

partial class LoggingSample3
{
    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "1.0.0.0")]
    private static readonly global::System.Action<global::Microsoft.Extensions.Logging.ILogger, string, global::System.Exception?> _LogNameCallback =
        global::Microsoft.Extensions.Logging.LoggerMessage.Define<string>(global::Microsoft.Extensions.Logging.LogLevel.Information, new global::Microsoft.Extensions.Logging.EventId(0, nameof(LogName)), "Hello {name}", skipEnabledCheck: true);

    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "1.0.0.0")]
    public partial void LogName(string name)
    {
        if (_logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Information))
        {
            _LogNameCallback(_logger, name, null);
        }
    }
}

However, this doesn't support the other method for high-performance logging, LoggerMessage.DefineScope. This means that, if you use scopes, you will have manual calls to LoggerMessage.DefineScope and the requisite delegate fields side-by-side with the new source generator approach:

[LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")]
public partial void LogName(string name);

private static readonly Func<ILogger, string, int, IDisposable?> ScopeCallback =
    LoggerMessage.DefineScope<string, int>("Value={Value};OtherValue={OtherValue}");

public IDisposable? Scope(string value, int otherValue) => ScopeCallback(_logger, value, otherValue);

API Proposal

I suggest adding a new attribute side-by-side with the existing LoggerMessageAttribute

namespace Microsoft.Extensions.Logging;

[AttributeUsage(AttributeTargets.Method)]
public sealed partial class LoggerMessageScopeAttribute : Attribute
{
    public LoggerMessageScopeAttribute();
    public string Message { get; set; } = "";
}

and having LoggingGenerator generate calls to LoggerMessage.DefineScope, similar to the way it currently generates them for LoggerMessage.Define.

API Usage

This will allow defining logger messages and scopes in the same way:

[LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")]
public partial void LogName(string name);

[LoggerMessageScope(Message = "Value={Value};OtherValue={OtherValue}")]
public partial IDisposable? Scope(string value, int otherValue);

Alternative Designs

No response

Risks

No response

[ghost]

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

Issue Details

---

### Background and motivation #51064 introduced a new LoggingGenerator source generator which allows writing code like ```csharp public partial class LoggingSample3 { private readonly ILogger _logger; public LoggingSample3(ILogger logger) { _logger = logger; } [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")] public partial void LogName(string name); } ``` and having an implementation using `LoggerMessage.Define` generated like ```csharp partial class LoggingSample3 { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "1.0.0.0")] private static readonly global::System.Action _LogNameCallback = global::Microsoft.Extensions.Logging.LoggerMessage.Define(global::Microsoft.Extensions.Logging.LogLevel.Information, new global::Microsoft.Extensions.Logging.EventId(0, nameof(LogName)), "Hello {name}", skipEnabledCheck: true); [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "1.0.0.0")] public partial void LogName(string name) { if (_logger.IsEnabled(global::Microsoft.Extensions.Logging.LogLevel.Information)) { _LogNameCallback(_logger, name, null); } } } ``` However, this doesn't support the other method for high-performance logging, `LoggerMessage.DefineScope`. This means that, if you use scopes, you will have manual calls to `LoggerMessage.DefineScope` and the requisite delegate fields side-by-side with the new source generator approach: ```csharp [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")] public partial void LogName(string name); private static readonly Func ScopeCallback = LoggerMessage.DefineScope("Value={Value};OtherValue={OtherValue}"); public IDisposable? Scope(string value, int otherValue) => ScopeCallback(_logger, value, otherValue); ``` ### API Proposal I suggest adding a new attribute side-by-side with the existing `LoggerMessageAttribute` ```csharp namespace Microsoft.Extensions.Logging; [AttributeUsage(AttributeTargets.Method)] public sealed partial class LoggerMessageScopeAttribute : Attribute { public LoggerMessageScopeAttribute(); public string Message { get; set; } = ""; } ``` and having LoggingGenerator generate calls to `LoggerMessage.DefineScope`, similar to the way it currently generates them for `LoggerMessage.Define`. ### API Usage This will allow defining logger messages and scopes in the same way: ```csharp [LoggerMessage(EventId = 0, Level = LogLevel.Information, Message = "Hello {name}")] public partial void LogName(string name); [LoggerMessageScope(Message = "Value={Value};OtherValue={OtherValue}")] public partial IDisposable? Scope(string value, int otherValue); ``` ### Alternative Designs _No response_ ### Risks _No response_

Author:	kimsey0
Assignees:	-
Labels:	`api-suggestion`, `area-Extensions-Logging`
Milestone:	-

[kimsey0]

@geeknoid and @maryamariyan, maybe you have some input on this based on your work on the current LoggingGenerator? Is this as straightforward as I see it?

[geeknoid]

Technically, this seems reasonable.

However, I thought that the logging scope thing was somewhat abandoned at this point. I think it was @davidfowl that mentioned this to me.

[kimsey0]

I didn't know scopes were being abandoned. There's nothing in the logging documentation nor the high-performance logging that indicates this. But if that's the case, this issue might not make sense. Perhaps instead the documentation should be updated to advice against using scopes.

[tarekgh]

This looks a duplicate of https://github.com/dotnet/runtime/issues/79028.

[kimsey0]

@tarekgh: Looking at #79028, this doesn't look to be a duplicate of it. #79028 proposes to add a scope parameter to generated [LoggerMessage] methods. This asks to add separately generated methods for LoggerMessage.DefineScope. Can we reopen it until we've determined if scopes are being deprecated altogether?

[tarekgh]

I am not aware the scope is abandoned. I see the scope is used heavily in all internal and external logger providers.

@kimsey0 do you think make sense merging the two proposals? I mean this one and https://github.com/dotnet/runtime/issues/79028? I am thinking of collecting all related scope scenarios together to support in the source gen. Ensure all of them work nicely together.

[geeknoid]

I'd love to hear what @davidfowl has to say on the general subject of logging scopes and their usefulness in general.

[davidfowl]

I've been personally attempting to abandoning scopes (https://github.com/dotnet/aspnetcore/pull/44873) in favor of activities and distributed tracing. I know it doesn't cover all scenarios but I think it works well for most cases. This just enrichment done with a more generic and less efficient api.

[kimsey0]

@tarekgh: I can't really see how the API proposed in #79028 would work. It suggests allowing a [LoggerScope] scope parameter in [LoggerMessage] methods, but doesn't address how to pass in arguments for the scope message format. I think it would be complicated and confusing to match parameters in the original declaration to placeholders in both the scope and log messages - what for example if both use the same placeholder with different casing? It seems limiting if it doesn't support arguments for the scope message at all. And in the first place, I don't understand the use case for a scope around just one message.

However, if you think both make sense to implement and to do together, I defer to your judgement.

[tarekgh]

if you think both make sense to implement and to do together, I defer to your judgement.

I am not pushing to have both at all. All I am trying to say is we need to ensure what makes sense to do with the scope in general and ensure we have a full story. We didn't have chance look deeply at https://github.com/dotnet/runtime/issues/79028. You may ignore it then for now.