Skip to content
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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Jun 23, 2022
Merged

Add exception.count in Runtime metrics #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
28bf236
Add `exception.counter` in Runtime metrics
xiang17 Jun 15, 2022
d3bade3
Update `CHANGELOG.md`
xiang17 Jun 15, 2022
aeeef72
Merge branch 'main' into xiang17/AddExceptionCounter
xiang17 Jun 15, 2022
4784c09
Use `Interlocked.Increment` for thread-safety
xiang17 Jun 16, 2022
533f11b
use `Counter` for exception.count
xiang17 Jun 17, 2022
38e9440
typo
xiang17 Jun 17, 2022
ffa0fbd
remove unused variable and rename option to `ExceptionCountEnabled`
xiang17 Jun 20, 2022
c04a2b4
Add directive for using System.Threading
xiang17 Jun 21, 2022
419459a
Merge branch 'main' into xiang17/AddExceptionCounter
xiang17 Jun 21, 2022
d232fa0
Merge branch 'main' into xiang17/AddExceptionCounter
xiang17 Jun 22, 2022
0c34be5
Merge branch 'main' into xiang17/AddExceptionCounter
xiang17 Jun 22, 2022
d25f9c2
Merge branch 'main' into xiang17/AddExceptionCounter
xiang17 Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/OpenTelemetry.Instrumentation.Runtime/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
([#430](/~https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/430))
* Remove Process related metrics from .NET Runtime metrics
([#446](/~https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/446))
* Add `exception.count` in Runtime metrics
([#431](/~https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/431))

## 0.2.0-alpha.1

Expand Down
9 changes: 9 additions & 0 deletions src/OpenTelemetry.Instrumentation.Runtime/RuntimeMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ public RuntimeMetrics(RuntimeMetricsOptions options)
{
this.meter.CreateObservableGauge($"{metricPrefix}assembly.count", () => (long)AppDomain.CurrentDomain.GetAssemblies().Length, description: "Number of Assemblies Loaded.");
}

if (options.IsExceptionCountEnabled)
{
var exceptionCounter = this.meter.CreateCounter<long>($"{metricPrefix}exception.count", description: "Number of exceptions thrown.");
Copy link
Member

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.

Copy link
Contributor Author

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.

  1. 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.

  1. 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?

AppDomain.CurrentDomain.FirstChanceException += (source, e) =>
{
exceptionCounter.Add(1);
};
}
}

/// <inheritdoc/>
Expand Down
13 changes: 12 additions & 1 deletion src/OpenTelemetry.Instrumentation.Runtime/RuntimeMetricsOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public class RuntimeMetricsOptions
/// </summary>
public bool? AssembliesEnabled { get; set; }

/// <summary>
/// Gets or sets a value indicating whether exception count metrics should be collected.
/// </summary>
public bool? ExceptionCountEnabled { get; set; }

/// <summary>
/// Gets a value indicating whether all metrics are enabled.
/// </summary>
Expand All @@ -55,7 +60,8 @@ public class RuntimeMetricsOptions
#if NETCOREAPP3_1_OR_GREATER
&& this.ThreadingEnabled == null
#endif
&& this.AssembliesEnabled == null;
&& this.AssembliesEnabled == null
&& this.ExceptionCountEnabled == null;

/// <summary>
/// Gets a value indicating whether garbage collection metrics is enabled.
Expand All @@ -80,5 +86,10 @@ public class RuntimeMetricsOptions
/// Gets a value indicating whether assembly metrics is enabled.
/// </summary>
internal bool IsAssembliesEnabled => this.AssembliesEnabled == true || this.IsAllEnabled;

/// <summary>
/// Gets a value indicating whether exception count metrics is enabled.
/// </summary>
internal bool IsExceptionCountEnabled => this.ExceptionCountEnabled == true || this.IsAllEnabled;
}
}