From 9b8178d99f226ae4806d91038169bd171616b8d7 Mon Sep 17 00:00:00 2001 From: Shubham Kaushal Date: Wed, 15 Oct 2025 16:01:01 +0000 Subject: [PATCH 1/4] making metrics export timeout configurable --- google/cloud/storage/grpc_plugin.h | 30 +++++++++++++++++++ .../storage/internal/grpc/default_options.cc | 1 + .../internal/grpc/metrics_exporter_impl.cc | 4 +-- .../grpc/metrics_exporter_impl_test.cc | 16 ++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/google/cloud/storage/grpc_plugin.h b/google/cloud/storage/grpc_plugin.h index 045c0f7fea0c5..848963dc723c5 100644 --- a/google/cloud/storage/grpc_plugin.h +++ b/google/cloud/storage/grpc_plugin.h @@ -102,6 +102,36 @@ struct GrpcMetricsPeriodOption { using Type = std::chrono::seconds; }; +/** + * gRPC telemetry export timeout. + * + * When `EnableGrpcMetrics` is enabled, this option controls the maximum time + * to wait for metrics to be exported to [Google Cloud Monitoring]. The default + * is 5 seconds. + * + * This timeout is particularly important for short-lived programs. Setting a + * lower timeout ensures metrics are flushed before the program exits. For + * long-running services, the default value is appropriate. + * + * @par Example: Configure for short-lived programs + * @code + * auto client = google::cloud::storage::MakeGrpcClient( + * google::cloud::Options{} + * .set( + * true) + * .set( + * std::chrono::seconds(5)) + * .set( + * std::chrono::seconds(2))); + * @endcode + * + * [Google Cloud Monitoring]: https://cloud.google.com/monitoring/docs + */ +struct GrpcMetricsExportTimeoutOption { + using Type = std::chrono::seconds; +}; + GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_experimental } // namespace cloud diff --git a/google/cloud/storage/internal/grpc/default_options.cc b/google/cloud/storage/internal/grpc/default_options.cc index e693ace81ad81..f1234599fda6f 100644 --- a/google/cloud/storage/internal/grpc/default_options.cc +++ b/google/cloud/storage/internal/grpc/default_options.cc @@ -36,6 +36,7 @@ namespace { auto constexpr kMinMetricsPeriod = std::chrono::seconds(5); auto constexpr kDefaultMetricsPeriod = std::chrono::seconds(60); +auto constexpr kDefaultMetricsExportTimeout = std::chrono::seconds(30); int DefaultGrpcNumChannels(std::string const& endpoint) { // When using Direct Connectivity the gRPC library already does load balancing diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_impl.cc b/google/cloud/storage/internal/grpc/metrics_exporter_impl.cc index 9e883d20852bd..2e6ae92ba5a6f 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_impl.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_impl.cc @@ -49,8 +49,8 @@ auto MakeReaderOptions(Options const& options) { opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions{}; reader_options.export_interval_millis = std::chrono::milliseconds( options.get()); - reader_options.export_timeout_millis = - std::chrono::milliseconds(std::chrono::seconds(30)); + reader_options.export_timeout_millis = std::chrono::milliseconds( + options.get()); return reader_options; } diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc index 59348cc42763a..0a8e5756602c7 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc @@ -110,6 +110,22 @@ TEST(GrpcMetricsExporter, EnabledWithTimeout) { std::chrono::milliseconds(0)); } +TEST(GrpcMetricsExporter, CustomExportTimeout) { + auto config = MakeMeterProviderConfig( + FullResource(), + TestOptions() + .set( + std::chrono::seconds(10)) + .set( + std::chrono::seconds(2))); + ASSERT_TRUE(config.has_value()); + EXPECT_EQ(config->project, Project("project-id-resource")); + EXPECT_EQ(config->reader_options.export_interval_millis, + std::chrono::seconds(10)); + EXPECT_EQ(config->reader_options.export_timeout_millis, + std::chrono::seconds(2)); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_internal From 078172d686320bfb4409161ac9d586c1c2807758 Mon Sep 17 00:00:00 2001 From: Shubham Kaushal Date: Thu, 16 Oct 2025 07:08:25 +0000 Subject: [PATCH 2/4] resolving comments --- google/cloud/storage/grpc_plugin.h | 11 +++++------ google/cloud/storage/internal/grpc/default_options.cc | 4 +++- .../internal/grpc/metrics_exporter_impl_test.cc | 10 ++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/google/cloud/storage/grpc_plugin.h b/google/cloud/storage/grpc_plugin.h index 848963dc723c5..167ecb8dcb609 100644 --- a/google/cloud/storage/grpc_plugin.h +++ b/google/cloud/storage/grpc_plugin.h @@ -107,7 +107,7 @@ struct GrpcMetricsPeriodOption { * * When `EnableGrpcMetrics` is enabled, this option controls the maximum time * to wait for metrics to be exported to [Google Cloud Monitoring]. The default - * is 5 seconds. + * is 30 seconds. * * This timeout is particularly important for short-lived programs. Setting a * lower timeout ensures metrics are flushed before the program exits. For @@ -115,14 +115,13 @@ struct GrpcMetricsPeriodOption { * * @par Example: Configure for short-lived programs * @code + * namespace gcs_ex = google::cloud::storage_experimental; * auto client = google::cloud::storage::MakeGrpcClient( * google::cloud::Options{} - * .set( - * true) - * .set( + * .set(true) + * .set( * std::chrono::seconds(5)) - * .set( + * .set( * std::chrono::seconds(2))); * @endcode * diff --git a/google/cloud/storage/internal/grpc/default_options.cc b/google/cloud/storage/internal/grpc/default_options.cc index f1234599fda6f..abd150c40d9ea 100644 --- a/google/cloud/storage/internal/grpc/default_options.cc +++ b/google/cloud/storage/internal/grpc/default_options.cc @@ -113,7 +113,9 @@ Options DefaultOptionsGrpc( .set( enable_grpc_metrics) .set( - kDefaultMetricsPeriod)); + kDefaultMetricsPeriod) + .set( + kDefaultMetricsExportTimeout)); if (options.get() < kMinMetricsPeriod) { options.set( diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc index 0a8e5756602c7..644d27e33d444 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc @@ -110,6 +110,16 @@ TEST(GrpcMetricsExporter, EnabledWithTimeout) { std::chrono::milliseconds(0)); } +TEST(GrpcMetricsExporter, DefaultExportTimeout) { + auto config = MakeMeterProviderConfig(FullResource(), TestOptions()); + ASSERT_TRUE(config.has_value()); + EXPECT_EQ(config->project, Project("project-id-resource")); + EXPECT_EQ(config->reader_options.export_interval_millis, + std::chrono::seconds(60)); + EXPECT_EQ(config->reader_options.export_timeout_millis, + std::chrono::seconds(30)); +} + TEST(GrpcMetricsExporter, CustomExportTimeout) { auto config = MakeMeterProviderConfig( FullResource(), From c10c4d537e2067febfcb610c8b94f1cfbcf2ed01 Mon Sep 17 00:00:00 2001 From: Shubham Kaushal Date: Thu, 16 Oct 2025 07:13:16 +0000 Subject: [PATCH 3/4] minor typo --- .../cloud/storage/internal/grpc/metrics_exporter_impl_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc index 644d27e33d444..ef21dcd3721a3 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc @@ -110,7 +110,7 @@ TEST(GrpcMetricsExporter, EnabledWithTimeout) { std::chrono::milliseconds(0)); } -TEST(GrpcMetricsExporter, DefaultExportTimeout) { +TEST(GrpcMetricsExporter, DefaultExportTime) { auto config = MakeMeterProviderConfig(FullResource(), TestOptions()); ASSERT_TRUE(config.has_value()); EXPECT_EQ(config->project, Project("project-id-resource")); @@ -120,7 +120,7 @@ TEST(GrpcMetricsExporter, DefaultExportTimeout) { std::chrono::seconds(30)); } -TEST(GrpcMetricsExporter, CustomExportTimeout) { +TEST(GrpcMetricsExporter, CustomExportTime) { auto config = MakeMeterProviderConfig( FullResource(), TestOptions() From 8fb97529c64a442824508039bd56335ceff9d189 Mon Sep 17 00:00:00 2001 From: Shubham Kaushal Date: Thu, 16 Oct 2025 09:03:28 +0000 Subject: [PATCH 4/4] code coverage --- .../grpc/metrics_exporter_impl_test.cc | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc index ef21dcd3721a3..b266f262c61e9 100644 --- a/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc +++ b/google/cloud/storage/internal/grpc/metrics_exporter_impl_test.cc @@ -136,6 +136,26 @@ TEST(GrpcMetricsExporter, CustomExportTime) { std::chrono::seconds(2)); } +TEST(GrpcMetricsExporter, ReaderOptionsAreSetFromConfig) { + auto const expected_interval = std::chrono::seconds(45); + auto const expected_timeout = std::chrono::seconds(25); + + auto config = MakeMeterProviderConfig( + FullResource(), + TestOptions() + .set(expected_interval) + .set( + expected_timeout)); + + ASSERT_TRUE(config.has_value()); + + // Verify the conversion from seconds to milliseconds happens correctly. + EXPECT_EQ(config->reader_options.export_interval_millis, + std::chrono::milliseconds(expected_interval)); + EXPECT_EQ(config->reader_options.export_timeout_millis, + std::chrono::milliseconds(expected_timeout)); +} + } // namespace GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace storage_internal