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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Support for http2/3 health checks? #14

Open
evantorrie opened this issue Apr 28, 2021 · 3 comments
Open

Question: Support for http2/3 health checks? #14

evantorrie opened this issue Apr 28, 2021 · 3 comments

Comments

@evantorrie
Copy link

Expected Behavior

Does NetCHASM support http2 (or later) health checks?

It's unclear from my scanning of the documentation whether NetCHASM supports making health checks to servers which support only http2 (i.e. no http/1.1 support).

Current Behavior

Change would be to add http2 support (if it does not exist).

Context

Such support would be useful for checking servers running gRPC services which are http2 based, but do not support http/1.1. We often have a situation now running Java gRPC services where a health check comes in for, say, GET /status.html HTTP/1.1 on the same port as the Java gRPC service. In Java gRPC, this results in an error and a long stack backtrace (see grpc/grpc-java#7692).

@uthiramohan
Copy link

NetCHASM uses the default version of HTTP provided by the libraries.

For example,
curl 7.62.0 and above uses HTTP 2 over TLS (HTTPS)
With versions before that HTTP 1.1 is used

@evantorrie
Copy link
Author

Not that I want it, but is there a way to force it to use HTTP 1.1 even when it's running with curl 7.62.0 and above?

@juen1jp
Copy link
Contributor

juen1jp commented May 26, 2021

As the code is currently written, it is not a configurable option. However, it would be trivial to do so.
You would want to insert the following line:
curl_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);

here:
https://github.com/yahoo/NetCHASM/blob/master/src/internal/HMWorkHealthCheckCurl.cpp#L234

That would replace the current HTTP/S calls using a curl config locked to 1.1.

A better solution would probably be to either request a feature to pass curl options directly (also wouldn't be too difficult todo) or it would be even easier to add a new checktype locked to 1.1. However, from an architectural perspective, it seems better to allow curl options to be passed through the config framework.

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

No branches or pull requests

3 participants