Open stephentoub opened 1 year ago
Tagging subscribers to this area: @dotnet/area-extensions-logging See info in area-owners.md if you want to be subscribed.
Author: | stephentoub |
---|---|
Assignees: | - |
Labels: | `code-analyzer`, `area-Extensions-Logging` |
Milestone: | - |
CC @Youssef1313
Happy to implement once the analyzer is approved.
Analyzer would detect when "work" is being done in an argument call to an ILogger
logging method. This basically boils down to anything other than a literal-expression, local-expression, parameter-expression, field-expression, property-expression, or indexer-expression (potentially recursively applied).
Note that this list includes identifying any parameters being wrapped in a new array to a params
method call.
The analyzer should not fire if it identifies that it is in a scope that has already checked on log levels.
A logging method is any of:
[LoggerMessage]
Microsoft.Extensions.Logging.LoggerExtensions
Microsoft.Extensions.Logging.ILogger.Log
Category: Performance Severity: Suggestion
@buyaa-n I did a quick (not yet tested) prototype (https://github.com/dotnet/roslyn-analyzers/pull/6643).
For now, I don't think I'll be able to complete it soon.
If anyone wants to pick it and continue on the code I wrote, this is totally fine.
Currently, the PR is missing:
The analyzer should not fire if it identifies that it is in a scope that has already checked on log levels.
@Youssef1313 thanks for letting us know and having a draft PR we can use to build this analyzer.
I'd like to finish this analyzer :)
@mpidash thanks for willing to help with that. Please ensure doing it as described in https://github.com/dotnet/runtime/issues/78402#issuecomment-1324132348.
We should consider an analyzer that flags use of
ToString()
in call sites to logging methods and suggests reconsidering the shape of the logging method such that it takes a strongly-typed value. In many situations, logging is disabled or set at a log level that will result in a particular event being disabled, and doing theToString()
at the call site is then often unnecessary allocation / expense. For example, given:the analyzer would flag the
.ToString()
and suggest theExcessivelyLargeTimeout
method should be changed to take aTimeSpan
instead of astring
, with the.ToString()
removed from the call site.Performance rules Category Severity = suggestion