Skip to content

Update Health Information documentation to reflect simplification and split of health and status #10863

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

Closed
wilkinsona opened this issue Nov 1, 2017 · 10 comments
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@wilkinsona
Copy link
Member

No description provided.

@wilkinsona wilkinsona added this to the 2.0.0.RC1 milestone Nov 1, 2017
@wilkinsona
Copy link
Member Author

The "HTTP Health Endpoint Format and Access Restrictions" section needs to be updated as well.

@dsyer
Copy link
Member

dsyer commented Nov 2, 2017

Could we maybe think about explicit migration guide for hypermedia users (if you were using the "/links" endpoint before, it should work as follows...)?

Also what about keeping /health as the "status only" endpoint. And putting it in the root context by default? A lot of existing systems probably rely on that behaviour (e.g. ones using k8s). At least some explicit documentation in the migration guide about what to do in such systems would be useful. (Maybe they can just set the actuator context path to "/"?)

@snicoll
Copy link
Member

snicoll commented Nov 2, 2017

what about keeping /health as the "status only" endpoint.

@dsyer there's a relatively lengthy discussion as why we've done this that way in #10181

@markfisher
Copy link

I understand the rationale behind the changes, but IMO the need to maintain backwards compatibility outweighs all of that. Instead, I would prefer to see a simple and well-documented way for users to customize the mappings beyond just the context path.

I also understand the rationale behind avoiding conflicts with other root-level mappings, but as developers increasingly build microservice apps with a smaller surface area of exposed endpoints (an extreme case being a Spring Cloud Function app that exposes only a single Function), it seems like an unnecessary burden to those developers since the collisions are not a concern for their apps.

@dsyer
Copy link
Member

dsyer commented Nov 2, 2017

Nobody discussed /health and /health/details in #10181 really (I saw /healthdetails but nobody liked it and neither do I).

@wilkinsona
Copy link
Member Author

To help people read the context for the move from / to /application, it was done under #6886.

The following might belong in the migration issue (#10313), but the discussion seems to have started here so let's carry on here for now at least.

As things stand, the minimal configuration to get high-level health information available at /health is:

endpoints.status.web.path=health
management.endpoints.web.base-path=/

This will also give you the info endpoint available at /info which can be turned off with the following configuration:

endpoints.info.web.enabled=false

If you want the detailed health information to be available at /health, the minimal configuration is:

endpoints.health.web.enabled=true
management.endpoints.web.base-path=/

This will also give you the status and info endpoints available at /status and /info respectively which can be turned off with the following configuration:

endpoints.status.web.enabled=false
endpoints.info.web.enabled=false

it seems like an unnecessary burden to those developers since the collisions are not a concern for their apps.

Whichever default we go for, it's going to be a burden for someone. We need to decide who should shoulder that burden. Previously, we've leaned towards the possibility of clashes as being the problem to avoid by default. Perhaps that should be outweighed by a desire to keep the endpoints at the root.

If we did move the endpoints to the root, we still wouldn't offer /health with the default configuration. Either endpoints.status.web.path=health or endpoints.health.web.enabled=true would still be required.

Nobody discussed /health and /health/details in #10181 really

/health/details is an interesting suggestion to sit alongside /health. It would still allow us to support #8685 with /health/details/dataSource, /health/details/mail, etc.

A problem with /health/details is that it's a step back towards the complexity of the old health endpoint. I don't think we'd want to allow two different endpoints to have overlapping mappings, so we'd be looking at having a single endpoint with two mappings that can be enabled independently. That feels quite complex to me, particularly in terms of the configuration that we'd have to present to the user.

If we could come up with a better name than /healthdetails, I think I'd prefer to have /health display what /status currently displays and have /somebettername display what /health currently displays once it's been enabled.

@markfisher
Copy link

Whichever default we go for, it's going to be a burden for someone. We need to decide who should shoulder that burden

First, this is not true. For an existing user with no collisions, not changing the current mappings would not introduce any new burden.

Second, IMO the "Spring way" to decide such a tradeoff should always give more weight to backwards compatibility.

@wilkinsona
Copy link
Member Author

First, this is not true. For an existing user with no collisions, not changing the current mappings would not introduce any new burden.

Fair enough, "someone" is probably an overstatement.

An additional burden (that I didn't mention above) which influenced the decision made in #6886 was configuring security. Having all of the actuator's endpoints available beneath a single, separate location makes securing them much easier. However, things have moved on in this regard since #6886 was considered. The new EndpointRequestMatcher introduced in M4 (46dfe38) does ease that burden quite a bit.

Second, IMO the "Spring way" to decide such a tradeoff should always give more weight to backwards compatibility.

Agreed, and I think we did that when we made the decisions that we did. Perhaps what we didn't do, but should have, is reconsider the weight that we gave to other things. For example, when EndpointRequestMatcher simplified the security side of things.

@snicoll
Copy link
Member

snicoll commented Nov 23, 2017

@wilkinsona I am not sure if there's much we have to do for this one. I had a look to the doc and it has been updated already.

@wilkinsona
Copy link
Member Author

We need to remove this section.

@wilkinsona wilkinsona self-assigned this Nov 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants