Improve MetricsEventSource error handling (#55466)

Previously exceptions that escaped user provided callbacks for observable instruments would have terminated all further metric collection. Now those exceptions are caught and reported, only potentially interfering with other observable instruments in the same collection interval.

Author: noahfalk

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs

@@ -41,6 +41,7 @@ internal sealed class AggregationManager
         private readonly Action<Exception> _collectionError;
         private readonly Action _timeSeriesLimitReached;
         private readonly Action _histogramLimitReached;
+        private readonly Action<Exception> _observableInstrumentCallbackError;
 
         public AggregationManager(
             int maxTimeSeries,
@@ -54,7 +55,8 @@ public AggregationManager(
             Action initialInstrumentEnumerationComplete,
             Action<Exception> collectionError,
             Action timeSeriesLimitReached,
-            Action histogramLimitReached)
+            Action histogramLimitReached,
+            Action<Exception> observableInstrumentCallbackError)
         {
             _maxTimeSeries = maxTimeSeries;
             _maxHistograms = maxHistograms;
@@ -68,6 +70,7 @@ public AggregationManager(
             _collectionError = collectionError;
             _timeSeriesLimitReached = timeSeriesLimitReached;
             _histogramLimitReached = histogramLimitReached;
+            _observableInstrumentCallbackError = observableInstrumentCallbackError;
 
             _listener = new MeterListener()
             {
@@ -351,7 +354,14 @@ private bool CheckHistogramAllowed()
 
         internal void Collect()
         {
-            _listener.RecordObservableInstruments();
+            try
+            {
+                _listener.RecordObservableInstruments();
+            }
+            catch (Exception e)
+            {
+                _observableInstrumentCallbackError(e);
+            }
 
             foreach (KeyValuePair<Instrument, InstrumentState> kv in _instrumentStates)
             {

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs

@@ -169,6 +169,12 @@ public void HistogramLimitReached(string sessionId)
             WriteEvent(13, sessionId);
         }
 
+        [Event(14, Keywords = Keywords.TimeSeriesValues)]
+        public void ObservableInstrumentCallbackError(string sessionId, string errorMessage)
+        {
+            WriteEvent(14, sessionId, errorMessage);
+        }
+
         /// <summary>
         /// Called when the EventSource gets a command from a EventListener or ETW.
         /// </summary>
@@ -303,7 +309,8 @@ public void OnEventCommand(EventCommandEventArgs command)
                             () => Log.InitialInstrumentEnumerationComplete(sessionId),
                             e => Log.Error(sessionId, e.ToString()),
                             () => Log.TimeSeriesLimitReached(sessionId),
-                            () => Log.HistogramLimitReached(sessionId));
+                            () => Log.HistogramLimitReached(sessionId),
+                            e => Log.ObservableInstrumentCallbackError(sessionId, e.ToString()));
 
                         _aggregationManager.SetCollectionPeriod(TimeSpan.FromSeconds(refreshIntervalSecs));
 

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

@@ -619,6 +619,33 @@ public void EventSourceEnforcesHistogramLimit()
             AssertCollectStartStopEventsPresent(events, IntervalSecs, 3);
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
+        [OuterLoop("Slow and has lots of console spew")]
+        public void EventSourceHandlesObservableCallbackException()
+        {
+            using Meter meter = new Meter("TestMeter15");
+            Counter<int> c = meter.CreateCounter<int>("counter1");
+            ObservableCounter<int> oc = meter.CreateObservableCounter<int>("observableCounter1",
+                (Func<int>)(() => { throw new Exception("Example user exception"); }));
+
+            EventWrittenEventArgs[] events;
+            using (MetricsEventListener listener = new MetricsEventListener(_output, MetricsEventListener.TimeSeriesValues, IntervalSecs, "TestMeter15"))
+            {
+                listener.WaitForCollectionStop(s_waitForEventTimeout, 1);
+                c.Add(5);
+                listener.WaitForCollectionStop(s_waitForEventTimeout, 2);
+                c.Add(12);
+                listener.WaitForCollectionStop(s_waitForEventTimeout, 3);
+                events = listener.Events.ToArray();
+            }
+
+            AssertBeginInstrumentReportingEventsPresent(events, c, oc);
+            AssertInitialEnumerationCompleteEventPresent(events);
+            AssertCounterEventsPresent(events, meter.Name, c.Name, "", "", "5", "12");
+            AssertObservableCallbackErrorPresent(events);
+            AssertCollectStartStopEventsPresent(events, IntervalSecs, 3);
+        }
+
         private void AssertBeginInstrumentReportingEventsPresent(EventWrittenEventArgs[] events, params Instrument[] expectedInstruments)
         {
             var beginReportEvents = events.Where(e => e.EventName == "BeginInstrumentReporting").Select(e =>
@@ -833,6 +860,17 @@ private void AssertCollectStartStopEventsPresent(EventWrittenEventArgs[] events,
             Assert.Equal(expectedPairs, startEventsSeen);
             Assert.Equal(expectedPairs, stopEventsSeen);
         }
+
+        private void AssertObservableCallbackErrorPresent(EventWrittenEventArgs[] events)
+        {
+            var errorEvents = events.Where(e => e.EventName == "ObservableInstrumentCallbackError").Select(e =>
+                new
+                {
+                    ErrorText = e.Payload[1].ToString(),
+                }).ToArray();
+            Assert.NotEmpty(errorEvents);
+            Assert.Contains("Example user exception", errorEvents[0].ErrorText);
+        }
     }
 
     class MetricsEventListener : EventListener