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.