Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Interpolated string overloads of ILogger extensions #111283

Closed
stephentoub opened this issue Jan 10, 2025 · 2 comments
Closed

[API Proposal]: Interpolated string overloads of ILogger extensions #111283

stephentoub opened this issue Jan 10, 2025 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging

Comments

@stephentoub
Copy link
Member

stephentoub commented Jan 10, 2025

Background and motivation

LoggerExtensions provides a plethora of methods like:

public static void LogDebug(this ILogger logger, string? message, params object?[] args)

We strongly encourage developers to use structured logging, either by having their messages to such calls be literals, with arguments filled in from the supplied params, or by using the logger generator to have strongly-typed logging methods. However, ridiculous amounts of code continue to just do the simple thing of using interpolated strings, e.g.

_logger.LogDebug($"Value: {value}");

and to do so in a way where the calls are not guarded by IsEnabled checks for the LogLevel. That means that all of the expense associated with generating the messages are incurred even if it's then immediately thrown away.

We can address that by adding interpolated string handler overloads, which can avoid the associated costs when the relevant log level isn't enabled.

API Proposal

namespace Microsoft.Extensions.Logging;

public static class LoggerExtensions
{
    +public static void Log(this ILogger logger, LogLevel logLevel, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);
    +public static void Log(this ILogger logger, LogLevel logLevel, EventId eventId, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);
    +public static void Log(this ILogger logger, LogLevel logLevel, EventId eventId, Exception? exception, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);

    +public static void LogTrace(this ILogger logger, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);
    +public static void LogTrace(this ILogger logger, EventId eventId, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);
    +public static void LogTrace(this ILogger logger, EventId eventId, Exception? exception, [InterpolatedStringHandlerArgument(nameof(logger))] ref LogInterpolatedStringHandler message);
    ... // copy above 3 overloads for LogDebug, LogInformation, LogWarning, LogError, and LogCritical

    +[EditorBrowsable(EditorBrowsableState.Never)]
    +[InterpolatedStringHandler]
    +public ref struct LogInterpolatedStringHandler
    +{
    +    public LogInterpolatedStringHandler(int literalLength, int formattedCount, ILogger logger, out bool shouldAppend);

    +    ... // exact same set of AppendLiteral / AppendFormatted overloads as are on Debug.AssertInterpolatedStringHandler
    +}
}

API Usage

_logger.LogInformation($"Value: {value}");

Alternative Designs

No response

Risks

No response

@stephentoub stephentoub added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging labels Jan 10, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 10, 2025
Copy link
Contributor

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

@stephentoub
Copy link
Member Author

Hmm, just realized that we'd need to be able to pass the LogLevel into the handler's ctor, and we don't have a good way to do that. We'd currently need a dedicated handler type for each LogLevel, which is prohibitive.
cc: @333fred

@stephentoub stephentoub closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Logging
Projects
None yet
Development

No branches or pull requests

1 participant