Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Redis Health Check #13589

Merged
merged 25 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3702139
Add RedisHealthCheck
hishamco Apr 22, 2023
a743935
Add RedisHealthCheck extension method
hishamco Apr 22, 2023
8b62e57
Call AddRedisCheck()
hishamco Apr 22, 2023
1aa2c94
Use IRedisService instead
hishamco Apr 22, 2023
82e9a55
Merge branch 'main' into hishamco/redis-health-check
hishamco May 14, 2023
632f4da
Address feedback
hishamco May 14, 2023
4c16f14
Add a separate Startup
hishamco May 14, 2023
f298f6e
Fix unable to load IRedisService while activating RedisHealthCheck
hishamco May 14, 2023
f5811ed
Fix the returned HealthCheckResult
hishamco May 14, 2023
08a4924
Merge branch 'main' into hishamco/redis-health-check
Piedone Mar 21, 2024
41804d1
Merge branch 'main' into hishamco/redis-health-check
hishamco Mar 26, 2024
8a23f3b
Merge branch 'main' into hishamco/redis-health-check
hishamco Mar 26, 2024
9e0cf80
Merge branch 'main' into hishamco/redis-health-check
hishamco Mar 27, 2024
5174d47
Add description for unhealthy status
hishamco Mar 27, 2024
38d06ab
Add docs
hishamco Mar 27, 2024
71d39d5
Address feedback
hishamco Mar 28, 2024
d7bb336
Merge branch 'main' into hishamco/redis-health-check
hishamco Mar 28, 2024
bfcf5ab
Fix the build
hishamco Mar 28, 2024
447fea6
Fix seconds argument
hishamco Mar 29, 2024
b4928d8
Merge branch 'main' into hishamco/redis-health-check
hishamco Apr 1, 2024
e93590a
Timeout as const
hishamco Apr 1, 2024
5f9a144
Use expression body
hishamco Apr 1, 2024
2f5242e
Merge branch 'main' into hishamco/redis-health-check
hishamco Apr 1, 2024
5720a64
Update the comment
hishamco Apr 1, 2024
6cac17e
Update src/OrchardCore.Modules/OrchardCore.Redis/HealthChecks/Startup.cs
hishamco Apr 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Localization;

namespace OrchardCore.Redis.HealthChecks;

public class RedisHealthCheck : IHealthCheck
{
private readonly IServiceProvider _serviceProvider;
private readonly IStringLocalizer S;

public RedisHealthCheck(IServiceProvider serviceProvider)
public RedisHealthCheck(IServiceProvider serviceProvider, IStringLocalizer<RedisHealthCheck> stringLocalizer)
{
_serviceProvider = serviceProvider;
S = stringLocalizer;
}

public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
Expand All @@ -22,7 +25,7 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
var redisService = _serviceProvider.GetService<IRedisService>();
if (redisService == null)
{
return HealthCheckResult.Unhealthy();
return HealthCheckResult.Unhealthy(description: S["The service '{0}' isn't registered.", nameof(IRedisService)]);
Piedone marked this conversation as resolved.
Show resolved Hide resolved
hishamco marked this conversation as resolved.
Show resolved Hide resolved
}

if (redisService.Connection == null)
Expand All @@ -35,7 +38,7 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
var time = await redisService.Database.PingAsync();
if (time > TimeSpan.FromSeconds(30))
{
return HealthCheckResult.Unhealthy();
return HealthCheckResult.Unhealthy(description: S["The Redis server isn't a live."]);
Piedone marked this conversation as resolved.
Show resolved Hide resolved
hishamco marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand All @@ -44,12 +47,12 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
}
else
{
return HealthCheckResult.Unhealthy();
return HealthCheckResult.Unhealthy(description: S["There's an issue in the Redis connection."]);
Piedone marked this conversation as resolved.
Show resolved Hide resolved
hishamco marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch
catch (Exception ex)
{
return HealthCheckResult.Unhealthy();
return HealthCheckResult.Unhealthy(description: ex.Message);
Piedone marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Piedone marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace OrchardCore.Redis.HealthChecks;
[RequireFeatures("OrchardCore.HealthChecks")]
public class Startup : StartupBase
{
// The order of this startup configuration should run early to register the redis check.
Piedone marked this conversation as resolved.
Show resolved Hide resolved
public override int Order => 100;
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment on why this is necessary, and what's the reason for 100 in particular.

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 might need to check this one, I remember this might be necessary to run the health check early

Copy link
Member

Choose a reason for hiding this comment

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

So, also document what's the reason for 100 in particular. This is a magic number; can it be 90? Can it be 110? Why not 1000?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be, no particular reason for using 10 but usually we're using 100 or more to avoid start ordering issue

Copy link
Member

Choose a reason for hiding this comment

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

OK, so please document it.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW there are many places where we use 1000 for instance without docs :)

Copy link
Member

Choose a reason for hiding this comment

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

And those should have been documented as well :). Having less-than-ideal pieces of code currently doesn't mean that we should keep writing the same.


public override void ConfigureServices(IServiceCollection services)
Expand Down
4 changes: 4 additions & 0 deletions src/docs/reference/modules/Redis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ Integrates Redis into Orchard Core. Provides the following features:
- Redis Lock: Distributed Lock using Redis.
- Redis DataProtection: Distributed DataProtection using Redis.

## Health Checks

This module provides a health check to report the status for the Redis server. Refer also to the [Health Checks Section](../../module/HealthChecks/README.md).
Piedone marked this conversation as resolved.
Show resolved Hide resolved

## Video

<iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/etH6IJOGUe8" title="YouTube video player" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowfullscreen></iframe>
Loading