Skip to content

Commit

Permalink
Replace [LogMethod] with [LoggerMessage] (#4248)
Browse files Browse the repository at this point in the history
Co-authored-by: Martin Taillefer <mataille@microsoft.com>
  • Loading branch information
geeknoid and Martin Taillefer authored Aug 9, 2023
1 parent 4ba092d commit ac7f0da
Show file tree
Hide file tree
Showing 86 changed files with 486 additions and 966 deletions.
3 changes: 3 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@
<!-- Opt in to build acceleration in VS (from 17.5 onwards): https://github.com/dotnet/project-system/blob/main/docs/build-acceleration.md -->
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
<AccelerateBuildsInVisualStudio>true</AccelerateBuildsInVisualStudio>

<!-- This repo introduces a replacement generator, we don't use the one from dotnet/runtime -->
<DisableMicrosoftExtensionsLoggingSourceGenerator>true</DisableMicrosoftExtensionsLoggingSourceGenerator>
</PropertyGroup>

<!-- https://github.com/dotnet/aspnetcore/blob/72b0269372a/eng/Common.props#L3-L6 -->
Expand Down
15 changes: 15 additions & 0 deletions Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,19 @@
<TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) $(_BlameArgs)</TestRunnerAdditionalArguments>
</PropertyGroup>
</Target>

<!-- This target will make sure that projects targeting netcoreapp3.1 will also have the Microsoft.Extensions.Logging.Abstractions analyzer removed. -->
<Target Name="_Microsoft_Extensions_Logging_AbstractionsRemoveAnalyzers"
Condition="'$(DisableMicrosoftExtensionsLoggingSourceGenerator)' == 'true'"
AfterTargets="ResolveReferences">
<ItemGroup>
<_Microsoft_Extensions_Logging_AbstractionsAnalyzer Include="@(Analyzer)" Condition="'%(Analyzer.NuGetPackageId)' == 'Microsoft.Extensions.Logging.Abstractions' Or
('%(Analyzer.AssemblyName)' == 'Microsoft.Extensions.Logging.Generators' and '%(Analyzer.NuGetPackageId)' == 'Microsoft.AspNetCore.App.Ref')" />
</ItemGroup>

<!-- Remove Microsoft.Extensions.Logging.Abstractions Analyzer -->
<ItemGroup>
<Analyzer Remove="@(_Microsoft_Extensions_Logging_AbstractionsAnalyzer)" />
</ItemGroup>
</Target>

This comment has been minimized.

Copy link
@RussKie

RussKie Aug 14, 2023

Member

I'm trying to update the R9 SDK to the latest, and observing quite puzzling issues:
image

It took me awhile to make connections to this issue. Applying the same target to the R9 SDK I could fix the issue, but that me wonder whether we could offer a better UX...

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal enum LogLevel
internal Func<Compilation, string, INamedTypeSymbol?> GetTypeByMetadataName3 = (c, n) => c.GetTypeByMetadataName(n);
internal Func<SemanticModel, BaseMethodDeclarationSyntax, CancellationToken, IMethodSymbol?> GetDeclaredSymbol = (sm, m, t) => sm.GetDeclaredSymbol(m, t);

private const string LogMethodAttribute = "Microsoft.Extensions.Telemetry.Logging.LogMethodAttribute";
private const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute";
private const int LogMethodAttrEventIdArg = 0;
private const int LogMethodAttrLevelArg = 1;
private const int LogMethodAttrMessageArg = 2;
Expand Down Expand Up @@ -147,7 +147,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var sm = docEditor.SemanticModel;
var comp = sm.Compilation;

var logMethodAttribute = GetTypeByMetadataName2(comp, LogMethodAttribute);
var logMethodAttribute = GetTypeByMetadataName2(comp, LoggerMessageAttribute);
if (logMethodAttribute is null)
{
// strange that we can't find the attribute, but supply a potential useful value instead
Expand Down Expand Up @@ -573,7 +573,7 @@ private async Task<Solution> InsertLoggingMethodSignatureAsync(
gen.LiteralExpression(details.Message),
};

var attr = gen.Attribute(LogMethodAttribute, attrArgs);
var attr = gen.Attribute(LoggerMessageAttribute, attrArgs);

logMethod = gen.AddAttributes(logMethod, attr);

Expand All @@ -588,13 +588,13 @@ private async Task<Solution> InsertLoggingMethodSignatureAsync(

/// <summary>
/// Iterate through the existing methods in the target class
/// and look at any method annotated with [LogMethod],
/// and look at any method annotated with [LoggerMessage],
/// get their event ids, and then return 1 larger than any event id
/// found.
/// </summary>
private int CalcEventId(Compilation comp, ClassDeclarationSyntax targetClass, CancellationToken cancellationToken)
{
var logMethodAttribute = GetTypeByMetadataName3(comp, LogMethodAttribute);
var logMethodAttribute = GetTypeByMetadataName3(comp, LoggerMessageAttribute);
if (logMethodAttribute is null)
{
// strange we can't find the attribute, but supply a potential useful value instead
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ private static void AnalyzeInvocation(OperationAnalysisContext context)
[ExcludeFromCodeCoverage]
private static bool IsLoggerMethod(ISymbol symbol)
{
return symbol.GetAttributes().Any(a => a.AttributeClass != null && IsLogMethodAttribute(a.AttributeClass));
return symbol.GetAttributes().Any(a => a.AttributeClass != null && IsLoggerMessageAttribute(a.AttributeClass));
}

private static bool IsLogMethodAttribute(ISymbol attributeSymbol)
private static bool IsLoggerMessageAttribute(ISymbol attributeSymbol)
{
return attributeSymbol.Name == "LogMethodAttribute"
&& attributeSymbol.ContainingNamespace.ToString() == "Microsoft.Extensions.Telemetry.Logging";
return attributeSymbol.Name == "LoggerMessageAttribute"
&& attributeSymbol.ContainingNamespace.ToString() == "Microsoft.Extensions.Logging";
}

private static IEnumerable<Diagnostic> AnalyzeLogger(IInvocationOperation invocation)
Expand Down
4 changes: 2 additions & 2 deletions src/Generators/Microsoft.Gen.ComplianceReports/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,14 @@ private static string FormatType(ITypeSymbol typeSymbol)
private List<ClassifiedLogMethod>? GetClassifiedLogMethods(ITypeSymbol typeSymbol)
{
List<ClassifiedLogMethod>? classifiedLogMethods = null;
if (_symbolHolder.LogMethodAttribute != null)
if (_symbolHolder.LoggerMessageAttribute != null)
{
var methods = typeSymbol.GetMembers().OfType<IMethodSymbol>();
foreach (IMethodSymbol method in methods)
{
foreach (var a in method.GetAttributes())
{
if (SymbolEqualityComparer.Default.Equals(_symbolHolder.LogMethodAttribute, a.AttributeClass))
if (SymbolEqualityComparer.Default.Equals(_symbolHolder.LoggerMessageAttribute, a.AttributeClass))
{
var clm = new ClassifiedLogMethod
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ namespace Microsoft.Gen.ComplianceReports;
/// </summary>
internal sealed record class SymbolHolder(
INamedTypeSymbol DataClassificationAttributeSymbol,
INamedTypeSymbol? LogMethodAttribute);
INamedTypeSymbol? LoggerMessageAttribute);
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.Gen.ComplianceReports;
internal static class SymbolLoader
{
private const string DataClassificationAttribute = "Microsoft.Extensions.Compliance.Classification.DataClassificationAttribute";
private const string LogMethodAttribute = "Microsoft.Extensions.Telemetry.Logging.LogMethodAttribute";
private const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute";

public static bool TryLoad(Compilation compilation, out SymbolHolder? symbolHolder)
{
Expand All @@ -23,7 +23,7 @@ public static bool TryLoad(Compilation compilation, out SymbolHolder? symbolHold

symbolHolder = new(
dataClassificationAttributeSymbol,
compilation.GetTypeByMetadataName(LogMethodAttribute));
compilation.GetTypeByMetadataName(LoggerMessageAttribute));

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Generators/Microsoft.Gen.Logging/LoggingGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
{
IncrementalValuesProvider<TypeDeclarationSyntax> typeDeclarations = context.SyntaxProvider
.ForAttributeWithMetadataName(
Parsing.SymbolLoader.LogMethodAttribute,
Parsing.SymbolLoader.LoggerMessageAttribute,
(syntaxNode, _) => syntaxNode.Parent is TypeDeclarationSyntax,
(context, _) => (TypeDeclarationSyntax)context.TargetNode.Parent!);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal static class AttributeProcessors
private const int LogLevelCritical = 5;

public static (int? eventId, int? level, string message, string? eventName, bool skipEnabledCheck)
ExtractLogMethodAttributeValues(AttributeData attr, SymbolHolder symbols)
ExtractLoggerMessageAttributeValues(AttributeData attr, SymbolHolder symbols)
{
// seven constructor arg shapes:
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.Gen.Logging.Parsing;

internal sealed class DiagDescriptors : DiagDescriptorsBase
{
private const string Category = "LogMethod";
private const string Category = "LoggerMessage";

public static DiagnosticDescriptor ShouldntMentionLogLevelInMessage { get; } = Make(
id: "LOGGEN000",
Expand Down
10 changes: 5 additions & 5 deletions src/Generators/Microsoft.Gen.Logging/Parsing/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public IReadOnlyList<LoggingType> GetLogTypes(IEnumerable<TypeDeclarationSyntax>
{
sm ??= _compilation.GetSemanticModel(typeDec.SyntaxTree);

var attrLoc = GetLogMethodAttribute(method, sm, symbols);
var attrLoc = GetLoggerMessageAttribute(method, sm, symbols);
if (attrLoc == null)
{
// doesn't have the magic attribute we like, so ignore
Expand Down Expand Up @@ -314,9 +314,9 @@ static bool IsAllowedKind(SyntaxKind kind) =>

(LoggingMethod lm, bool keepMethod) ProcessMethod(MethodDeclarationSyntax method, IMethodSymbol methodSymbol, Location attrLoc)
{
var attr = ParserUtilities.GetSymbolAttributeAnnotationOrDefault(symbols.LogMethodAttribute, methodSymbol)!;
var attr = ParserUtilities.GetSymbolAttributeAnnotationOrDefault(symbols.LoggerMessageAttribute, methodSymbol)!;

var (eventId, level, message, eventName, skipEnabledCheck) = AttributeProcessors.ExtractLogMethodAttributeValues(attr, symbols);
var (eventId, level, message, eventName, skipEnabledCheck) = AttributeProcessors.ExtractLoggerMessageAttributeValues(attr, symbols);

var lm = new LoggingMethod
{
Expand Down Expand Up @@ -512,14 +512,14 @@ private static bool HasXmlDocumentation(MethodDeclarationSyntax method)
return lp;
}

private Location? GetLogMethodAttribute(MethodDeclarationSyntax methodSyntax, SemanticModel sm, SymbolHolder symbols)
private Location? GetLoggerMessageAttribute(MethodDeclarationSyntax methodSyntax, SemanticModel sm, SymbolHolder symbols)
{
foreach (var mal in methodSyntax.AttributeLists)
{
foreach (var methodAttr in mal.Attributes)
{
var attrCtor = sm.GetSymbolInfo(methodAttr, _cancellationToken).Symbol;
if (attrCtor != null && SymbolEqualityComparer.Default.Equals(attrCtor.ContainingType, symbols.LogMethodAttribute))
if (attrCtor != null && SymbolEqualityComparer.Default.Equals(attrCtor.ContainingType, symbols.LoggerMessageAttribute))
{
return methodAttr.GetLocation();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.Gen.Logging.Parsing;
[ExcludeFromCodeCoverage]
internal sealed record class SymbolHolder(
Compilation Compilation,
INamedTypeSymbol LogMethodAttribute,
INamedTypeSymbol LoggerMessageAttribute,
INamedTypeSymbol LogPropertiesAttribute,
INamedTypeSymbol? LogPropertyIgnoreAttribute,
INamedTypeSymbol ITagCollectorSymbol,
Expand Down
4 changes: 2 additions & 2 deletions src/Generators/Microsoft.Gen.Logging/Parsing/SymbolLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.Gen.Logging.Parsing;

internal static class SymbolLoader
{
internal const string LogMethodAttribute = "Microsoft.Extensions.Telemetry.Logging.LogMethodAttribute";
internal const string LoggerMessageAttribute = "Microsoft.Extensions.Logging.LoggerMessageAttribute";
internal const string LogPropertiesAttribute = "Microsoft.Extensions.Telemetry.Logging.LogPropertiesAttribute";
internal const string LogPropertyIgnoreAttribute = "Microsoft.Extensions.Telemetry.Logging.LogPropertyIgnoreAttribute";
internal const string ITagCollectorType = "Microsoft.Extensions.Telemetry.Logging.ITagCollector";
Expand Down Expand Up @@ -50,7 +50,7 @@ internal static class SymbolLoader
{
var loggerSymbol = compilation.GetTypeByMetadataName(ILoggerType);
var logLevelSymbol = compilation.GetTypeByMetadataName(LogLevelType);
var logMethodAttributeSymbol = compilation.GetTypeByMetadataName(LogMethodAttribute);
var logMethodAttributeSymbol = compilation.GetTypeByMetadataName(LoggerMessageAttribute);
var logPropertiesAttributeSymbol = compilation.GetTypeByMetadataName(LogPropertiesAttribute);
var tagCollectorSymbol = compilation.GetTypeByMetadataName(ITagCollectorType);
var logPropertyIgnoreAttributeSymbol = compilation.GetTypeByMetadataName(LogPropertyIgnoreAttribute);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.ObjectPool;
using Microsoft.Extensions.Telemetry.Logging;
using Microsoft.Extensions.Telemetry.Metering;
using Microsoft.Shared.Diagnostics;

Expand Down Expand Up @@ -197,12 +196,12 @@ private void CopyTo(Box<T> to)
}
}

[LogMethod(LogLevel.Debug, "Can't parse header '{headerName}' due to '{error}'.")]
[LoggerMessage(LogLevel.Debug, "Can't parse header '{headerName}' due to '{error}'.")]
private partial void LogParsingError(string headerName, string error);

[LogMethod(LogLevel.Debug, "Using a default value for header '{headerName}'.")]
[LoggerMessage(LogLevel.Debug, "Using a default value for header '{headerName}'.")]
private partial void LogDefaultUsage(string headerName);

[LogMethod(LogLevel.Debug, "Header '{headerName}' not found.")]
[LoggerMessage(LogLevel.Debug, "Header '{headerName}' not found.")]
private partial void LogNotFound(string headerName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ IEnumerator IEnumerable.GetEnumerator()

#endregion

[LogMethod(3, LogLevel.Error, ReadingRequestBodyError)]
[LoggerMessage(3, LogLevel.Error, ReadingRequestBodyError)]
public static partial void ErrorReadingRequestBody(this ILogger logger, Exception ex);

[LogMethod(4, LogLevel.Error, ReadingResponseBodyError)]
[LoggerMessage(4, LogLevel.Error, ReadingResponseBodyError)]
public static partial void ErrorReadingResponseBody(this ILogger logger, Exception ex);

#pragma warning disable LOGGEN000
[LogMethod(5, LogLevel.Warning,
[LoggerMessage(5, LogLevel.Warning,
$"HttpLogging middleware is injected into application pipeline, but {nameof(LogLevel)} '{{logLevel}}' is disabled in logger. " +
"Remove {methodName}() call from pipeline configuration in that case.")]
public static partial void MiddlewareIsMisused(this ILogger logger, LogLevel logLevel, string methodName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Telemetry.Logging;

namespace Microsoft.AspNetCore.Telemetry.Internal;

Expand All @@ -12,13 +11,13 @@ internal static partial class Log
/// <summary>
/// Logs `Http Route not found for Activity {activityName}.` at `Debug` level.
/// </summary>
[LogMethod(2, LogLevel.Debug, "Http Route not found for Activity {activityName}.")]
[LoggerMessage(2, LogLevel.Debug, "Http Route not found for Activity {activityName}.")]
public static partial void HttpRouteNotFound(this ILogger logger, string activityName);

/// <summary>
/// Logs `Configured HttpTracingOptions: {options}` at `Information` level.
/// </summary>
[LogMethod(3, LogLevel.Information, "Configured HttpTracingOptions: {options}")]
[LoggerMessage(3, LogLevel.Information, "Configured HttpTracingOptions: {options}")]
internal static partial void ConfiguredHttpTracingOptions(this ILogger logger, HttpTracingOptions options);
}
#pragma warning restore S109 // Magic numbers should not be used
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@

using System.Text;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Telemetry.Logging;

namespace Microsoft.Extensions.Diagnostics.HealthChecks;

internal static partial class Log
{
[LogMethod(0, LogLevel.Warning, "Process reporting unhealthy: {status}. Health check entries are {entries}")]
[LoggerMessage(0, LogLevel.Warning, "Process reporting unhealthy: {status}. Health check entries are {entries}")]
public static partial void Unhealthy(
ILogger logger,
HealthStatus status,
StringBuilder entries);

[LogMethod(1, LogLevel.Debug, "Process reporting healthy: {status}.")]
[LoggerMessage(1, LogLevel.Debug, "Process reporting healthy: {status}.")]
public static partial void Healthy(
ILogger logger,
HealthStatus status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@

using System;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Telemetry.Logging;

namespace Microsoft.Extensions.Diagnostics.Probes;

internal static partial class Log
{
[LogMethod(LogLevel.Error, "Error updating health status through TCP endpoint")]
[LoggerMessage(LogLevel.Error, "Error updating health status through TCP endpoint")]
public static partial void SocketExceptionCaughtTcpEndpoint(this ILogger logger, Exception e);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Telemetry.Logging;

namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring.Internal;

[SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "Generators.")]
[SuppressMessage("Major Code Smell", "S109:Magic numbers should not be used", Justification = "Generators.")]
internal static partial class Log
{
[LogMethod(1, LogLevel.Error, "Unable to gather utilization statistics.")]
[LoggerMessage(1, LogLevel.Error, "Unable to gather utilization statistics.")]
public static partial void HandledGatherStatisticsException(ILogger logger, Exception e);

[LogMethod(2, LogLevel.Error, "Publisher `{publisher}` was unable to publish utilization statistics.")]
[LoggerMessage(2, LogLevel.Error, "Publisher `{publisher}` was unable to publish utilization statistics.")]
public static partial void HandlePublishUtilizationException(ILogger logger, Exception e, string publisher);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Telemetry.Logging;

namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring.Internal;

Expand All @@ -11,12 +10,12 @@ namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring.Internal;

internal static partial class Log
{
[LogMethod(1, LogLevel.Error, "Windows performance counter `{counterName}` does not exist.")]
[LoggerMessage(1, LogLevel.Error, "Windows performance counter `{counterName}` does not exist.")]
public static partial void CounterDoesNotExist(ILogger logger, string counterName);

[LogMethod(2, LogLevel.Information, "Resource Utilization is running inside a Job Object. For more information about Job Objects see https://aka.ms/job-objects")]
[LoggerMessage(2, LogLevel.Information, "Resource Utilization is running inside a Job Object. For more information about Job Objects see https://aka.ms/job-objects")]
public static partial void RunningInsideJobObject(ILogger logger);

[LogMethod(3, LogLevel.Information, "Resource Utilization is running outside of Job Object. For more information about Job Objects see https://aka.ms/job-objects")]
[LoggerMessage(3, LogLevel.Information, "Resource Utilization is running outside of Job Object. For more information about Job Objects see https://aka.ms/job-objects")]
public static partial void RunningOutsideJobObject(ILogger logger);
}
Loading

0 comments on commit ac7f0da

Please sign in to comment.