Adding Activity.AddException #102905
Adding Activity.AddException #102905
Conversation
|
Note regarding the |
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs
Outdated
Show resolved
Hide resolved
| /// <para>- Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback that adds "exception.message", "exception.stacktrace", or "exception.type" tags | ||
| /// will not have these tags overwritten, except by any subsequent <see cref="ActivityListener"/> that explicitly overwrites them.</para> | ||
| /// </remarks> | ||
| public Activity AddException(Exception exception, TagList tags = default, DateTimeOffset timestamp = default) |
There was a problem hiding this comment.
@tarekgh Sorry just noticed this but should we use in TagList tags here? It is kind of big struct that might save some copy cycles.
There was a problem hiding this comment.
I raised the question in #53641 (comment).
There was a problem hiding this comment.
I wouldn't be too worried either way. Presumably to get to this point the dev already threw and caught an Exception costing them 10s of microseconds. Spending an extra 10? nanoseconds copying a struct seems neglible.
There was a problem hiding this comment.
I already opened a PR #103009 for it.
The other point is we always pass TagList by ref in other APIs. Consistency is good beside getting minor perf is not bad.
fixes #53641