feat: to support GRPC-GCP Extension, include additional options in grpcOptions

[alexander-fenster]

@crwilcox pointed out that when we filter the almighty options object for specific gRPC-related options, we take options prefixed with grpc. and then pass then to gRPC as is. It turns out gRPC does not expect this prefix (see the snippet below), so let's remove it.

https://github.com/grpc/grpc-node/blob/8df65a91a24b8c9e3e74d6c957cfa5905c902f86/packages/grpc-native-core/src/client.js#L367-L387

[crwilcox]

I applied this and ran the tests we have for spanner. It seems there are options passed here from spanner (possibly other clients) that aren't valid for grpc.

(node:16412) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 879)
(node:16412) UnhandledPromiseRejectionWarning: TypeError: Channel options must be an object with string keys and integer or string values
    at ServiceClient.Client (/Users/crwilcox/workspace/google-cloud-node/nodejs-spanner/node_modules/grpc/src/client.js:400:23)
    at new ServiceClient (/Users/crwilcox/workspace/google-cloud-node/nodejs-spanner/node_modules/grpc/src/client.js:823:12)
    at /Users/crwilcox/workspace/google-cloud-node/nodejs-spanner/node_modules/google-gax/build/src/grpc.js:307:20
    at process._tickCallback (internal/process/next_tick.js:68:7)

This might be the reason for the filtering. The grpc options requires that it needs to be string keys with string/integer values.

[alexander-fenster]

Since you have applied the fix, may I ask you to dump both options and grpcOptions in the code I changed? Something like console.log(JSON.stringify(options, null, ' ') + '\n' + JSON.stringify(grpcOptions, null, ' '));

[alexander-fenster]

I actually believe Spanner sets some options that previously were never seen by gRPC and now they are processed (and something goes wrong).

[crwilcox]

Editing as there was an issue with the code I ran for it

 GrpcClient.prototype.createStub = function (CreateStub, options) {
        var serviceAddress = options.servicePath + ':' + options.port;
        return this._getCredentials(options).then(function (credentials) {
            var grpcOptions = {};
            var fensterGrpcOptions = {};
            Object.keys(options).forEach(function (key) {
                if (key.indexOf('grpc.') === 0) {
                     grpcOptions[key] = options[key];
                }
                
               const match = key.match(/^grpc\.(.*)$/);
                if (match) {
                    fensterGrpcOptions[match[1]] = options[key];
                }
            });
            console.log("GRPC: " + JSON.stringify(grpcOptions, null, ' '))
            console.log("OPTIONS: " + JSON.stringify(options, null, ' '))
            console.log("FENSTER-GRPCOPTIONS: " + JSON.stringify(fensterGrpcOptions, null, ' '))
            return new CreateStub(serviceAddress, credentials, grpcOptions, options);
        });
    };
    /**

FENSTER-GRPCOPTIONS: {}
GRPC: {}
OPTIONS: {
 "clientConfig": {},
 "port": 443,
 "servicePath": "spanner.googleapis.com",
 "credentials": {
  "client_email": "bogus",
  "private_key": "bogus"
 },
 "projectId": "bogus",
 "scopes": [
  "https://www.googleapis.com/auth/cloud-platform",
  "https://www.googleapis.com/auth/spanner.admin"
 ]
}

[JustinBeckwith]

I have no context here :) Happy to let @crwilcox and @kinwa91 make a call.

[jkwlui]

Approving the code change ✅
I am not super familiar with the options that grpc accepts though.. I'll let @crwilcox investigate more and figure out if this is a good change.

[crwilcox]

Changed title to better match what the purpose of this work is.

cc: @WeiranFang

[alexander-fenster]

The code now does not match the tests (or vice versa).

[crwilcox]

Holding merge until after a new version of GRPC releases.