Ensure we don't add duplicate Health Checks (#707)

When adding Health Checks, we need to ensure we don't add the same health check twice. ASP.NET will throw an exception if 2 health checks have the same name. Use the HostApplicationBuilder Properties to ensure an Aspire Component doesn't add the same health check twice. Fix #705
dotnet · Nov 6, 2023 · b7bff4d · b7bff4d
1 parent 12c7a14
commit b7bff4d
Show file tree

Hide file tree

Showing 21 changed files with 158 additions and 47 deletions.
diff --git a/src/Components/Aspire.Azure.Data.Tables/Aspire.Azure.Data.Tables.csproj b/src/Components/Aspire.Azure.Data.Tables/Aspire.Azure.Data.Tables.csproj
@@ -13,6 +13,7 @@
 
   <ItemGroup>
     <Compile Include="..\Common\AzureComponent.cs" Link="AzureComponent.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Azure.Messaging.ServiceBus/Aspire.Azure.Messaging.ServiceBus.csproj b/src/Components/Aspire.Azure.Messaging.ServiceBus/Aspire.Azure.Messaging.ServiceBus.csproj
@@ -13,6 +13,7 @@
 
   <ItemGroup>
     <Compile Include="..\Common\AzureComponent.cs" Link="AzureComponent.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Azure.Security.KeyVault/Aspire.Azure.Security.KeyVault.csproj b/src/Components/Aspire.Azure.Security.KeyVault/Aspire.Azure.Security.KeyVault.csproj
@@ -13,6 +13,7 @@
 
   <ItemGroup>
     <Compile Include="..\Common\AzureComponent.cs" Link="AzureComponent.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Azure.Storage.Blobs/Aspire.Azure.Storage.Blobs.csproj b/src/Components/Aspire.Azure.Storage.Blobs/Aspire.Azure.Storage.Blobs.csproj
@@ -13,6 +13,7 @@
 
   <ItemGroup>
     <Compile Include="..\Common\AzureComponent.cs" Link="AzureComponent.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Azure.Storage.Queues/Aspire.Azure.Storage.Queues.csproj b/src/Components/Aspire.Azure.Storage.Queues/Aspire.Azure.Storage.Queues.csproj
@@ -13,6 +13,7 @@
 
   <ItemGroup>
     <Compile Include="..\Common\AzureComponent.cs" Link="AzureComponent.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Microsoft.Data.SqlClient/Aspire.Microsoft.Data.SqlClient.csproj b/src/Components/Aspire.Microsoft.Data.SqlClient/Aspire.Microsoft.Data.SqlClient.csproj
@@ -8,6 +8,10 @@
     <PackageIconFullPath>$(SharedDir)SQL_256x.png</PackageIconFullPath>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="AspNetCore.HealthChecks.SqlServer" />
     <PackageReference Include="Microsoft.Data.SqlClient" />

diff --git a/src/Components/Aspire.Microsoft.Data.SqlClient/AspireSqlServerSqlClientExtensions.cs b/src/Components/Aspire.Microsoft.Data.SqlClient/AspireSqlServerSqlClientExtensions.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Aspire;
 using Aspire.Microsoft.Data.SqlClient;
 using HealthChecks.SqlServer;
 using Microsoft.Data.SqlClient;
@@ -106,16 +107,15 @@ string GetConnectionString()
 
         if (settings.HealthChecks)
         {
-            builder.Services.AddHealthChecks()
-                .Add(new HealthCheckRegistration(
-                    serviceKey is null ? "SqlServer" : $"SqlServer_{connectionName}",
-                    sp => new SqlServerHealthCheck(new SqlServerHealthCheckOptions()
-                    {
-                        ConnectionString = settings.ConnectionString ?? string.Empty
-                    }),
-                    failureStatus: default,
-                    tags: default,
-                    timeout: default));
+            builder.TryAddHealthCheck(new HealthCheckRegistration(
+                serviceKey is null ? "SqlServer" : $"SqlServer_{connectionName}",
+                sp => new SqlServerHealthCheck(new SqlServerHealthCheckOptions()
+                {
+                    ConnectionString = settings.ConnectionString ?? string.Empty
+                }),
+                failureStatus: default,
+                tags: default,
+                timeout: default));
         }
     }
 }
diff --git a/...osoft.EntityFrameworkCore.SqlServer/Aspire.Microsoft.EntityFrameworkCore.SqlServer.csproj b/...osoft.EntityFrameworkCore.SqlServer/Aspire.Microsoft.EntityFrameworkCore.SqlServer.csproj
@@ -8,6 +8,10 @@
     <PackageIconFullPath>$(SharedDir)SQL_256x.png</PackageIconFullPath>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" />
     <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />

diff --git a/...spire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs b/...spire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics.CodeAnalysis;
+using Aspire;
 using Aspire.Microsoft.EntityFrameworkCore.SqlServer;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Configuration;
@@ -90,7 +91,9 @@ public static class AspireSqlServerEFCoreSqlClientExtensions
 
         if (settings.HealthChecks)
         {
-            builder.Services.AddHealthChecks().AddDbContextCheck<TContext>();
+            builder.TryAddHealthCheck(
+                name: typeof(TContext).Name,
+                static hcBuilder => hcBuilder.AddDbContextCheck<TContext>());
         }
 
         void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder)

diff --git a/...Npgsql.EntityFrameworkCore.PostgreSQL/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.csproj b/...Npgsql.EntityFrameworkCore.PostgreSQL/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.csproj
@@ -10,6 +10,7 @@
 
   <ItemGroup>
     <Compile Include="..\Aspire.Npgsql\NpgsqlCommon.cs" />
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs b/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics.CodeAnalysis;
+using Aspire;
 using Aspire.Npgsql.EntityFrameworkCore.PostgreSQL;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.Configuration;
@@ -95,7 +96,9 @@ public static partial class AspireEFPostgreSqlExtensions
         if (settings.HealthChecks)
         {
             // calling MapHealthChecks is the responsibility of the app, not Component
-            builder.Services.AddHealthChecks().AddDbContextCheck<TContext>();
+            builder.TryAddHealthCheck(
+                name: typeof(TContext).Name,
+                static hcBuilder => hcBuilder.AddDbContextCheck<TContext>());
         }
 
         if (settings.Tracing)

diff --git a/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj b/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj
@@ -8,6 +8,10 @@
     <PackageIconFullPath>$(SharedDir)PostgreSQL_logo.3colors.540x557.png</PackageIconFullPath>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="AspNetCore.HealthChecks.NpgSql" />
     <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />

diff --git a/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs b/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Data.Common;
+using Aspire;
 using Aspire.Npgsql;
 using HealthChecks.NpgSql;
 using Microsoft.Extensions.Configuration;
@@ -71,18 +72,17 @@ private static void AddNpgsqlDataSource(IHostApplicationBuilder builder, string
         // https://www.npgsql.org/doc/connection-string-parameters.html#pooling
         if (settings.HealthChecks)
         {
-            builder.Services.AddHealthChecks()
-                .Add(new HealthCheckRegistration(
-                    serviceKey is null ? "PostgreSql" : $"PostgreSql_{connectionName}",
-                    sp => new NpgSqlHealthCheck(new NpgSqlHealthCheckOptions()
-                    {
-                        DataSource = serviceKey is null
-                            ? sp.GetRequiredService<NpgsqlDataSource>()
-                            : sp.GetRequiredKeyedService<NpgsqlDataSource>(serviceKey)
-                    }),
-                    failureStatus: default,
-                    tags: default,
-                    timeout: default));
+            builder.TryAddHealthCheck(new HealthCheckRegistration(
+                serviceKey is null ? "PostgreSql" : $"PostgreSql_{connectionName}",
+                sp => new NpgSqlHealthCheck(new NpgSqlHealthCheckOptions()
+                {
+                    DataSource = serviceKey is null
+                        ? sp.GetRequiredService<NpgsqlDataSource>()
+                        : sp.GetRequiredKeyedService<NpgsqlDataSource>(serviceKey)
+                }),
+                failureStatus: default,
+                tags: default,
+                timeout: default));
         }
 
         if (settings.Tracing)

diff --git a/src/Components/Aspire.RabbitMQ.Client/Aspire.RabbitMQ.Client.csproj b/src/Components/Aspire.RabbitMQ.Client/Aspire.RabbitMQ.Client.csproj
@@ -8,6 +8,10 @@
     <NoWarn>$(NoWarn);SYSLIB1100;SYSLIB1101</NoWarn>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="AspNetCore.HealthChecks.Rabbitmq" />
     <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />

diff --git a/src/Components/Aspire.RabbitMQ.Client/AspireRabbitMQExtensions.cs b/src/Components/Aspire.RabbitMQ.Client/AspireRabbitMQExtensions.cs
@@ -3,6 +3,7 @@
 
 using System.Diagnostics;
 using System.Net.Sockets;
+using Aspire;
 using Aspire.RabbitMQ.Client;
 using HealthChecks.RabbitMQ;
 using Microsoft.Extensions.Configuration;
@@ -119,9 +120,7 @@ IConnectionFactory CreateConnectionFactory(IServiceProvider sp)
 
         if (settings.HealthChecks)
         {
-            var hcBuilder = builder.Services.AddHealthChecks();
-
-            hcBuilder.Add(new HealthCheckRegistration(
+            builder.TryAddHealthCheck(new HealthCheckRegistration(
                 serviceKey is null ? "RabbitMQ.Client" : $"RabbitMQ.Client_{connectionName}",
                 sp =>
                 {

diff --git a/src/Components/Aspire.StackExchange.Redis/Aspire.StackExchange.Redis.csproj b/src/Components/Aspire.StackExchange.Redis/Aspire.StackExchange.Redis.csproj
@@ -9,6 +9,10 @@
     <NoWarn>$(NoWarn);SYSLIB1100;SYSLIB1101</NoWarn>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="AspNetCore.HealthChecks.Redis" />
     <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />

diff --git a/src/Components/Aspire.StackExchange.Redis/AspireRedisExtensions.cs b/src/Components/Aspire.StackExchange.Redis/AspireRedisExtensions.cs
@@ -4,6 +4,7 @@
 global using System.Net.Security; // needed to work around https://github.com/dotnet/runtime/issues/94065
 
 using System.Text;
+using Aspire;
 using Aspire.StackExchange.Redis;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
@@ -108,13 +109,16 @@ private static void AddRedis(IHostApplicationBuilder builder, string configurati
 
         if (settings.HealthChecks)
         {
-            builder.Services.AddHealthChecks()
-                .AddRedis(
+            var healthCheckName = serviceKey is null ? "StackExchange.Redis" : $"StackExchange.Redis_{connectionName}";
+
+            builder.TryAddHealthCheck(
+                healthCheckName,
+                hcBuilder => hcBuilder.AddRedis(
                     // The connection factory tries to open the connection and throws when it fails.
                     // That is why we don't invoke it here, but capture the state (in a closure)
                     // and let the health check invoke it and handle the exception (if any).
                     connectionMultiplexerFactory: sp => serviceKey is null ? sp.GetRequiredService<IConnectionMultiplexer>() : sp.GetRequiredKeyedService<IConnectionMultiplexer>(serviceKey),
-                    name: serviceKey is null ? "StackExchange.Redis" : $"StackExchange.Redis_{connectionName}");
+                    healthCheckName));
         }
 
         static TextWriter? CreateLogger(IServiceProvider serviceProvider)

diff --git a/src/Components/Common/AzureComponent.cs b/src/Components/Common/AzureComponent.cs
@@ -103,23 +103,22 @@ internal void AddClient(
         {
             string namePrefix = $"Azure_{typeof(TClient).Name}";
 
-            builder.Services.AddHealthChecks()
-                .Add(new HealthCheckRegistration(
-                   serviceKey is null ? namePrefix : $"{namePrefix}_{serviceKey}",
-                   serviceProvider =>
-                   {
-                       // From https://devblogs.microsoft.com/azure-sdk/lifetime-management-and-thread-safety-guarantees-of-azure-sdk-net-clients/:
-                       // "The main rule of Azure SDK client lifetime management is: treat clients as singletons".
-                       // So it's fine to root the client via the health check.
-                       TClient client = serviceKey is null
-                            ? serviceProvider.GetRequiredService<TClient>()
-                            : serviceProvider.GetRequiredKeyedService<TClient>(serviceKey);
-
-                       return CreateHealthCheck(client, settings);
-                   },
-                   failureStatus: default,
-                   tags: default,
-                   timeout: default));
+            builder.TryAddHealthCheck(new HealthCheckRegistration(
+                serviceKey is null ? namePrefix : $"{namePrefix}_{serviceKey}",
+                serviceProvider =>
+                {
+                    // From https://devblogs.microsoft.com/azure-sdk/lifetime-management-and-thread-safety-guarantees-of-azure-sdk-net-clients/:
+                    // "The main rule of Azure SDK client lifetime management is: treat clients as singletons".
+                    // So it's fine to root the client via the health check.
+                    TClient client = serviceKey is null
+                        ? serviceProvider.GetRequiredService<TClient>()
+                        : serviceProvider.GetRequiredKeyedService<TClient>(serviceKey);
+
+                    return CreateHealthCheck(client, settings);
+                },
+                failureStatus: default,
+                tags: default,
+                timeout: default));
         }
 
         if (GetTracingEnabled(settings))

diff --git a/src/Components/Common/HealthChecksExtensions.cs b/src/Components/Common/HealthChecksExtensions.cs
@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Diagnostics.HealthChecks;
+using Microsoft.Extensions.Hosting;
+
+namespace Aspire;
+
+internal static class HealthChecksExtensions
+{
+    /// <summary>
+    /// Adds a HealthCheckRegistration if one hasn't already been added to the builder.
+    /// </summary>
+    public static void TryAddHealthCheck(this IHostApplicationBuilder builder, HealthCheckRegistration healthCheckRegistration)
+    {
+        builder.TryAddHealthCheck(healthCheckRegistration.Name, hcBuilder => hcBuilder.Add(healthCheckRegistration));
+    }
+
+    /// <summary>
+    /// Invokes the <paramref name="addHealthCheck"/> action if the given <paramref name="name"/> hasn't already been added to the builder.
+    /// </summary>
+    public static void TryAddHealthCheck(this IHostApplicationBuilder builder, string name, Action<IHealthChecksBuilder> addHealthCheck)
+    {
+        var healthCheckKey = $"Aspire.HealthChecks.{name}";
+        if (!builder.Properties.ContainsKey(healthCheckKey))
+        {
+            builder.Properties[healthCheckKey] = true;
+            addHealthCheck(builder.Services.AddHealthChecks());
+        }
+    }
+}
diff --git a/tests/Aspire.StackExchange.Redis.Tests/Aspire.StackExchange.Redis.Tests.csproj b/tests/Aspire.StackExchange.Redis.Tests/Aspire.StackExchange.Redis.Tests.csproj
@@ -6,6 +6,9 @@
 
   <ItemGroup>
     <ProjectReference Include="..\..\src\Components\Aspire.StackExchange.Redis\Aspire.StackExchange.Redis.csproj" />
+    <ProjectReference Include="..\..\src\Components\Aspire.StackExchange.Redis.DistributedCaching\Aspire.StackExchange.Redis.DistributedCaching.csproj" />
+    <ProjectReference Include="..\..\src\Components\Aspire.StackExchange.Redis.OutputCaching\Aspire.StackExchange.Redis.OutputCaching.csproj" />
+
     <ProjectReference Include="..\Aspire.Components.Common.Tests\Aspire.Components.Common.Tests.csproj" />
   </ItemGroup>
 

diff --git a/tests/Aspire.StackExchange.Redis.Tests/AspireRedisExtensionsTests.cs b/tests/Aspire.StackExchange.Redis.Tests/AspireRedisExtensionsTests.cs
@@ -1,8 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.AspNetCore.OutputCaching;
+using Microsoft.Extensions.Caching.Distributed;
+using Microsoft.Extensions.Caching.StackExchangeRedis;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Diagnostics.HealthChecks;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Options;
 using StackExchange.Redis;
@@ -181,4 +185,42 @@ public void AbortOnConnectFailDefaults(bool useKeyed, IEnumerable<KeyValuePair<s
 
         Assert.Equal(expectedAbortOnConnect, options.AbortOnConnectFail);
     }
+
+    /// <summary>
+    /// Verifies that both distributed and output caching components can be added to the same builder and their HealthChecks don't conflict.
+    /// See https://github.com/dotnet/aspire/issues/705
+    /// </summary>
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void MultipleRedisComponentsCanBeAdded(bool useKeyed)
+    {
+        var builder = Host.CreateEmptyApplicationBuilder(null);
+
+        if (useKeyed)
+        {
+            builder.AddKeyedRedisDistributedCache("redis");
+            builder.AddKeyedRedisOutputCache("redis");
+        }
+        else
+        {
+            builder.AddRedisDistributedCache("redis");
+            builder.AddRedisOutputCache("redis");
+        }
+
+        var host = builder.Build();
+
+        // Note that IDistributedCache and OutputCacheStore don't support keyed services - so only the Redis ConnectionMultiplexer is keyed.
+
+        var distributedCache = host.Services.GetRequiredService<IDistributedCache>();
+        Assert.IsAssignableFrom<RedisCache>(distributedCache);
+
+        var cacheStore = host.Services.GetRequiredService<IOutputCacheStore>();
+        Assert.StartsWith("Redis", cacheStore.GetType().Name);
+
+        // Explicitly ensure the HealthCheckService can be retrieved. It validates the registrations in its constructor.
+        // See https://github.com/dotnet/aspnetcore/blob/94ad7031db6744409de24f75777a59620cb94d9a/src/HealthChecks/HealthChecks/src/DefaultHealthCheckService.cs#L33-L36
+        var healthCheckService = host.Services.GetRequiredService<HealthCheckService>();
+        Assert.NotNull(healthCheckService);
+    }
 }