-
Notifications
You must be signed in to change notification settings - Fork 282
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
Add exception.count
in Runtime metrics
#431
Add exception.count
in Runtime metrics
#431
Conversation
Codecov Report
@@ Coverage Diff @@
## main #431 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 163 163
Lines 4898 4918 +20
=====================================
- Misses 4898 4918 +20
|
src/OpenTelemetry.Instrumentation.Runtime/RuntimeMetricsOptions.cs
Outdated
Show resolved
Hide resolved
exception.counter
in Runtime metricsexception.count
in Runtime metrics
|
||
if (options.IsExceptionCountEnabled) | ||
{ | ||
var exceptionCounter = this.meter.CreateCounter<long>($"{metricPrefix}exception.count", description: "Number of exceptions thrown."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description is not indicating if this is including FirstChance exceptions or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it's not necessary to mention it in the short description after reading through the the description for FirstChanceException and the comparison between first chance vs second chance exception.
- FirstChanceException indeed is the count of "exceptions thrown", just with two limits: "in managed code" and "before runtime searches for handler":
Occurs when an exception is thrown in managed code, before the runtime searches the call stack for an exception handler in the application domain.
- In comparison, second chance exception happens when the application doesn't handle it, i.e. the application crashes, then there is no need to "count" it because it'll always be 0 until it got to 1 and crashes immediately later. The user wouldn't have doubts on whether it's first chance or second chance exception if he/she knows about this concept.
If the user does get question on more detailed definition, he/she can refer to the detailed markdown doc, as for any other metrics in the Runtime metrics.
Is my understanding correct? Do you think it's still needed to add first chance in the description field for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Could improve descriptions in future PRs.
Adding feature for #335.
Changes
Add
exception.counter
in Runtime metrics.For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes