Suchiman / SerilogAnalyzer

Roslyn-based analysis for code using the Serilog logging library. Checks for common mistakes and usage problems.
Apache License 2.0
309 stars 29 forks source link

Support for Microsoft.Extensions.Logging #15

Open cd21h opened 7 years ago

cd21h commented 7 years ago

Please add analysis of ILogger messages from Microsoft.Extensions.Logging.

E.g. I'm using Serilog with Microsoft.Extensions.Logging, but log messages with interpolation are not highlighted correctly.

Inspections should run if both MS Logging extensions and Serilog are referenced or it can be controlled by some configuration option.

Thanks

Suchiman commented 7 years ago

I'm not very familiar with Microsoft.Extensions.Logging but i will have a look if the whitelist can be simply expanded to include it or if Microsoft.Extensions.Logging requires different checks than Serilog.

nblumhardt commented 7 years ago

Alongside this, adding support for NLog (4.5+) would also be interesting to discuss. Just remembered about this issue while reading https://github.com/NLog/NLog.StructuredEvents/issues/46. (CC @304NotModified.)

nblumhardt commented 7 years ago

Also- LibLog, which supports message template syntax too.

olsh commented 6 years ago

It seems like there are analyzers for Microsoft.Extensions.Logging https://github.com/aspnet/Logging/tree/dev/src/Microsoft.Extensions.Logging.Analyzers

And they will be shipped in Core 2.2 https://github.com/aspnet/Logging/issues/712

olsh commented 5 years ago

I implemented a plugin for R# and Rider which highlights templates for Serilog, NLog, and Microsoft.Extensions.Logging, at the moment it doesn't have some analyzers and code fixes. https://github.com/olsh/resharper-structured-logging https://plugins.jetbrains.com/plugin/12083-structured-logging https://plugins.jetbrains.com/plugin/12832-structured-logging

jons-aura commented 4 years ago

What would it take to have this added? I saw reference to expanding a whitelist, is that all it would take? The project I'm working on takes ILogger instances as constructor parameters and logs through those and this extension doesn't seem to recognize them.

jons-aura commented 4 years ago

I did some local digging. So far I've removed/disabled the code below because Serilog itself doesn't show up in the sub projects because they only rely on ILogger from Microsoft.Extensions.Logging

var messageTemplateAttribute = context.SemanticModel.Compilation.GetTypeByMetadataName("Serilog.Core.MessageTemplateFormatMethodAttribute");
if (messageTemplateAttribute == null)
{
    return;
}

I changed the method attributes check to

-if (attributeData == null)
+if (attributeData == null && !((method as IMethodSymbol).ReceiverType.ToString() == "Microsoft.Extensions.Logging.ILogger"))

I changed the code that looks for the template parameter to check by parameter name instead of an attribute

-string messageTemplateName = attributeData.ConstructorArguments.First().Value as string;
// ... and where used
-if (parameter.Name == messageTemplateName)
+if (parameter.Name.Contains("message"))

It's all ugly bodge work but I get analysis messages now and nothing has crashed yet.

digitalsigi commented 3 years ago

Thx for the reply and the Info to #52.

I'm not very familiar with Microsoft.Extensions.Logging but i will have a look if the whitelist can be simply expanded to include it or if Microsoft.Extensions.Logging requires different checks than Serilog.

From my experience - examples:

Serilog: Log.Information("Template {Var}",var) (I think a reference to a static Object)

Mircrosoft: logger.LogInformation("Template {Var}",var) (Referece to a DI injected Object IIogger with for shure different names in each project / solution)

I do not know how VS extensions work, but I believe that parsing the template is exactly the same.

304NotModified commented 3 years ago

It seems like there are analyzers for Microsoft.Extensions.Logging https://github.com/aspnet/Logging/tree/dev/src/Microsoft.Extensions.Logging.Analyzers

And they will be shipped in Core 2.2 https://github.com/aspnet/Logging/issues/712

about this, it hasn't been shipped yet: https://github.com/dotnet/extensions/issues/3434