grpc v1.15 will break health check protocol

[ahmetb]

grpc 1.15 has added a new rpc to health.proto:

  rpc Watch(HealthCheckRequest) returns (stream HealthCheckResponse);	

Since we implement the service Health directly and its rpc Check, addition of this method will break at least some parts of microservices-demo.

For Go, this 100% breaks all services with [email protected]. So I went ahead and pinned all grpc-go dependencies to =v1.14.0 constraint in Gopkg.toml.

For dynamic languages like Node, Python: I would not be surprised if next time we update requirements.txt/package.json, health servers will stop starting because rpc Watch isn't implemented.

[askmeegs]

I believe this was fixed (for grpc-go) grpc/grpc-go#2318

And the go services are now running grpc 1.17 https://github.com/GoogleCloudPlatform/microservices-demo/blob/master/src/productcatalogservice/Gopkg.toml#L58

[dfawley]

Note that you must embed the Unimplemented<service_name>Server in your service implementation to avoid being broken by newly-added methods.

[ahmetb]

Let’s keep reopen. @dfawley is this only for Go and other compiled languages -or does it include dynamic languages as well?

[dfawley]

AFAIK only Go had this problem - in general, adding RPCs to a service should be considered a backward compatible change.

[ahmetb]

BTW I think we already return codes.Unimplemented for rpc Watch(). Go is probably covered from that side –until another breaking change is to be added.

[ahmetb]

@dfawley I'm looking for type UnimplementedHealthServer here https://godoc.org/google.golang.org/grpc/health/grpc_health_v1 but doesn't seem to be added there. (I see it in some other OSS projects.)

[dfawley]

Sorry for the confusion - grpc/grpc-go#3025 will add it. We needed to upgrade the protoc-gen-go version to pick up the new feature.

[daniel-sanche]

@ahmetb Is this issue still relevant?

[ahmetb]

I think we can close. grpc-go addressed this.