72

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 ?

Ali Faris
  • 17,754
  • 10
  • 45
  • 70
  • 4
    No, I don't think so, my problem related to `c#` – Ali Faris Jan 24 '21 at 19:23
  • 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 argument – godot Jan 24 '21 at 19:25
  • @godot tried that but the warning still exist – Ali Faris Jan 24 '21 at 19:26
  • It is a feature of Serilog, see discussion [here](https://gitter.im/serilog/serilog?at=5d81e159a38dae3a6371ae37). – Martin Staufcik Jan 24 '21 at 19:31
  • 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. – Guru Stron Jan 24 '21 at 19:38
  • If you use serilog - check out [this post](https://nblumhardt.com/2015/01/c-6-string-interpolation-and-serilog/) – Guru Stron Jan 24 '21 at 19:42
  • Report to JetBrains as a bug https://youtrack.jetbrains.com/issues/RIDER and you might get an answer there. – Lex Li Jan 24 '21 at 23:02

2 Answers2

135

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.

Pang
  • 9,564
  • 146
  • 81
  • 122
Rolf Staflin
  • 2,092
  • 2
  • 22
  • 19
  • 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. – Yamuk Feb 20 '21 at 20:22
  • Yamaç Kurtuluş, I'm using [NLog](https://nlog-project.org/), 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). – Rolf Staflin Feb 21 '21 at 10:48
  • 10
    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. – svonidze May 31 '21 at 15:13
  • @svonidze 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. – Newteq Developer May 31 '21 at 18:23
  • @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 interpolation – svonidze Jun 01 '21 at 19:40
  • 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. – Newteq Developer Jun 04 '21 at 09:38
  • 6
    @svonidze 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. – EKW Jun 10 '21 at 15:20
  • 1
    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. – Harry Nov 12 '21 at 20:04
  • @EKW "elided"?? – thargenediad Apr 21 '22 at 13:17
  • 3
    @thargenediad https://www.merriam-webster.com/dictionary/elide – Rolf Staflin Apr 22 '22 at 14:36
  • @Harry 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'` – EKW May 04 '22 at 16:05
  • 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. – Chris Claude May 11 '22 at 11:01
-4

This is a false positive in the Serilog extension for Rider but other way to remove this warning is to disable the warning once (or globally in your class file).

// ReSharper disable once TemplateIsNotCompileTimeConstantProblem
_logger.LogInformation(messageTemplate);

Not the best solution but it's an option too.

Now, check Rof's answer about Why the warning.

Carlos Campos
  • 61
  • 6