Adding Akka.Persistence health checks - #662
Conversation
| /// <summary> | ||
| /// Used to help build journal configurations | ||
| /// </summary> | ||
| public sealed class AkkaPersistenceJournalBuilder |
There was a problem hiding this comment.
I am going to make some significant structural changes to how Akka.Persistence.Hosting works. There's two competing paradigms (builder vs. options) when they should really be one and the same. I think this should be largely a superficial change.
Merged declarative (JournalOptions/SnapshotOptions) and fluent
(builder actions) configuration patterns into a single coherent API.
Changes:
- Added AkkaPersistenceSnapshotBuilder with health check support
- Added new overloads: WithJournal(options, builder),
WithSnapshot(options, builder), and
WithJournalAndSnapshot(options, options, builder, builder)
- Deprecated WithJournal(string, builder) in favor of unified approach
- Updated EventAdapterSpecs to demonstrate new unified API pattern
- Added comprehensive test coverage in UnifiedApiSpecs.cs
Benefits:
- Single canonical way to configure persistence plugins
- Plugin properties and event adapters/health checks in one call
- Binary compatible - all existing APIs continue to work
- Follows extend-only design principles per API compatibility guidelines
- Ready for Akka.NET persistence health check integration
API Example:
builder.WithJournal(
new SqlServerJournalOptions { ConnectionString = "..." },
journal => journal
.AddWriteEventAdapter<MyAdapter>("adapter", types)
.WithHealthCheck());
…de duplication
- Fixed health check plugin IDs to use full paths (akka.persistence.journal.{id}) instead of just identifiers
- Corrected test resource InternalDefaultConfig to avoid double-nesting of configuration paths
- Added comprehensive health check verification to unified API tests
- Refactored WithJournal/WithSnapshot/WithJournalAndSnapshot methods to use method chaining, establishing single source of truth for each operation
- All 15 persistence hosting tests now pass with health checks returning accurate results
…theweb/Akka.Hosting into persistence-healthchecks
Aaronontheweb
left a comment
There was a problem hiding this comment.
Detailed my changes
| /// or <see cref="PersistenceHealthStatus.Degraded"/>. Defaults to degraded.</param> | ||
| /// <param name="name">Optional name to add to the health check.</param> | ||
| /// <returns></returns> | ||
| public AkkaPersistenceJournalBuilder WithHealthCheck(HealthStatus unHealthyStatus = HealthStatus.Degraded, |
There was a problem hiding this comment.
Registers the Akka.Persistence health check definition is inside the AsyncWriteJournal - by default it just looks as the state of the CircuitBreaker, which is a "good enough" heuristic typically.
|
|
||
| // add the health checks if specified | ||
| if(HealthCheckRegistration != null) | ||
| Builder.WithHealthCheck(HealthCheckRegistration); |
There was a problem hiding this comment.
Add the health checks to the AkkaConfigurationBuilder if they've been defined.
| /// .WithHealthCheck()); | ||
| /// </code> | ||
| /// </example> | ||
| public static AkkaConfigurationBuilder WithJournalAndSnapshot( |
There was a problem hiding this comment.
DX improvement: the JournalOptions and JournalBuilder should be part of the same "configuration" - the way it's defined in v1.5.50 right now is as two separate islands of configuration that reference the same thing via a shared pluginId.
We should, instead, make it clear to end-users that both of these components work against the same "thing".
We have also followed this pattern for the snapshot store too.
| /// <summary> | ||
| /// Shared test resources for unified API specs | ||
| /// </summary> | ||
| public static class UnifiedApiTestResources |
There was a problem hiding this comment.
Tests the "unified" Akka.Persistence.Hosting APIs and tries to get test coverage over the PersistenceHostingExtensions
| /// <summary> | ||
| /// Test WithJournalAndSnapshot with builder actions | ||
| /// </summary> | ||
| public sealed class JournalAndSnapshotWithBuildersSpec : Akka.Hosting.TestKit.TestKit |
There was a problem hiding this comment.
End to end integration test - also asserts that the health checks on each plugin work.
|
|
||
| namespace Akka.Persistence.Hosting; | ||
|
|
||
| internal static class HealthCheckExt |
There was a problem hiding this comment.
Maps the Akka.Persistence health check data structures into Microsoft.Extensions.HealthChecks data structures
| public async Task<HealthCheckResult> CheckHealthAsync(AkkaHealthCheckContext context, CancellationToken cancellationToken = default) | ||
| { | ||
| var persistence = Persistence.Instance.Apply(context.ActorSystem); | ||
| var journalResult = await persistence.CheckJournalHealthAsync(_journalPluginId, cancellationToken); |
There was a problem hiding this comment.
Runs the health checks we added in akkadotnet/akka.net#7842 and pipes them through the MSFT.EXT.HealthCheck functionality
| public async Task<HealthCheckResult> CheckHealthAsync(AkkaHealthCheckContext context, CancellationToken cancellationToken = default) | ||
| { | ||
| var persistence = Persistence.Instance.Apply(context.ActorSystem); | ||
| var ssResult = await persistence.CheckSnapshotStoreHealthAsync(_snapshotStorePluginId, cancellationToken); |
There was a problem hiding this comment.
Does the same as above, but for snapshots.
| { | ||
| var pluginId = $"akka.persistence.snapshot-store.{SnapshotStoreId}"; | ||
| var registration = new AkkaHealthCheckRegistration( | ||
| name ?? $"Akka.Persistence.SnapshotStore.{SnapshotStoreId}", |
There was a problem hiding this comment.
default health check name if left blank.
| <XunitRunneVisualstudio>3.1.5</XunitRunneVisualstudio> | ||
| <AkkaVersion>1.5.50</AkkaVersion> | ||
| <XunitRunneVisualstudio>3.1.5</XunitRunneVisualstudio> | ||
| <AkkaVersion>1.5.51</AkkaVersion> |
There was a problem hiding this comment.
Upgraded to Akka.NET v1.5.51 in order to get access to akkadotnet/akka.net#7842
Fixes akkadotnet#665 The Adapters property created confusion by providing two competing patterns for configuring event adapters. This deprecates the property in favor of the unified callback pattern introduced in PR akkadotnet#662. Changes: - Mark JournalOptions.Adapters as [Obsolete] with clear migration message - Remove internal usage of the property (Adapters.AppendAdapters call) - Add regression test DeprecatedAdaptersPropertySpec that verifies: * Deprecated property is ignored and does not configure adapters * Callback pattern continues to work correctly * Uses #pragma warning disable to test deprecated API Benefits: - Single, consistent pattern for configuring adapters and health checks - Cleaner API surface with options containing only configuration data - Clear migration path with deprecation warning Migration: Before (deprecated): var options = new JournalOptions(); options.Adapters.AddWriteEventAdapter<Foo>(...); builder.WithJournal(options); After (recommended): var options = new JournalOptions(); builder.WithJournal(options, journal => journal.AddWriteEventAdapter<Foo>(...)); All 17 tests pass.
Changes
Counterpart to akkadotnet/akka.net#7842
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):