-
Notifications
You must be signed in to change notification settings - Fork 246
A90: Add List Method to gRPC Health Service #468
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
Conversation
This proposal introduces a new `List` RPC endpoint for the Health service, allowing clients to retrieve the statuses of all monitored services or a filtered list of specific services. This feature simplifies integration with status-reporting dashboards and enhances observability for microservices.
@a11r care to give it a review whenever you can? :) |
Co-authored-by: Benjamin Krämer <benjamin.kraemer@alien-scripts.de>
A##-list-health.md
Outdated
## Implementation | ||
|
||
1. Add the `List` RPC endpoint and the associated request/response messages in the Health service .proto file. | ||
2. Update client libraries to support the `List` endpoint. Provide examples demonstrating how to call the endpoint and handle its response. |
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 think there's another important step before this one, which is to implement the new method in the health service implementations in each gRPC language.
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.
Would you mind pointing me in the right direction? I'm happy to write code in pretty much all the languages supported by gRPC, but I just want to make sure what are those places I need to change or guidelines to take into account.
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.
In C-core, you'll need to implement the List
method in a similar way to how the existing Check
method is implemented:
@ejona86 can advise for Java, @dfawley for Go, and @murgatroid99 for Node.
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.
In Node the health service is implemented here: https://github.com/grpc/grpc-node/blob/master/packages/grpc-health-check/src/health.ts#L82. The List
method would need to be added. It should mostly just be straightforwardly iterating through the statusMap
.
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.
This is the implementation of Check
in our service in Go:
https://github.com/grpc/grpc-go/blob/8ae4b7db9171c735093092577886ce0dddbb8094/health/server.go#L54
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.
Thank you all for the guidance.
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
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.
Seems fair. I could bikeshed, but I don't have any real concerns.
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
A##-list-health.md
Outdated
## Implementation | ||
|
||
1. Add the `List` RPC endpoint and the associated request/response messages in the Health service .proto file. | ||
2. Update client libraries to support the `List` endpoint. Provide examples demonstrating how to call the endpoint and handle its response. |
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.
In C-core, you'll need to implement the List
method in a similar way to how the existing Check
method is implemented:
@ejona86 can advise for Java, @dfawley for Go, and @murgatroid99 for Node.
A##-list-health.md
Outdated
## Implementation | ||
|
||
1. Add the `List` RPC endpoint and the associated request/response messages in the Health service .proto file. | ||
2. Update client libraries to support the `List` endpoint. Provide examples demonstrating how to call the endpoint and handle its response. |
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.
This is the implementation of Check
in our service in Go:
https://github.com/grpc/grpc-go/blob/8ae4b7db9171c735093092577886ce0dddbb8094/health/server.go#L54
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
…rocess Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
gRPC protobuf change: grpc/grpc-proto#143 gRPC Go change: grpc/grpc-go#8155 |
Co-authored-by: Marcos Huck <marcoshuck@users.noreply.github.com>
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.
Just a couple of minor nits remaining!
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
I think we're ready to go! 😃 @markdroth |
Context
This proposal introduces a new
List
RPC endpoint for the Health service, allowing clients to retrieve the statuses of all monitored services. This feature simplifies integration with status-reporting dashboards and enhances observability for microservices.Change
This PR submits the proposal document for review and potential approval by the gRPC maintainers.
Additional information
I started the implementation of the health service (including just the gRPC service endpoint spec) in this PR: grpc/grpc-proto#143