agoda-com / AgodaAnalyzers

A set of opinionated Roslyn analyzers for C#
Apache License 2.0
21 stars 12 forks source link

String Interpolation used in logs instead of message template #183

Closed joeldickson closed 3 weeks ago

joeldickson commented 2 months ago

Feature request

Type

Should be able to check and warn on string interpolation or concatenation in logs and warn with a code fix to change to message template.

Also will do a fix for both serilog and ilogger as they have different signatures.

Using Message Templates vs String Interpolation in Logging

Approach Comparison

  1. String Interpolation:
    logger.Information($"Log message: {variable1}");

    or Concatination too

    logger.Warning("Person " + firstName + " " + lastName + " encountered an issue");
  2. Message Template:
    logger.Information("Log message: {Variable1}", variable1);

Benefits of Message Templates

  1. Structured Logging: Captures variables as separate entities, not just part of a string.
  2. Performance: Can be more efficient, especially if the log isn't written due to log level settings.
  3. Improved Parsing: Easier to parse and query logs later, especially when output in formats like JSON.
  4. Reduced Allocations: Often avoids allocating new strings, beneficial for high-volume logging.
  5. Framework Support: Optimized for modern logging frameworks like Serilog or NLog.
szaboopeeter commented 2 months ago

Doesn't CA2254 cover this?

joeldickson commented 1 month ago

just repeating internal slack thread for docs

CA2254 doesnt have a code fix and it's description in the IDE is confusing as to what is wrong

joeldickson commented 1 month ago

Just noting here too, claude managed to generate a test case that i didnt cover :)

i think its an obscure one, but wanted to leave it here incase we decide we want to cover it

 yield return new TestCaseData(new TestCase
 {
     Usings = "using Microsoft.Extensions.Logging;",
     SetupCode = "private readonly ILogger _logger;private string message;",
     LogStatement = "_logger.LogWarning($\"Warning: {message}\" + $\" at {DateTime.Now}\");",
     ExpectedFix = "_logger.LogWarning(\"Warning: {Message} at {Timestamp}\", message, DateTime.Now);",
     ExpectedDiagnostics = new[]
     {
         new DiagnosticResult(AG0040LogTemplateAnalyzer.Rule)
             .WithSpan(13, 31, 13, 78)
             .WithArguments("string interpolation")
     }
 }).SetName("ILogger with mixed string interpolation and concatenation");