Skip to content

Conversation

@Hnogared
Copy link
Contributor

@Hnogared Hnogared commented Nov 9, 2025

Add keyed services support for the Dapr health check.

Summary by CodeRabbit

  • New Features

    • Health checks can target a named/keyed Dapr client instance, with automatic fallback to the default when no key is provided.
  • Documentation

    • Added XML docs describing the keyed-service option and clarified README wording and examples for configuration-based and builder-based setup.
  • Tests

    • Added unit tests verifying keyed Dapr client resolution and that a keyed client’s healthy result takes precedence over a failing default.

@Hnogared Hnogared self-assigned this Nov 9, 2025
@Hnogared Hnogared requested a review from a team as a code owner November 9, 2025 15:42
@Hnogared Hnogared requested review from Spacemonkay and removed request for a team November 9, 2025 15:42
@Hnogared Hnogared added the type:chore Indicates some housework that needs to be done. label Nov 9, 2025
@Hnogared Hnogared linked an issue Nov 9, 2025 that may be closed by this pull request
6 tasks
@Hnogared Hnogared changed the title chore(Dapr): Add keyed services support WIP chore(Dapr): Add keyed services support Nov 9, 2025
@Hnogared Hnogared changed the title WIP chore(Dapr): Add keyed services support WIP chore(Dapr): Add keyed services support for health check Nov 9, 2025
@samtrion samtrion force-pushed the chore/dapr-keyed-services branch from 8ed9534 to 4e9cdc5 Compare November 9, 2025 21:10
@codecov
Copy link

codecov bot commented Nov 9, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.30%. Comparing base (4bbf124) to head (ef3d4c8).

Files with missing lines Patch % Lines
src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs 66.66% 0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (75.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1012      +/-   ##
==========================================
- Coverage   95.32%   95.30%   -0.03%     
==========================================
  Files         203      203              
  Lines        3487     3490       +3     
  Branches      596      597       +1     
==========================================
+ Hits         3324     3326       +2     
  Misses        116      116              
- Partials       47       48       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from 4e9cdc5 to ef3d4c8 Compare November 10, 2025 20:01
@Hnogared Hnogared changed the title WIP chore(Dapr): Add keyed services support for health check chore(Dapr): Add keyed services support for health check Nov 10, 2025
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from ef3d4c8 to 3c2592c Compare November 14, 2025 19:32
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Walkthrough

Dapr health check now resolves the DaprClient using a keyed service when DaprOptions.KeyedService is provided (non-empty); otherwise it falls back to the default DaprClient from the service provider. Added the KeyedService option, tests for keyed resolution, and README wording/examples updates.

Changes

Cohort / File(s) Summary
Options
src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs
Added public string? KeyedService { get; set; } with XML documentation explaining keyed resolution behavior; no other public API changes.
Health check implementation
src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs
Modified ExecuteHealthCheckAsync to select DaprClient via GetRequiredKeyedService(options.KeyedService) when options.KeyedService is non-empty/whitespace; otherwise uses GetRequiredService<DaprClient>().
Unit tests
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs
Added tests CheckHealthAsync_WithKeyedService_ShouldUseKeyedService and CheckHealthAsync_WithKeyedService_PrefersKeyedOverDefault to verify keyed resolution and precedence over default client.
Documentation
src/NetEvolve.HealthChecks.Dapr/README.md
Editorial updates: hyphenation and clearer "configuration-based" vs "builder-based" variants; added example snippets and configuration examples; no behavior changes.

Sequence Diagram(s)

sequenceDiagram
    participant HC as DaprHealthCheck
    participant SP as IServiceProvider
    participant DC as DaprClient

    HC->>SP: Read DaprOptions (options)
    alt options.KeyedService provided (non-empty)
        HC->>SP: GetRequiredKeyedService(key)
        SP-->>DC: keyed DaprClient
    else no key provided
        HC->>SP: GetRequiredService<DaprClient>()
        SP-->>DC: default DaprClient
    end
    HC->>DC: perform probe (with timeout/cancellation)
    DC-->>HC: probe result / exception
    HC->>HC: build HealthCheckResult (healthy/unhealthy)
    note right of HC `#EEF6F6`: Keyed resolution used when key present
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify GetRequiredKeyedService extension/method is available and correctly imported.
  • Confirm trimming/null/whitespace handling for KeyedService.
  • Review new unit tests for proper registration and assertions.

Poem

🐰 I found a key beneath a log,
a client hopped to greet the bog.
I ping, I wait, a healthy cheer,
keyed friend chosen when it’s near,
carrots clapped — the check is clear.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding keyed services support for the Dapr health check, which is the primary feature across all modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/dapr-keyed-services

Comment @coderabbitai help to get the list of available commands and usage tips.

@Hnogared Hnogared added the state:ready for merge Indicates that a pull request has been reviewed and approved, and is ready to be merged into the mai label Nov 14, 2025
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from 90fe3ac to b36f229 Compare November 14, 2025 20:13
@Hnogared
Copy link
Contributor Author

@CodeRabbit generate docstrings

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #1101

coderabbitai bot added a commit that referenced this pull request Nov 14, 2025
Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1)

194-226: Good test coverage for keyed service path.

The test properly validates that a keyed DaprClient is resolved and used when KeyedService is specified. The test setup is clean and assertions are appropriate.

Consider adding an additional test case to validate that when both a default and keyed service are registered, the correct one is selected:

[Test]
public async Task CheckHealthAsync_WithKeyedService_PrefersKeyedOverDefault()
{
    // Arrange
    const string testName = "Test";
    const string serviceKey = "test-key";
    
    var options = new DaprOptions { KeyedService = serviceKey, Timeout = 100 };
    
    var optionsMonitor = Substitute.For<IOptionsMonitor<DaprOptions>>();
    _ = optionsMonitor.Get(testName).Returns(options);
    
    var defaultClient = Substitute.For<DaprClient>();
    _ = defaultClient.CheckHealthAsync(Arg.Any<CancellationToken>()).Returns(false); // Returns unhealthy
    
    var keyedClient = Substitute.For<DaprClient>();
    _ = keyedClient.CheckHealthAsync(Arg.Any<CancellationToken>()).Returns(true); // Returns healthy
    
    var serviceProvider = new ServiceCollection()
        .AddSingleton(defaultClient)
        .AddKeyedSingleton(serviceKey, keyedClient)
        .BuildServiceProvider();
    
    var healthCheck = new DaprHealthCheck(serviceProvider, optionsMonitor);
    var context = new HealthCheckContext
    {
        Registration = new HealthCheckRegistration(testName, healthCheck, HealthStatus.Unhealthy, null),
    };
    
    // Act
    var result = await healthCheck.CheckHealthAsync(context, CancellationToken.None);
    
    // Assert - Should be Healthy (from keyed service), not Degraded (from default)
    using (Assert.Multiple())
    {
        _ = await Assert.That(result.Status).IsEqualTo(HealthStatus.Healthy);
        _ = await Assert.That(result.Description).IsEqualTo($"{testName}: Healthy");
    }
}

This would more explicitly verify that the keyed service takes precedence when configured.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between e7c4b53 and b36f229.

📒 Files selected for processing (3)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (1 hunks)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)
🔇 Additional comments (2)
src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1)

10-17: LGTM! Well-documented property addition.

The KeyedService property is properly typed as nullable and includes clear XML documentation explaining the resolution behavior. The documentation accurately describes when GetRequiredKeyedService vs. GetRequiredService will be used.

src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (1)

23-25: LGTM! Clean implementation of keyed service resolution.

The conditional logic correctly resolves DaprClient via keyed services when KeyedService is specified, falling back to the default service resolution when not. The use of IsNullOrWhiteSpace appropriately handles null, empty, or whitespace-only values. The project targets .NET 8.0 and 9.0, both of which support keyed services, so the implementation is framework-compatible.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (2)

194-226: Good test coverage for the keyed service happy path.

The test correctly verifies that a keyed DaprClient is resolved and used when DaprOptions.KeyedService is configured. The mock setup and assertions are appropriate.

Consider adding edge case tests for:

  • KeyedService set but the keyed service is not registered (to verify proper error handling)
  • KeyedService with whitespace or empty string values

228-266: Excellent precedence verification.

This test effectively validates that the keyed DaprClient takes priority over the default instance when both are registered. The differentiation via return values (false for default, true for keyed) clearly demonstrates which client was resolved.

The setup code is nearly identical to the previous test. Consider extracting a helper method to reduce duplication:

private static (DaprHealthCheck, HealthCheckContext) CreateHealthCheckWithKeyedService(
    string testName,
    string serviceKey,
    DaprClient keyedClient,
    DaprClient? defaultClient = null)
{
    var options = new DaprOptions { KeyedService = serviceKey, Timeout = 100 };
    var optionsMonitor = Substitute.For<IOptionsMonitor<DaprOptions>>();
    _ = optionsMonitor.Get(testName).Returns(options);

    var services = new ServiceCollection();
    if (defaultClient is not null)
    {
        _ = services.AddSingleton(defaultClient);
    }
    _ = services.AddKeyedSingleton(serviceKey, keyedClient);
    var serviceProvider = services.BuildServiceProvider();

    var healthCheck = new DaprHealthCheck(serviceProvider, optionsMonitor);
    var context = new HealthCheckContext
    {
        Registration = new HealthCheckRegistration(testName, healthCheck, HealthStatus.Unhealthy, null),
    };

    return (healthCheck, context);
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between b36f229 and 62ab529.

📒 Files selected for processing (1)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)

@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from 62ab529 to a6f40a6 Compare November 14, 2025 21:28
Hnogared added a commit that referenced this pull request Nov 14, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (1)

14-19: Consider completing the XML documentation.

The documentation is helpful, but could be improved:

  • The <returns> tag describes HealthCheckResult, but the method returns ValueTask<HealthCheckResult>.
  • The cancellationToken parameter is not documented.

Apply this diff to enhance the documentation:

 /// <summary>
 /// Performs the Dapr health probe and produces a HealthCheckResult for the specified check name.
 /// </summary>
 /// <param name="name">The health check name used when constructing the result.</param>
 /// <param name="options">DaprOptions that configure the probe (e.g., Timeout and KeyedService).</param>
-/// <returns>`HealthCheckResult` representing a healthy state only if the Dapr client reports healthy and the probe result is true; otherwise an unhealthy result for the given name.</returns>
+/// <param name="cancellationToken">The cancellation token to observe during the health check operation.</param>
+/// <returns>A ValueTask containing a HealthCheckResult representing a healthy state only if the Dapr client reports healthy and the probe result is true; otherwise an unhealthy result for the given name.</returns>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 62ab529 and e485d39.

📒 Files selected for processing (3)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (4 hunks)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs
🔇 Additional comments (3)
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1)

194-266: LGTM! Excellent test coverage for keyed service behavior.

Both test methods effectively verify the new keyed service functionality:

  • CheckHealthAsync_WithKeyedService_ShouldUseKeyedService confirms that keyed resolution works when only a keyed service is registered.
  • CheckHealthAsync_WithKeyedService_PrefersKeyedOverDefault validates that keyed services take precedence over default services when both are available.

The test setup, mocking, and assertions are appropriate and thorough.

src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (2)

29-31: LGTM! Keyed service resolution logic is correct.

The conditional resolution appropriately checks for a configured keyed service name and falls back to the default service when not specified. The use of string.IsNullOrWhiteSpace properly handles null, empty, and whitespace-only values.


33-38: LGTM! Health check execution is properly implemented.

The health check correctly:

  • Invokes the Dapr client's health endpoint with the provided cancellation token
  • Applies the configured timeout
  • Uses ConfigureAwait(false) for optimal async behavior
  • Combines both the timeout success flag and the health result in the final determination

Hnogared added a commit that referenced this pull request Nov 14, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from e485d39 to 049c0e6 Compare November 14, 2025 22:20
@Hnogared Hnogared enabled auto-merge (squash) November 14, 2025 22:21
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between e485d39 and 049c0e6.

📒 Files selected for processing (3)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (4 hunks)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs
🔇 Additional comments (3)
src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (3)

20-27: Verify the intended use of the failureStatus parameter.

The failureStatus parameter is marked as unused (line 23) but is part of the method signature. At line 38, the return statement calls HealthCheckState(isHealthy && result, name) without using failureStatus. Please confirm whether:

  1. This parameter is required by the ConfigurableHealthCheck source generator but intentionally unused in this implementation, or
  2. The failureStatus should be passed to HealthCheckState to properly report failure status.

If failureStatus should be used, consider applying a diff similar to:

-return HealthCheckState(isHealthy && result, name);
+return HealthCheckState(isHealthy && result, name, failureStatus);

29-31: LGTM: Keyed service resolution logic is correct.

The conditional logic properly resolves DaprClient as a keyed service when KeyedService is specified, falling back to the default service otherwise. This implementation aligns with the PR objectives.


33-38: LGTM: Health check invocation and timeout handling are correct.

The health check properly:

  • Invokes CheckHealthAsync with the cancellation token
  • Applies timeout via WithTimeoutAsync using the configured timeout value
  • Uses ConfigureAwait(false) for optimal library performance
  • Combines the health status and result correctly in the return statement

Hnogared added a commit that referenced this pull request Nov 16, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from 049c0e6 to ea1ff0b Compare November 16, 2025 15:43
Hnogared added a commit that referenced this pull request Nov 16, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from ea1ff0b to bdb2f84 Compare November 16, 2025 15:46
Hnogared added a commit that referenced this pull request Nov 16, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from bcab984 to 1ecd456 Compare November 16, 2025 16:46
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1)

194-266: Consider adding edge case tests for completeness.

While the current tests cover the primary scenarios well, consider adding tests for the following edge cases to ensure robust behavior:

  1. Empty string KeyedService: Verify that an empty string is treated the same as null (falls back to default service)
  2. Missing keyed service: Verify behavior when KeyedService is set to a key that doesn't exist in the service provider (should throw InvalidOperationException from GetRequiredKeyedService)
  3. Whitespace-only KeyedService: Clarify whether whitespace should be treated as a valid key or handled like empty/null

Example test skeleton for empty string handling:

[Test]
public async Task CheckHealthAsync_WithEmptyKeyedService_ShouldUseDefaultService()
{
    // Arrange
    const string testName = "Test";
    var options = new DaprOptions { KeyedService = string.Empty, Timeout = 100 };
    
    var optionsMonitor = Substitute.For<IOptionsMonitor<DaprOptions>>();
    _ = optionsMonitor.Get(testName).Returns(options);
    
    var mockClient = Substitute.For<DaprClient>();
    _ = mockClient.CheckHealthAsync(Arg.Any<CancellationToken>()).Returns(true);
    
    var serviceProvider = new ServiceCollection()
        .AddSingleton(mockClient)
        .BuildServiceProvider();
    
    var healthCheck = new DaprHealthCheck(serviceProvider, optionsMonitor);
    var context = new HealthCheckContext
    {
        Registration = new HealthCheckRegistration(testName, healthCheck, HealthStatus.Unhealthy, null),
    };
    
    // Act
    var result = await healthCheck.CheckHealthAsync(context, CancellationToken.None);
    
    // Assert
    _ = await Assert.That(result.Status).IsEqualTo(HealthStatus.Healthy);
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between bcab984 and 1ecd456.

📒 Files selected for processing (3)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (2 hunks)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs
🔇 Additional comments (2)
tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (2)

194-226: LGTM! Solid test for keyed service usage.

The test correctly verifies that when KeyedService is specified, the health check resolves and uses the keyed DaprClient. The test structure follows the AAA pattern and includes appropriate assertions.


228-266: LGTM! Excellent test for keyed service precedence.

This test effectively demonstrates that the keyed service takes precedence over the default service when KeyedService is specified. The scenario with a failing default client and healthy keyed client clearly validates the expected behavior.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1ecd456 and a500d85.

📒 Files selected for processing (2)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • src/NetEvolve.HealthChecks.Dapr/README.md (2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build & Tests
src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs

[error] 10-10: csharpier check failed. File is not formatted. Expected around line 10: '///

Gets or sets the key used to resolve the DaprClient from the service provider.'

🪛 LanguageTool
src/NetEvolve.HealthChecks.Dapr/README.md

[style] ~53-~53: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ch. This approach is recommended if you want to configure the health check programmatic...

(REP_WANT_TO_VB)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
src/NetEvolve.HealthChecks.Dapr/README.md (1)

29-29: LGTM! Documentation improvements enhance clarity.

The editorial changes improve consistency and readability:

  • Properly hyphenating compound adjectives ("configuration-based", "builder-based")
  • Clearer explanatory text for each variant

Also applies to: 32-33, 52-53

Hnogared added a commit that referenced this pull request Nov 16, 2025
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Hnogared Hnogared force-pushed the chore/dapr-keyed-services branch from 0e10a96 to 907c334 Compare November 16, 2025 17:58
Hnogared and others added 7 commits November 17, 2025 21:41
* chore(Dapr): Add keyed services support

* chore(Dapr): Add unit test for keyed service retrieval

* 📝 Add docstrings to `chore/dapr-keyed-services`

Docstrings generation was requested by @Hnogared.

* #1012 (comment)

The following files were modified:

* `src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs`

---------

Signed-off-by: Hnogared <[email protected]>
Co-authored-by: Hnogared <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@samtrion samtrion force-pushed the chore/dapr-keyed-services branch from 907c334 to 41ee22b Compare November 17, 2025 20:41
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/NetEvolve.HealthChecks.Dapr/README.md (2)

41-50: Add KeyedService to the configuration example.

The JSON configuration example should include the new KeyedService property to help users understand how to configure keyed service resolution.

Apply this diff:

 {
   ..., // other configuration
   "HealthChecks": {
     "DaprSidecar": {
+      "KeyedService": "<service-key>", // optional, resolves DaprClient using keyed service when specified
       "Timeout": "<timeout>" // optional, default is 100 milliseconds
     }
   }
 }

57-61: Add KeyedService to the builder example.

The builder-based example should demonstrate the new KeyedService property, as it's the primary feature added in this PR.

Apply this diff:

 builder.AddDapr(options =>
 {
+    options.KeyedService = "<service-key>"; // optional, resolves DaprClient using keyed service when specified
     options.Timeout = "<timeout>"; // optional, default is 100 milliseconds
 });
🧹 Nitpick comments (1)
src/NetEvolve.HealthChecks.Dapr/README.md (1)

52-53: Consider rewording to reduce repetition.

The phrasing "if you want to configure" is repeated from line 33. While clear, varying the sentence structure would improve readability.

Consider:

-The second approach is to use the builder-based approach. This approach is recommended if you want to configure the health check programmatically with dynamic values.
+The second approach is to use the builder-based approach. This approach is recommended for programmatic configuration with dynamic values.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between a500d85 and 41ee22b.

📒 Files selected for processing (4)
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs (2 hunks)
  • src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1 hunks)
  • src/NetEvolve.HealthChecks.Dapr/README.md (2 hunks)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/NetEvolve.HealthChecks.Tests.Unit/Dapr/DaprHealthCheckTests.cs
  • src/NetEvolve.HealthChecks.Dapr/DaprHealthCheck.cs
🧰 Additional context used
🪛 LanguageTool
src/NetEvolve.HealthChecks.Dapr/README.md

[style] ~53-~53: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ch. This approach is recommended if you want to configure the health check programmatic...

(REP_WANT_TO_VB)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: TestGroup 'Z05TestGroup' / Testing .NET solution
  • GitHub Check: TestGroup 'Z03TestGroup' / Testing .NET solution
  • GitHub Check: TestGroup 'Z04TestGroup' / Testing .NET solution
  • GitHub Check: TestGroup 'Z01TestGroup' / Testing .NET solution
  • GitHub Check: TestGroup 'Z02TestGroup' / Testing .NET solution
  • GitHub Check: TestGroup 'Z00TestGroup' / Testing .NET solution
  • GitHub Check: Analyze (csharp)
  • GitHub Check: submit-nuget
🔇 Additional comments (1)
src/NetEvolve.HealthChecks.Dapr/DaprOptions.cs (1)

10-17: LGTM! Property addition is well-documented.

The KeyedService property is properly defined with clear XML documentation. The remarks accurately describe the conditional resolution behavior (using keyed service when specified, falling back to default service when null/empty/whitespace). Previous formatting and documentation concerns have been addressed.

@samtrion samtrion disabled auto-merge November 17, 2025 20:45
@samtrion samtrion merged commit f04440f into main Nov 17, 2025
29 of 31 checks passed
@samtrion samtrion deleted the chore/dapr-keyed-services branch November 17, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:ready for merge Indicates that a pull request has been reviewed and approved, and is ready to be merged into the mai type:chore Indicates some housework that needs to be done.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Keyed Service Support for Dapr Health Check

3 participants