dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.28k stars 4.73k forks source link

Add support for logging with more than 6 arguments at runtime #55525

Open maryamariyan opened 3 years ago

maryamariyan commented 3 years ago

Background and Motivation

As part of .NET 6 we updated the new LoggerMessage.Define overloads to use LogDefineOptions. (PR here and issue https://github.com/dotnet/runtime/issues/50913)

Later in .NET 7, for supporting more arguments, we could add a new Define<T> API (where T is a delegate) that would take n arguments, log level, event ID, format string and LogDefineOptions to log message.

Theoretically we need an analyzer that makes sure the arguments of the new Define API are proper (ILogger etc.). The source generator would not be using the new Define (where T is delegate) API. (not source gen friendly).

We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process . This template will help us gather the information we need to start the review process. First, please describe the purpose and value of the new API here.

Proposed API

Something like:

public static partial class LoggerMessage
{
+    public static void Define<T>(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId, string formatString, Microsoft.Extensions.Logging.LogOptions options) where T : delegate {​ throw null; }​
}

T would be a delegate but an analyzer would need to make sure it takes ILogger, Exception, etc. System.Action<Microsoft.Extensions.Logging.ILogger, T1, T2, ..., Tn, System.Exception?>

TODO

cc: @davidfowl @eerhardt

ghost commented 3 years ago

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

Issue Details
## Background and Motivation As part of .NET 6 we updated the new `LoggerMessage.Define` overloads to use `LogDefineOptions`. (PR [here](https://github.com/dotnet/runtime/pull/54581) and issue [here](https://github.com/dotnet/runtime/issues/52276)) Later in .NET 7, for supporting more arguments, we could add a new `Define` API (where T is a delegate) that would take n arguments, log level, event ID, format string and `LogDefineOptions` to log message. Theoretically we need an analyzer that makes sure the arguments of the new Define API are proper (ILogger etc.). The source generator would not be using the new Define (where T is delegate) API. (not source gen friendly). We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process . This template will help us gather the information we need to start the review process. First, please describe the purpose and value of the new API here. ## Proposed API Something like: ```diff public static partial class LoggerMessage { + public static void Define(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId, string formatString, Microsoft.Extensions.Logging.LogOptions options) where T : delegate {​ throw null; }​ } ``` `T` would be a delegate but an analyzer would need to make sure it takes ILogger, Exception, etc. `System.Action` ## TODO - [ ] Go through the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/framework-design-guidelines-digest.md) and [API Review Process](https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md) to further investigate sample usages and investigate alternative designs and risks. cc: @davidfowl @eerhardt
Author: maryamariyan
Assignees: -
Labels: `api-suggestion`, `area-Extensions-Logging`
Milestone: -
abatishchev commented 5 months ago

@tarekgh any update on this, please?

tarekgh commented 5 months ago

We couldn't get into this yet because we are occupied with other high-priority tasks.