FP S6664 Reduce the number of Debug logging calls shouldn't take trace level into account

Rules and Languages Report False-positive / False-negative...
,
1

Sonarqube: server community v25.9.0.112764

Language: csharp

Rule: S6664 Reduce the number of Debug logging calls within this code block

I think this rule should exclude trace-level logs.

The purpose of trace-level logging is to help developpers understand how the execution flows accross their code. As a developper your are supposed to place quite a large numbers of them by design, especially for methods which implement sequences of operations.

How to reproduce?

try {

  logger.LogTrace(Constants.Progress.ProgressAuthentication);
  context.Authentication = await authenticationService.GetAuthentication(context.Tenant, source.Token).ConfigureAwait(false);

  logger.LogTrace(Constants.Progress.ProgressKey);
  context.Key = await keyService.GetByCustomer(context.Authentication.Customer, source.Token).ConfigureAwait(false);

  logger.LogTrace(Constants.Progress.ProgressDownload);
  context.Directory = await payloadService.DownloadPayload(context.Operation, context.Tenant, source.Token).ConfigureAwait(false);

  logger.LogTrace(Constants.Progress.ProgressMetadata);
  context.Metadata = await payloadService.ReadMetadata(context.Directory, source.Token).ConfigureAwait(false);

  logger.LogTrace(Constants.Progress.ProgressUpload);
  context.SendDate = DateTimeOffset.UtcNow;
  await remoteService.UploadPayload(context.Directory, source.Token).ConfigureAwait(false);

  logger.LogTrace(Constants.Progress.ProgressSuccess);
  await payloadService.NotifySuccess(context, cancellationToken).ConfigureAwait(false);

} catch(Exception exception) {
  // do stuff
}
3

I would argue this is not a FP, and tracing statements should be included in this matrix. Logging statements tend to obscure the actual code. To me, your code is a good example why S6664 makes a lot of sense.

In general, I would reject a PR containing so many logging statements.

1 Like
4

Logging statements are actual code; implying otherwise sounds pretty dangerous to me.

The code above can be coupled with a logging scope to be able to see the evolution of values in the context object after each step of the operation. One can enable and disable trace-level logs by changing the log-level of a single logger category, and without activating low level logging in dependency classes.

Since it’s the purpose of trace-level logging statements to follow the code flow, you’re supposed to write a bunch of them. I think they should be excluded, because unlike other level statements, using a large number of them was what they were designed for. I wonder what alternatives you suggest in order to punctually provide detailed tracing in applications you can’t attach a debugger to?

If the only argument against it is code readbility, it feels a bit idealistic in comparison.

5

Hi @ArwynFr,

Thank you for the detailed report and for pushing back — after looking into this more carefully, we agree with you.

All the logging frameworks we support (Microsoft.Extensions.Logging, NLog, Serilog, log4net, Castle.Core) consistently document the Trace level as being intentionally more verbose than Debug — it is the “noisiest level”, disabled by default, and designed to be used liberally for detailed execution tracing. Counting Trace calls toward the same threshold as Debug contradicts the documented intent of the level.

We will exclude Trace-level calls from this rule.

Thanks again for raising this!