[feature] HTTP/2 and gRPC health-checks support

[ashald]

Given the raising popularity of HTTP/2 and gRPC it'd be nice to be able to health-check such services natively with Consul.

[skyrocknroll]

It would be really great to have this addition. Right now we are using tcp health that creates Exception at the server side. One workaround is we can use script check.

[ghost]

That would be fine +

[matthewfarwell]

I was just wondering, what sort of form would this take?

My proposal is that we would have a grpc service which takes empty message and returns something else (maybe even empty). If the return was OK, then the service is ok, otherwise it isn't.

One issue would be how do we transmit information? Currently, we always return JSON, even in case of error, so that this is stored in Consul and we can diagnose any problems. Maybe metadata is the way to go for this?

[skyrocknroll]

There is already an standard defined by GRPC. https://github.com/grpc/grpc/blob/master/doc/health-checking.md . It would great if we follow that.

[ashald]

We just released https://github.com/ncbi/gprobe and going to use it with script check in Consul. Our plan is to contribute same feature into Consul codebase if @slackpad will be willing to accept such a PR

[slackpad]

Hey @ashald this looks like a simple addition - we'd welcome a PR fo it!

[ashald]

Great! @edio already has it on his todo list!

[Anricx]

So will it be support in future version?

[koiuo]

I have feature implemented, however, there's issue with dependencies that blocks me from creating a PR

TLDR

vendored x/net/trace dependency conflicts with grpc dependency.

Long version:

consul has a dependency to hashicorp/go-discover

hashicorp/go-discover has gce provider, which vendors package golang.org/x/net/trace

golang.org/x/net/trace/trace.go has init() function with the following content

func init() {
	http.HandleFunc("/debug/requests", func(w http.ResponseWriter, req *http.Request) {
...

The same package is imported by grpc-go (not vendored).

This, as far as I understand, leads to a situation, where the same init() function is executed twice. Which, in turn, causes panic on consul initialization

panic: http: multiple registrations for /debug/requests

goroutine 1 [running]:
net/http.(*ServeMux).Handle(0x1f7eb80, 0x14f8430, 0xf, 0x1e10100, 0x1544bd8)
        /usr/lib/go/src/net/http/server.go:2270 +0x627
net/http.(*ServeMux).HandleFunc(0x1f7eb80, 0x14f8430, 0xf, 0x1544bd8)
        /usr/lib/go/src/net/http/server.go:2302 +0x55
net/http.HandleFunc(0x14f8430, 0xf, 0x1544bd8)
        /usr/lib/go/src/net/http/server.go:2314 +0x4b
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/golang.org/x/net/trace.init.0()
        /home/edio/development/go/src/github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/golang.org/x/net/trace/trace.go:114 +0x42
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/golang.org/x/net/trace.init()
        <autogenerated>:1 +0x549
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/google.golang.org/grpc.init()
        <autogenerated>:1 +0x82
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/github.com/googleapis/gax-go.init()
        <autogenerated>:1 +0x4e
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/cloud.google.com/go/internal.init()
        <autogenerated>:1 +0x53
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/cloud.google.com/go/compute/metadata.init()
        <autogenerated>:1 +0x8c
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce/vendor/golang.org/x/oauth2/google.init()
        <autogenerated>:1 +0x8c
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover/provider/gce.init()
        <autogenerated>:1 +0x5d
github.com/hashicorp/consul/vendor/github.com/hashicorp/go-discover.init()
        <autogenerated>:1 +0x7d
github.com/hashicorp/consul/agent.init()
        <autogenerated>:1 +0x172
github.com/hashicorp/consul/command/agent.init()
        <autogenerated>:1 +0x89
github.com/hashicorp/consul/command.init()
        <autogenerated>:1 +0x48
main.init()
        <autogenerated>:1 +0x58

I'm not that familiar with the codebase to propose any solution.

Just bringing this up for your attention and looking for any advice on resolving/workarounding this.

[koiuo]

just FYI, unvendoring x/net/trace from go-discover solves the issue: tests pass, agent starts without panicking.
Do you think we could do the change to go-discover?

[slackpad]

Hi @edio I just pushed a change via #3780 that monkey patches go-discover to remove these registrations.