From 2f407590408e2ff99bba8c1271f01f7618556130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez?= Date: Wed, 9 Oct 2019 15:44:15 -0300 Subject: [PATCH 1/5] Track exceptions in Application Insights publisher Previous version ignored reporting any exception contained in reports. Most of the Xabaril health checks do send a value for HealthCheckResult.Exception when the check fails, but they weren't being reported to App Insights. Note that there is lot of room for improvements on this class, so far I've only added a simple additional method to track exceptions. --- .../ApplicationInsightsPublisher.cs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs index fd4ddff4e7..c2b2a7ce32 100644 --- a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs +++ b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs @@ -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) { _instrumentationKey = instrumentationKey; _saveDetailedReport = saveDetailedReport; @@ -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) + { + foreach (var reportEntry in report.Entries.Where(entry => entry.Value.Exception != null)) + { + client.TrackException(reportEntry.Value.Exception, + properties: new Dictionary() + { + { nameof(Environment.MachineName), Environment.MachineName }, + { nameof(Assembly), Assembly.GetEntryAssembly().GetName().Name }, + { HEALTHCHECK_NAME, reportEntry.Key } + }, + metrics: new Dictionary() + { + { 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)) From 4d93ae545bcaac05e3b7c4f2fd6e0ed686993f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez?= Date: Mon, 21 Oct 2019 10:42:16 -0300 Subject: [PATCH 2/5] Assign track exceptions argument in App Insights publisher --- .../ApplicationInsightsPublisher.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs index c2b2a7ce32..05c8531124 100644 --- a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs +++ b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs @@ -30,6 +30,7 @@ public ApplicationInsightsPublisher(string instrumentationKey = default, bool sa _instrumentationKey = instrumentationKey; _saveDetailedReport = saveDetailedReport; _excludeHealthyReports = excludeHealthyReports; + _trackExceptions = trackExceptions; } public Task PublishAsync(HealthReport report, CancellationToken cancellationToken) { From 856c5004db34f2bb185f32b9e479476ae2c6e17b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez?= Date: Mon, 21 Oct 2019 10:43:37 -0300 Subject: [PATCH 3/5] Add Application Insights Publisher unit tests This commit only includes tests for tracking exceptions in ApplicationInsightsPublisher. Tests for existing tracking methods are left until this one is approved or rejected. If approved I'll add the tests in a new pull request, as adding those is out of the scope of the current pull request #302. --- .../ApplicationInsightsPublisher.cs | 2 +- .../ApplicationInsightsPublisherTests.cs | 129 ++++++++++++++++++ test/UnitTests/UnitTests.csproj | 2 + 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 test/UnitTests/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisherTests.cs diff --git a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs index 05c8531124..5f34a36e10 100644 --- a/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs +++ b/src/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisher.cs @@ -10,7 +10,7 @@ namespace HealthChecks.Publisher.ApplicationInsights { - class ApplicationInsightsPublisher + public class ApplicationInsightsPublisher : IHealthCheckPublisher { const string EVENT_NAME = "AspNetCoreHealthCheck"; diff --git a/test/UnitTests/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisherTests.cs b/test/UnitTests/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisherTests.cs new file mode 100644 index 0000000000..f235fddcca --- /dev/null +++ b/test/UnitTests/HealthChecks.Publisher.ApplicationInsights/ApplicationInsightsPublisherTests.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using HealthChecks.Publisher.ApplicationInsights; +using Microsoft.ApplicationInsights; +using Microsoft.ApplicationInsights.Channel; +using Microsoft.ApplicationInsights.DataContracts; +using Microsoft.ApplicationInsights.Extensibility; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Xunit; +using Xunit.Abstractions; + +namespace UnitTests.HealthChecks.Publisher.ApplicationInsights +{ + public class application_insights_publisher_should + { + private ApplicationInsightsPublisher publisher; + private readonly HealthReport report; + private readonly CancellationToken cancellationToken = CancellationToken.None; + private readonly FakeTelemetryChannel telemetryChannel; + private ITestOutputHelper testOutputHelper; + + public application_insights_publisher_should(ITestOutputHelper testOutputHelper) + { + this.testOutputHelper = testOutputHelper; + + telemetryChannel = new FakeTelemetryChannel(); + +#pragma warning disable CS0618 // Type or member is obsolete + TelemetryConfiguration.Active.TelemetryChannel = telemetryChannel; +#pragma warning restore CS0618 // Type or member is obsolete + + Dictionary entries = new Dictionary(); + + var properties = new Dictionary + { + { "foo", "bar" } + }; + + entries.Add("healthy", new HealthReportEntry(HealthStatus.Healthy, "healthy", TimeSpan.FromMilliseconds(25), exception: null, data: properties)); + entries.Add("unhealthy", new HealthReportEntry(HealthStatus.Unhealthy, "unhealthy", TimeSpan.FromMilliseconds(25), exception: new ArgumentNullException(), data: properties)); + entries.Add("degraded", new HealthReportEntry(HealthStatus.Degraded, "degraded", TimeSpan.FromMilliseconds(25), exception: null, data: properties)); + entries.Add("degraded_with_exception", new HealthReportEntry(HealthStatus.Degraded, "degraded with exception", TimeSpan.FromMilliseconds(25), exception: new NullReferenceException(), data: properties)); + + report = new HealthReport(entries, TimeSpan.FromMilliseconds(100)); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task track_exceptions_by_default(bool saveDetailedReport, bool excludeHealthyReports) + { + // Arrange + publisher = new ApplicationInsightsPublisher(string.Empty, saveDetailedReport, excludeHealthyReports); + + // Act + await publisher.PublishAsync(report, cancellationToken); + + // Assert + var itemsWithException = telemetryChannel.Items.Where(i => i is ExceptionTelemetry); + Assert.Equal(2, itemsWithException.Count()); + + foreach (var item in itemsWithException.Select(i => i as ExceptionTelemetry)) + { + // Assert properties + Assert.True(item.Properties.TryGetValue("AspNetCoreHealthCheckName", out string name)); + Assert.Equal(Environment.MachineName, item.Properties[nameof(Environment.MachineName)]); + Assert.Equal(Assembly.GetEntryAssembly().GetName().Name, item.Properties[nameof(Assembly)]); + + // Assert metrics + Assert.Equal(0, item.Metrics["AspNetCoreHealthCheckStatus"]); + Assert.Equal(25, item.Metrics["AspNetCoreHealthCheckDuration"]); + + // Assert proper exception is reported based on name + switch (name) + { + case "unhealthy": + Assert.IsType(item.Exception); + break; + + case "degraded_with_exception": + Assert.IsType(item.Exception); + break; + + default: + throw new Exception($"Unrecognized AspNetCoreHealthCheckName: {name}"); + } + } + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task dont_track_exceptions_if_parameter_is_false(bool saveDetailedReport, bool excludeHealthyReports) + { + // Arrange + publisher = new ApplicationInsightsPublisher(string.Empty, saveDetailedReport, excludeHealthyReports, trackExceptions: false); + + // Act + await publisher.PublishAsync(report, cancellationToken); + + // Assert + var itemsWithException = telemetryChannel.Items.Where(i => i is ExceptionTelemetry); + Assert.Empty(itemsWithException); + } + + private class FakeTelemetryChannel : ITelemetryChannel + { + public List Items { get; set; } = new List(); + + public void Send(ITelemetry item) => Items.Add(item); + + // These are not used for testing purposes, so they are left as not implemented methods. + // The only one that is used is get DeveloperMode, which is set to a sane default. + + public bool? DeveloperMode { get => true; set => throw new NotImplementedException(); } + public string EndpointAddress { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public void Dispose() => throw new NotImplementedException(); + public void Flush() => throw new NotImplementedException(); + } + } +} \ No newline at end of file diff --git a/test/UnitTests/UnitTests.csproj b/test/UnitTests/UnitTests.csproj index 95be864d96..c6080fc3d4 100644 --- a/test/UnitTests/UnitTests.csproj +++ b/test/UnitTests/UnitTests.csproj @@ -13,6 +13,7 @@ + @@ -42,6 +43,7 @@ + From 9a85635f528eb60be542ba0ddab87b5e81198e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez?= Date: Mon, 21 Oct 2019 11:09:25 -0300 Subject: [PATCH 4/5] Add track exceptions parameter to App Insights publisher DI While at it add some simple unit tests for this extension method. --- ...ionInsightsHealthCheckBuilderExtensions.cs | 4 ++-- .../Publisher.ApplicationInsightsUnitTests.cs | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 test/UnitTests/DependencyInjection/Publisher.ApplicationInsights/Publisher.ApplicationInsightsUnitTests.cs diff --git a/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs b/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs index e94cab0843..ec7ed178ce 100644 --- a/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs +++ b/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs @@ -17,12 +17,12 @@ public static class ApplicationInsightsHealthCheckBuilderExtensions /// Specifies if save an Application Insights event for each HealthCheck or just save one event with the global status for all the HealthChecks. Optional: If true saves an Application Insights event for each HealthCheck /// Specifies if save an Application Insights event only for reports indicating an unhealthy status /// The . - public static IHealthChecksBuilder AddApplicationInsightsPublisher(this IHealthChecksBuilder builder, string instrumentationKey = default, bool saveDetailedReport = false, bool excludeHealthyReports = false) + public static IHealthChecksBuilder AddApplicationInsightsPublisher(this IHealthChecksBuilder builder, string instrumentationKey = default, bool saveDetailedReport = false, bool excludeHealthyReports = false, bool trackExceptions = true) { builder.Services .AddSingleton(sp => { - return new ApplicationInsightsPublisher(instrumentationKey, saveDetailedReport, excludeHealthyReports); + return new ApplicationInsightsPublisher(instrumentationKey, saveDetailedReport, excludeHealthyReports, trackExceptions); }); return builder; diff --git a/test/UnitTests/DependencyInjection/Publisher.ApplicationInsights/Publisher.ApplicationInsightsUnitTests.cs b/test/UnitTests/DependencyInjection/Publisher.ApplicationInsights/Publisher.ApplicationInsightsUnitTests.cs new file mode 100644 index 0000000000..297174c009 --- /dev/null +++ b/test/UnitTests/DependencyInjection/Publisher.ApplicationInsights/Publisher.ApplicationInsightsUnitTests.cs @@ -0,0 +1,23 @@ +using HealthChecks.Publisher.ApplicationInsights; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Xunit; + +namespace UnitTests.DependencyInjection.Publisher.ApplicationInsights +{ + public class application_insights_publisher_registration_should + { + [Fact] + public void add_publisher_when_properly_configured() + { + var services = new ServiceCollection(); + services.AddHealthChecks() + .AddApplicationInsightsPublisher(); + + var serviceProvider = services.BuildServiceProvider(); + var publisher = serviceProvider.GetService(); + + Assert.IsType(publisher); + } + } +} From 0e62a0b3262dbb4a195f463f6e4534e06226f942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez?= Date: Mon, 21 Oct 2019 11:09:25 -0300 Subject: [PATCH 5/5] Add track exceptions parameter to App Insights publisher DI While at it add some simple unit tests for this extension method. --- .../ApplicationInsightsHealthCheckBuilderExtensions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs b/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs index ec7ed178ce..55dbca4bbb 100644 --- a/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs +++ b/src/HealthChecks.Publisher.ApplicationInsights/DependencyInjection/ApplicationInsightsHealthCheckBuilderExtensions.cs @@ -16,6 +16,7 @@ public static class ApplicationInsightsHealthCheckBuilderExtensions /// Specified Application Insights instrumentation key. Optional. If null TelemetryConfiguration.Active is used. /// Specifies if save an Application Insights event for each HealthCheck or just save one event with the global status for all the HealthChecks. Optional: If true saves an Application Insights event for each HealthCheck /// Specifies if save an Application Insights event only for reports indicating an unhealthy status + /// Specifies if save an Application Insights exception report for each HealthCheck that contains an exception. Optional: If true saves an Application Insights exception report. /// The . public static IHealthChecksBuilder AddApplicationInsightsPublisher(this IHealthChecksBuilder builder, string instrumentationKey = default, bool saveDetailedReport = false, bool excludeHealthyReports = false, bool trackExceptions = true) {