Consider passing Options to all Client methods

[devjgm]

(filing an issue so we don't forget about this)

We sometimes need to the ability to pass method-specific options to a client function call. For example:

google-cloud-cpp/google/cloud/spanner/client.h

Lines 250 to 253 in 0b2f1e5

	StatusOr<std::vector<ReadPartition>> PartitionRead(
	Transaction transaction, std::string table, KeySet keys,
	std::vector<std::string> columns, ReadOptions read_options = {},
	PartitionOptions const& partition_options = PartitionOptions{});

Rather than having a bunch of separate options classes, we may just want to pass a google::cloud::Options opts = {} argument as the last argument to all client methods. This would help us be future-proof as it would let us add more options to a method call without breaking existing calls.

We could use something like this to add per-method timeouts.

We may want to call this Context rather than Options if there's a meaningful semantic difference between the two.

We'll need to do this in the generator too.

[dbolduc]

(mostly repeating what @devjgm, but maybe adding an original thought or two)

Overview

This would offer a general framework for implementing a solution to something like #4926. (In addition to timeouts, it could be used to add Metadata, serve some service specific function, etc.).

The Client calls would all gain an Options parameter:

// calls like:
Status Foo(FooRequest const& request);
// ...become:
Status Foo(FooRequest const& request, Options const& options = {});

The Connection calls would also gain an Options parameter.

Details / Open Questions

Jotting down some thoughts from a previous discussion with the team:

• We probably don't want to default a parameter for a virtual member function in Connection.
• We should consider making these non-pure virtual functions. This could prevent breaks of Connection mocks when a new RPC is added to a service for users that do NOT use our provided mocks + the Google Mock framework.
• Implementing this for GA services constitutes a breaking change. (Maybe the code will compile, but the implementation details will change, making any current mocks useless).
  • We will need to add a configuration setting to the generator to enable/disable the generation of the Options parameter.
  • How and when to migrate currently GA libraries to using per-call options is an open question.

While we probably have enough direction to start implementing the API surface, I think we need a clearer picture of what a per-call option implementation looks like.

• gRPC configuration
  • Could be applied to configure the context just before it is passed to the stub layer
  • Could be applied in our generic loop helpers (e.g. RetryLoop)
  • Could be applied as another stub decorator. We would need to think about whether this decorator takes effect before or after other decorators (e.g. auth).
  • Would these Options apply for poll and cancel operations?
• REST configuration (currently just Storage)
  • Has storage::DownloadStallTimeoutOption and storage::TransferStallTimeoutOption
  • They are applied to the CurlClient. (So all requests with the same client have the same timeouts. Is this a problem? I don't know.)
  • They set CURLOPT_CONNECTTIMEOUT - (transfer) (download)
• Service-specific configuration
  • Do they exist?
  • If so, I think they would be handwritten. The developer can decide how to implement them when the time comes.

Appendix

Just to give an idea of what I currently have in mind (setting deadlines) (for gRPC):

/// FILE: google/cloud/common_options.h
namespace google::cloud {
struct DeadlineOption {
  using Type = std::chrono::time_point<std::chrono::system_clock>;
};
}  // namespace

/// FILE: google/cloud/grpc_options.h
namespace google::cloud::internal {
struct GrpcSetupOption {
  using Type = std::function<void(grpc::ClientContext&)>;
};

// TODO : Whether this alters a ClientContext& or returns a new, configured
//              ClientContext can be figured out at a later date.
void ConfigureContext(grpc::ClientContext& context, Options const& opts) {
  // TODO : Which option takes precedence, and whether they are exclusive
  //        can be decided later.
  if (opts.has<GrpcSetupOption>()) {
    opts.get<GrpcSetupOption>()(context);
  } else if (opts.has<DeadlineOption>()) {
    context.set_deadline(opts.get<DeadlineOption>());
  }
}
}  // namespace

[devbww]

For the record, I currently have #7669 out for review, which adds the notion of "call-tree-specific options" (you can call it "context" if you like). And then I have a PR queued up behind that which adds the Options options = {} arguments to the generated Client ctor and calls (and another in progress behind that to add them to the Spanner data client).

The Connection calls would also gain an Options parameter.

In that world this is no longer necessary, which removes many of the open questions above.

[coryan]

This is implemented for generated libraries, and we have separate bugs for each library.

[coryan]

See #8164