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

Track exceptions in Application Insights publisher #302

Merged
merged 5 commits into from
Nov 6, 2019
Merged

Track exceptions in Application Insights publisher #302

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f40759
Track exceptions in Application Insights publisher
inkel Oct 9, 2019
4d93ae5
Assign track exceptions argument in App Insights publisher
inkel Oct 21, 2019
856c500
Add Application Insights Publisher unit tests
inkel Oct 21, 2019
9a85635
Add track exceptions parameter to App Insights publisher DI
inkel Oct 21, 2019
0e62a0b
Add track exceptions parameter to App Insights publisher DI
inkel Oct 21, 2019
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
26 changes: 25 additions & 1 deletion src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ class ApplicationInsightsPublisher
private static readonly object sync_root = new object();
private readonly bool _saveDetailedReport;
private readonly bool _excludeHealthyReports;
private readonly bool _trackExceptions;

public ApplicationInsightsPublisher(string instrumentationKey = default, bool saveDetailedReport = false, bool excludeHealthyReports = false)
public ApplicationInsightsPublisher(string instrumentationKey = default, bool saveDetailedReport = false, bool excludeHealthyReports = false, bool trackExceptions = true)
inkel marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How you can change TrackException parameters? probably you need to add a new overload or default parameter on ApplicationInsightsHealthCheckBuilderExtensions.cs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, I was going to add a default on ApplicationInsightsHealthCheckBuilderExtensions. Again, the plan is adding a default bool trackExceptions = true rather than false because I think having a report on exceptions happened is something one would like for default (at least I know it's what I wanted).

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've add the parameter to the extension method in 0c2e389.

{
_instrumentationKey = instrumentationKey;
_saveDetailedReport = saveDetailedReport;
Expand All @@ -48,8 +49,31 @@ public Task PublishAsync(HealthReport report, CancellationToken cancellationToke
SaveGeneralizedReport(report, client);
}

if (_trackExceptions)
{
TrackExceptions(report, client);
}

return Task.CompletedTask;
}
private void TrackExceptions(HealthReport report, TelemetryClient client)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the health report contains exception, you can save also detailed report and exception? probably detailed and generalized report will contain exception information without create any other method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. Initially, I was going to TelemetryClient.TrackException only in the detailed report, but I think it's information you would like to have even in the summary report, that's why I've added the TrackExceptions method.

Also, note that for detailed report you could choose to not report healthy results; it probably doesn't make much sense but it could happen that a report is healthy but it also includes an exception and this is why the TrackExceptions method is called regardless you choose a summary or detailed report.

{
foreach (var reportEntry in report.Entries.Where(entry => entry.Value.Exception != null))
{
client.TrackException(reportEntry.Value.Exception,
properties: new Dictionary<string, string>()
{
{ nameof(Environment.MachineName), Environment.MachineName },
{ nameof(Assembly), Assembly.GetEntryAssembly().GetName().Name },
{ HEALTHCHECK_NAME, reportEntry.Key }
},
metrics: new Dictionary<string, double>()
{
{ METRIC_STATUS_NAME, reportEntry.Value.Status == HealthStatus.Healthy ? 1 : 0 },
{ METRIC_DURATION_NAME, reportEntry.Value.Duration.TotalMilliseconds }
});
}
}
private void SaveDetailedReport(HealthReport report, TelemetryClient client)
{
foreach (var reportEntry in report.Entries.Where(entry => !_excludeHealthyReports || entry.Value.Status != HealthStatus.Healthy))
Expand Down