Skip to content

Rename the health check endpoint to /healthz - #467

Merged
David Fowler (davidfowl) merged 1 commit into
mainfrom
davidfowl/healthz
Nov 2, 2023
Merged

Rename the health check endpoint to /healthz#467
David Fowler (davidfowl) merged 1 commit into
mainfrom
davidfowl/healthz

Conversation

@davidfowl

Copy link
Copy Markdown
Contributor

Fixes #447

@davidfowl

Copy link
Copy Markdown
Contributor Author

After this poll, I'm sure everyone will change this endpoint 😄

@brendandburns

Brendan Burns (brendandburns) commented Oct 26, 2023

Copy link
Copy Markdown
Contributor

As I discussed w/ David, I'm team /healthy and /alive

@mitchdenny

Copy link
Copy Markdown
Member

+1 for healthz given the broader cloud native ecosystem is invested in this convention.

@IEvangelist David Pine (IEvangelist) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤣 at the /healthz...


// All health checks must pass for app to be considered ready to accept traffic after starting
app.MapHealthChecks("/readiness");
app.MapHealthChecks("/healthz");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
app.MapHealthChecks("/healthz");
app.MapHealthChecks("/healthy");


// All health checks must pass for app to be considered ready to accept traffic after starting
app.MapHealthChecks("/readiness");
app.MapHealthChecks("/healthz");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
app.MapHealthChecks("/healthz");
app.MapHealthChecks("/healthy");


// All health checks must pass for app to be considered ready to accept traffic after starting
app.MapHealthChecks("/readiness");
app.MapHealthChecks("/healthz");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
app.MapHealthChecks("/healthz");
app.MapHealthChecks("/healthy");

@DamianEdwards

Copy link
Copy Markdown
Member

David Fowler (@davidfowl) are we doing this for preview.1 still?

@davidfowl

Copy link
Copy Markdown
Contributor Author

Yep, I will go with /health and /alive

@IEvangelist

Copy link
Copy Markdown
Member

Yep, I will go with /health and /alive

Why not /healthy? It's more aligned with /alive if that's a preference, but /health feels disjointed from /alive. If you're healthy, you're alive, but they're not dually synonymous. You could be alive, but unwell... Whereas, /health could be anything, what does this actually report? It is a bit, or some status object? Is it an enum with three possible values; Healthy, Degraded, and Unhealthy? If the latter is the case, then I'd assume that we wouldn't want /alive as I'd expect that to be either true or false.

When you say "and", are you using both, as in multiple routes that resolve with the same check? Just curious about this naming convention. I agree with Brendan Burns (@brendandburns) on this one, but still a bit conflicted.

@davidfowl

David Fowler (davidfowl) commented Oct 31, 2023

Copy link
Copy Markdown
Contributor Author

/health does a deep health check and checks all dependencies. It'll eventually also return detailed health status (it'll return a single result today, healthy, unhealthy, degraded)
/alive does a self-check without checking dependencies.

@davidfowl

Copy link
Copy Markdown
Contributor Author

/backport to release/8.0-preview1

@github-actions

github-actions Bot commented Nov 2, 2023

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-preview1: https://github.com/dotnet/aspire/actions/runs/6737243070

@github-actions

github-actions Bot commented Nov 2, 2023

Copy link
Copy Markdown
Contributor

David Fowler (@davidfowl) backporting to release/8.0-preview1 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions

github-actions Bot commented Nov 2, 2023

Copy link
Copy Markdown
Contributor

David Fowler (@davidfowl) an error occurred while backporting to release/8.0-preview1, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Jose Perez Rodriguez (joperezr) added a commit that referenced this pull request Nov 14, 2023
@danmoseley Dan Moseley (danmoseley) added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Nov 16, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider changing the health check endpoint to /healthz

6 participants