Message template should be compile time constant
Asked Answered
V

1

75

I have this code

[HttpGet("average/{videoGuid}")]
public async Task<IActionResult> AverageRatingOfVideo([FromRoute] string videoGuid)
{
    _logger.LogInformation($"Finding average rating of video : {videoGuid}");
    var avg = await _ratingService.GetVideoRatingAverageAsync(videoGuid);
    return Ok(avg);
}

and I'm getting a warning here $"Finding average rating of video : {videoGuid}"

Message template should be compile time constant

I'm using Rider, there is no suggestion to fix this warning.

I can't understand why this gives me a warning, how could I fix this ?

Varnado answered 24/1, 2021 at 19:15 Comment(7)
No, I don't think so, my problem related to c#Varnado
Try to extract this $"Finding average rating of video : {videoGuid}" to some variable, like var msg = $"Finding average rating of video : {videoGuid}"; and use this message as LogInformation argumentHuffman
@Huffman tried that but the warning still existVarnado
It is a feature of Serilog, see discussion here.Annadiane
try _logger.LogInformation("Finding average rating of video : {videoGuid}", videoGuid) or _logger.LogInformation("Finding average rating of video : " + videoGuid). I would say that the reason for it is structured logging which uses the same curly brackets for templating and analyzer missing the interpolated string part.Leghorn
If you use serilog - check out this postLeghorn
Report to JetBrains as a bug youtrack.jetbrains.com/issues/RIDER and you might get an answer there.Ensign
D
140

The way to get rid of the warning is to supply the variable videoGuid separately, like this:

_logger.LogInformation("Finding average rating of video : {VideoGuid}", videoGuid);

Here, I first removed the $ sign, thereby turning off the string interpolation performed by C#. The {videoGuid} in the string now becomes a "property" instead, and so I pass the variable as a second argument to LogInformation. Rider also complains that properties in strings should start with a capital letter, so I changed it to {VideoGuid}.

Now for the real question: Why is there a warning?

The answer is that string interpolation prevents structured logging. When you pass the variables after the message, you make it possible for the logger to save them separately. If you just save the log to a file you may not see a difference, but if you later decide to log to a database or in some JSON format, you can just change your logging sink and you will be able to search through the logs much easier without changing all the log statements in your code.

There's a good discussion of this over on Software Engineering Stack Exchange.

Determiner answered 28/1, 2021 at 13:58 Comment(12)
This is a feature of Serilog right? Are you using it or did they introduce structured logging in .net 5? It might be good to clarify in any case.Clang
Yamaç Kurtuluş, I'm using NLog, at least for the moment. I'm guessing the IntelliJ team keeps a list of frameworks supporting structured logging somewhere to know when to activate this warning (or maybe they have a more sophisticated algorithm – there must be a lot of effort and code behind many of the warnings that their tools generate).Determiner
for me the solution (which Rider/MS/C# creators offer?) does not make sense, it may mislead code since the named arguments are just ordered arguments - no name match, so the code logger.LogDebug("hey {Foo}, hi {Bar}", bar, foo); will log hey bar, hi foo, even more renaming variables bar and foo will make code strange ...("hey {Foo}, hi {Bar}", big, bang); - Rider/ReSharper does not support rename in {} as well atm.Rudiger
@Rudiger the usage of {} in the string can simply be seen as the same as what you would use for string.Format(). You're able to add named variables to make it easier to read but you could just as well do logger.LogDebug("hey {0}, hi {1}", bar, foo) - it will be the same. Keep that in mind, it might help you.Jasper
@NewteqDeveloper it is clear, that is what I meant under "the named arguments are just ordered arguments - no name match", it does not replace beauty and clearness of string interpolationRudiger
Oh yes, I understand. I agree with you – it does not replace the clearness that string interpolation does, however, as the answerer mentioned, it helps a ton with structured logging. If you’re only ever going to write logs in a string format to a text file, I would say that you can completely ignore this warning. After all, it’s just a warning. But, to allow the structured logging, it will help to fix the warning as mentioned.Jasper
@Rudiger It is absolutely critical, if you're a library developer, that you do not ignore this warning, because you will flood downstream logging systems with messages that can't be properly elided or compressed. Whether or not is is "cleaner" isn't relevant in this case, because non-constant templates are wrong when using a structured logging framework.Doddered
Does it work with something like Logger.LogInformation("Server state is {Server.State}.", Server.State);? I hope it does, because I've changed a lot of lines like this ;) Compiler doesn't complain, I wonder if the strings will be interpolated correctly in the log.Kyle
@Doddered "elided"??Zedoary
@Zedoary merriam-webster.com/dictionary/elideDeterminer
@Kyle I'd try to avoid punctuation in variables (and messages, really). No guarantee that the log target will support that. If you're using Seq, I'm not sure if it is unsupported, but you'd likely eventually end up with a query like select * from stream where Server.State = 'foo' and 'Server.State' = 'bar'Doddered
I had a log like this _logger.LogInformation( $"Processing notifications for client {client.CompanyName} and user {user.UserId}"); but while debugging this is what it showed Processing notifications for client {client.CompanyName} and user {user.UserId}. It didn't care about replacing the correct values.Whitelaw

© 2022 - 2024 — McMap. All rights reserved.