Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cecac51
Drafted design doc for logging generator
maryamariyan Apr 1, 2021
27bf951
Address design feedback
maryamariyan Apr 5, 2021
7681c39
Update proposed/logging-generator.md
maryamariyan Apr 6, 2021
b2a9c85
update builder proposal
maryamariyan Apr 6, 2021
b6b62d9
Apply latest feedback
maryamariyan Apr 6, 2021
a14e540
update sample code
maryamariyan Apr 6, 2021
464b2d5
Add generated code to document
maryamariyan Apr 6, 2021
1900b6c
cleanup
maryamariyan Apr 6, 2021
9c5ebc4
update conclusion
maryamariyan Apr 6, 2021
2d45b32
nit: code formatting
maryamariyan Apr 6, 2021
dd292d5
- Added benefits compared to directly using LoggerMessage.Define
maryamariyan Apr 8, 2021
36cb284
- Updated new samples
maryamariyan Apr 8, 2021
28dc4ff
Remove `EmitDefaultMessage` from proposal
maryamariyan Apr 8, 2021
ec1b13f
Cleaned up the design doc
maryamariyan Apr 9, 2021
58909d7
Adding overload needed with builder approach
maryamariyan Apr 9, 2021
ba29842
Rename builder
maryamariyan Apr 9, 2021
e7b640b
Merge remote-tracking branch 'origin/main' into logging-generator
maryamariyan Apr 9, 2021
43a64cb
Merge remote-tracking branch 'origin/main' into logging-generator
maryamariyan Apr 9, 2021
ea2d415
Update INDEX.md
maryamariyan Apr 9, 2021
93a8266
Updates with Define limitations
maryamariyan Apr 10, 2021
a2e8912
Updates with LoggerMessageAttribute usage
maryamariyan Apr 10, 2021
ced717f
- Update pascal casing to case insensitive support
maryamariyan Apr 10, 2021
d12991f
Merge remote-tracking branch 'origin/main' into logging-generator
maryamariyan Apr 10, 2021
f2ff366
update index file
maryamariyan Apr 10, 2021
3c90676
update index.md
maryamariyan Apr 12, 2021
172614e
Add known issues.
maryamariyan Apr 12, 2021
2ef72b3
- Update SYSLIBXXXX numbers
maryamariyan Apr 12, 2021
ddeecdb
update index.md
maryamariyan Apr 12, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
- Update pascal casing to case insensitive support
- Mention message/level required but event ID could be skipped
  • Loading branch information
maryamariyan committed Apr 10, 2021
commit ced717fc77d218b3793aa68b4ccd3ac3fe59fe50
18 changes: 6 additions & 12 deletions accepted/2021/logging-generator.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ namespace Microsoft.Extensions.Logging
}
```

When using `LoggerMessageAttribute`, the source generator would be making sure both `Level` and `Message` are provided by the user. For library authors it makes sense to have `EventId` required as well. Or, the event IDs could potentially default to zero or implicit event IDs would get generated. This way the diagnostics for `EventId` uniqueness checks still recommend best practices while allowing app developers to cut corners for productivity.

### Usage Examples:

A log method can be a plain static method, an extension method, or an instance method (where the ILogger comes from a field on the containing type).
Expand Down Expand Up @@ -181,14 +183,9 @@ static partial void LogMethod(ILogger logger, System.Exception ex, System.Except
static partial void LogMethod(ILogger logger, System.Exception ex, System.Exception ex2);
```

### Options

The generator supports a global option that it recognizes:

#### `PascalCaseArguments` : YES/NO

This will convert argument names to pascal case (from city to City) within the generated code such that when the ILogger enumerates the state, the argument will be in pascal case, which can make the logs nicer to consume. This defaults to YES.
### Case-insensitive parameter/template name support

The generator does case-insensitive comparison between parameters in message template and log message argument names so when the ILogger enumerates the state, the argument will be picked up by message template, which can make the logs nicer to consume:

```csharp
public partial class LoggingSample6
Expand All @@ -200,7 +197,7 @@ public partial class LoggingSample6
_logger = logger;
}

[LoggerMessage(EventId = 10, Level = LogLevel.Infomration, Message = "Welcome to {city} {province}!")]
[LoggerMessage(EventId = 10, Level = LogLevel.Infomration, Message = "Welcome to {City} {Province}!")]
public partial void LogMethodSupportsPascalCasingOfNames(string city, string province);

public void TestLogging()
Expand Down Expand Up @@ -520,9 +517,7 @@ For more clarity, the documentation in the future would need to mention that the

- Question: Why would we want to enforce event ID uniqueness checks if existing log APIs still allow for using default event ID values? For example with `LogInformation` we have some overloads, one taking `EventId` and the other one not. But the proposal with `LoggerMessageAttribute` only requires taking an `EventId`.

Answer: The enforced restriction added via `LoggerMessage` attribute aims at providing best practices for library authors more so than it does for app developers, who in most cases do not care about event IDs.

This is a great feedback. In order to make this approach less restrictive, would could also be allowing `LoggerMessageAttribute` to skip taking event IDs in combination with an analyzer that generates an error to recommend best practices while allowing app developers to cut corners for productivity. When turned off, the event IDs would default to zero or implicit event IDs would get generated.
Answer: The enforced restriction added via `LoggerMessage` attribute aims at providing best practices for library authors more so than it does for app developers, who in most cases do not care about event IDs. But it should be possible to suppress warnings regarding event ID uniqueness using the source generator.

## Supporting string interpolation builder in logging

Expand Down Expand Up @@ -708,7 +703,6 @@ Similar to using `LoggerMessageAttribute`, we showed that the builder approach a
- Guided developer experience - the generator gives warnings to help developers do the right thing
- Support for an arbitrary # of logging parameters. current approach tops out at 6
- Support for dynamic log level, current approach doesn't support this
- `PascalCaseArguments : YES/NO` : Optionally support for pascal casing logging arguments

### Conclusion

Expand Down