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

Expand Health Checks with database connection check #108

Open
Hechamon opened this issue Mar 6, 2024 · 7 comments
Open

Expand Health Checks with database connection check #108

Hechamon opened this issue Mar 6, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@Hechamon
Copy link

Hechamon commented Mar 6, 2024

Is your feature request related to a problem? Please describe.

#105 added a basic health check endpoint, which could be expanded upon

Describe the solution you'd like

BaGetter is only really healthy if the database is at least reachable, so let's check that
The two ways of doing this which I can see are the following

@Regenhardt
Copy link

We can also add a button, or one button per service, to the statistics page where the services are listed to let the user trigger the health checks from the web app.

@Regenhardt
Copy link

Regenhardt commented Mar 21, 2024

Should the health check just return Healthy/Degraded/Unhealthy, or the actual services too? I could implement it so that if ListConfiguredServices is true, it returns a detailed report about which service is healthy, and otherwise just return Healthy/Degraded/Unhealthy and nothing else.

Or more uniform: Always a json object, but it contains either the general status only or includes services too:

{
  "BaGetter": "Healthy",
  "Sqlite": "Healthy"
}

Not sure if "BaGetter" is clear enough or if something like "OverallStatus" or completely different would be better, as that value represents the aggregated result of all health checks.

@Regenhardt
Copy link

Or just "Status".

@seriouz
Copy link

seriouz commented Mar 21, 2024

I would go with Status and the ListConfiguredServices idea:

{
   "Status": "Healthy"
}

If atleast one service fails Status is unhealthy too.

{
   "Status": "Unhealthy",
   "MySql": "Unhealthy"
}

Or the app is somehow unhealty itself but the DB is healthy:

{
   "Status": "Unealthy",
   "MySql": "Healthy"
}

@Regenhardt
Copy link

Just tried it: the status property is automatically unhealthy when at least one other health check is unhealthy.

@Regenhardt
Copy link

Regenhardt commented Mar 21, 2024

This first feature includes db checks and usage of all configured and registered checks.

This means we can now easily add additional health checks for single providers and they will be used, but we don't have to add all at once.

I propose to add another extension method to the core to add a storage health check that makes sure it's registered with the correct name, which each individual provider's activating extension can then call with its own specific implementation, similar to the AddSqlServer etc. methods. Not entirely sure about this one yet.

I see AWS has its own health check package and function, maybe other providers have that too.

@Regenhardt Regenhardt added the enhancement New feature or request label Mar 24, 2024
Copy link

This issue is stale because it has been open for 90 days with no activity. Remove the stale label, comment, or this will be closed in 5 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants