From 293313c8f451e82f865cf98eead1849d25dc7b0b Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Mon, 10 Aug 2026 15:12:35 -0400 Subject: [PATCH 1/2] impl(bigtable): add benchmark command line arg to set metrics period --- google/cloud/bigtable/benchmarks/benchmark.cc | 4 ++++ .../cloud/bigtable/benchmarks/benchmark_options.cc | 11 +++++++++++ google/cloud/bigtable/benchmarks/benchmark_options.h | 2 ++ .../bigtable/benchmarks/benchmark_options_test.cc | 12 +++++++++++- .../bigtable/benchmarks/bigtable_benchmark_test.cc | 12 ++++++++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/google/cloud/bigtable/benchmarks/benchmark.cc b/google/cloud/bigtable/benchmarks/benchmark.cc index b58aeb9d3af2b..30922d01bb11d 100644 --- a/google/cloud/bigtable/benchmarks/benchmark.cc +++ b/google/cloud/bigtable/benchmarks/benchmark.cc @@ -129,6 +129,10 @@ void Benchmark::DeleteTable() { Table Benchmark::MakeTable(Options connection_opts) const { auto connection_options = MergeOptions(std::move(connection_opts), opts_); + if (options_.metrics_period.has_value()) { + connection_options.set( + *options_.metrics_period); + } auto table_opts = Options{}.set(options_.app_profile_id); return Table( MakeDataConnection({InstanceResource(Project(options_.project_id), diff --git a/google/cloud/bigtable/benchmarks/benchmark_options.cc b/google/cloud/bigtable/benchmarks/benchmark_options.cc index 47da15650bf25..e85159c874ebc 100644 --- a/google/cloud/bigtable/benchmarks/benchmark_options.cc +++ b/google/cloud/bigtable/benchmarks/benchmark_options.cc @@ -100,6 +100,11 @@ google::cloud::StatusOr ParseBenchmarkOptions( [&options](std::string const& val) { options.enable_metrics = ParseBoolean(val).value_or(true); }}, + {"--metrics-period", + "the frequency at which client-side metrics are exported", + [&options](std::string const& val) { + options.metrics_period = ParseDuration(val); + }}, }; auto usage = BuildUsage(desc, argv[0]); @@ -164,6 +169,12 @@ google::cloud::StatusOr ParseBenchmarkOptions( << "). Check your --test-duration option.\n"; return make_status(os, GCP_ERROR_INFO()); } + if (options.metrics_period && options.metrics_period->count() <= 0) { + std::ostringstream os; + os << "Invalid metrics period seconds (" << options.metrics_period->count() + << "). Check your --metrics-period option.\n"; + return make_status(os, GCP_ERROR_INFO()); + } return options; } diff --git a/google/cloud/bigtable/benchmarks/benchmark_options.h b/google/cloud/bigtable/benchmarks/benchmark_options.h index d9e157318c545..b8a5bf3fd3bf4 100644 --- a/google/cloud/bigtable/benchmarks/benchmark_options.h +++ b/google/cloud/bigtable/benchmarks/benchmark_options.h @@ -18,6 +18,7 @@ #include "google/cloud/bigtable/benchmarks/constants.h" #include "google/cloud/status_or.h" #include +#include #include #include @@ -42,6 +43,7 @@ struct BenchmarkOptions { bool exit_after_parse = false; bool include_read_rows = false; bool enable_metrics = true; + std::optional metrics_period; }; google::cloud::StatusOr ParseBenchmarkOptions( diff --git a/google/cloud/bigtable/benchmarks/benchmark_options_test.cc b/google/cloud/bigtable/benchmarks/benchmark_options_test.cc index 246d3cfe53e8f..d3f0efb6fe412 100644 --- a/google/cloud/bigtable/benchmarks/benchmark_options_test.cc +++ b/google/cloud/bigtable/benchmarks/benchmark_options_test.cc @@ -35,7 +35,7 @@ TEST(BenchmarkOptions, Basic) { {"self-test", "--project-id=test-project", "--instance-id=test-instance", "--app-profile-id=test-app-profile-id", "--table-size=10000", "--test-duration=300s", "--use-embedded-server=true", - "--include-read-rows=true"}, + "--include-read-rows=true", "--metrics-period=10s"}, ""); ASSERT_STATUS_OK(options); EXPECT_FALSE(options->exit_after_parse); @@ -46,6 +46,8 @@ TEST(BenchmarkOptions, Basic) { EXPECT_EQ(300, options->test_duration.count()); EXPECT_EQ(true, options->use_embedded_server); EXPECT_EQ(true, options->include_read_rows); + EXPECT_THAT(options->metrics_period, + ::testing::Optional(std::chrono::seconds(10))); } TEST(BenchmarkOptions, Defaults) { @@ -64,6 +66,7 @@ TEST(BenchmarkOptions, Defaults) { options->test_duration.count()); EXPECT_EQ(false, options->use_embedded_server); EXPECT_EQ(10, options->parallel_requests); + EXPECT_FALSE(options->metrics_period.has_value()); } TEST(BenchmarkOptions, Initialization) { @@ -105,6 +108,13 @@ TEST(BenchmarkOptions, Validate) { EXPECT_FALSE(ParseBenchmarkOptions( {"self-test", "--project-id=a", "--instance-id=b", "--test-duration=0"}, "")); + EXPECT_FALSE(ParseBenchmarkOptions( + {"self-test", "--project-id=a", "--instance-id=b", "--metrics-period=0s"}, + "")); + EXPECT_FALSE( + ParseBenchmarkOptions({"self-test", "--project-id=a", "--instance-id=b", + "--metrics-period=-5s"}, + "")); } } // namespace diff --git a/google/cloud/bigtable/benchmarks/bigtable_benchmark_test.cc b/google/cloud/bigtable/benchmarks/bigtable_benchmark_test.cc index afb4b406b6a56..9d602db11d319 100644 --- a/google/cloud/bigtable/benchmarks/bigtable_benchmark_test.cc +++ b/google/cloud/bigtable/benchmarks/bigtable_benchmark_test.cc @@ -53,6 +53,18 @@ TEST(BenchmarkTest, Create) { SUCCEED() << "Benchmark object successfully destroyed"; } +TEST(BenchmarkTest, MakeTableWithMetricsPeriod) { + char arg8[] = "--metrics-period=10s"; + char* argv[] = {arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8}; + int argc = sizeof(argv) / sizeof(argv[0]); + auto options = ParseArgs(argc, argv, ""); + ASSERT_STATUS_OK(options); + + Benchmark bm(*options); + auto table = bm.MakeTable(); + SUCCEED(); +} + TEST(BenchmarkTest, Populate) { char* argv[] = {arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7}; int argc = sizeof(argv) / sizeof(argv[0]); From 43c4a89cf3a06ae3a1984984466005a781c893d2 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Mon, 10 Aug 2026 17:41:33 -0400 Subject: [PATCH 2/2] Update google/cloud/bigtable/benchmarks/benchmark.cc Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- google/cloud/bigtable/benchmarks/benchmark.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigtable/benchmarks/benchmark.cc b/google/cloud/bigtable/benchmarks/benchmark.cc index 30922d01bb11d..955f6505a6f53 100644 --- a/google/cloud/bigtable/benchmarks/benchmark.cc +++ b/google/cloud/bigtable/benchmarks/benchmark.cc @@ -130,7 +130,7 @@ void Benchmark::DeleteTable() { Table Benchmark::MakeTable(Options connection_opts) const { auto connection_options = MergeOptions(std::move(connection_opts), opts_); if (options_.metrics_period.has_value()) { - connection_options.set( + connection_options.set( *options_.metrics_period); } auto table_opts = Options{}.set(options_.app_profile_id);