Address health check review feedback

This commit is contained in:
Sipke Schoorstra 2026-05-21 00:09:46 +02:00
parent 182f524d83
commit f149c27ab0
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
8 changed files with 103 additions and 41 deletions

View file

@ -7,9 +7,13 @@ Elsa hosts can opt in to Elsa-specific readiness checks on top of the normal ASP
Register ASP.NET Core health checks and then add Elsa readiness checks:
```csharp
using System;
services
.AddHealthChecks()
.AddElsaReadinessChecks(includeDistributedLocks: true);
.AddElsaReadinessChecks(
includeDistributedLocks: true,
configureOptions: options => options.DistributedLockAcquisitionTimeout = TimeSpan.FromSeconds(1));
```
Use `includeDistributedLocks: true` when the host enables distributed runtime behavior or relies on distributed locks for workflow coordination. Leave it `false` for simple single-node hosts that do not want a lock probe.
@ -25,6 +29,10 @@ services
Map separate endpoints for liveness and readiness:
```csharp
using Elsa.Extensions;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Diagnostics.HealthChecks;
app.MapHealthChecks("/health/live", new()
{
Predicate = _ => false
@ -32,7 +40,7 @@ app.MapHealthChecks("/health/live", new()
app.MapHealthChecks("/health/ready", new()
{
Predicate = check => check.Tags.Contains("readiness"),
Predicate = check => check.Tags.Contains(HealthCheckExtensions.ReadinessTag),
ResultStatusCodes =
{
[HealthStatus.Degraded] = StatusCodes.Status503ServiceUnavailable,
@ -72,4 +80,4 @@ readinessProbe:
Do not point liveness at Elsa readiness checks. A paused, draining, or temporarily database-degraded runtime should be removed from service by readiness without forcing Kubernetes to restart the process.
When `includeDistributedLocks: true` is enabled, set the readiness probe `timeoutSeconds` higher than the distributed-lock acquisition timeout. The built-in distributed-lock readiness check currently waits up to 1 second to acquire its probe lock, so the example uses `timeoutSeconds: 2` to avoid intermittent Kubernetes probe timeouts while the lock provider is still healthy.
When `includeDistributedLocks: true` is enabled, set the readiness probe `timeoutSeconds` higher than the configured distributed-lock acquisition timeout. The built-in distributed-lock readiness check waits up to 1 second by default to acquire its probe lock, so the example uses `timeoutSeconds: 2` to avoid intermittent Kubernetes probe timeouts while the lock provider is still healthy.

View file

@ -177,7 +177,7 @@ app.MapHealthChecks("/health/live", new()
});
app.MapHealthChecks("/health/ready", new()
{
Predicate = check => check.Tags.Contains("readiness"),
Predicate = check => check.Tags.Contains(HealthCheckExtensions.ReadinessTag),
ResultStatusCodes =
{
[HealthStatus.Degraded] = StatusCodes.Status503ServiceUnavailable,

View file

@ -1,4 +1,5 @@
using Elsa.Workflows.Runtime.HealthChecks;
using Elsa.Workflows.Runtime.Options;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
@ -9,7 +10,12 @@ namespace Elsa.Extensions;
/// </summary>
public static class HealthCheckExtensions
{
private static readonly string[] ReadinessTags = ["elsa", "readiness"];
/// <summary>
/// Tag applied to Elsa readiness checks.
/// </summary>
public const string ReadinessTag = "readiness";
private static readonly string[] ReadinessTags = ["elsa", ReadinessTag];
/// <summary>
/// Adds conservative Elsa-specific readiness probes for the workflow runtime and its core stores.
@ -17,11 +23,16 @@ public static class HealthCheckExtensions
/// <param name="builder">The health checks builder.</param>
/// <param name="includePersistence">Whether to probe workflow management/runtime stores.</param>
/// <param name="includeDistributedLocks">Whether to probe the configured distributed lock provider.</param>
/// <param name="configureOptions">Configures Elsa readiness probe behavior.</param>
public static IHealthChecksBuilder AddElsaReadinessChecks(
this IHealthChecksBuilder builder,
bool includePersistence = true,
bool includeDistributedLocks = false)
bool includeDistributedLocks = false,
Action<ElsaReadinessHealthCheckOptions>? configureOptions = null)
{
if (configureOptions != null)
builder.Services.Configure(configureOptions);
builder.AddCheck<ElsaRuntimeHealthCheck>("elsa-runtime", tags: ReadinessTags);
if (includePersistence)

View file

@ -1,18 +1,22 @@
using Elsa.Common;
using Elsa.Workflows.Runtime.Options;
using Medallion.Threading;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
namespace Elsa.Workflows.Runtime.HealthChecks;
/// <summary>
/// Verifies that the configured distributed lock provider can acquire and release a probe lock.
/// </summary>
public class ElsaDistributedLockHealthCheck(IServiceProvider serviceProvider, ILogger<ElsaDistributedLockHealthCheck> logger) : IHealthCheck
public class ElsaDistributedLockHealthCheck(
IServiceProvider serviceProvider,
IOptions<ElsaReadinessHealthCheckOptions> options,
ILogger<ElsaDistributedLockHealthCheck> logger) : IHealthCheck
{
private static readonly string LockName = $"elsa-health-check-{Environment.MachineName}-{Guid.NewGuid():N}";
private static readonly TimeSpan LockAcquisitionTimeout = TimeSpan.FromSeconds(1);
/// <inheritdoc />
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
@ -28,7 +32,7 @@ public class ElsaDistributedLockHealthCheck(IServiceProvider serviceProvider, IL
});
}
await using var handle = await distributedLockProvider.TryAcquireLockAsync(LockName, LockAcquisitionTimeout, cancellationToken);
await using var handle = await distributedLockProvider.TryAcquireLockAsync(LockName, options.Value.DistributedLockAcquisitionTimeout, cancellationToken);
if (handle == null)
{
return HealthCheckResult.Degraded("Elsa distributed lock provider was reachable, but the probe lock was not acquired.", data: new Dictionary<string, object>

View file

@ -18,45 +18,46 @@ public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider
/// <inheritdoc />
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
{
var failedStore = "";
var probes = new List<string>();
var skippedProbes = new List<string>();
var probeResults = await Task.WhenAll(
ProbeAsync("workflow-definitions", serviceProvider.GetService<IWorkflowDefinitionStore>(), async (store, ct) => await store.FindAsync(new WorkflowDefinitionFilter { Id = ProbeId }, ct)),
ProbeAsync("workflow-instances", serviceProvider.GetService<IWorkflowInstanceStore>(), async (store, ct) => await store.CountAsync(new WorkflowInstanceFilter { Id = ProbeId }, ct)),
ProbeAsync("triggers", serviceProvider.GetService<ITriggerStore>(), async (store, ct) => await store.FindAsync(new TriggerFilter { Id = ProbeId }, ct)),
ProbeAsync("bookmark-queue", serviceProvider.GetService<IBookmarkQueueStore>(), async (store, ct) => await store.FindAsync(new BookmarkQueueFilter { Id = ProbeId }, ct)));
try
var probes = probeResults.Where(x => !x.Skipped).Select(x => x.StoreName).ToList();
var skippedProbes = probeResults.Where(x => x.Skipped).Select(x => x.StoreName).ToList();
var failedProbe = probeResults.FirstOrDefault(x => x.Exception != null);
if (failedProbe != null)
{
await ProbeAsync("workflow-definitions", serviceProvider.GetService<IWorkflowDefinitionStore>(), async (store, ct) => await store.FindAsync(new WorkflowDefinitionFilter { Id = ProbeId }, ct));
await ProbeAsync("workflow-instances", serviceProvider.GetService<IWorkflowInstanceStore>(), async (store, ct) => await store.CountAsync(new WorkflowInstanceFilter { Id = ProbeId }, ct));
await ProbeAsync("triggers", serviceProvider.GetService<ITriggerStore>(), async (store, ct) => await store.FindAsync(new TriggerFilter { Id = ProbeId }, ct));
await ProbeAsync("bookmark-queue", serviceProvider.GetService<IBookmarkQueueStore>(), async (store, ct) => await store.FindAsync(new BookmarkQueueFilter { Id = ProbeId }, ct));
var data = CreateData();
return probes.Count == 0
? HealthCheckResult.Degraded("No Elsa workflow persistence stores are registered.", data: data)
: HealthCheckResult.Healthy("Elsa workflow stores are reachable.", data);
}
catch (Exception e) when (!e.IsFatal())
{
logger.LogWarning(e, "Elsa workflow store {StoreName} is not reachable.", failedStore);
return HealthCheckResult.Unhealthy($"Elsa workflow store '{failedStore}' is not reachable.", data: new Dictionary<string, object>
{
["category"] = "persistence",
["failedStore"] = failedStore,
["failedProbe"] = failedStore
});
data["failedStore"] = failedProbe.StoreName;
data["failedProbe"] = failedProbe.StoreName;
logger.LogWarning(failedProbe.Exception, "Elsa workflow store {StoreName} is not reachable.", failedProbe.StoreName);
return HealthCheckResult.Unhealthy($"Elsa workflow store '{failedProbe.StoreName}' is not reachable.", data: data);
}
async Task ProbeAsync<TStore>(string storeName, TStore? store, Func<TStore, CancellationToken, Task> probe) where TStore : class
{
failedStore = storeName;
var healthyData = CreateData();
return probes.Count == 0
? HealthCheckResult.Degraded("No Elsa workflow persistence stores are registered.", data: healthyData)
: HealthCheckResult.Healthy("Elsa workflow stores are reachable.", healthyData);
async Task<ProbeResult> ProbeAsync<TStore>(string storeName, TStore? store, Func<TStore, CancellationToken, Task> probe) where TStore : class
{
if (store == null)
{
skippedProbes.Add(storeName);
return;
return new ProbeResult(storeName, true, null);
}
probes.Add(storeName);
await probe(store, cancellationToken);
try
{
await probe(store, cancellationToken);
return new ProbeResult(storeName, false, null);
}
catch (Exception e) when (!e.IsFatal())
{
return new ProbeResult(storeName, false, e);
}
}
Dictionary<string, object> CreateData()
@ -75,4 +76,6 @@ public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider
return data;
}
}
private sealed record ProbeResult(string StoreName, bool Skipped, Exception? Exception);
}

View file

@ -0,0 +1,12 @@
namespace Elsa.Workflows.Runtime.Options;
/// <summary>
/// Configures Elsa workflow runtime readiness health checks.
/// </summary>
public class ElsaReadinessHealthCheckOptions
{
/// <summary>
/// The maximum time the distributed-lock readiness probe waits to acquire its probe lock.
/// </summary>
public TimeSpan DistributedLockAcquisitionTimeout { get; set; } = TimeSpan.FromSeconds(1);
}

View file

@ -1,15 +1,17 @@
using Elsa.Workflows.Runtime.HealthChecks;
using Elsa.Workflows.Runtime.Options;
using Medallion.Threading;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using NSubstitute;
namespace Elsa.Workflows.Runtime.UnitTests.HealthChecks;
public class ElsaDistributedLockHealthCheckTests
{
private static readonly TimeSpan ExpectedLockAcquisitionTimeout = TimeSpan.FromSeconds(1);
private static readonly TimeSpan ExpectedLockAcquisitionTimeout = TimeSpan.FromMilliseconds(250);
private readonly IDistributedLockProvider _distributedLockProvider = Substitute.For<IDistributedLockProvider>();
private readonly IDistributedLock _distributedLock = Substitute.For<IDistributedLock>();
private readonly ElsaDistributedLockHealthCheck _sut;
@ -19,7 +21,7 @@ public class ElsaDistributedLockHealthCheckTests
_distributedLockProvider.CreateLock(Arg.Any<string>()).Returns(_distributedLock);
_distributedLock.TryAcquireAsync(Arg.Any<TimeSpan>(), Arg.Any<CancellationToken>())
.Returns(new ValueTask<IDistributedSynchronizationHandle?>(Substitute.For<IDistributedSynchronizationHandle>()));
_sut = new ElsaDistributedLockHealthCheck(CreateServiceProvider(_distributedLockProvider), NullLogger<ElsaDistributedLockHealthCheck>.Instance);
_sut = new ElsaDistributedLockHealthCheck(CreateServiceProvider(_distributedLockProvider), CreateOptions(), NullLogger<ElsaDistributedLockHealthCheck>.Instance);
}
[Fact]
@ -59,7 +61,7 @@ public class ElsaDistributedLockHealthCheckTests
[Fact]
public async Task ReturnsDegradedWhenProviderIsNotRegistered()
{
var sut = new ElsaDistributedLockHealthCheck(CreateServiceProvider(), NullLogger<ElsaDistributedLockHealthCheck>.Instance);
var sut = new ElsaDistributedLockHealthCheck(CreateServiceProvider(), CreateOptions(), NullLogger<ElsaDistributedLockHealthCheck>.Instance);
var result = await sut.CheckHealthAsync(new HealthCheckContext());
@ -76,4 +78,9 @@ public class ElsaDistributedLockHealthCheckTests
return services.BuildServiceProvider();
}
private static IOptions<ElsaReadinessHealthCheckOptions> CreateOptions() => Microsoft.Extensions.Options.Options.Create(new ElsaReadinessHealthCheckOptions
{
DistributedLockAcquisitionTimeout = ExpectedLockAcquisitionTimeout
});
}

View file

@ -66,4 +66,21 @@ public class ElsaWorkflowPersistenceHealthCheckTests
Assert.Equal("triggers,bookmark-queue", result.Data["probes"]);
Assert.Equal("workflow-definitions,workflow-instances", result.Data["skippedProbes"]);
}
[Fact]
public async Task ReturnsDegradedWithSkippedProbesWhenNoStoresAreRegistered()
{
_serviceProvider.GetService(typeof(IWorkflowDefinitionStore)).Returns((object?)null);
_serviceProvider.GetService(typeof(IWorkflowInstanceStore)).Returns((object?)null);
_serviceProvider.GetService(typeof(ITriggerStore)).Returns((object?)null);
_serviceProvider.GetService(typeof(IBookmarkQueueStore)).Returns((object?)null);
var result = await _sut.CheckHealthAsync(new HealthCheckContext());
Assert.Equal(HealthStatus.Degraded, result.Status);
Assert.Equal("No Elsa workflow persistence stores are registered.", result.Description);
Assert.Equal("persistence", result.Data["category"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers,bookmark-queue", result.Data["skippedProbes"]);
Assert.False(result.Data.ContainsKey("probes"));
}
}