Skip to content

Commit

Permalink
Improve MetricEventSource error message (#55319)
Browse files Browse the repository at this point in the history
The Error event wasn't capturing inner exception info
in its message. Switching from Exception.Message to
Exception.ToString() captures more useful messages.
We no longer split out the stack separately, but that
wasn't important. The error messages are likely only
useful for humans anyway.
  • Loading branch information
noahfalk authored Jul 8, 2021
1 parent 75fb0c3 commit 5f689e2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ public void EndInstrumentReporting(string sessionId, string meterName, string? m
}

[Event(9, Keywords = Keywords.TimeSeriesValues | Keywords.Messages | Keywords.InstrumentPublishing)]
public void Error(string sessionId, string errorMessage, string errorStack)
public void Error(string sessionId, string errorMessage)
{
WriteEvent(9, sessionId, errorMessage, errorStack);
WriteEvent(9, sessionId, errorMessage);
}

[Event(10, Keywords = Keywords.TimeSeriesValues | Keywords.InstrumentPublishing)]
Expand Down Expand Up @@ -203,7 +203,7 @@ public void OnEventCommand(EventCommandEventArgs command)
// This limitation shouldn't really matter because browser also doesn't support out-of-proc EventSource communication
// which is the intended scenario for this EventSource. If it matters in the future AggregationManager can be
// modified to have some other fallback path that works for browser.
Log.Error("", "System.Diagnostics.Metrics EventSource not supported on browser", "");
Log.Error("", "System.Diagnostics.Metrics EventSource not supported on browser");
return;
}
#endif
Expand Down Expand Up @@ -301,7 +301,7 @@ public void OnEventCommand(EventCommandEventArgs command)
i => Log.EndInstrumentReporting(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
i => Log.InstrumentPublished(sessionId, i.Meter.Name, i.Meter.Version, i.Name, i.GetType().Name, i.Unit, i.Description),
() => Log.InitialInstrumentEnumerationComplete(sessionId),
e => Log.Error(sessionId, e.Message, e.StackTrace?.ToString() ?? ""),
e => Log.Error(sessionId, e.ToString()),
() => Log.TimeSeriesLimitReached(sessionId),
() => Log.HistogramLimitReached(sessionId));

Expand All @@ -328,7 +328,7 @@ public void OnEventCommand(EventCommandEventArgs command)

private bool LogError(Exception e)
{
Log.Error(_sessionId, e.Message, e.StackTrace?.ToString() ?? "");
Log.Error(_sessionId, e.ToString());
// this code runs as an exception filter
// returning false ensures the catch handler isn't run
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1001,8 +1001,7 @@ private void AssertOnError()
if (errorEvent != null)
{
string message = errorEvent.Payload[1].ToString();
string stackTrace = errorEvent.Payload[2].ToString();
Assert.True(errorEvent == null, "Unexpected Error event: " + message + Environment.NewLine + stackTrace);
Assert.True(errorEvent == null, "Unexpected Error event: " + message);
}
}
}
Expand Down

0 comments on commit 5f689e2

Please sign in to comment.