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

Support for Microsoft.Extensions.Logging #15

Open
cd21h opened this issue Jan 24, 2017 · 9 comments
Open

Support for Microsoft.Extensions.Logging #15

cd21h opened this issue Jan 24, 2017 · 9 comments

Comments

@cd21h
Copy link

cd21h commented Jan 24, 2017

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
Copy link
Owner

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
Copy link
Contributor

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

@nblumhardt
Copy link
Contributor

Also- LibLog, which supports message template syntax too.

@olsh
Copy link

olsh commented Apr 23, 2018

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
Copy link

olsh commented Mar 11, 2019

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
Copy link

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
Copy link

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
Copy link

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
Copy link

304NotModified commented Feb 1, 2021

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: dotnet/extensions#3434

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants