[docs] Add exception logging best practices #6037
Conversation
docs/logs/README.md
Outdated
| > [!NOTE] | ||
| > When using the compile-time source generator the first `Exception` parameter | ||
| > detected is automatically given special handling. It **SHOULD NOT** be part of | ||
| > the message template. |
There was a problem hiding this comment.
Few users may find it difficult to understand what we mean as message template. An example or calling it out would help.
There was a problem hiding this comment.
I added a link to runtime docs for more details. Think that will be enough?
| > the message template. | ||
|
|
||
| :heavy_check_mark: You should use the dedicated overloads to log exceptions when | ||
| using the logging extensions methods. |
There was a problem hiding this comment.
since we recommended against using extension methods in earlier section of the doc, lets not talk about it further :)
There was a problem hiding this comment.
That's a tough one. I don't disagree with you necessarily, but I see this mistake being made with users migrating to ILogger who are doing find/replace type of things (not taking the source generator). Perhaps we should soften the recommendation for source generator? It is great for high-perf. But users may not care and prefer the more concise syntax. There are also cases where high-perf probably doesn't matter. For example, startup/shutdown logging.
There was a problem hiding this comment.
Even .NET docs show both slow way and fast way, so we may not be able to completely get rid of extension methods from the docs, agree!
| You want to use the correct `Exception` APIs because the OpenTelemetry | ||
| Specification [defines dedicated | ||
| attributes](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/exceptions/exceptions-logs.md) | ||
| for `Exception` details. The following examples show what **NOT** to do. In |
There was a problem hiding this comment.
If a user does not want to get the stack details (due to cost etc.), then there is no opt-out mechanism today, so this maybe a viable workaround folks can use today. i.e they send ex.msg, and ex.type as normal parameters, and not pass in the exception object...
There was a problem hiding this comment.
That's a good point. Probably doesn't belong in best practices though. What if we put that somewhere with more advanced content or somewhere showing customizations?
There was a problem hiding this comment.
That'd be good (move it elsewhere)
Changes
Merge requirement checklist