Add health checks to Node.js services #26
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googlebot
added
the
cla: yes
ahmetb
suggested changes
Aug 20, 2018
src/currencyservice/.gitignore
Outdated
| @@ -1 +1,2 @@ | |||
| proto/ | |||
There was a problem hiding this comment.
You should probably undo this.
We want the app to be buildable from source just after git-clone, so we should keep the duplicate protos –it's fine for now (until/if we decide to use Bazel).
pb/demo.proto
Outdated
| @@ -156,6 +157,7 @@ message CurrencyConversionRequest { | |||
|
|
|||
| service PaymentService { | |||
| rpc Charge(ChargeRequest) returns (ChargeResponse) {} | |||
| rpc Check(Empty) returns (Empty) {} | |||
There was a problem hiding this comment.
This would come from the standard gRPC HC protocol: https://github.com/grpc/grpc/blob/master/src/proto/grpc/health/v1/health.proto
Also: https://github.com/grpc/grpc/blob/master/doc/health-checking.md
So we need to implement the grpc.health.v1.Health's rpc Check instead. This would
- download health.proto to pb/
- update genproto scripts for node
- register for Health service in app
- implement rpc Check(HealthCheckRequest) returns (HealthCheckResponse); from the health.proto
this gets currencyservice to work but paymentservice is still crashing in the docker container. Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
|
I pushed a commit that fixes genproto.sh path issue in these services + ran the scripts.
|
|
This works fine now. |
ahmetb
approved these changes
Aug 21, 2018
Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
D-Mwanth
pushed a commit
to D-Mwanth/microservices-demo
that referenced
this pull request
Mar 6, 2024
* Move Node healthchecks to gRPC * gitignore proto files * Switch to standard health RPC * Fix lint * Update client.js * Add protos back + update them * node services: fix & run genproto.sh this gets currencyservice to work but paymentservice is still crashing in the docker container. Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com> * Fix docker breaking * update dockerfiles with released health probe Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FYI: I've tested these locally, but not with Docker.