SerilogAnalyzer icon indicating copy to clipboard operation
SerilogAnalyzer copied to clipboard

Support for Microsoft.Extensions.Logging

Open cd21h opened this issue 8 years ago • 9 comments

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

cd21h avatar Jan 24 '17 14:01 cd21h

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.

Suchiman avatar Jan 26 '17 01:01 Suchiman

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 avatar Feb 13 '17 21:02 nblumhardt

Also- LibLog, which supports message template syntax too.

nblumhardt avatar Apr 12 '17 23:04 nblumhardt

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 avatar Apr 23 '18 10:04 olsh

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

olsh avatar Mar 11 '19 10:03 olsh

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 avatar Jun 11 '20 15:06 jons-aura

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.

jons-aura avatar Jun 11 '20 16:06 jons-aura

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.

digitalsigi avatar Nov 05 '20 09:11 digitalsigi

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

304NotModified avatar Feb 01 '21 12:02 304NotModified