-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PRR for structured ComponentFlagz #5611 #5612
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
base: master
Are you sure you want to change the base?
Conversation
This feature should not cause rollout failures. If it does, we can disable the feature. In the worst | ||
case, it is possible it could cause runtime failures, but it is highly unlikely we would not detect this | ||
with existing tests. | ||
with existing tests. The endpoint is isolated and does not affect core workloads. |
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'm not convinced this is the case. How is the endpoint isolated such that it cannot affect core workloads.
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.
What I meant to say was that this endpoint is not in the critical path of a core Kubernetes service / component but more for extra observability into the component's state..
|
||
This enhancement proposes data that can be used to determine the health of the component. | ||
(though this endpoint is not intended to be used for alerting.) | ||
It is not intended for use by workloads or for real-time monitoring or alerting. |
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.
Same comment as above. I'm not convinced that and endpoint that be used to determine the health of a component won't be used for observability purposes like monitoring/alerting. I'm not sure our design intent really matters when it comes to how users will actually use the endpoint.
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.
Hmm having this for monitoring is anticipated but to me, there's no clear alerting use case for this endpoint. I'll remove the last line "It is not intended for use by workloads or for real-time monitoring or alerting." then
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpbetz, richabanker The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold
For #5550 to be merged first