grpc-js: Apply timeouts from service configs

[murgatroid99]

This is the grpc-js side of the timeout part of proposal A31: the mechanism for applying a timeout from a service config to each individual request. The xDS side uses xDS v3-specific fields, so I will do that after #1765 is merged.

I changed the method config definition to more closely match the spec. The validation code still accepts the same inputs it accepted before, so that shouldn't break anything.

This puts the deadline filter in a weird state. It already had some implicit coupling with the deadline exposed by the call stream class, but now that deadline can change and the filter needs to take action when that happens. I don't really know what to do about that, so I'm deferring that to another time.

[max-mathieu]

Hi 👋

Is this going to be added to grpc-js 1.3.x ? I see it got merged in May but isn't in 1.3.7 (latest release). If not, what's the ETA for 1.4.x?
We are trying to add client timeouts across our app (we are facing the exact same issue as #1769 (comment) ) and it be cleaner/safer/more scalable to not pass a deadline option with every call, and from the docs, I thought I could use grpc.service_config for that

In the meantime, would you consider showing some warnings when validating service_config that some of the options passed are not supported by the library? Seems like retries are not supported (which is documented in https://github.com/grpc/grpc-node/blob/master/PACKAGE-COMPARISON.md), but deadlines are listed as supported there... Just thinking that I may not be the only one that will spend hours trying to understand why my service config is not taken into account when the main branch here seems to support it :)

Thanks!

[murgatroid99]

This is part of 1.4. We currently don't intend to backport it to 1.3. 1.4 will be released as soon as possible, whenever that ends up being.

The focus of the current service config validation code is validating it according to the spec, so that we don't incorrectly reject service configs retrieved over the network. But I can look into adding some logs there.

Regarding the package comparison doc, "deadlines" there refers to providing deadlines when making requests, not to configuring timeouts in service configs.

You may be right that other users have faced similar problems, but nobody has ever filed an issue or otherwise contacted us about that, so I wouldn't know about it. In fact, the goal of this PR wasn't even to support this use case, it was to support using timeouts with service configs retrieved from an xDS server.

[max-mathieu]

Thanks @murgatroid99 , appreciate the fast answer!
In the meantime, we've wrapped our calls to pass a deadline with every call, so we should be fine for now -- and I'll keep tabs on 1.4. Happy to do some beta testing if you're looking for volunteers :)

[murgatroid99]

Some comments from my team regarding the usage you described:

The locally provided service config API is really primarily intended to be used in cases where (a) you want to set a default service config for the channel to be used when the resolver cannot find a service config or does not support service configs and (b) use in testing.

There's no reason it won't work in the case you describe, although that's not necessarily the intended use of the API.

if you rely on this and then a remote service config comes along, then you either:

1. ignore it, because you disabled the remote config locally
   or
2. use its values, which might be missing a deadline on some RPC you were relying upon

"safer" is to ensure your RPCs always have explicit deadlines determined by the caller. Add an interceptor and panic if one isn't set if you're worried you will forget to set one.