SerilogAnalyzer icon indicating copy to clipboard operation
SerilogAnalyzer copied to clipboard

New rule proposal: Log event messages should be fragments, not sentences

Open prlcutting opened this issue 5 years ago • 8 comments

I've seen it written that log messages should be fragments, and not sentences, and therefore we should avoid including a dot/period at the end of a log message, e.g.:

var firstName = "John";

Log.Information("First name is {FirstName}.", firstName);  // Bad - trailing period
Log.Information("First name is {FirstName}", firstName);  // Good - no trailing period

How about a rule to catch this, and a quick-fix to remove the trailing period?

I'm basing this on Ben Foster's excellent Serilog Best Practices blog post (see Sentences vs. Fragments section). I'm sure I've also read similar from the Serilog man himself (@nblumhardt), but despite seaching, couldn't find a reference to cite.

prlcutting avatar Aug 14 '20 13:08 prlcutting

:+1: this would be nice

nblumhardt avatar Aug 16 '20 22:08 nblumhardt

@Suchiman - would you be open to including this new rule proposal in your SerilogAnalyzer project? If so, would you be interested in a PR for it?

prlcutting avatar Aug 22 '20 11:08 prlcutting

@prlcutting Yes, i'll gladly take a PR!

Suchiman avatar Aug 22 '20 16:08 Suchiman

I too have seen Ben Foster's blog post (and several other mentions of this best practice that all point to his blog post as the source of truth), but can anyone (@nblumhardt ?) explain in a little more detail a practical example of why this is a bad thing? Am I supposed to also not start log messages with a capital letter?

Balfa avatar Jul 28 '22 12:07 Balfa

@Balfa 👋 Having a consistent convention makes it more pleasant to read logs created by multiple developers; the details of the convention itself are mostly just opinion. I'm pretty sure I picked this up from older codebases and others I worked with.

When log messages get short and pithy I think the periods are just distracting:

Starting up.
Unhandled exception.

vs.

Starting up
Unhandled exception

YMMV :-)

nblumhardt avatar Jul 28 '22 21:07 nblumhardt

Thanks, that makes sense 🙂 It still seems a bit weird how we use capital letters at the start of log fragments, but not full stops at the end.

Balfa avatar Jul 29 '22 15:07 Balfa

I struggle with this rule a lot. It doesn't make sense to me. Maybe that's because of my use case.

My application is a .NET console app. I have two log sinks: File and console (stdout):

  • The file logs are for bug reports. by default, debug logs are written to the files.
  • The console is for info and above (no debug). It's intended to be more concise while providing useful, human readable information.

Especially for the console output, I need my logs to be human vs machine readable. I don't really benefit from the structural aspects of the logs that Serilog provides. Here's an example of a piece of code that checks for some invalid configuration. For each piece of invalid configuration, I log some diagnostic information. At the end, I provide a helpful message to the user that guides them on how to address the issues. This is a combination of two sentences.

if (_guideProcessor.DuplicateScores.Any())
{
    foreach (var (profileName, duplicates) in _guideProcessor.DuplicateScores)
    foreach (var (trashId, dupeScores) in duplicates)
    {
        _log.Warning(
            "Custom format with trash ID {TrashId} is duplicated {Count} times in quality profile " +
            "{ProfileName} with the following scores: {Scores}",
            trashId, dupeScores.Count, profileName, dupeScores);
    }

    _log.Warning(
        "When the same CF is specified multiple times with different scores in the same quality profile, " +
        "only the score from the first occurrence is used. To resolve the duplication warnings above, " +
        "remove the duplicate trash IDs from your YAML config.");

    _console.Output.WriteLine("");
}

I obviously get an analysis warning on the second Warning() log in my example above. But I don't see what I'm doing wrong here. Am I violating best practices? Are logs not meant to be written this way in Serilog? I'm not sure what to think and the analyzer just confuses the situation even more.

rcdailey avatar Oct 13 '22 01:10 rcdailey

I wouldn't pretend to know all the history of Serilog, but in regards to logging in general - the rule doesn't make sense to me. What do you mean messages are fragments, not sentences? IMO they absolutely are sentences for other humans to read. You start with a capital letter, follow basic grammar & punctuation so that other people are not put off by what you wrote.

The only argument for following that rule is if it's very widespread in general & followed not only by those who use Serilog. Then it would be the issue of consistency. But I highly doubt that it is so widespread (but again, not an expert, maybe it is that widespread).

wolf8196 avatar Mar 16 '23 08:03 wolf8196