From 5cd8ee25f81bab472eff1be47b3737115d9cc738 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 19 Sep 2022 15:29:46 -0700 Subject: [PATCH] GcpObservabilityConfig: New changes (#31038) * GcpObservabilityConfig: New changes * Fix IWYU --- src/cpp/ext/gcp/BUILD | 1 + src/cpp/ext/gcp/observability.cc | 7 ++-- src/cpp/ext/gcp/observability_config.h | 21 +++------- test/cpp/ext/gcp/observability_config_test.cc | 40 ++++++++++++------- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/cpp/ext/gcp/BUILD b/src/cpp/ext/gcp/BUILD index b686607ce0e..ab54e0a7ac0 100644 --- a/src/cpp/ext/gcp/BUILD +++ b/src/cpp/ext/gcp/BUILD @@ -40,6 +40,7 @@ grpc_cc_library( external_deps = [ "absl/status", "absl/status:statusor", + "absl/types:optional", "opencensus-trace", "opencensus-trace-stackdriver_exporter", "opencensus-stats-stackdriver_exporter", diff --git a/src/cpp/ext/gcp/observability.cc b/src/cpp/ext/gcp/observability.cc index e5a2132b8d0..9cb1e46388d 100644 --- a/src/cpp/ext/gcp/observability.cc +++ b/src/cpp/ext/gcp/observability.cc @@ -24,6 +24,7 @@ #include #include "absl/status/statusor.h" +#include "absl/types/optional.h" #include "opencensus/exporters/stats/stackdriver/stackdriver_exporter.h" #include "opencensus/exporters/trace/stackdriver/stackdriver_exporter.h" #include "opencensus/trace/sampler.h" @@ -54,17 +55,17 @@ absl::Status GcpObservabilityInit() { } grpc::RegisterOpenCensusPlugin(); grpc::RegisterOpenCensusViewsForExport(); - if (!config->cloud_trace.disabled) { + if (config->cloud_trace.has_value()) { opencensus::trace::TraceConfig::SetCurrentTraceParams( {kMaxAttributes, kMaxAnnotations, kMaxMessageEvents, kMaxLinks, opencensus::trace::ProbabilitySampler( - config->cloud_trace.sampling_rate)}); + config->cloud_trace->sampling_rate)}); opencensus::exporters::trace::StackdriverOptions trace_opts; trace_opts.project_id = config->project_id; opencensus::exporters::trace::StackdriverExporter::Register( std::move(trace_opts)); } - if (!config->cloud_monitoring.disabled) { + if (config->cloud_monitoring.has_value()) { opencensus::exporters::stats::StackdriverOptions stats_opts; stats_opts.project_id = config->project_id; opencensus::exporters::stats::StackdriverExporter::Register( diff --git a/src/cpp/ext/gcp/observability_config.h b/src/cpp/ext/gcp/observability_config.h index 0db45b79efc..56efa1a16f9 100644 --- a/src/cpp/ext/gcp/observability_config.h +++ b/src/cpp/ext/gcp/observability_config.h @@ -20,6 +20,7 @@ #include #include "absl/status/statusor.h" +#include "absl/types/optional.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" @@ -29,49 +30,39 @@ namespace internal { struct GcpObservabilityConfig { struct CloudLogging { - bool disabled = false; - static const grpc_core::JsonLoaderInterface* JsonLoader( const grpc_core::JsonArgs&) { static const auto* loader = - grpc_core::JsonObjectLoader() - .OptionalField("disabled", &CloudLogging::disabled) - .Finish(); + grpc_core::JsonObjectLoader().Finish(); return loader; } }; struct CloudMonitoring { - bool disabled = false; - static const grpc_core::JsonLoaderInterface* JsonLoader( const grpc_core::JsonArgs&) { static const auto* loader = - grpc_core::JsonObjectLoader() - .OptionalField("disabled", &CloudMonitoring::disabled) - .Finish(); + grpc_core::JsonObjectLoader().Finish(); return loader; } }; struct CloudTrace { - bool disabled = false; float sampling_rate = 0; static const grpc_core::JsonLoaderInterface* JsonLoader( const grpc_core::JsonArgs&) { static const auto* loader = grpc_core::JsonObjectLoader() - .OptionalField("disabled", &CloudTrace::disabled) .OptionalField("sampling_rate", &CloudTrace::sampling_rate) .Finish(); return loader; } }; - CloudLogging cloud_logging; - CloudMonitoring cloud_monitoring; - CloudTrace cloud_trace; + absl::optional cloud_logging; + absl::optional cloud_monitoring; + absl::optional cloud_trace; std::string project_id; static const grpc_core::JsonLoaderInterface* JsonLoader( diff --git a/test/cpp/ext/gcp/observability_config_test.cc b/test/cpp/ext/gcp/observability_config_test.cc index 974dda3e3d2..96144dc787a 100644 --- a/test/cpp/ext/gcp/observability_config_test.cc +++ b/test/cpp/ext/gcp/observability_config_test.cc @@ -32,14 +32,9 @@ namespace { TEST(GcpObservabilityConfigJsonParsingTest, Basic) { const char* json_str = R"json({ - "cloud_logging": { - "disabled": true - }, - "cloud_monitoring": { - "disabled": true - }, + "cloud_logging": {}, + "cloud_monitoring": {}, "cloud_trace": { - "disabled": true, "sampling_rate": 0.05 }, "project_id": "project" @@ -50,10 +45,10 @@ TEST(GcpObservabilityConfigJsonParsingTest, Basic) { auto config = grpc_core::LoadFromJson( *json, grpc_core::JsonArgs(), &errors); ASSERT_TRUE(errors.ok()) << errors.status(); - EXPECT_TRUE(config.cloud_logging.disabled); - EXPECT_TRUE(config.cloud_monitoring.disabled); - EXPECT_TRUE(config.cloud_trace.disabled); - EXPECT_FLOAT_EQ(config.cloud_trace.sampling_rate, 0.05); + EXPECT_TRUE(config.cloud_logging.has_value()); + EXPECT_TRUE(config.cloud_monitoring.has_value()); + EXPECT_TRUE(config.cloud_trace.has_value()); + EXPECT_FLOAT_EQ(config.cloud_trace->sampling_rate, 0.05); EXPECT_EQ(config.project_id, "project"); } @@ -66,13 +61,28 @@ TEST(GcpObservabilityConfigJsonParsingTest, Defaults) { auto config = grpc_core::LoadFromJson( *json, grpc_core::JsonArgs(), &errors); ASSERT_TRUE(errors.ok()) << errors.status(); - EXPECT_FALSE(config.cloud_logging.disabled); - EXPECT_FALSE(config.cloud_monitoring.disabled); - EXPECT_FALSE(config.cloud_trace.disabled); - EXPECT_FLOAT_EQ(config.cloud_trace.sampling_rate, 0); + EXPECT_FALSE(config.cloud_logging.has_value()); + EXPECT_FALSE(config.cloud_monitoring.has_value()); + EXPECT_FALSE(config.cloud_trace.has_value()); EXPECT_TRUE(config.project_id.empty()); } +TEST(GcpObservabilityConfigJsonParsingTest, SamplingRateDefaults) { + const char* json_str = R"json({ + "cloud_trace": { + "sampling_rate": 0.05 + } + })json"; + auto json = grpc_core::Json::Parse(json_str); + ASSERT_TRUE(json.ok()) << json.status(); + grpc_core::ErrorList errors; + auto config = grpc_core::LoadFromJson( + *json, grpc_core::JsonArgs(), &errors); + ASSERT_TRUE(errors.ok()) << errors.status(); + ASSERT_TRUE(config.cloud_trace.has_value()); + EXPECT_FLOAT_EQ(config.cloud_trace->sampling_rate, 0.05); +} + TEST(GcpEnvParsingTest, NoEnvironmentVariableSet) { auto config = GcpObservabilityConfig::ReadFromEnv(); EXPECT_EQ(config.status(),