Skip to content

Conversation

@Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Sep 30, 2025

Changes

Counterpart to akkadotnet/akka.net#7842

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

/// <summary>
/// Used to help build journal configurations
/// </summary>
public sealed class AkkaPersistenceJournalBuilder
Copy link
Member Author

Choose a reason for hiding this comment

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

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
Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

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,
Copy link
Member Author

Choose a reason for hiding this comment

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

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);
Copy link
Member Author

Choose a reason for hiding this comment

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

Add the health checks to the AkkaConfigurationBuilder if they've been defined.

/// .WithHealthCheck());
/// </code>
/// </example>
public static AkkaConfigurationBuilder WithJournalAndSnapshot(
Copy link
Member Author

Choose a reason for hiding this comment

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

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
Copy link
Member Author

Choose a reason for hiding this comment

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

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
Copy link
Member Author

Choose a reason for hiding this comment

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

End to end integration test - also asserts that the health checks on each plugin work.


namespace Akka.Persistence.Hosting;

internal static class HealthCheckExt
Copy link
Member Author

Choose a reason for hiding this comment

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

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);
Copy link
Member Author

Choose a reason for hiding this comment

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

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);
Copy link
Member Author

Choose a reason for hiding this comment

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

Does the same as above, but for snapshots.

{
var pluginId = $"akka.persistence.snapshot-store.{SnapshotStoreId}";
var registration = new AkkaHealthCheckRegistration(
name ?? $"Akka.Persistence.SnapshotStore.{SnapshotStoreId}",
Copy link
Member Author

Choose a reason for hiding this comment

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

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>
Copy link
Member Author

Choose a reason for hiding this comment

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

Upgraded to Akka.NET v1.5.51 in order to get access to akkadotnet/akka.net#7842

@Aaronontheweb Aaronontheweb marked this pull request as ready for review October 1, 2025 19:04
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit 6cf713e into akkadotnet:dev Oct 1, 2025
2 checks passed
@Aaronontheweb Aaronontheweb deleted the persistence-healthchecks branch October 1, 2025 19:21
@Aaronontheweb Aaronontheweb mentioned this pull request Oct 1, 2025
Aaronontheweb added a commit to Aaronontheweb/Akka.Hosting that referenced this pull request Oct 2, 2025
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.
This was referenced Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants