-
-
Notifications
You must be signed in to change notification settings - Fork 254
Improve bit Boilerplate health checks (#11314) #11315
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces and wires comprehensive health checks across server projects, adds health-check endpoints (/health, /alive, /healthz) with caching and UI response, fixes extension method name, extends service worker URL handling, adds health-check packages, implements a FluentStorage health check, and updates AppHost with health endpoints plus container/image/port settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SW as Service Worker
participant Web as Server Web
participant API as Server API
participant HC as HealthChecks Pipeline
User->>SW: Request /alive | /health | /healthz
alt URL matches /healthz|/health|/alive
SW-->>User: Bypass SW (network)
else other URLs
SW->>User: Handle per SW strategy
end
User->>Web: GET /alive (Dev)
Web->>HC: Evaluate health (Predicate: tag "live")
HC-->>User: 200/503 JSON
User->>API: GET /health (Dev)
API->>HC: Evaluate health (default predicate)
HC-->>User: 200/503 JSON
User->>API: GET /healthz (Dev)
API->>HC: Evaluate all checks
HC-->>User: 200/503 UIResponse JSON
sequenceDiagram
autonumber
participant Builder as HostApplicationBuilder
participant H as HealthChecksBuilder
participant DB as EF Core
participant HG as Hangfire
participant FS as FluentStorage
Builder->>H: AddDefaultHealthChecks()
Note right of H: Adds OutputCache policy "HealthChecks"<br/>Adds DiskStorage check (min 5GB free, tag "live")
Builder->>H: AddDbContextCheck<AppDbContext>(tags: ["live"])
H->>DB: Register context check
Builder->>H: AddHangfire(min servers 1, tags: ["live"])
H->>HG: Register server availability check
Builder->>H: AddCheck<FluentStorageHealthCheck>("storage", tags: ["live"])
H->>FS: Register custom check
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Pull Request Overview
This PR improves the health check implementation in the bit Boilerplate project by enhancing health check endpoints, adding new health check types, and fixing configuration issues.
- Fixes health check method naming and configuration
- Adds comprehensive health checks for database, storage, and Hangfire background jobs
- Implements proper caching and response formatting for health check endpoints
- Configures Aspire orchestration with explicit port configurations and health check integration
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vs-spell.dic | Adds health check related terms to spell check dictionary |
| Program.Services.cs (Web) | Adds default health checks configuration to web project |
| Program.Middlewares.cs (Web/API) | Fixes method name typo from MappAppHealthChecks to MapAppHealthChecks |
| WebApplicationExtensions.cs | Enhances health check endpoints with caching, UI response writer, and improved organization |
| WebApplicationBuilderExtensions.cs | Refactors health checks to be more specific and adds output caching configuration |
| Project files | Adds necessary health check NuGet packages |
| Program.cs (AppHost) | Configures explicit ports and health check endpoints in Aspire orchestration |
| FluentStorageHealthCheck.cs | New custom health check for storage connectivity validation |
| service-worker.published.js | Updates service worker to handle new health check endpoints |
...ilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs
Show resolved
Hide resolved
...plate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/FluentStorageHealthCheck.cs
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs
Show resolved
Hide resolved
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.
Actionable comments posted: 2
🧹 Nitpick comments (11)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Web/wwwroot/service-worker.published.js (1)
98-99: Tighten regex to avoid over-matching (e.g., “/healthy.png”).
Current patterns match any path containing “/health” or “/alive”. Anchor to end/boundary to reduce collateral matches.- /\/health/, - /\/alive/, + /\/health(?:$|[\/?#])/, + /\/alive(?:$|[\/?#])/,src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
192-201: Make disk check path deterministic & min-free space configurable; ensure cache policy applies only to UI.
- Use AppContext.BaseDirectory to derive drive root (Directory.GetCurrentDirectory can vary).
- Allow min free MB via config with sensible default.
- Verify the “HealthChecks” OutputCache policy is applied only to the UI endpoint (e.g., /healthz), not liveness/readiness.
- public static IHealthChecksBuilder AddDefaultHealthChecks<TBuilder>(this TBuilder builder) + public static IHealthChecksBuilder AddDefaultHealthChecks<TBuilder>(this TBuilder builder) where TBuilder : IHostApplicationBuilder { - builder.Services.AddOutputCache(configureOptions: static caching => - caching.AddPolicy("HealthChecks", - build: static policy => policy.Expire(TimeSpan.FromSeconds(10)))); - - return builder.Services.AddHealthChecks() - .AddDiskStorageHealthCheck(opt => opt.AddDrive(Path.GetPathRoot(Directory.GetCurrentDirectory())!, minimumFreeMegabytes: 5 * 1024), tags: ["live"]); + builder.Services.AddOutputCache(configureOptions: static caching => + caching.AddPolicy("HealthChecks", + build: static policy => policy.Expire(TimeSpan.FromSeconds(10)))); + + var minFreeMb = builder.Configuration.GetValue<int?>("HealthChecks:MinFreeDiskMB") ?? 5 * 1024; + var driveRoot = Path.GetPathRoot(AppContext.BaseDirectory)!; + + return builder.Services.AddHealthChecks() + .AddDiskStorageHealthCheck(opt => opt.AddDrive(driveRoot, minimumFreeMegabytes: minFreeMb), tags: ["live"]); }If desired, we can also switch to Configure to avoid a second AddOutputCache call.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (6)
31-35: Add explicit MySQL image for reproducibilitySQL Server and Postgres are pinned; MySQL should be too for parity.
Diff:
.WithDataVolume() + .WithImage(mySqlImage, mySqlTag) .AddDatabase("mysqldb");Add once near the top:
const string mySqlImage = "mysql"; const string mySqlTag = "8.4"; // TODO: choose the vetted LTS tag and pin/digest
44-46: Avoid fixed dev ports or make them parametersAzulite defaults (10000–10002) are fine, but these collide easily on shared dev hosts. Consider parameters or letting Aspire pick free host ports.
Example:
// const int azBlobPort = 10000; // make configurable via template symbol or env // azurite.WithBlobPort(azBlobPort).WithQueuePort(azQueuePort).WithTablePort(azTablePort);
60-66: Dev-only health probe: align path via a single constantGood to gate on Development. Use a shared const to prevent drift between Web/API.
Diff:
-if (builder.Environment.IsDevelopment()) +if (builder.Environment.IsDevelopment()) { - serverWebProject.WithHttpHealthCheck("/alive"); + serverWebProject.WithHttpHealthCheck(HealthPath); }Add once near the top:
const string HealthPath = "/alive";
13-19: Pin SQL Server image tag and hoist to constants
Hard-codingmssql/server:2025-latestrisks surprise breaks. Define image and tag as constants and update the call:- .WithImage("mssql/server", "2025-latest") + .WithImage(sqlServerImage, sqlServerTag) const string sqlServerImage = "mssql/server"; const string sqlServerTag = "2025-latest"; // TODO: pin to a vetted tag or digestVerify no remaining hard-coded references and confirm the new constants are used:
#!/bin/bash # Check for any leftover hard-coded SQL Server images rg -nP '\.WithImage\(\s*"mssql/server"\s*,' # Confirm usage of the new constants rg -nP '\.WithImage\(\s*sqlServerImage\s*,\s*sqlServerTag\s*\)'
22-27: Centralize and pin Postgres image/tag for pgvector
Define at the top of Program.cs (e.g. above builder):const string pgImage = "pgvector/pgvector"; const string pgTag = "pg17"; // TODO: pin to a vetted tag or digestThen replace the literal call:
- .WithImage("pgvector/pgvector", "pg17") // pgvector supports embedded vector search. + .WithImage(pgImage, pgTag) // pgvector supports embedded vector search.
71-76: Extract/aliveto a shared constant
MapAppHealthChecks already registers/alive(WebApplicationExtensions.cs:29); define a constant for this endpoint and use it inWithHttpHealthCheck(...).src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj (1)
13-14: Limit transitive exposure of health-check UI/client packagesThese are implementation details of the shared layer; avoid leaking them transitively to consumers.
Diff:
- <PackageReference Include="AspNetCore.HealthChecks.System" /> - <PackageReference Include="AspNetCore.HealthChecks.UI.Client" /> + <PackageReference Include="AspNetCore.HealthChecks.System" PrivateAssets="all" /> + <PackageReference Include="AspNetCore.HealthChecks.UI.Client" PrivateAssets="all" />If any project relies on them transitively, add explicit references there.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)
52-53: Feature-gate AspNetCore.HealthChecks.Hangfire; central net9 versions confirmed
- Wrap AspNetCore.HealthChecks.Hangfire in the existing Hangfire feature toggle to keep the template lean. Central version pins for AspNetCore.HealthChecks.Hangfire (9.0.0) and Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore (9.0.8) already target .NET 9 in Directory.Packages.props; no further changes needed.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/FluentStorageHealthCheck.cs (1)
11-26: Avoid side effects in health checks; validate config and add context dataCreating folders in a health check can mutate state, mask permission issues, and add latency. Prefer a pure readiness probe: validate the configured path, check existence, and return Unhealthy if missing; bootstrap creation elsewhere (startup/IHostedService).
- public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) + public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) { try { - if (await blobStorage.ExistsAsync(settings.UserProfileImagesDir, cancellationToken) is false) - { - await blobStorage.CreateFolderAsync(settings.UserProfileImagesDir, cancellationToken: cancellationToken); - } - - return HealthCheckResult.Healthy("Storage is healthy"); + if (string.IsNullOrWhiteSpace(settings.UserProfileImagesDir)) + { + return HealthCheckResult.Unhealthy("Storage path is not configured."); + } + + var exists = await blobStorage.ExistsAsync(settings.UserProfileImagesDir, cancellationToken); + if (!exists) + { + return HealthCheckResult.Unhealthy($"Folder '{settings.UserProfileImagesDir}' does not exist."); + } + + return HealthCheckResult.Healthy( + "Storage is healthy", + new Dictionary<string, object?> + { + ["path"] = settings.UserProfileImagesDir + }); } catch (Exception exp) { return HealthCheckResult.Unhealthy("Storage is unhealthy", exp); } }If you want auto-creation, move it to a startup bootstrapper:
public sealed class StorageBootstrapper : IHostedService { private readonly IBlobStorage _storage; private readonly ServerApiSettings _settings; public StorageBootstrapper(IBlobStorage storage, ServerApiSettings settings) => (_storage, _settings) = (storage, settings); public async Task StartAsync(CancellationToken ct) { if (!string.IsNullOrWhiteSpace(_settings.UserProfileImagesDir) && await _storage.ExistsAsync(_settings.UserProfileImagesDir, ct) is false) { await _storage.CreateFolderAsync(_settings.UserProfileImagesDir, cancellationToken: ct); } } public Task StopAsync(CancellationToken ct) => Task.CompletedTask; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Web/wwwroot/service-worker.published.js(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/FluentStorageHealthCheck.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationExtensions.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Services.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/vs-spell.dic(1 hunks)
⏰ 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: build and test
🔇 Additional comments (9)
src/Templates/Boilerplate/Bit.Boilerplate/vs-spell.dic (1)
104-105: LGTM — added terms reduce false positives in spell checks.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
25-28: Health-check package versions verified — all are latest 9.0.x
Hangfire, System, and UI.Client are at 9.0.0; EntityFrameworkCore is at 9.0.8 — no newer stable patch releases exist.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Services.cs (1)
36-36: Health checks registration in Server.Web — LGTMPlacement after AddServerSharedServices() is sensible; endpoints are dev-gated elsewhere. No issues spotted.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationExtensions.cs (3)
18-38: Cache policy TTL for “HealthChecks” is set to 10 seconds—that meets the recommended ≤5–10 s threshold, so no changes needed.
2-2: No action needed: AspNetCore.HealthChecks.UI.Client is already referenced
The package is declared in Directory.Packages.props and referenced in Boilerplate.Server.Shared.csproj.
12-12: MapAppHealthChecks call sites updated and verifiedNo occurrences of the old
MappAppHealthChecksremain;app.MapAppHealthChecks()is now consistently used in both Server.Web and Server.Api.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (1)
112-112: Typo fix to MapAppHealthChecks — LGTMCall now matches the renamed extension; ordering after antiforgery is fine.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (1)
5-5: Import is appropriate for env checks
builder.Environment.IsDevelopment()requires this; good.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)
41-41: LGTM: health checks namespace import is appropriate
closes #11314
Summary by CodeRabbit
New Features
Bug Fixes
Chores