-
Notifications
You must be signed in to change notification settings - Fork 565
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
Significant changes and corrections to the SE health doc #8125
Conversation
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.
Doc LGTM. Needs code review.
|
||
==== Adding configuration to a custom observer | ||
In addition to preparing the health observer builder with hard-coded settings, your code can also apply configuration for health so your user could set any health behavior there as well. | ||
|
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.
Was confused by "your user" here. Maybe rephrase this? Maybe "your code can also apply configuration by directly referring to a config node" or something like that.
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.
I've rephrased this in a way that, I hope, is clearer.
|
||
Order is important. Here, the code first sets `details` to `true` explicitly and later applies configuration. If your end user sets `details` in the `server.features.observe.observers.health` config to `false`, that setting overrides the hard-coded `true` setting in the code _because of where in the code you apply the configuration_. Try changing the `details` value to `false` in the config file and then stop, rebuild, and rerun the application. Access the health endpoint and notice that the output is no longer detailed. | ||
|
||
In general, most applications should apply settings from config _after_ assigning any settings in the code so users have the final say, but there might be exceptions in your particular case. |
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.
I see the "users" again. I'd would use "external config" or simply "config" instead, but I understand the notion better now.
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.
Rephrased this a bit.
Description
Resolves #7945
The health doc was significantly out of step with the late coding changes for 4.0.0. This PR addresses a number of problems, including what config key prefix is correct and the code examples to create a custom observe feature and a custom health observer in which to add custom health checks.
Documentation
This is a doc PR.