Skip to content

Conversation

marcoshuck
Copy link
Contributor

@marcoshuck marcoshuck commented Dec 17, 2024

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

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.
@marcoshuck marcoshuck changed the title Add List endpoint to gRPC Health service A## - Add List endpoint to gRPC Health service Dec 17, 2024
@marcoshuck
Copy link
Contributor Author

@a11r care to give it a review whenever you can? :)

Co-authored-by: Benjamin Krämer <benjamin.kraemer@alien-scripts.de>
@markdroth markdroth requested review from ejona86 and dfawley February 21, 2025 00:52
## 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.
Copy link
Member

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.

Copy link
Contributor Author

@marcoshuck marcoshuck Feb 22, 2025

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.

Copy link
Member

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:

https://github.com/grpc/grpc/blob/8418c8e805f6f98e16de6c5812c5b7fe3d035c06/src/cpp/server/health/default_health_check_service.cc#L150

@ejona86 can advise for Java, @dfawley for Go, and @murgatroid99 for Node.

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.

@markdroth markdroth changed the title A## - Add List endpoint to gRPC Health service A90: Add List Method to gRPC Health Service Feb 21, 2025
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Signed-off-by: Marcos Huck <marcos@huck.com.ar>
Copy link
Member

@ejona86 ejona86 left a 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. :shipit:

Signed-off-by: Marcos Huck <marcos@huck.com.ar>
@marcoshuck marcoshuck requested review from markdroth and ejona86 March 3, 2025 18:19
## 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.
Copy link
Member

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:

https://github.com/grpc/grpc/blob/8418c8e805f6f98e16de6c5812c5b7fe3d035c06/src/cpp/server/health/default_health_check_service.cc#L150

@ejona86 can advise for Java, @dfawley for Go, and @murgatroid99 for Node.

## 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.
Copy link
Member

Choose a reason for hiding this comment

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

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>
@marcoshuck
Copy link
Contributor Author

gRPC protobuf change: grpc/grpc-proto#143

gRPC Go change: grpc/grpc-go#8155

Co-authored-by: Marcos Huck <marcoshuck@users.noreply.github.com>
Copy link
Member

@markdroth markdroth left a 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>
@marcoshuck
Copy link
Contributor Author

I think we're ready to go! 😃 @markdroth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants