Adding Activity.AddException (#102905)

* Adding Activity.AddException

* Feedback

* Feedback 2

* Remove un-needed text from the comment

Author: tarekgh

src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs

@@ -50,6 +50,7 @@ public string? Id
         public string? TraceStateString { get { throw null; } set { } }
         public System.Diagnostics.Activity AddBaggage(string key, string? value) { throw null; }
         public System.Diagnostics.Activity AddEvent(System.Diagnostics.ActivityEvent e) { throw null; }
+        public System.Diagnostics.Activity AddException(System.Exception exception, System.Diagnostics.TagList tags = default, System.DateTimeOffset timestamp = default) { throw null; }
         public System.Diagnostics.Activity AddLink(System.Diagnostics.ActivityLink link) { throw null; }
         public System.Diagnostics.Activity AddTag(string key, string? value) { throw null; }
         public System.Diagnostics.Activity AddTag(string key, object? value) { throw null; }
@@ -276,11 +277,13 @@ public readonly struct ActivityCreationOptions<T>
         public string? TraceState { get { throw null; } init { throw null; } }
     }
     public delegate System.Diagnostics.ActivitySamplingResult SampleActivity<T>(ref System.Diagnostics.ActivityCreationOptions<T> options);
+    public delegate void ExceptionRecorder(System.Diagnostics.Activity activity, System.Exception exception, ref System.Diagnostics.TagList tags);
     public sealed class ActivityListener : IDisposable
     {
         public ActivityListener() { throw null; }
         public System.Action<System.Diagnostics.Activity>? ActivityStarted { get { throw null; } set { throw null; } }
         public System.Action<System.Diagnostics.Activity>? ActivityStopped { get { throw null; } set { throw null; } }
+        public System.Diagnostics.ExceptionRecorder? ExceptionRecorder { get { throw null; } set { throw null; } }
         public System.Func<System.Diagnostics.ActivitySource, bool>? ShouldListenTo { get { throw null; } set { throw null; } }
         public System.Diagnostics.SampleActivity<string>? SampleUsingParentId { get { throw null; } set { throw null; } }
         public System.Diagnostics.SampleActivity<ActivityContext>? Sample { get { throw null; } set { throw null; } }

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs

@@ -517,6 +517,75 @@ public Activity AddEvent(ActivityEvent e)
             return this;
         }
 
+        /// <summary>
+        /// Add an <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>
+        /// <para>- 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.</para>
+        /// <para>- 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.</para>
+        /// <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)
+        {
+            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>

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivityEvent.cs

@@ -28,12 +28,16 @@ public ActivityEvent(string name) : this(name, DateTimeOffset.UtcNow, tags: null
         /// <param name="name">Event name.</param>
         /// <param name="timestamp">Event timestamp. Timestamp MUST only be used for the events that happened in the past, not at the moment of this call.</param>
         /// <param name="tags">Event Tags.</param>
-        public ActivityEvent(string name, DateTimeOffset timestamp = default, ActivityTagsCollection? tags = null)
+        public ActivityEvent(string name, DateTimeOffset timestamp = default, ActivityTagsCollection? tags = null) : this(name, timestamp, tags, tags is null ? 0 : tags.Count) { }
+
+        internal ActivityEvent(string name, DateTimeOffset timestamp, ref TagList tags) : this(name, timestamp, tags, tags.Count) { }
+
+        private ActivityEvent(string name, DateTimeOffset timestamp, IEnumerable<KeyValuePair<string, object?>>? tags, int tagsCount)
         {
             Name = name ?? string.Empty;
             Timestamp = timestamp != default ? timestamp : DateTimeOffset.UtcNow;
 
-            _tags = tags?.Count > 0 ? new Activity.TagsLinkedList(tags) : null;
+            _tags = tagsCount > 0 ? new Activity.TagsLinkedList(tags!) : null;
         }
 
         /// <summary>

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivityListener.cs

@@ -10,6 +10,11 @@ namespace System.Diagnostics
     /// </summary>
     public delegate ActivitySamplingResult SampleActivity<T>(ref ActivityCreationOptions<T> options);
 
+    /// <summary>
+    /// Define the callback to be used in <see cref="ActivityListener"/> to receive notifications when exceptions are added to the <see cref="Activity"/>.
+    /// </summary>
+    public delegate void ExceptionRecorder(Activity activity, Exception exception, ref TagList tags);
+
     /// <summary>
     /// ActivityListener allows listening to the start and stop Activity events and give the opportunity to decide creating the Activity for sampling scenarios.
     /// </summary>
@@ -32,6 +37,11 @@ public ActivityListener()
         /// </summary>
         public Action<Activity>? ActivityStopped { get; set; }
 
+        /// <summary>
+        /// Set or get the callback used to listen to <see cref="Activity"/> events when exceptions are added.
+        /// </summary>
+        public ExceptionRecorder? ExceptionRecorder { get; set; }
+
         /// <summary>
         /// Set or get the callback used to decide if want to listen to <see cref="Activity"/> objects events which created using <see cref="ActivitySource"/> object.
         /// </summary>

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs

@@ -393,6 +393,18 @@ internal void NotifyActivityStop(Activity activity)
                 listeners.EnumWithAction((listener, obj) => listener.ActivityStopped?.Invoke((Activity)obj), activity);
             }
         }
+
+        internal void NotifyActivityAddException(Activity activity, Exception exception, ref TagList tags)
+        {
+            Debug.Assert(activity != null);
+
+            // _listeners can get assigned to null in Dispose.
+            SynchronizedList<ActivityListener>? listeners = _listeners;
+            if (listeners != null && listeners.Count > 0)
+            {
+                listeners.EnumWithExceptionNotification(activity, exception, ref tags);
+            }
+        }
     }
 
     // SynchronizedList<T> is a helper collection which ensure thread safety on the collection
@@ -498,5 +510,36 @@ public void EnumWithAction(Action<T, object> action, object arg)
             }
         }
 
+        public void EnumWithExceptionNotification(Activity activity, Exception exception, ref TagList tags)
+        {
+            if (typeof(T) != typeof(ActivityListener))
+            {
+                return;
+            }
+
+            uint version = _version;
+            int index = 0;
+
+            while (index < _list.Count)
+            {
+                T item;
+                lock (_list)
+                {
+                    if (version != _version)
+                    {
+                        version = _version;
+                        index = 0;
+                        continue;
+                    }
+
+                    item = _list[index];
+                    index++;
+                }
+
+                // Important to notify outside the lock.
+                // This is the whole point we are having this wrapper class.
+                (item as ActivityListener)!.ExceptionRecorder?.Invoke(activity, exception, ref tags);
+            }
+        }
     }
 }

src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs

@@ -1626,6 +1626,102 @@ public void AddLinkTest()
             Assert.Equal(99, tag.Value);
         }
 
+        [Fact]
+        public void AddExceptionTest()
+        {
+            using ActivitySource aSource = new ActivitySource("AddExceptionTest");
+
+            ActivityListener listener = new ActivityListener();
+            listener.ShouldListenTo = (activitySource) => object.ReferenceEquals(activitySource, aSource);
+            listener.Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData;
+            ActivitySource.AddActivityListener(listener);
+
+            using Activity? activity = aSource.StartActivity("Activity1");
+            Assert.NotNull(activity);
+            Assert.Empty(activity.Events);
+
+            const string ExceptionEventName = "exception";
+            const string ExceptionMessageTag = "exception.message";
+            const string ExceptionStackTraceTag = "exception.stacktrace";
+            const string ExceptionTypeTag = "exception.type";
+
+            Exception exception = new ArgumentOutOfRangeException("Some message");
+            activity.AddException(exception);
+            List<ActivityEvent> events = activity.Events.ToList();
+            Assert.Equal(1, events.Count);
+            Assert.Equal(ExceptionEventName, events[0].Name);
+            Assert.Equal(new TagList { { ExceptionMessageTag, exception.Message}, { ExceptionStackTraceTag, exception.ToString()}, { ExceptionTypeTag, exception.GetType().ToString() } }, events[0].Tags);
+
+            try { throw new InvalidOperationException("Some other message"); } catch (Exception e) { exception = e; }
+            activity.AddException(exception);
+            events = activity.Events.ToList();
+            Assert.Equal(2, events.Count);
+            Assert.Equal(ExceptionEventName, events[1].Name);
+            Assert.Equal(new TagList { { ExceptionMessageTag, exception.Message}, { ExceptionStackTraceTag, exception.ToString()}, { ExceptionTypeTag, exception.GetType().ToString() } }, events[1].Tags);
+
+            listener.ExceptionRecorder = (Activity activity, Exception exception, ref TagList theTags) => theTags.Add("foo", "bar");
+            activity.AddException(exception, new TagList { { "hello", "world" } });
+            events = activity.Events.ToList();
+            Assert.Equal(3, events.Count);
+            Assert.Equal(ExceptionEventName, events[2].Name);
+            Assert.Equal(new TagList
+                            {
+                                { "hello", "world" },
+                                { "foo", "bar" },
+                                { ExceptionMessageTag, exception.Message },
+                                { ExceptionStackTraceTag, exception.ToString() },
+                                { ExceptionTypeTag, exception.GetType().ToString() }
+                            },
+                            events[2].Tags);
+
+            listener.ExceptionRecorder = (Activity activity, Exception exception, ref TagList theTags) =>
+                                            {
+                                                theTags.Add("exception.escaped", "true");
+                                                theTags.Add("exception.message", "Overridden message");
+                                                theTags.Add("exception.stacktrace", "Overridden stacktrace");
+                                                theTags.Add("exception.type", "Overridden type");
+                                            };
+            activity.AddException(exception, new TagList { { "hello", "world" } });
+            events = activity.Events.ToList();
+            Assert.Equal(4, events.Count);
+            Assert.Equal(ExceptionEventName, events[3].Name);
+            Assert.Equal(new TagList
+                            {
+                                { "hello", "world" },
+                                { "exception.escaped", "true" },
+                                { "exception.message", "Overridden message" },
+                                { "exception.stacktrace", "Overridden stacktrace" },
+                                { "exception.type", "Overridden type" }
+                            },
+                            events[3].Tags);
+
+            ActivityListener listener1 = new ActivityListener();
+            listener1.ShouldListenTo = (activitySource) => object.ReferenceEquals(activitySource, aSource);
+            listener1.Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllData;
+            ActivitySource.AddActivityListener(listener1);
+            listener1.ExceptionRecorder = (Activity activity, Exception exception, ref TagList theTags) =>
+                                            {
+                                                theTags.Remove(new KeyValuePair<string, object?>("exception.message", "Overridden message"));
+                                                theTags.Remove(new KeyValuePair<string, object?>("exception.stacktrace", "Overridden stacktrace"));
+                                                theTags.Remove(new KeyValuePair<string, object?>("exception.type", "Overridden type"));
+                                                theTags.Add("secondListener", "win");
+                                            };
+            activity.AddException(exception, new TagList { { "hello", "world" } });
+            events = activity.Events.ToList();
+            Assert.Equal(5, events.Count);
+            Assert.Equal(ExceptionEventName, events[4].Name);
+            Assert.Equal(new TagList
+                            {
+                                { "hello", "world" },
+                                { "exception.escaped", "true" },
+                                { "secondListener", "win" },
+                                { "exception.message", exception.Message },
+                                { "exception.stacktrace", exception.ToString() },
+                                { "exception.type", exception.GetType().ToString() },
+                            },
+                            events[4].Tags);
+        }
+
         [Fact]
         public void TestIsAllDataRequested()
         {