Address health check review feedback

This commit is contained in:
Sipke Schoorstra 2026-05-21 02:13:19 +02:00
parent 33d94ab384
commit b7076fd0cf
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
7 changed files with 145 additions and 72 deletions

View file

@ -27,6 +27,8 @@ services
.AddElsaReadinessChecks(includePersistence: false);
```
Persistence probing stops after the first failed store by default to keep readiness responses bounded during dependency outages. Set `ContinuePersistenceProbesAfterFailure = true` when richer per-store diagnostics are more important than the fastest readiness response.
Map separate endpoints for liveness and readiness:
```csharp

View file

@ -40,6 +40,10 @@ public static class HealthCheckExtensions
if (configureOptions != null)
optionsBuilder.Configure(configureOptions);
optionsBuilder
.Validate(x => x.DistributedLockAcquisitionTimeout > TimeSpan.Zero, "Distributed lock acquisition timeout must be greater than zero.")
.ValidateOnStart();
builder.AddCheck<ElsaRuntimeHealthCheck>("elsa-runtime", tags: ReadinessTags);
if (includePersistence)

View file

@ -24,26 +24,17 @@ public class ElsaDistributedLockHealthCheck(
var distributedLockProvider = serviceProvider.GetService<IDistributedLockProvider>();
if (distributedLockProvider == null)
{
return HealthCheckResult.Degraded("Elsa distributed lock provider is not registered.", data: new Dictionary<string, object>
{
["category"] = "distributed-locks"
});
return HealthCheckResult.Degraded("Elsa distributed lock provider is not registered.", data: CreateData());
}
var lockName = $"elsa-health-check-{Guid.NewGuid():N}";
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>
{
["category"] = "distributed-locks"
});
return HealthCheckResult.Degraded("Elsa distributed lock provider was reachable, but the probe lock was not acquired.", data: CreateData());
}
return HealthCheckResult.Healthy("Elsa distributed lock provider is reachable.", new Dictionary<string, object>
{
["category"] = "distributed-locks"
});
return HealthCheckResult.Healthy("Elsa distributed lock provider is reachable.", CreateData());
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
@ -52,10 +43,12 @@ public class ElsaDistributedLockHealthCheck(
catch (Exception e) when (!e.IsFatal())
{
logger.LogWarning(e, "Elsa distributed lock provider is not reachable.");
return HealthCheckResult.Unhealthy("Elsa distributed lock provider is not reachable.", data: new Dictionary<string, object>
return HealthCheckResult.Unhealthy("Elsa distributed lock provider is not reachable.", data: CreateData());
}
static Dictionary<string, object> CreateData() => new()
{
["category"] = "distributed-locks"
});
}
};
}
}

View file

@ -2,16 +2,21 @@ using Elsa.Common;
using Elsa.Workflows.Management;
using Elsa.Workflows.Management.Filters;
using Elsa.Workflows.Runtime.Filters;
using Elsa.Workflows.Runtime.Options;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
namespace Elsa.Workflows.Runtime.HealthChecks;
/// <summary>
/// Performs small read-only probes against the workflow management and runtime stores.
/// </summary>
public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider, ILogger<ElsaWorkflowPersistenceHealthCheck> logger) : IHealthCheck
public class ElsaWorkflowPersistenceHealthCheck(
IServiceProvider serviceProvider,
IOptions<ElsaReadinessHealthCheckOptions> options,
ILogger<ElsaWorkflowPersistenceHealthCheck> logger) : IHealthCheck
{
private const string ProbeId = "00000000-0000-0000-0000-000000000000";
@ -19,26 +24,43 @@ public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider
public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
{
// These probes verify store reachability only; returned entities and counts are intentionally ignored.
var probeResults = new List<ProbeResult>
{
await ProbeAsync("workflow-definitions", serviceProvider.GetService<IWorkflowDefinitionStore>(), async (store, ct) =>
var probeResults = new List<ProbeResult>();
if (!await AddProbeAsync("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) =>
}))
return CreateResult();
if (!await AddProbeAsync("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) =>
}))
return CreateResult();
if (!await AddProbeAsync("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) =>
}))
return CreateResult();
if (!await AddProbeAsync("bookmark-queue", serviceProvider.GetService<IBookmarkQueueStore>(), async (store, ct) =>
{
await store.FindAsync(new BookmarkQueueFilter { Id = ProbeId }, ct);
})
};
}))
return CreateResult();
return CreateResult();
async Task<bool> AddProbeAsync<TStore>(string storeName, TStore? store, Func<TStore, CancellationToken, Task> probe) where TStore : class
{
var result = await ProbeAsync(storeName, store, probe);
probeResults.Add(result);
return result.Exception == null || options.Value.ContinuePersistenceProbesAfterFailure;
}
HealthCheckResult CreateResult()
{
var attemptedProbes = probeResults.Where(x => !x.Skipped).Select(x => x.StoreName).ToList();
var successfulProbes = probeResults.Where(x => !x.Skipped && x.Exception == null).Select(x => x.StoreName).ToList();
var skippedProbes = probeResults.Where(x => x.Skipped).Select(x => x.StoreName).ToList();
@ -59,6 +81,29 @@ public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider
? HealthCheckResult.Degraded("No Elsa workflow persistence stores are registered.", data: healthyData)
: HealthCheckResult.Healthy("Elsa workflow stores are reachable.", healthyData);
Dictionary<string, object> CreateData()
{
var data = new Dictionary<string, object>
{
["category"] = "persistence"
};
if (successfulProbes.Count > 0)
data["successfulProbes"] = string.Join(",", successfulProbes);
if (attemptedProbes.Count > 0)
data["attemptedProbes"] = string.Join(",", attemptedProbes);
if (failedProbes.Count > 0)
data["failedProbes"] = string.Join(",", failedProbes);
if (skippedProbes.Count > 0)
data["skippedProbes"] = string.Join(",", skippedProbes);
return data;
}
}
async Task<ProbeResult> ProbeAsync<TStore>(string storeName, TStore? store, Func<TStore, CancellationToken, Task> probe) where TStore : class
{
if (store == null)
@ -80,28 +125,6 @@ public class ElsaWorkflowPersistenceHealthCheck(IServiceProvider serviceProvider
return new ProbeResult(storeName, false, e);
}
}
Dictionary<string, object> CreateData()
{
var data = new Dictionary<string, object>
{
["category"] = "persistence"
};
if (successfulProbes.Count > 0)
data["probes"] = string.Join(",", successfulProbes);
if (attemptedProbes.Count > 0)
data["attemptedProbes"] = string.Join(",", attemptedProbes);
if (failedProbes.Count > 0)
data["failedProbes"] = string.Join(",", failedProbes);
if (skippedProbes.Count > 0)
data["skippedProbes"] = string.Join(",", skippedProbes);
return data;
}
}
private sealed record ProbeResult(string StoreName, bool Skipped, Exception? Exception);

View file

@ -9,4 +9,9 @@ public class ElsaReadinessHealthCheckOptions
/// The maximum time the distributed-lock readiness probe waits to acquire its probe lock.
/// </summary>
public TimeSpan DistributedLockAcquisitionTimeout { get; set; } = TimeSpan.FromSeconds(1);
/// <summary>
/// Whether persistence readiness probing should continue after the first failed store probe.
/// </summary>
public bool ContinuePersistenceProbesAfterFailure { get; set; }
}

View file

@ -2,6 +2,7 @@ using Elsa.Workflows.Management;
using Elsa.Workflows.Management.Filters;
using Elsa.Workflows.Runtime.Filters;
using Elsa.Workflows.Runtime.HealthChecks;
using Elsa.Workflows.Runtime.Options;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Logging.Abstractions;
using NSubstitute;
@ -27,7 +28,7 @@ public class ElsaWorkflowPersistenceHealthCheckTests
_serviceProvider.GetService(typeof(IWorkflowInstanceStore)).Returns(_workflowInstanceStore);
_serviceProvider.GetService(typeof(ITriggerStore)).Returns(_triggerStore);
_serviceProvider.GetService(typeof(IBookmarkQueueStore)).Returns(_bookmarkQueueStore);
_sut = new ElsaWorkflowPersistenceHealthCheck(_serviceProvider, NullLogger<ElsaWorkflowPersistenceHealthCheck>.Instance);
_sut = CreateSut();
}
[Fact]
@ -37,7 +38,7 @@ public class ElsaWorkflowPersistenceHealthCheckTests
Assert.Equal(HealthStatus.Healthy, result.Status);
Assert.Equal("persistence", result.Data["category"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers,bookmark-queue", result.Data["probes"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers,bookmark-queue", result.Data["successfulProbes"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers,bookmark-queue", result.Data["attemptedProbes"]);
await _workflowDefinitionStore.Received(1).FindAsync(
Arg.Is<WorkflowDefinitionFilter>(x => x.Id == "00000000-0000-0000-0000-000000000000"),
@ -64,7 +65,7 @@ public class ElsaWorkflowPersistenceHealthCheckTests
}
[Fact]
public async Task ReturnsUnhealthyWithFailedStoreWhenAStoreProbeFails()
public async Task ReturnsUnhealthyWithFailedStoreAndStopsProbingWhenAStoreProbeFails()
{
_triggerStore.FindAsync(Arg.Any<TriggerFilter>(), Arg.Any<CancellationToken>()).Returns<ValueTask<Elsa.Workflows.Runtime.Entities.StoredTrigger?>>(_ => throw new InvalidOperationException("store unavailable"));
@ -75,9 +76,25 @@ public class ElsaWorkflowPersistenceHealthCheckTests
Assert.Equal("persistence", result.Data["category"]);
Assert.Equal("triggers", result.Data["failedStore"]);
Assert.Equal("triggers", result.Data["failedProbe"]);
Assert.Equal("workflow-definitions,workflow-instances,bookmark-queue", result.Data["probes"]);
Assert.Equal("workflow-definitions,workflow-instances", result.Data["successfulProbes"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers", result.Data["attemptedProbes"]);
Assert.Equal("triggers", result.Data["failedProbes"]);
await _bookmarkQueueStore.DidNotReceive().FindAsync(Arg.Any<BookmarkQueueFilter>(), Arg.Any<CancellationToken>());
}
[Fact]
public async Task ReturnsUnhealthyWithAllProbeDataWhenContinuationIsEnabled()
{
var sut = CreateSut(continueAfterFailure: true);
_triggerStore.FindAsync(Arg.Any<TriggerFilter>(), Arg.Any<CancellationToken>()).Returns<ValueTask<Elsa.Workflows.Runtime.Entities.StoredTrigger?>>(_ => throw new InvalidOperationException("store unavailable"));
var result = await sut.CheckHealthAsync(new HealthCheckContext());
Assert.Equal(HealthStatus.Unhealthy, result.Status);
Assert.Equal("workflow-definitions,workflow-instances,bookmark-queue", result.Data["successfulProbes"]);
Assert.Equal("workflow-definitions,workflow-instances,triggers,bookmark-queue", result.Data["attemptedProbes"]);
Assert.Equal("triggers", result.Data["failedProbes"]);
await _bookmarkQueueStore.Received(1).FindAsync(Arg.Any<BookmarkQueueFilter>(), Arg.Any<CancellationToken>());
}
[Fact]
@ -90,7 +107,7 @@ public class ElsaWorkflowPersistenceHealthCheckTests
Assert.Equal(HealthStatus.Healthy, result.Status);
Assert.Equal("persistence", result.Data["category"]);
Assert.Equal("triggers,bookmark-queue", result.Data["probes"]);
Assert.Equal("triggers,bookmark-queue", result.Data["successfulProbes"]);
Assert.Equal("triggers,bookmark-queue", result.Data["attemptedProbes"]);
Assert.Equal("workflow-definitions,workflow-instances", result.Data["skippedProbes"]);
}
@ -109,7 +126,17 @@ public class ElsaWorkflowPersistenceHealthCheckTests
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"));
Assert.False(result.Data.ContainsKey("successfulProbes"));
}
private ElsaWorkflowPersistenceHealthCheck CreateSut(bool continueAfterFailure = false)
{
var options = Microsoft.Extensions.Options.Options.Create(new ElsaReadinessHealthCheckOptions
{
ContinuePersistenceProbesAfterFailure = continueAfterFailure
});
return new ElsaWorkflowPersistenceHealthCheck(_serviceProvider, options, NullLogger<ElsaWorkflowPersistenceHealthCheck>.Instance);
}
private static async Task<T> TrackProbeAsync<T>(ProbeConcurrencyTracker tracker, T result)

View file

@ -22,6 +22,25 @@ public class HealthCheckExtensionsTests
Assert.NotNull(serviceProvider.GetRequiredService<IOptions<ElsaReadinessHealthCheckOptions>>());
}
[Theory]
[InlineData(0)]
[InlineData(-1)]
public void AddElsaReadinessChecksRejectsNonPositiveDistributedLockTimeout(int timeoutMilliseconds)
{
var services = new ServiceCollection();
services
.AddHealthChecks()
.AddElsaReadinessChecks(
includePersistence: false,
includeDistributedLocks: true,
configureOptions: options => options.DistributedLockAcquisitionTimeout = TimeSpan.FromMilliseconds(timeoutMilliseconds));
using var serviceProvider = services.BuildServiceProvider();
Assert.Throws<OptionsValidationException>(() => serviceProvider.GetRequiredService<IOptions<ElsaReadinessHealthCheckOptions>>().Value);
}
[Fact]
public void AddElsaReadinessChecksUsesElsaSpecificReadinessTag()
{