-
Notifications
You must be signed in to change notification settings - Fork 15
HealthCheck Extensions Project #529
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Diagnostics.HealthChecks; | ||
| using ActiveMQ.Artemis.Client.Extensions.DependencyInjection; | ||
|
|
||
| namespace ActiveMQ.Artemis.Client.Extensions.HealthCheck | ||
| { | ||
| /// <summary> | ||
| /// Extension methods for adding ActiveMQ Artemis health checks to the application. | ||
| /// </summary> | ||
| public static class ActiveMqHealthCheckExtensions | ||
| { | ||
| /// <summary> | ||
| /// Adds ActiveMQ Artemis health checks to the specified <see cref="IHealthChecksBuilder"/>. | ||
| /// </summary> | ||
| /// <param name="builder">The <see cref="IHealthChecksBuilder"/> to add the health check to.</param> | ||
| /// <param name="name">The name of the ActiveMQ connection to check.</param> | ||
| /// <param name="factory">Optional factory function to create custom connection for health check.</param> | ||
| /// <returns>The <see cref="IHealthChecksBuilder"/> for further configuration.</returns> | ||
| public static IHealthChecksBuilder AddActiveMqHealthCheck(this IHealthChecksBuilder builder, string name, string[] tags = null) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also suggest slightly changing the signature. If we could pass not only the health check name but also the IActiveMqBuilder (the same one we're using in the DI package) we would gain access to the connection or its events. So this could like sth like this: public static IHealthChecksBuilder AddActiveMqHealthCheck(
this IHealthChecksBuilder builder,
string name,
IActiveMqBuilder activeMqBuilder,
string[] tags = null)
{
// Register a connection callback to wire up health check event handlers
activeMqBuilder.ConfigureConnection((provider, connection) =>
{
var healthCheckService = provider.GetRequiredKeyedService<ArtemisHealthCheckService>(activeMqBuilder.Name);
connection.ConnectionRecovered += healthCheckService.ConnectionRecovered;
connection.ConnectionClosed += healthCheckService.ConnectionClosed;
connection.ConnectionRecoveryError += healthCheckService.ConnectionRecoveryError;
});
// Ensure the ArtemisHealthCheckService is registered as a keyed singleton
builder.Services.AddKeyedSingleton<ArtemisHealthCheckService>(activeMqBuilder.Name);
// Register the health check using the same keyed instance to ensure consistency
builder.Add(new HealthCheckRegistration(
name,
provider => provider.GetRequiredKeyedService<ArtemisHealthCheckService>(activeMqBuilder.Name),
HealthStatus.Unhealthy,
tags));
return builder;
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think if I directly extend IActionBuilder like this to enable it?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of this kind of indirect reference. It may be problematic and not easy for users to figure out.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok reverted to previous version with parameter |
||
| { | ||
| // Register the health check factory in the DI container | ||
| builder.Services.AddSingleton(sp => new ArtemisHealthCheckService()); | ||
|
|
||
| // Register the health check using the typed approach | ||
| return builder.AddTypeActivatedCheck<ArtemisHealthCheckService>(name, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm not mistaken, this registration will create its own instance of ArtemisHealthCheckService, regardless of what you registered above as a singleton.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right by registering it this way we can have it for the necessary instances |
||
| failureStatus: HealthStatus.Unhealthy, | ||
| tags: tags ?? new[] { "activemq" }); | ||
| } | ||
|
|
||
| public static void ConfigureConnection(IServiceProvider _, IConnection connection) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then we wouldn't need this method. |
||
| { | ||
| connection.ConnectionClosed += (sender, args) => ArtemisHealthCheckService.ConnectionClosed(sender, args); | ||
| connection.ConnectionRecovered += (sender, args) => ArtemisHealthCheckService.ConnectionRecovered(sender, args); | ||
| connection.ConnectionRecoveryError += (sender, args) => ArtemisHealthCheckService.ConnectionRecoveryError(sender, args); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Extensions.Diagnostics.HealthChecks; | ||
|
|
||
| namespace ActiveMQ.Artemis.Client.Extensions.HealthCheck | ||
| { | ||
|
|
||
| public class ArtemisHealthCheckService : IHealthCheck | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make this internal.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| { | ||
| private static bool IsOpen = true; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This state should not be static. It may fall apart if we have more than one connection in a single application (which is the case in most production use cases I've seen). Instead, we should register this service per individual ActiveMQ connection.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should, in fact, be false. When the connection opens, it should trigger a change in state.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add method
for the connected state change |
||
| private static string Description = "Connection is open"; | ||
|
|
||
| public static void ConnectionRecovered(object? sender, ConnectionRecoveredEventArgs e, string description = "Connection recovered") | ||
| { | ||
| IsOpen = true; | ||
| Description = description; | ||
| } | ||
|
|
||
| public static void ConnectionRecoveryError(object? sender, ConnectionRecoveryErrorEventArgs e, string description = "Connection recovery failed") | ||
| { | ||
| IsOpen = false; | ||
| Description = description; | ||
| } | ||
|
|
||
| public static void ConnectionClosed(object? sender, ConnectionClosedEventArgs e, string description = "Connection closed") | ||
| { | ||
| IsOpen = false; | ||
| Description = description; | ||
| } | ||
|
|
||
| public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) | ||
| { | ||
| HealthCheckResult result = new(IsOpen ? HealthStatus.Healthy : HealthStatus.Unhealthy, description: Description); | ||
| return await Task.FromResult(result); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <Summary>Microsoft.Extensions.Diagnostics.HealthChecks support for ActiveMQ Artemis .NET Client</Summary> | ||
| <Description>Microsoft.Extensions.Diagnostics.HealthChecks support for ActiveMQ Artemis .NET Client.</Description> | ||
| <PackageProjectUrl>https://havret.github.io/dotnet-activemq-artemis-client/</PackageProjectUrl> | ||
| <PackageId>ArtemisNetClient.Extensions.HealthCheck</PackageId> | ||
| <PackageLicenseUrl>https://github.com/Havret/activemq-artemis-extensions/blob/master/LICENSE</PackageLicenseUrl> | ||
| <RepositoryType>git</RepositoryType> | ||
| <RepositoryUrl>https://github.com/Havret/activemq-artemis-extensions</RepositoryUrl> | ||
| <PublishRepositoryUrl>true</PublishRepositoryUrl> | ||
| <IncludeSymbols>true</IncludeSymbols> | ||
| <SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
| <Authors>Havret</Authors> | ||
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| <RootNamespace>ActiveMQ.Artemis.Client.Extensions.HealthCheck</RootNamespace> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="8.0.18" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\ArtemisNetClient.Extensions.DependencyInjection\ArtemisNetClient.Extensions.DependencyInjection.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.