Hi @jdpedrie . I am trying to solve the spanner issue you submitted in gRPC repo by hooking a channel management support via call invoker, which is implement in the link above. I am testing it with zhouyihaiding/grpc-gcp-test composer package.
Spanner client needs to use spanner.grpc.config to create a call invoker with the API \Grpc\Gcp\ApiConfig.

I think I find the place: https://github.com/googleapis/gapic-generator/blob/master/src/main/resources/com/google/api/codegen/php/client_impl.snip#L367. But by doing so, all gapic client will be updated.

Oh I think I use the wrong SpannerClient. It should be the Spanner/src/SpannerClient.php instead of Spanner/src/V1/Spanner.php

My apologies for the delayed response @ZhouyihaiDing. I was out of town for the past few days. I'll take a look today!

Thanks, @jdpedrie !
I am testing the implementation downloaded via composer from a test repository if you are interested. https://github.com/ZhouyihaiDing/grpc-gcp-test
Thank you!

As a quick note in regards to src/V1/SpannerClient.php (the generated client) vs. src/SpannerClient.php (veneer client) - It actually strikes me as preferable to make the change in the generated client.

If we solve this there, we could get the benefit of this configuration amongst all our components (some of which only have generated clients). Also - the veneer wraps the generated client, so we can satisfy users who use either client.

Thanks! That makes sense. I am going to add more unit tests if there is no $api.grpc.config set.
Maybe we can do it gradually? Spanner first and then all other clients(generated client) because currently only spanner client is more likely to hit the 100 streams bound?

Sounds like a solid plan 👍

Thanks! I am going to use enable-gcp-optimizer.
Since all travis tests have passed, I am going to substitute the test composer library to the real composer library.
Thanks!

Sure thing, will review today

It's not good to let the user sets the flag to enable the grpc-gcp library by themselves. We should handle it inside cloud-php by ourself.

I removed the flag enableGcpOptimizer exposed to the client config. The support is enabled by default now and can be disabled by an environment flag. Later we can use the grpc.spanner.config file to enable/disable the support(like adding a line enable_gcp: true). Since the user doesn't the way we enable/disable the support, it won't affect the user.

Update:
When enable by default, seems all tests are passed according to the travis.

@ZhouyihaiDing, I think I may have misunderstood our previous discussion. I thought we were going to tackle this in the spanner generated client first, then figure out a process for the others.

Hi @dwsupplee .

I think it is still the plan. We first enable the gcp library for the spanner client. It is enabled inside cloud-php so the user doesn't need to config it.
The environment variable is only used for cloud-php to enable/disable it so we don't need to remove/add the core code for the first release. When the first release stays stable, we can remove this env var and use "rollback" if something goes wrong in the new lease.

I will also create a same PR to the repo GoogleCloudPlatform/google-cloud-php-spanner after this PR gets merged.

cc: @wenbozhu

Hi @dwsupplee @jdpedrie @chingor13

I think the gcp library is ready to be merged and used in the cloud-php which spanner client will use it first due to the issue #1052.

I modified the PR and removed the flag from the client config. It should only be seens and enabled/disabled by the cloud-php. The environment variable is supposed to be used for the first gcp library release. Later we can remove it and use gcp library all the time. If there are failures caused by the new release, we only need to rollback the library version inside composer.json. If you have some suggestions about it, please let me know.

Can you please take a look at this PR and merge it if there are no other concerns?

Hi @dwsupplee , to answer you the concern on GAX: there's been a long discussion between us and the gcp client library team (specifically spanner team), and we came into a conclusion that it's better that this extension library is used directly by them, instead of the GAX layer, and it's also done this way in other languages like Python and C#. This extension is highly coupled with the grpc library. Currently we make this as an add-on to fix some GCP specific issues, in future some of the extension logic may even be moved down to the grpc core layer. Also, the gcp extension features are not meant to work on all cloud APIs, currently this channel management work only applies to storage APIs like spanner and bigtable. So we believe the better place to put this library would be the client library directly.

@WeiranFang thanks for the explanation and context :).

Moving the logic down to the gRPC core layer sounds great. I didn't mean to suggest that the bulk of that logic should live in GAX, but rather the orchestration of enabling the optimizer could.

For example, we could consider adding the configuration file to the resources directory for the GAPIC. From there, when a user instantiates a GAPIC (or we do, as part of wrapping it for the handwritten layer), GAX would check to see if the configuration file exists and then enable optimization. That would give us fine-grained control over which services use the optimization (it could be controlled by whether or not the configuration file exists) and would make sure we help out our users who directly use the GAPICs just as easily as those using the handwritten veneer layer.

With that said, if you see the configuration file being no longer needed as part of moving the logic into gRPC core - then I am definitely happy with moving forward with what we have now.

Hi @dwsupplee , per discussion we had before, I have submitted PR in googleapis/gax-php#207 and googleapis/gapic-generator#2231 . Please take a look whenever you get chance. Thank you!

Now that we've ported this over, I think all we need from this PR is to introduce the configuration file, update the GAPIC, and make sure we are updated to the latest version of GAX (when it is released). 👍

Nice work on this @WeiranFang and @ZhouyihaiDing.

@WeiranFang / @ZhouyihaiDing since we moved the remaining work for this into another PR, I'll close this out. Please re-open if needed.