Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom HTTP health check endpoint path #2572

Closed
antonioiubatti93 opened this issue Mar 8, 2022 · 5 comments 路 Fixed by #2587
Closed

Custom HTTP health check endpoint path #2572

antonioiubatti93 opened this issue Mar 8, 2022 · 5 comments 路 Fixed by #2587

Comments

@antonioiubatti93
Copy link
Contributor

馃殌 Feature

Thank you @brumhard and @johanbrandhorst for having the following HTTP health check endpoint available through the gRPC gateway, this is very useful 馃帀

To further enhance that, I would suggest to customize the endpoint path in order to use any path other than /healthz, so that any existing health check endpoints shall not undergo any disruption.

@johanbrandhorst
Copy link
Collaborator

This sounds like a reasonable request. I'm not actually sure what the best way to accomplish this is... we might need a new WithHealthEndpointPath or something that does what WithHealthzEndpoint does but with a path parameter. Then WithHealthzEndpoint can be implemented as WithHealthEndpointPath(healthCheckClient, "/healthz"). @brumhard any thoughts?

@brumhard
Copy link
Contributor

brumhard commented Mar 9, 2022

I already thought about that while implementing the PR but didn't come up with a nice solution and then lost track somehow. My idea was some kind of nested functional option, but I think your solution looks nicer than WithHealthzEndpoint(healthCheckClient, WithPath("/health")). I can implement the WithHealthEndpointPath if you want 馃檪

@antonioiubatti93
Copy link
Contributor Author

@johanbrandhorst @brumhard those are indeed the two solutions I would have suggested myself 馃憤

Imho functional options may be overkill to simply apply a non-default endpoint path in this case, so I would favor more the idea of having a WithHealthEndpointPath mux option, and reuse it for WithHealthzEndpoint.
However, should anyone need to apply other non-default configuration, having functional options might become handy (I'm thinking about the pathParams map[string]string for example).

I can implement the WithHealthEndpointPath if you want

@brumhard that would be awesome, thank you 馃槃 ofc I'm also available to give it a try myself if you prefer 馃憤 In that case, I'll try to take care of it according to your preferences on the above and ping you guys in review.

@brumhard
Copy link
Contributor

@antonioiubatti93 sry was a bit afk last week but glad that you got it working to your needs 馃憤馃徎

@antonioiubatti93
Copy link
Contributor Author

@brumhard no problem, I had actually very little to do, the base feature was very nice already 馃槃

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 a pull request may close this issue.

3 participants