-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Adding Activity.AddException #102905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding Activity.AddException #102905
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -517,6 +517,71 @@ public Activity AddEvent(ActivityEvent e) | |
| return this; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Add a <see cref="ActivityEvent" /> object containing the exception information to the <see cref="Events" /> list. | ||
| /// </summary> | ||
| /// <param name="exception">The exception to add to the attached events list.</param> | ||
| /// <param name="tags">The tags to add to the exception event.</param> | ||
| /// <param name="timestamp">The timestamp to add to the exception event.</param> | ||
| /// <returns><see langword="this" /> for convenient chaining.</returns> | ||
| /// <remarks> | ||
| /// - The name of the event will be "exception", and it will include the tags "exception.message", "exception.stacktrace", and "exception.type", in addition to the tags provided in the <paramref name="tags"/> parameter. | ||
tarekgh marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| /// - Any registered <see cref="ActivityListener"/> with the <see cref="ActivityListener.ExceptionRecorder"/> callback will be notified about this exception addition before the <see cref="ActivityEvent" /> object is added to the <see cref="Events" /> list. | ||
| /// </remarks> | ||
| public Activity AddException(Exception exception, TagList tags = default, DateTimeOffset timestamp = default) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tarekgh Sorry just noticed this but should we use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I raised the question in #53641 (comment).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| { | ||
| if (exception == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(exception)); | ||
| } | ||
|
|
||
| TagList exceptionTags = tags; | ||
|
|
||
| Source.NotifyActivityAddException(this, exception, ref exceptionTags); | ||
|
|
||
| const string ExceptionEventName = "exception"; | ||
| const string ExceptionMessageTag = "exception.message"; | ||
| const string ExceptionStackTraceTag = "exception.stacktrace"; | ||
| const string ExceptionTypeTag = "exception.type"; | ||
|
|
||
| bool hasMessage = false; | ||
| bool hasStackTrace = false; | ||
| bool hasType = false; | ||
|
|
||
| for (int i = 0; i < exceptionTags.Count; i++) | ||
| { | ||
| if (exceptionTags[i].Key == ExceptionMessageTag) | ||
| { | ||
| hasMessage = true; | ||
| } | ||
| else if (exceptionTags[i].Key == ExceptionStackTraceTag) | ||
| { | ||
| hasStackTrace = true; | ||
| } | ||
| else if (exceptionTags[i].Key == ExceptionTypeTag) | ||
| { | ||
| hasType = true; | ||
| } | ||
| } | ||
|
|
||
| if (!hasMessage) | ||
| { | ||
| exceptionTags.Add(new KeyValuePair<string, object?>(ExceptionMessageTag, exception.Message)); | ||
| } | ||
|
|
||
| if (!hasStackTrace) | ||
| { | ||
| exceptionTags.Add(new KeyValuePair<string, object?>(ExceptionStackTraceTag, exception.ToString())); | ||
| } | ||
|
|
||
| if (!hasType) | ||
| { | ||
| exceptionTags.Add(new KeyValuePair<string, object?>(ExceptionTypeTag, exception.GetType().ToString())); | ||
| } | ||
|
|
||
| return AddEvent(new ActivityEvent(ExceptionEventName, timestamp, ref exceptionTags)); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Add an <see cref="ActivityLink"/> to the <see cref="Links"/> list. | ||
| /// </summary> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.