Skip to content
This repository has been archived by the owner on Dec 8, 2018. It is now read-only.

Allow health checks to use any DI lifetime #466

Merged
merged 2 commits into from Aug 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions build/dependencies.props
Expand Up @@ -31,6 +31,7 @@
<MicrosoftExtensionsRazorViewsSourcesPackageVersion>2.2.0-preview1-34967</MicrosoftExtensionsRazorViewsSourcesPackageVersion>
<MicrosoftExtensionsStackTraceSourcesPackageVersion>2.2.0-preview1-34967</MicrosoftExtensionsStackTraceSourcesPackageVersion>
<MicrosoftExtensionsTypeNameHelperSourcesPackageVersion>2.2.0-preview1-34967</MicrosoftExtensionsTypeNameHelperSourcesPackageVersion>
<MicrosoftExtensionsValueStopwatchSourcesPackageVersion>2.2.0-preview1-34967</MicrosoftExtensionsValueStopwatchSourcesPackageVersion>
<MicrosoftNETCoreApp20PackageVersion>2.0.9</MicrosoftNETCoreApp20PackageVersion>
<MicrosoftNETCoreApp21PackageVersion>2.1.2</MicrosoftNETCoreApp21PackageVersion>
<MicrosoftNETCoreApp22PackageVersion>2.2.0-preview1-26618-02</MicrosoftNETCoreApp22PackageVersion>
Expand Down
1 change: 0 additions & 1 deletion samples/HealthChecksSample/DBHealthStartup.cs
Expand Up @@ -21,7 +21,6 @@ public void ConfigureServices(IServiceCollection services)
{
// Registers required services for health checks
services.AddHealthChecks()

// Add a health check for a SQL database
.AddCheck(new SqlConnectionHealthCheck("MyDatabase", Configuration["ConnectionStrings:DefaultConnection"]));
}
Expand Down
8 changes: 2 additions & 6 deletions samples/HealthChecksSample/LivenessProbeStartup.cs
Expand Up @@ -17,7 +17,6 @@ public void ConfigureServices(IServiceCollection services)
// Registers required services for health checks
services
.AddHealthChecks()
.AddCheck("identity", () => Task.FromResult(HealthCheckResult.Healthy()))
.AddCheck(new SlowDependencyHealthCheck());
}

Expand Down Expand Up @@ -53,11 +52,8 @@ public void Configure(IApplicationBuilder app, IHostingEnvironment env)
// The liveness check uses an 'identity' health check that always returns healthy
app.UseHealthChecks("/health/live", new HealthCheckOptions()
{
// Filters the set of health checks run by this middleware
HealthCheckNames =
{
"identity",
},
// Exclude all checks, just return a 200.
Predicate = (check) => false,
Copy link
Member Author

Choose a reason for hiding this comment

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

Changing this to an arbitrary predicate rather than an explicit 'filter by name' seems like a good usability improvement. Since the set of checks has to be resolved per-request a lot of motivations for providing 'filter by name' seemed to melt away.

});

app.Run(async (context) =>
Expand Down
4 changes: 3 additions & 1 deletion samples/HealthChecksSample/Program.cs
Expand Up @@ -15,7 +15,7 @@ static Program()
{
_scenarios = new Dictionary<string, Type>(StringComparer.OrdinalIgnoreCase)
{
{ "", typeof(BasicStartup) },
{ "", typeof(DBHealthStartup) },
{ "basic", typeof(BasicStartup) },
{ "writer", typeof(CustomWriterStartup) },
{ "liveness", typeof(LivenessProbeStartup) },
Expand Down Expand Up @@ -48,6 +48,8 @@ public static IWebHost BuildWebHost(string[] args)
.UseConfiguration(config)
.ConfigureLogging(builder =>
{
builder.SetMinimumLevel(LogLevel.Trace);
builder.AddConfiguration(config);
builder.AddConsole();
})
.UseKestrel()
Expand Down
8 changes: 8 additions & 0 deletions samples/HealthChecksSample/appsettings.json
@@ -1,5 +1,13 @@
{
"ConnectionStrings": {
"DefaultConnection": "Server=(localdb)\\MSSQLLocalDB;Database=HealthCheckSample;Trusted_Connection=True;MultipleActiveResultSets=true;ConnectRetryCount=0"
},
"Logging": {
"LogLevel": {
"Default": "Debug"
},
"Console": {
"IncludeScopes": "true"
}
}
}
Expand Up @@ -16,7 +16,6 @@ public class HealthCheckMiddleware
private readonly RequestDelegate _next;
private readonly HealthCheckOptions _healthCheckOptions;
private readonly IHealthCheckService _healthCheckService;
private readonly IHealthCheck[] _checks;

public HealthCheckMiddleware(
RequestDelegate next,
Expand All @@ -41,8 +40,6 @@ public class HealthCheckMiddleware
_next = next;
_healthCheckOptions = healthCheckOptions.Value;
_healthCheckService = healthCheckService;

_checks = FilterHealthChecks(_healthCheckService.Checks, healthCheckOptions.Value.HealthCheckNames);
}

/// <summary>
Expand All @@ -58,7 +55,7 @@ public async Task InvokeAsync(HttpContext httpContext)
}

// Get results
var result = await _healthCheckService.CheckHealthAsync(_checks, httpContext.RequestAborted);
var result = await _healthCheckService.CheckHealthAsync(_healthCheckOptions.Predicate, httpContext.RequestAborted);

// Map status to response code - this is customizable via options.
if (!_healthCheckOptions.ResultStatusCodes.TryGetValue(result.Status, out var statusCode))
Expand Down
Expand Up @@ -15,15 +15,19 @@ namespace Microsoft.AspNetCore.Diagnostics.HealthChecks
public class HealthCheckOptions
{
/// <summary>
/// Gets a set of health check names used to filter the set of health checks run.
/// Gets or sets a predicate that is used to filter the set of health checks executed.
/// </summary>
/// <remarks>
/// If <see cref="HealthCheckNames"/> is empty, the <see cref="HealthCheckMiddleware"/> will run all
/// If <see cref="Predicate"/> is <c>null</c>, the <see cref="HealthCheckMiddleware"/> will run all
/// registered health checks - this is the default behavior. To run a subset of health checks,
/// add the names of the desired health checks.
/// provide a function that filters the set of checks.
/// </remarks>
public ISet<string> HealthCheckNames { get; } = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
public Func<IHealthCheck, bool> Predicate { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Filter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, we considered it and thought this was less overloaded. If we use filter (or another term) other places in the framework then I want to change this.


/// <summary>
/// Gets a dictionary mapping the <see cref="HealthCheckStatus"/> to an HTTP status code applied to the response.
/// This property can be used to configure the status codes returned for each status.
/// </summary>
public IDictionary<HealthCheckStatus, int> ResultStatusCodes { get; } = new Dictionary<HealthCheckStatus, int>()
{
{ HealthCheckStatus.Healthy, StatusCodes.Status200OK },
Expand Down
202 changes: 107 additions & 95 deletions src/Microsoft.Extensions.Diagnostics.HealthChecks/HealthCheckService.cs
Expand Up @@ -6,127 +6,139 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;

namespace Microsoft.Extensions.Diagnostics.HealthChecks
{
/// <summary>
/// Default implementation of <see cref="IHealthCheckService"/>.
/// </summary>
public class HealthCheckService : IHealthCheckService
internal class HealthCheckService : IHealthCheckService
{
private readonly IServiceScopeFactory _scopeFactory;
private readonly ILogger<HealthCheckService> _logger;

/// <summary>
/// A <see cref="IReadOnlyDictionary{TKey, T}"/> containing all the health checks registered in the application.
/// </summary>
/// <remarks>
/// The key maps to the <see cref="IHealthCheck.Name"/> property of the health check, and the value is the <see cref="IHealthCheck"/>
/// instance itself.
/// </remarks>
public IReadOnlyDictionary<string, IHealthCheck> Checks { get; }

/// <summary>
/// Constructs a <see cref="HealthCheckService"/> from the provided collection of <see cref="IHealthCheck"/> instances.
/// </summary>
/// <param name="healthChecks">The <see cref="IHealthCheck"/> instances that have been registered in the application.</param>
public HealthCheckService(IEnumerable<IHealthCheck> healthChecks) : this(healthChecks, NullLogger<HealthCheckService>.Instance) { }

/// <summary>
/// Constructs a <see cref="HealthCheckService"/> from the provided collection of <see cref="IHealthCheck"/> instances, and the provided logger.
/// </summary>
/// <param name="healthChecks">The <see cref="IHealthCheck"/> instances that have been registered in the application.</param>
/// <param name="logger">A <see cref="ILogger{T}"/> that can be used to log events that occur during health check operations.</param>
public HealthCheckService(IEnumerable<IHealthCheck> healthChecks, ILogger<HealthCheckService> logger)
public HealthCheckService(IServiceScopeFactory scopeFactory, ILogger<HealthCheckService> logger)
{
healthChecks = healthChecks ?? throw new ArgumentNullException(nameof(healthChecks));
_scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));

// Scan the list for duplicate names to provide a better error if there are duplicates.
var names = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var duplicates = new List<string>();
foreach (var check in healthChecks)
// We're specifically going out of our way to do this at startup time. We want to make sure you
// get any kind of health-check related error as early as possible. Waiting until someone
// actually tries to **run** health checks would be real baaaaad.
using (var scope = _scopeFactory.CreateScope())
{
if (!names.Add(check.Name))
{
duplicates.Add(check.Name);
}
var healthChecks = scope.ServiceProvider.GetRequiredService<IEnumerable<IHealthCheck>>();
EnsureNoDuplicates(healthChecks);
}
}

public Task<CompositeHealthCheckResult> CheckHealthAsync(CancellationToken cancellationToken = default) =>
CheckHealthAsync(predicate: null, cancellationToken);

if (duplicates.Count > 0)
public async Task<CompositeHealthCheckResult> CheckHealthAsync(
Func<IHealthCheck, bool> predicate,
CancellationToken cancellationToken = default)
{
using (var scope = _scopeFactory.CreateScope())
{
throw new ArgumentException($"Duplicate health checks were registered with the name(s): {string.Join(", ", duplicates)}", nameof(healthChecks));
var healthChecks = scope.ServiceProvider.GetRequiredService<IEnumerable<IHealthCheck>>();

var results = new Dictionary<string, HealthCheckResult>(StringComparer.OrdinalIgnoreCase);
foreach (var healthCheck in healthChecks)
{
if (predicate != null && !predicate(healthCheck))
{
continue;
}

cancellationToken.ThrowIfCancellationRequested();

// If the health check does things like make Database queries using EF or backend HTTP calls,
// it may be valuable to know that logs it generates are part of a health check. So we start a scope.
using (_logger.BeginScope(new HealthCheckLogScope(healthCheck.Name)))
{
HealthCheckResult result;
try
{
Log.HealthCheckBegin(_logger, healthCheck);
var stopwatch = ValueStopwatch.StartNew();
result = await healthCheck.CheckHealthAsync(cancellationToken);
Log.HealthCheckEnd(_logger, healthCheck, result, stopwatch.GetElapsedTime());
}
catch (Exception ex)
{
Log.HealthCheckError(_logger, healthCheck, ex);
result = new HealthCheckResult(HealthCheckStatus.Failed, ex, ex.Message, data: null);
}

// This can only happen if the result is default(HealthCheckResult)
if (result.Status == HealthCheckStatus.Unknown)
{
// This is different from the case above. We throw here because a health check is doing something specifically incorrect.
throw new InvalidOperationException($"Health check '{healthCheck.Name}' returned a result with a status of Unknown");
}

results[healthCheck.Name] = result;
}
}

return new CompositeHealthCheckResult(results);
}
}

Checks = healthChecks.ToDictionary(c => c.Name, StringComparer.OrdinalIgnoreCase);
private static void EnsureNoDuplicates(IEnumerable<IHealthCheck> healthChecks)
{
// Scan the list for duplicate names to provide a better error if there are duplicates.
var duplicateNames = healthChecks
.GroupBy(c => c.Name, StringComparer.OrdinalIgnoreCase)
.Where(g => g.Count() > 1)
.Select(g => g.Key)
.ToList();

if (_logger.IsEnabled(LogLevel.Debug))
if (duplicateNames.Count > 0)
{
foreach (var check in Checks)
{
_logger.LogDebug("Health check '{healthCheckName}' has been registered", check.Key);
}
throw new ArgumentException($"Duplicate health checks were registered with the name(s): {string.Join(", ", duplicateNames)}", nameof(healthChecks));
}
}

/// <summary>
/// Runs all the health checks in the application and returns the aggregated status.
/// </summary>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> which can be used to cancel the health checks.</param>
/// <returns>
/// A <see cref="Task{T}"/> which will complete when all the health checks have been run,
/// yielding a <see cref="CompositeHealthCheckResult"/> containing the results.
/// </returns>
public Task<CompositeHealthCheckResult> CheckHealthAsync(CancellationToken cancellationToken = default) =>
CheckHealthAsync(Checks.Values, cancellationToken);

/// <summary>
/// Runs the provided health checks and returns the aggregated status
/// </summary>
/// <param name="checks">The <see cref="IHealthCheck"/> instances to be run.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> which can be used to cancel the health checks.</param>
/// <returns>
/// A <see cref="Task{T}"/> which will complete when all the health checks have been run,
/// yielding a <see cref="CompositeHealthCheckResult"/> containing the results.
/// </returns>
public async Task<CompositeHealthCheckResult> CheckHealthAsync(IEnumerable<IHealthCheck> checks, CancellationToken cancellationToken = default)
private static class Log
{
var results = new Dictionary<string, HealthCheckResult>(Checks.Count, StringComparer.OrdinalIgnoreCase);
foreach (var check in checks)
public static class EventIds
{
cancellationToken.ThrowIfCancellationRequested();
// If the health check does things like make Database queries using EF or backend HTTP calls,
// it may be valuable to know that logs it generates are part of a health check. So we start a scope.
using (_logger.BeginScope(new HealthCheckLogScope(check.Name)))
{
HealthCheckResult result;
try
{
_logger.LogTrace("Running health check: {healthCheckName}", check.Name);
result = await check.CheckHealthAsync(cancellationToken);
_logger.LogTrace("Health check '{healthCheckName}' completed with status '{healthCheckStatus}'", check.Name, result.Status);
}
catch (Exception ex)
{
// We don't log this as an error because a health check failing shouldn't bring down the active task.
_logger.LogError(ex, "Health check '{healthCheckName}' threw an unexpected exception", check.Name);
result = new HealthCheckResult(HealthCheckStatus.Failed, ex, ex.Message, data: null);
}
public static readonly EventId HealthCheckBegin = new EventId(100, "HealthCheckBegin");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

public static readonly EventId HealthCheckEnd = new EventId(101, "HealthCheckEnd");
public static readonly EventId HealthCheckError = new EventId(102, "HealthCheckError");
}

// This can only happen if the result is default(HealthCheckResult)
if (result.Status == HealthCheckStatus.Unknown)
{
// This is different from the case above. We throw here because a health check is doing something specifically incorrect.
var exception = new InvalidOperationException($"Health check '{check.Name}' returned a result with a status of Unknown");
_logger.LogError(exception, "Health check '{healthCheckName}' returned a result with a status of Unknown", check.Name);
throw exception;
}
private static readonly Action<ILogger, string, Exception> _healthCheckBegin = LoggerMessage.Define<string>(
LogLevel.Debug,
EventIds.HealthCheckBegin,
"Running health check {HealthCheckName}");

results[check.Name] = result;
}
private static readonly Action<ILogger, string, double, HealthCheckStatus, Exception> _healthCheckEnd = LoggerMessage.Define<string, double, HealthCheckStatus>(
LogLevel.Debug,
EventIds.HealthCheckEnd,
"Health check {HealthCheckName} completed after {ElapsedMilliseconds}ms with status {HealthCheckStatus}");

private static readonly Action<ILogger, string, Exception> _healthCheckError = LoggerMessage.Define<string>(
LogLevel.Error,
EventIds.HealthCheckError,
"Health check {HealthCheckName} threw an unhandled exception");

public static void HealthCheckBegin(ILogger logger, IHealthCheck healthCheck)
{
_healthCheckBegin(logger, healthCheck.Name, null);
}

public static void HealthCheckEnd(ILogger logger, IHealthCheck healthCheck, HealthCheckResult result, TimeSpan duration)
{
_healthCheckEnd(logger, healthCheck.Name, duration.TotalMilliseconds, result.Status, null);
}

public static void HealthCheckError(ILogger logger, IHealthCheck healthCheck, Exception exception)
{
_healthCheckError(logger, healthCheck.Name, exception);
}
return new CompositeHealthCheckResult(results);
}
}
}