LoggerMessageAttribute should have a constructor that takes EventId, LogLevel, and Message

[eerhardt]

When using the logging source generator, it is too much clutter to have to put EventId =, Level =. and Message = in my code:

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Debug, Message = HealthCheckEndText)]
        private partial void HealthCheckEndHealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Warning, Message = HealthCheckEndText)]
        private partial void HealthCheckEndDegraded(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Error, Message = HealthCheckEndText)]
        private partial void HealthCheckEndUnhealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription, Exception? exception);

vs

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Debug, HealthCheckEndText)]
        private partial void HealthCheckEndHealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Warning, HealthCheckEndText)]
        private partial void HealthCheckEndDegraded(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Error, HealthCheckEndText)]
        private partial void HealthCheckEndUnhealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription, Exception? exception);

We should add a constructor to LoggerMessage that takes these 3 parameters, so users don't have to specify the properties on every LoggerMessage attribute.

[System.AttributeUsageAttribute(System.AttributeTargets.Method)]
public sealed partial class LoggerMessageAttribute : System.Attribute
{
    public LoggerMessageAttribute() { }
+    public LoggerMessageAttribute(string message)
+    public LoggerMessageAttribute(LogLevel level, string message) { }
+    public LoggerMessageAttribute(int eventId, string message)
+    public LoggerMessageAttribute(int eventId, LogLevel level, string message) { }
    public int EventId { get { throw null; } set { } }
    public string? EventName { get { throw null; } set { } }
    public Microsoft.Extensions.Logging.LogLevel Level { get { throw null; } set { } }
    public string Message { get { throw null; } set { } }
}

cc @maryamariyan @davidfowl @shirhatti

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

When using the logging source generator, it is too much clutter to have to put EventId =, Level =. and Message = in my code:

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Debug, Message = HealthCheckEndText)]
        private partial void HealthCheckEndHealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Warning, Message = HealthCheckEndText)]
        private partial void HealthCheckEndDegraded(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventId = EventIds.HealthCheckEndId, Level = LogLevel.Error, Message = HealthCheckEndText)]
        private partial void HealthCheckEndUnhealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription, Exception? exception);

vs

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Debug, HealthCheckEndText)]
        private partial void HealthCheckEndHealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Warning, HealthCheckEndText)]
        private partial void HealthCheckEndDegraded(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription);

        [LoggerMessage(EventIds.HealthCheckEndId, LogLevel.Error, HealthCheckEndText)]
        private partial void HealthCheckEndUnhealthy(string HealthCheckName, HealthStatus HealthStatus, double ElapsedMilliseconds, string? HealthCheckDescription, Exception? exception);

We should add a constructor to LoggerMessage that takes these 3 parameters, so users don't have to specify the properties on every LoggerMessage attribute.

cc @maryamariyan @davidfowl @shirhatti

Author:	eerhardt
Assignees:	-
Labels:	area-Extensions-Logging
Milestone:	-

[shirhatti]

We had this exact conversation during API review. The point I raised is that we may want to additional ctors (for scenarios like #52223) and the determination was made to move all validation to the source generator and only keep the parameter-less ctor.

I'm not opposed to it adding them back, but just want to point out that we've flip-flopped on this already

[davidfowl]

Seems like we're saying we'd have 2 constructors, default and this one?

[shirhatti]

What about the variant without LogLevel? And one/two more for #52223?

[eerhardt]

We had this exact conversation during API review.
just want to point out that we've flip-flopped on this already

I've been using this generator as a user would this week (I'm app building for preview4 - see dotnet/aspnetcore#32414). It feels like too much clutter in my code, and makes it hard to read. Notice the difference in the top post. I imagine most devs would feel similarly that this is too much clutter.

[bartonjs]

Video

We think that only one simplified constructor is necessary. The generator needs to account for the fact that the EventId/LogLevel/String can now be specified both as the ctor and as the property set -- in the same attribute instance. ([LoggerMessage(1, ..., ..., EventId = 2)] is actually event ID 2)

partial class LoggerMessageAttribute
{
    public LoggerMessageAttribute(int eventId, LogLevel level, string message) { }
}