feat(storage): promote gRPC plugin to GA #14712

ddelgrosso1 · 2024-09-17T20:06:28Z

This change is

codecov · 2024-09-17T21:16:30Z

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.50%. Comparing base (39a3faf) to head (cf5a7b5).

Files with missing lines	Patch %	Lines
...gle/cloud/storage/examples/storage_grpc_samples.cc	80.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14712   +/-   ##
=======================================
  Coverage   93.50%   93.50%           
=======================================
  Files        2323     2324    +1     
  Lines      207947   207948    +1     
=======================================
+ Hits       194434   194439    +5     
+ Misses      13513    13509    -4

Flag	Coverage Δ
	`93.50% <93.75%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ddelgrosso1 · 2024-09-19T15:26:18Z

Based on previous work by Carlos from April 2024. See: #13858

google/cloud/storage/examples/storage_grpc_samples.cc

Move the gRPC plugin functions out of the `google::cloud::storage_experimental` namespace and remove the `experimental-` prefix from the CMake and Bazel targets. I left shims in place for backwards compatibility. The `AsyncClient` remains in the `storage_experimental` namespace, it is fully functional, but we may change some APIs.

snippet-bot · 2024-09-25T15:39:28Z

Here is the summary of changes.

You are about to delete 1 region tag.

google/cloud/storage/examples/storage_grpc_samples.cc:61, tag storage_grpc_quickstart_dp

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

frankyn

I believe this is blocked until subset of RPCs are updated to throw unimplemented error?

frankyn · 2024-09-25T16:20:12Z

cmake/GoogleCloudCppFeatures.cmake

@@ -38,7 +38,7 @@ set(GOOGLE_CLOUD_CPP_EXPERIMENTAL_LIBRARIES
 )

 set(GOOGLE_CLOUD_CPP_TRANSITION_LIBRARIES # cmake-format: sort
-)
+                                          "storage_grpc")


What is the difference between transition libraris to GA libraries list below?

frankyn · 2024-09-25T16:23:04Z

google/cloud/storage/examples/storage_grpc_samples.cc

@@ -58,22 +58,11 @@ void GrpcReadWrite(std::string const& bucket_name) {
 }
 //! [grpc-read-write] [END storage_grpc_quickstart]

-//! [grpc-with-dp] [START storage_grpc_quickstart_dp]


This needs to be removed from docs to properly clean it up. I'll send a link offline.

Pinged TW about updating the docs.

Do you want to edit this text too?

google-cloud-cpp/google/cloud/storage/doc/storage-grpc.dox

Lines 21 to 24 in a60754e

1. The default endpoint (`storage.googleapis.com`) works from any hosting

environment (on-prem, GKE, GCE, other cloud providers, etc.). For best

performance on GCE or GKE consider using

`google-c2p:///storage.googleapis.com`.

dbolduc · 2024-09-26T16:51:24Z

CHANGELOG.md

@@ -174,6 +174,12 @@ the APIs in these libraries are stable, and are ready for production use.

 - [Managed Kafka API](/google/cloud/managedkafka/README.md)

+### New Libraries


This is not in the right place. We should not be editing the release notes for the v2.26.0 release.

If you want to get ahead of the CHANGELOG update, add a Storage section here, similar to the existing KMS section:

google-cloud-cpp/CHANGELOG.md

Line 22 in a60754e

dbolduc · 2024-09-26T16:55:18Z

CHANGELOG.md

+### New Libraries
+
+- [Google Cloud Storage](/google/cloud/storage/README.md) - the gRPC plugin is
+  now GA. The [Using the gRPC plugin][storage-grpc] guide describe this feature


nit: s/describe/describes/

Maybe describe some of the benefits of the gRPC plugin to actually entice a customer to adopt the new feature?

The guide says: "When using GCS from Google Compute Engine (GCE) this plugin can enable higher total throughput across large workloads that run on hundreds or thousands of VMs."

dbolduc · 2024-09-26T16:55:31Z

CHANGELOG.md

-`experimental-storage_grpc` feature, not the `storage` feature.
+`storage_grpc` feature, not the `storage` feature.


we can't change the past.

dbolduc · 2024-09-26T16:57:35Z

ci/etc/cloudcxxrc

@@ -18,7 +18,7 @@
 ALWAYS_RESET_CMAKE_CACHE=NO

 # By default compile most features.
-ENABLED_FEATURES=__ga_libraries__,__experimental_libraries__,opentelemetry,experimental-storage_grpc
+ENABLED_FEATURES=__ga_libraries__,__experimental_libraries__,opentelemetry,storage_grpc


s/,storage_grpc//

It is included in __ga_libraries__ now.

dbolduc · 2024-09-26T16:59:50Z

ci/verify_quickstart/CMakeLists.txt

@@ -56,7 +56,7 @@ function (add_cmake_quickstart_target feature library target)
        PREFIX "${PROJECT_BINARY_DIR}/cmake-${library}-prefix"
        SOURCE_DIR
            "${PROJECT_SOURCE_DIR}/../../google/cloud/${library}/quickstart"
-        BINARY_DIR "${PROJECT_BINARY_DIR}/cmake-${library}"
+        BINARY_DIR "${PROJECT_BINARY_DIR}/cmake-${feature}"


this is a good change, thanks. We might be able to consolidate the commands that runs the GCS+gRPC quickstart in cmake-install.sh. But that is something we can look at in a future PR.

dbolduc · 2024-09-26T17:06:52Z

google/cloud/storage/benchmarks/async_throughput_benchmark.cc

@@ -320,7 +320,7 @@ auto MakeAsyncClients(Configuration const& cfg,

 auto MakeClient(ClientConfig const& cc, int background_threads) {
  if (cc.transport == "GRPC") {
-    return google::cloud::storage_experimental::DefaultGrpcClient(
+    return gcs::MakeGrpcClient(


comment: I would have made the library code changes in one PR, then the build changes in another PR. All good, though.

dbolduc · 2024-09-26T17:09:25Z

google/cloud/storage/examples/storage_grpc_samples.cc

@@ -58,22 +58,11 @@ void GrpcReadWrite(std::string const& bucket_name) {
 }
 //! [grpc-read-write] [END storage_grpc_quickstart]

-//! [grpc-with-dp] [START storage_grpc_quickstart_dp]


Do you want to edit this text too?

google-cloud-cpp/google/cloud/storage/doc/storage-grpc.dox

Lines 21 to 24 in a60754e

1. The default endpoint (`storage.googleapis.com`) works from any hosting

environment (on-prem, GKE, GCE, other cloud providers, etc.). For best

performance on GCE or GKE consider using

`google-c2p:///storage.googleapis.com`.

dbolduc · 2024-09-26T17:13:27Z

google/cloud/storage/grpc_plugin.h

 */
-google::cloud::storage::Client DefaultGrpcClient(Options opts = {});
+[[deprecated(
+    "use ::google::cloud::storage::MakeGrpcClient() instead")]] inline google::


Why the inline? Consider dropping it and defining this function in the source file.

dbolduc · 2024-09-26T17:15:14Z

google/cloud/storage/quickstart/BUILD.bazel

@@ -32,7 +32,7 @@ cc_binary(
        "quickstart_grpc.cc",
    ],
    deps = [
-        "@google_cloud_cpp//:experimental-storage_grpc",


I don't think this will build if we:

cd google/cloud/storage/quickstart bazel build :quickstart_grpc

Can you try it and confirm? If it doesn't build, we should keep depending on the experimental target and leave a TODO to update to the non-experimental target when the next version of google-cloud-cpp is released and picked up in this repo by renovate-bot.

dbolduc · 2024-09-26T17:16:33Z

google/cloud/storage/quickstart/README.md

-benefit from GCS+gRPC. Furthermore, this GCS feature is not generally available
-in GCP, please talk to your sales rep if you have an interest in using this
-feature.
+The Google Cloud Storage client library includes an plugin to use gRPC as the


s/an plugin/a plugin/

product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Sep 17, 2024

ddelgrosso1 temporarily deployed to external September 17, 2024 20:39 — with GitHub Actions Inactive

ddelgrosso1 force-pushed the grpc-ga-sept-2024 branch from df7e6e8 to be8bc4b Compare September 19, 2024 15:23

ddelgrosso1 force-pushed the grpc-ga-sept-2024 branch 2 times, most recently from 5ee84b7 to cf5a7b5 Compare September 20, 2024 17:48

danielduhh reviewed Sep 24, 2024

View reviewed changes

google/cloud/storage/examples/storage_grpc_samples.cc Outdated Show resolved Hide resolved

coryan and others added 13 commits September 25, 2024 12:51

continue from Carlos previous work

78041b7

remove duplicate entry in readme

e458125

fix typo in cmake-split-install.sh

26d55db

remove storage_control from transitional

832704c

remove more references to experimental-storage_grpc

e614c8c

fix naming in storage_grpc.cmake

0d8ad8e

more grpc build fixes

b8235fa

update store_grpc abi dump

155f528

change naming in quickstart build, remove more experimental references

89bd8c1

checkers

375e8f2

fix quickstart

8fdfdd7

update customer facing references to direct path

a60754e

ddelgrosso1 force-pushed the grpc-ga-sept-2024 branch from cf5a7b5 to a60754e Compare September 25, 2024 15:14

ddelgrosso1 marked this pull request as ready for review September 25, 2024 15:39

ddelgrosso1 requested review from a team as code owners September 25, 2024 15:39

frankyn reviewed Sep 25, 2024

View reviewed changes

ddelgrosso1 temporarily deployed to external September 25, 2024 21:42 — with GitHub Actions Inactive

dbolduc reviewed Sep 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storage): promote gRPC plugin to GA #14712

feat(storage): promote gRPC plugin to GA #14712

ddelgrosso1 commented Sep 17, 2024 •

edited by scotthart

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

ddelgrosso1 commented Sep 19, 2024 •

edited

Loading

snippet-bot bot commented Sep 25, 2024

frankyn left a comment

frankyn Sep 25, 2024

frankyn Sep 25, 2024

ddelgrosso1 Sep 25, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

dbolduc Sep 26, 2024

	1. The default endpoint (`storage.googleapis.com`) works from any hosting
	environment (on-prem, GKE, GCE, other cloud providers, etc.). For best
	performance on GCE or GKE consider using
	`google-c2p:///storage.googleapis.com`.

		@@ -174,6 +174,12 @@ the APIs in these libraries are stable, and are ready for production use.

		- [Managed Kafka API](/google/cloud/managedkafka/README.md)

		### New Libraries

		`experimental-storage_grpc` feature, not the `storage` feature.
		`storage_grpc` feature, not the `storage` feature.

feat(storage): promote gRPC plugin to GA #14712

Are you sure you want to change the base?

feat(storage): promote gRPC plugin to GA #14712

Conversation

ddelgrosso1 commented Sep 17, 2024 • edited by scotthart Loading

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

ddelgrosso1 commented Sep 19, 2024 • edited Loading

snippet-bot bot commented Sep 25, 2024

frankyn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddelgrosso1 commented Sep 17, 2024 •

edited by scotthart

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

ddelgrosso1 commented Sep 19, 2024 •

edited

Loading