Skip to content

Commit c1b1868

Browse files
Copilotgewarren
andauthored
Update CA1873 to recommend source-generated logging with LoggerMessageAttribute (#50584)
* Initial plan * Update CA1873 to recommend source-generated logging with LoggerMessageAttribute Co-authored-by: gewarren <[email protected]> * Fix code examples to properly avoid expensive operations in logging Co-authored-by: gewarren <[email protected]> * Apply suggestions from code review --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: gewarren <[email protected]>
1 parent d66bda9 commit c1b1868

File tree

9 files changed

+141
-97
lines changed

9 files changed

+141
-97
lines changed

docs/fundamentals/code-analysis/quality-rules/ca1873.md

Lines changed: 7 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -30,117 +30,27 @@ In many situations, logging is disabled or set to a log level that results in an
3030

3131
## Rule description
3232

33-
When logging methods are called, their arguments are evaluated regardless of whether the logging level is enabled. This can result in expensive operations being executed even when the log message won't be written. For better performance, guard expensive logging calls with a check to <xref:Microsoft.Extensions.Logging.ILogger.IsEnabled%2A> or use the `LoggerMessage` pattern.
33+
When logging methods are called, their arguments are evaluated regardless of whether the logging level is enabled. This can result in expensive operations being executed even when the log message won't be written. For better performance, guard expensive logging calls with a check to <xref:Microsoft.Extensions.Logging.ILogger.IsEnabled%2A> or use source-generated logging with <xref:Microsoft.Extensions.Logging.LoggerMessageAttribute>.
3434

3535
## How to fix violations
3636

3737
To fix a violation of this rule, use one of the following approaches:
3838

3939
- Guard the logging call with a check to <xref:Microsoft.Extensions.Logging.ILogger.IsEnabled%2A>.
40-
- Use the `LoggerMessage` pattern with <xref:Microsoft.Extensions.Logging.LoggerMessageAttribute>.
40+
- Use source-generated logging with <xref:Microsoft.Extensions.Logging.LoggerMessageAttribute>.
4141
- Ensure expensive operations aren't performed in logging arguments unless necessary.
4242

4343
## Example
4444

4545
The following code snippet shows violations of CA1873:
4646

47-
```csharp
48-
using Microsoft.Extensions.Logging;
49-
50-
class Example
51-
{
52-
private readonly ILogger _logger;
53-
54-
public Example(ILogger<Example> logger)
55-
{
56-
_logger = logger;
57-
}
58-
59-
public void ProcessData(int[] data)
60-
{
61-
// Violation: expensive operation in logging argument.
62-
_logger.LogDebug($"Processing {string.Join(", ", data)} items");
63-
64-
// Violation: object creation in logging argument.
65-
_logger.LogTrace("Data: {Data}", new { Count = data.Length, Items = data });
66-
}
67-
}
68-
```
69-
70-
```vb
71-
Imports Microsoft.Extensions.Logging
72-
73-
Class Example
74-
Private ReadOnly _logger As ILogger
75-
76-
Public Sub New(logger As ILogger(Of Example))
77-
_logger = logger
78-
End Sub
79-
80-
Public Sub ProcessData(data As Integer())
81-
' Violation: expensive operation in logging argument.
82-
_logger.LogDebug($"Processing {String.Join(", ", data)} items")
83-
84-
' Violation: object creation in logging argument.
85-
_logger.LogTrace("Data: {Data}", New With {.Count = data.Length, .Items = data})
86-
End Sub
87-
End Class
88-
```
47+
:::code language="csharp" source="./snippets/ca1873/csharp/CA1873Example/Violation.cs" id="ViolationExample":::
48+
:::code language="vb" source="./snippets/ca1873/vb/CA1873Example/Violation.vb" id="ViolationExample":::
8949

90-
The following code snippet fixes the violations:
50+
The following code snippet fixes the violations by using source-generated logging:
9151

92-
```csharp
93-
using Microsoft.Extensions.Logging;
94-
95-
class Example
96-
{
97-
private readonly ILogger _logger;
98-
99-
public Example(ILogger<Example> logger)
100-
{
101-
_logger = logger;
102-
}
103-
104-
public void ProcessData(int[] data)
105-
{
106-
// Fixed: guard with IsEnabled check.
107-
if (_logger.IsEnabled(LogLevel.Debug))
108-
{
109-
_logger.LogDebug($"Processing {string.Join(", ", data)} items");
110-
}
111-
112-
// Fixed: guard with IsEnabled check.
113-
if (_logger.IsEnabled(LogLevel.Trace))
114-
{
115-
_logger.LogTrace("Data: {Data}", new { Count = data.Length, Items = data });
116-
}
117-
}
118-
}
119-
```
120-
121-
```vb
122-
Imports Microsoft.Extensions.Logging
123-
124-
Class Example
125-
Private ReadOnly _logger As ILogger
126-
127-
Public Sub New(logger As ILogger(Of Example))
128-
_logger = logger
129-
End Sub
130-
131-
Public Sub ProcessData(data As Integer())
132-
' Fixed: guard with IsEnabled check.
133-
If _logger.IsEnabled(LogLevel.Debug) Then
134-
_logger.LogDebug($"Processing {String.Join(", ", data)} items")
135-
End If
136-
137-
' Fixed: guard with IsEnabled check.
138-
If _logger.IsEnabled(LogLevel.Trace) Then
139-
_logger.LogTrace("Data: {Data}", New With {.Count = data.Length, .Items = data})
140-
End If
141-
End Sub
142-
End Class
143-
```
52+
:::code language="csharp" source="./snippets/ca1873/csharp/CA1873Example/Fix.cs" id="FixExample":::
53+
:::code language="vb" source="./snippets/ca1873/vb/CA1873Example/Fix.vb" id="FixExample":::
14454

14555
## When to suppress warnings
14656

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<OutputType>Exe</OutputType>
5+
<TargetFramework>net10.0</TargetFramework>
6+
<ImplicitUsings>enable</ImplicitUsings>
7+
<Nullable>enable</Nullable>
8+
</PropertyGroup>
9+
10+
<ItemGroup>
11+
<PackageReference Include="Microsoft.Extensions.Logging" Version="10.0.1" />
12+
</ItemGroup>
13+
14+
</Project>
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
using Microsoft.Extensions.Logging;
2+
3+
// <FixExample>
4+
partial class FixExample
5+
{
6+
private readonly ILogger _logger;
7+
8+
public FixExample(ILogger<FixExample> logger)
9+
{
10+
_logger = logger;
11+
}
12+
13+
public void ProcessData(int[] data)
14+
{
15+
// Fixed: use source-generated logging.
16+
// The data array is passed directly; no expensive operation executed unless log level is enabled.
17+
LogProcessingData(data);
18+
19+
// Fixed: use source-generated logging.
20+
LogTraceData(data.Length, data);
21+
}
22+
23+
[LoggerMessage(Level = LogLevel.Debug, Message = "Processing {Data} items")]
24+
private partial void LogProcessingData(int[] data);
25+
26+
[LoggerMessage(Level = LogLevel.Trace, Message = "Data: Count={Count}, Items={Items}")]
27+
private partial void LogTraceData(int count, int[] items);
28+
}
29+
// </FixExample>
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// See https://aka.ms/new-console-template for more information
2+
Console.WriteLine("Hello, World!");
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using Microsoft.Extensions.Logging;
2+
3+
// <ViolationExample>
4+
class ViolationExample
5+
{
6+
private readonly ILogger _logger;
7+
8+
public ViolationExample(ILogger<ViolationExample> logger)
9+
{
10+
_logger = logger;
11+
}
12+
13+
public void ProcessData(int[] data)
14+
{
15+
// Violation: expensive operation in logging argument.
16+
_logger.LogDebug($"Processing {string.Join(", ", data)} items");
17+
18+
// Violation: object creation in logging argument.
19+
_logger.LogTrace("Data: {Data}", new { Count = data.Length, Items = data });
20+
}
21+
}
22+
// </ViolationExample>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<OutputType>Exe</OutputType>
5+
<RootNamespace>CA1873Example</RootNamespace>
6+
<TargetFramework>net10.0</TargetFramework>
7+
</PropertyGroup>
8+
9+
<ItemGroup>
10+
<PackageReference Include="Microsoft.Extensions.Logging" Version="10.0.1" />
11+
</ItemGroup>
12+
13+
</Project>
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
Imports Microsoft.Extensions.Logging
2+
3+
' <FixExample>
4+
Partial Class FixExample
5+
Private ReadOnly _logger As ILogger
6+
7+
Public Sub New(logger As ILogger(Of FixExample))
8+
_logger = logger
9+
End Sub
10+
11+
Public Sub ProcessData(data As Integer())
12+
' Fixed: use source-generated logging.
13+
' The data array is passed directly; no expensive operation executed unless log level is enabled.
14+
LogProcessingData(data)
15+
16+
' Fixed: use source-generated logging.
17+
LogTraceData(data.Length, data)
18+
End Sub
19+
20+
<LoggerMessage(Level:=LogLevel.Debug, Message:="Processing {Data} items")>
21+
Private Partial Sub LogProcessingData(data As Integer())
22+
End Sub
23+
24+
<LoggerMessage(Level:=LogLevel.Trace, Message:="Data: Count={Count}, Items={Items}")>
25+
Private Partial Sub LogTraceData(count As Integer, items As Integer())
26+
End Sub
27+
End Class
28+
' </FixExample>
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Imports System
2+
3+
Module Program
4+
Sub Main(args As String())
5+
Console.WriteLine("Hello World!")
6+
End Sub
7+
End Module
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
Imports Microsoft.Extensions.Logging
2+
3+
' <ViolationExample>
4+
Class ViolationExample
5+
Private ReadOnly _logger As ILogger
6+
7+
Public Sub New(logger As ILogger(Of ViolationExample))
8+
_logger = logger
9+
End Sub
10+
11+
Public Sub ProcessData(data As Integer())
12+
' Violation: expensive operation in logging argument.
13+
_logger.LogDebug($"Processing {String.Join(", ", data)} items")
14+
15+
' Violation: object creation in logging argument.
16+
_logger.LogTrace("Data: {Data}", New With {.Count = data.Length, .Items = data})
17+
End Sub
18+
End Class
19+
' </ViolationExample>

0 commit comments

Comments
 (0)