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

Add SkipEnabledCheck on LoggerMessageAttribute #54305

Merged
merged 7 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -381,34 +381,45 @@ private void GenLogMethod(LoggerMethod lm, string nestedIndentation)
GenParameters(lm);

_builder.Append($@")
{nestedIndentation}{{
{nestedIndentation}{{");

string enabledCheckIndentation = lm.SkipEnabledCheck ? "" : " ";
if (!lm.SkipEnabledCheck)
{
_builder.Append($@"
{nestedIndentation}if ({logger}.IsEnabled({level}))
{nestedIndentation}{{");
}

if (UseLoggerMessageDefine(lm))
{
_builder.Append($@"
{nestedIndentation}__{lm.Name}Callback({logger}, ");
{nestedIndentation}{enabledCheckIndentation}__{lm.Name}Callback({logger}, ");

GenCallbackArguments(lm);

_builder.Append(@$"{exceptionArg});");
_builder.Append($"{exceptionArg});");
}
else
{
_builder.Append($@"
{nestedIndentation}{logger}.Log(
{level},
new global::Microsoft.Extensions.Logging.EventId({lm.EventId}, {eventName}),
");
GenHolder(lm);
_builder.Append($@",
{exceptionArg},
__{lm.Name}Struct.Format);");
{nestedIndentation}{enabledCheckIndentation}{logger}.Log(
{nestedIndentation}{enabledCheckIndentation}{level},
{nestedIndentation}{enabledCheckIndentation}new global::Microsoft.Extensions.Logging.EventId({lm.EventId}, {eventName}),
{nestedIndentation}{enabledCheckIndentation}");
GenHolder(lm);
_builder.Append($@",
{nestedIndentation}{enabledCheckIndentation}{exceptionArg},
{nestedIndentation}{enabledCheckIndentation}__{lm.Name}Struct.Format);");
}

if (!lm.SkipEnabledCheck)
{
_builder.Append($@"
{nestedIndentation}}}");
}

_builder.Append($@"
{nestedIndentation}}}
{nestedIndentation}}}");

static string GetException(LoggerMethod lm)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public Parser(Compilation compilation, Action<Diagnostic> reportDiagnostic, Canc
}

/// <summary>
/// Gets the set of logging classes or structs containing methods to output.
/// Gets the set of logging classes containing methods to output.
/// </summary>
public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSyntax> classes)
{
Expand Down Expand Up @@ -105,7 +105,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
continue;
}

(int eventId, int? level, string message, string? eventName) = ExtractAttributeValues(ma.ArgumentList!, sm);
(int eventId, int? level, string message, string? eventName, bool skipEnabledCheck) = ExtractAttributeValues(ma.ArgumentList!, sm);

IMethodSymbol? methodSymbol = sm.GetDeclaredSymbol(method, _cancellationToken);
if (methodSymbol != null)
Expand All @@ -119,6 +119,7 @@ public IReadOnlyList<LoggerClass> GetLogClasses(IEnumerable<ClassDeclarationSynt
EventName = eventName,
IsExtensionMethod = methodSymbol.IsExtensionMethod,
Modifiers = method.Modifiers.ToString(),
SkipEnabledCheck = skipEnabledCheck
};

ExtractTemplates(message, lm.TemplateMap, lm.TemplateList);
Expand Down Expand Up @@ -435,12 +436,13 @@ bool IsAllowedKind(SyntaxKind kind) =>
return (loggerField, false);
}

private (int eventId, int? level, string message, string? eventName) ExtractAttributeValues(AttributeArgumentListSyntax args, SemanticModel sm)
private (int eventId, int? level, string message, string? eventName, bool skipEnabledCheck) ExtractAttributeValues(AttributeArgumentListSyntax args, SemanticModel sm)
{
int eventId = 0;
int? level = null;
string? eventName = null;
string message = string.Empty;
bool skipEnabledCheck = false;
foreach (AttributeArgumentSyntax a in args.Arguments)
{
// argument syntax takes parameters. e.g. EventId = 0
Expand All @@ -459,9 +461,13 @@ bool IsAllowedKind(SyntaxKind kind) =>
case "Message":
message = sm.GetConstantValue(a.Expression, _cancellationToken).ToString();
break;
case "SkipEnabledCheck":
var flag = (bool?)sm.GetConstantValue(a.Expression, _cancellationToken).Value;
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
skipEnabledCheck = flag == null ? false : flag.Value;
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}
return (eventId, level, message, eventName);
return (eventId, level, message, eventName, skipEnabledCheck);
}

private void Diag(DiagnosticDescriptor desc, Location? location, params object?[]? messageArgs)
Expand Down Expand Up @@ -612,6 +618,7 @@ internal class LoggerMethod
public bool IsExtensionMethod;
public string Modifiers = string.Empty;
public string LoggerField = string.Empty;
public bool SkipEnabledCheck;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ public LoggerMessageAttribute() { }
public string? EventName { get { throw null; } set { } }
public Microsoft.Extensions.Logging.LogLevel Level { get { throw null; } set { } }
public string Message { get { throw null; } set { } }
public bool SkipEnabledCheck { get { throw null; } set { } }
}
public partial class Logger<T> : Microsoft.Extensions.Logging.ILogger, Microsoft.Extensions.Logging.ILogger<T>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,10 @@ public LoggerMessageAttribute() { }
/// Gets the message text for the logging method.
/// </summary>
public string Message { get; set; } = "";

/// <summary>
/// Gets the flag to skip IsEnabled check for the logging method.
/// </summary>
public bool SkipEnabledCheck { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,32 @@ public void EventNameTests()
Assert.Equal("CustomEventName", logger.LastEventId.Name);
}

[Fact]
public void SkipEnabledCheckTests()
{
var logger = new MockLogger();

logger.Reset();
logger.Enabled = false;
Assert.False(logger.IsEnabled(LogLevel.Information));
SkipEnabledCheckExtensions.LoggerMethodWithFalseSkipEnabledCheck(logger);
Assert.Null(logger.LastException);
Assert.Null(logger.LastFormattedString);
Assert.Equal((LogLevel)(-1), logger.LastLogLevel);
Assert.Equal(0, logger.CallCount);
Assert.Equal(default, logger.LastEventId);

logger.Reset();
logger.Enabled = false;
Assert.False(logger.IsEnabled(LogLevel.Debug));
SkipEnabledCheckExtensions.LoggerMethodWithTrueSkipEnabledCheck(logger);
Assert.Null(logger.LastException);
Assert.Equal("Message: When using SkipEnabledCheck, the generated code skips logger.IsEnabled(logLevel) check before calling log. To be used when consumer has already guarded logger method in an IsEnabled check.", logger.LastFormattedString);
Assert.Equal(LogLevel.Debug, logger.LastLogLevel);
Assert.Equal(1, logger.CallCount);
Assert.Equal("LoggerMethodWithTrueSkipEnabledCheck", logger.LastEventId.Name);
}

[Fact]
public void NestedClassTests()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,26 @@ public partial class Nested
Assert.Empty(diagnostics);
}

[Theory]
[InlineData("false")]
[InlineData("true")]
[InlineData("null")]
public async Task UsingSkipEnabledCheck(string skipEnabledCheckValue)
{
IReadOnlyList<Diagnostic> diagnostics = await RunGenerator($@"
partial class C
{{
public partial class WithLoggerMethodUsingSkipEnabledCheck
{{
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = ""M1"", SkipEnabledCheck = {skipEnabledCheckValue})]
static partial void M1(ILogger logger);
}}
}}
");

Assert.Empty(diagnostics);
}

[Fact]
public async Task MissingExceptionType()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.Extensions.Logging.Generators.Tests.TestClasses
{
internal static partial class SkipEnabledCheckExtensions
{
[LoggerMessage(EventId = 0, Level = LogLevel.Debug, Message = "Message: When using SkipEnabledCheck, the generated code skips logger.IsEnabled(logLevel) check before calling log. To be used when consumer has already guarded logger method in an IsEnabled check.", SkipEnabledCheck = true)]
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
internal static partial void LoggerMethodWithTrueSkipEnabledCheck(ILogger logger);

[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "M1", SkipEnabledCheck = false)]
internal static partial void LoggerMethodWithFalseSkipEnabledCheck(ILogger logger);
}
}