From 2ae15acfeb42f8d775f7acffcf2dc75efaaa3421 Mon Sep 17 00:00:00 2001 From: yijiem Date: Wed, 10 Apr 2024 18:33:00 -0700 Subject: [PATCH] remove synchronous gauge API --- src/core/lib/channel/metrics.cc | 64 ------------------------- src/core/lib/channel/metrics.h | 40 ---------------- src/cpp/ext/otel/otel_plugin.cc | 5 -- src/cpp/ext/otel/otel_plugin.h | 8 ---- test/core/channel/metrics_test.cc | 74 ----------------------------- test/core/util/fake_stats_plugin.cc | 18 ------- test/core/util/fake_stats_plugin.h | 68 -------------------------- 7 files changed, 277 deletions(-) diff --git a/src/core/lib/channel/metrics.cc b/src/core/lib/channel/metrics.cc index 7f687975bc9..3d8deec6188 100644 --- a/src/core/lib/channel/metrics.cc +++ b/src/core/lib/channel/metrics.cc @@ -160,70 +160,6 @@ GlobalInstrumentsRegistry::RegisterDoubleHistogram( return handle; } -GlobalInstrumentsRegistry::GlobalInt64GaugeHandle -GlobalInstrumentsRegistry::RegisterInt64Gauge( - absl::string_view name, absl::string_view description, - absl::string_view unit, absl::Span label_keys, - absl::Span optional_label_keys, - bool enable_by_default) { - auto& instruments = GetInstrumentList(); - for (const auto& descriptor : instruments) { - if (descriptor.name == name) { - Crash( - absl::StrFormat("Metric name %s has already been registered.", name)); - } - } - uint32_t index = instruments.size(); - GPR_ASSERT(index < std::numeric_limits::max()); - GlobalInstrumentDescriptor descriptor; - descriptor.value_type = ValueType::kInt64; - descriptor.instrument_type = InstrumentType::kGauge; - descriptor.index = index; - descriptor.enable_by_default = enable_by_default; - descriptor.name = name; - descriptor.description = description; - descriptor.unit = unit; - descriptor.label_keys = {label_keys.begin(), label_keys.end()}; - descriptor.optional_label_keys = {optional_label_keys.begin(), - optional_label_keys.end()}; - instruments.push_back(std::move(descriptor)); - GlobalInt64GaugeHandle handle; - handle.index = index; - return handle; -} - -GlobalInstrumentsRegistry::GlobalDoubleGaugeHandle -GlobalInstrumentsRegistry::RegisterDoubleGauge( - absl::string_view name, absl::string_view description, - absl::string_view unit, absl::Span label_keys, - absl::Span optional_label_keys, - bool enable_by_default) { - auto& instruments = GetInstrumentList(); - for (const auto& descriptor : instruments) { - if (descriptor.name == name) { - Crash( - absl::StrFormat("Metric name %s has already been registered.", name)); - } - } - uint32_t index = instruments.size(); - GPR_ASSERT(index < std::numeric_limits::max()); - GlobalInstrumentDescriptor descriptor; - descriptor.value_type = ValueType::kDouble; - descriptor.instrument_type = InstrumentType::kGauge; - descriptor.index = index; - descriptor.enable_by_default = enable_by_default; - descriptor.name = name; - descriptor.description = description; - descriptor.unit = unit; - descriptor.label_keys = {label_keys.begin(), label_keys.end()}; - descriptor.optional_label_keys = {optional_label_keys.begin(), - optional_label_keys.end()}; - instruments.push_back(std::move(descriptor)); - GlobalDoubleGaugeHandle handle; - handle.index = index; - return handle; -} - GlobalInstrumentsRegistry::GlobalCallbackInt64GaugeHandle GlobalInstrumentsRegistry::RegisterCallbackInt64Gauge( absl::string_view name, absl::string_view description, diff --git a/src/core/lib/channel/metrics.h b/src/core/lib/channel/metrics.h index 1dd79696552..cbcd8182c2d 100644 --- a/src/core/lib/channel/metrics.h +++ b/src/core/lib/channel/metrics.h @@ -21,7 +21,6 @@ #include #include -#include "absl/container/flat_hash_map.h" #include "absl/functional/any_invocable.h" #include "absl/functional/function_ref.h" #include "absl/strings/string_view.h" @@ -59,7 +58,6 @@ class GlobalInstrumentsRegistry { kUndefined, kCounter, kHistogram, - kGauge, kCallbackGauge, }; using InstrumentID = uint32_t; @@ -85,8 +83,6 @@ class GlobalInstrumentsRegistry { struct GlobalDoubleCounterHandle : public GlobalInstrumentHandle {}; struct GlobalUInt64HistogramHandle : public GlobalInstrumentHandle {}; struct GlobalDoubleHistogramHandle : public GlobalInstrumentHandle {}; - struct GlobalInt64GaugeHandle : public GlobalInstrumentHandle {}; - struct GlobalDoubleGaugeHandle : public GlobalInstrumentHandle {}; struct GlobalCallbackInt64GaugeHandle : public GlobalInstrumentHandle {}; struct GlobalCallbackDoubleGaugeHandle : public GlobalInstrumentHandle {}; using GlobalCallbackHandle = absl::variant label_keys, absl::Span optional_label_keys, bool enable_by_default); - static GlobalInt64GaugeHandle RegisterInt64Gauge( - absl::string_view name, absl::string_view description, - absl::string_view unit, absl::Span label_keys, - absl::Span optional_label_keys, - bool enable_by_default); - static GlobalDoubleGaugeHandle RegisterDoubleGauge( - absl::string_view name, absl::string_view description, - absl::string_view unit, absl::Span label_keys, - absl::Span optional_label_keys, - bool enable_by_default); static GlobalCallbackInt64GaugeHandle RegisterCallbackInt64Gauge( absl::string_view name, absl::string_view description, absl::string_view unit, absl::Span label_keys, @@ -221,22 +207,6 @@ class StatsPlugin { GlobalInstrumentsRegistry::GlobalDoubleHistogramHandle handle, double value, absl::Span label_values, absl::Span optional_label_values) = 0; - // Sets an int64 \a value to the gauge specifed by \a handle. \a - // label_values and \a optional_label_values specify attributes that are - // associated with this measurement and must match with their corresponding - // keys in GlobalInstrumentsRegistry::RegisterInt64Gauge(). - virtual void SetGauge( - GlobalInstrumentsRegistry::GlobalInt64GaugeHandle handle, int64_t value, - absl::Span label_values, - absl::Span optional_label_values) = 0; - // Sets a double \a value to the gauge specifed by \a handle. \a - // label_values and \a optional_label_values specify attributes that are - // associated with this measurement and must match with their corresponding - // keys in GlobalInstrumentsRegistry::RegisterDoubleGauge(). - virtual void SetGauge( - GlobalInstrumentsRegistry::GlobalDoubleGaugeHandle handle, double value, - absl::Span label_values, - absl::Span optional_label_values) = 0; // Adds a callback to be invoked when the stats plugin wants to // populate the corresponding metrics (see callback->metrics() for list). virtual void AddCallback(RegisteredMetricCallback* callback) = 0; @@ -305,16 +275,6 @@ class GlobalStatsPluginRegistry { optional_values); } } - // Sets a value to a gauge in all stats plugins within the group. See the - // StatsPlugin interface for more documentation and valid types. - template - void SetGauge(HandleType handle, ValueType value, - absl::Span label_values, - absl::Span optional_values) { - for (auto& state : plugins_state_) { - state.plugin->SetGauge(handle, value, label_values, optional_values); - } - } // Registers a callback to be used to populate callback metrics. // The callback will update the specified metrics. The callback diff --git a/src/cpp/ext/otel/otel_plugin.cc b/src/cpp/ext/otel/otel_plugin.cc index aac4dd0222c..599472d5ed7 100644 --- a/src/cpp/ext/otel/otel_plugin.cc +++ b/src/cpp/ext/otel/otel_plugin.cc @@ -488,11 +488,6 @@ OpenTelemetryPlugin::OpenTelemetryPlugin( descriptor.value_type)); } break; - case grpc_core::GlobalInstrumentsRegistry::InstrumentType::kGauge: - grpc_core::Crash( - "Non-callback gauge is not supported and will be deleted in " - "the future."); - break; case grpc_core::GlobalInstrumentsRegistry::InstrumentType:: kCallbackGauge: switch (descriptor.value_type) { diff --git a/src/cpp/ext/otel/otel_plugin.h b/src/cpp/ext/otel/otel_plugin.h index 3d1fdd3440e..fc05e189e9a 100644 --- a/src/cpp/ext/otel/otel_plugin.h +++ b/src/cpp/ext/otel/otel_plugin.h @@ -396,14 +396,6 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin { grpc_core::GlobalInstrumentsRegistry::GlobalDoubleHistogramHandle handle, double value, absl::Span label_values, absl::Span optional_values) override; - void SetGauge( - grpc_core::GlobalInstrumentsRegistry::GlobalInt64GaugeHandle /*handle*/, - int64_t /*value*/, absl::Span /*label_values*/, - absl::Span /*optional_values*/) override {} - void SetGauge( - grpc_core::GlobalInstrumentsRegistry::GlobalDoubleGaugeHandle /*handle*/, - double /*value*/, absl::Span /*label_values*/, - absl::Span /*optional_values*/) override {} void AddCallback(grpc_core::RegisteredMetricCallback* callback) ABSL_LOCKS_EXCLUDED(mu_) override; void RemoveCallback(grpc_core::RegisteredMetricCallback* callback) diff --git a/test/core/channel/metrics_test.cc b/test/core/channel/metrics_test.cc index 324c1c2d804..c2267d3333e 100644 --- a/test/core/channel/metrics_test.cc +++ b/test/core/channel/metrics_test.cc @@ -199,80 +199,6 @@ TEST_F(MetricsTest, DoubleHistogram) { ::testing::Optional(::testing::UnorderedElementsAre(1.23, 2.34, 3.45))); } -TEST_F(MetricsTest, Int64Gauge) { - const absl::string_view kLabelKeys[] = {"label_key_1", "label_key_2"}; - const absl::string_view kOptionalLabelKeys[] = {"optional_label_key_1", - "optional_label_key_2"}; - auto int64_gauge_handle = GlobalInstrumentsRegistry::RegisterInt64Gauge( - "int64_gauge", "A simple int64 gauge.", "unit", kLabelKeys, - kOptionalLabelKeys, true); - constexpr absl::string_view kLabelValues[] = {"label_value_1", - "label_value_2"}; - constexpr absl::string_view kOptionalLabelValues[] = { - "optional_label_value_1", "optional_label_value_2"}; - constexpr absl::string_view kDomain1To4 = "domain1.domain2.domain3.domain4"; - constexpr absl::string_view kDomain2To4 = "domain2.domain3.domain4"; - constexpr absl::string_view kDomain3To4 = "domain3.domain4"; - auto plugin1 = MakeStatsPluginForTarget(kDomain1To4); - auto plugin2 = MakeStatsPluginForTarget(kDomain2To4); - auto plugin3 = MakeStatsPluginForTarget(kDomain3To4); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain1To4, "")) - .SetGauge(int64_gauge_handle, 1, kLabelValues, kOptionalLabelValues); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain2To4, "")) - .SetGauge(int64_gauge_handle, 2, kLabelValues, kOptionalLabelValues); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain3To4, "")) - .SetGauge(int64_gauge_handle, 3, kLabelValues, kOptionalLabelValues); - EXPECT_THAT(plugin1->GetGaugeValue(int64_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(1)); - EXPECT_THAT(plugin2->GetGaugeValue(int64_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(2)); - EXPECT_THAT(plugin3->GetGaugeValue(int64_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(3)); -} - -TEST_F(MetricsTest, DoubleGauge) { - const absl::string_view kLabelKeys[] = {"label_key_1", "label_key_2"}; - const absl::string_view kOptionalLabelKeys[] = {"optional_label_key_1", - "optional_label_key_2"}; - auto double_gauge_handle = GlobalInstrumentsRegistry::RegisterDoubleGauge( - "double_gauge", "A simple double gauge.", "unit", kLabelKeys, - kOptionalLabelKeys, true); - constexpr absl::string_view kLabelValues[] = {"label_value_1", - "label_value_2"}; - constexpr absl::string_view kOptionalLabelValues[] = { - "optional_label_value_1", "optional_label_value_2"}; - constexpr absl::string_view kDomain1To4 = "domain1.domain2.domain3.domain4"; - constexpr absl::string_view kDomain2To4 = "domain2.domain3.domain4"; - constexpr absl::string_view kDomain3To4 = "domain3.domain4"; - auto plugin1 = MakeStatsPluginForTarget(kDomain1To4); - auto plugin2 = MakeStatsPluginForTarget(kDomain2To4); - auto plugin3 = MakeStatsPluginForTarget(kDomain3To4); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain1To4, "")) - .SetGauge(double_gauge_handle, 1.23, kLabelValues, kOptionalLabelValues); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain2To4, "")) - .SetGauge(double_gauge_handle, 2.34, kLabelValues, kOptionalLabelValues); - GlobalStatsPluginRegistry::GetStatsPluginsForChannel( - StatsPluginChannelScope(kDomain3To4, "")) - .SetGauge(double_gauge_handle, 3.45, kLabelValues, kOptionalLabelValues); - EXPECT_THAT(plugin1->GetGaugeValue(double_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(1.23)); - EXPECT_THAT(plugin2->GetGaugeValue(double_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(2.34)); - EXPECT_THAT(plugin3->GetGaugeValue(double_gauge_handle, kLabelValues, - kOptionalLabelValues), - ::testing::Optional(3.45)); -} - TEST_F(MetricsTest, Int64CallbackGauge) { const absl::string_view kLabelKeys[] = {"label_key_1", "label_key_2"}; const absl::string_view kOptionalLabelKeys[] = {"optional_label_key_1", diff --git a/test/core/util/fake_stats_plugin.cc b/test/core/util/fake_stats_plugin.cc index a9f10bbba8b..2ef159d5b75 100644 --- a/test/core/util/fake_stats_plugin.cc +++ b/test/core/util/fake_stats_plugin.cc @@ -175,24 +175,6 @@ GlobalInstrumentsRegistryTestPeer::FindDoubleHistogramHandleByName( GlobalInstrumentsRegistry::InstrumentType::kHistogram); } -absl::optional -GlobalInstrumentsRegistryTestPeer::FindInt64GaugeHandleByName( - absl::string_view name) { - return FindInstrument( - GlobalInstrumentsRegistry::GetInstrumentList(), name, - GlobalInstrumentsRegistry::ValueType::kInt64, - GlobalInstrumentsRegistry::InstrumentType::kGauge); -} - -absl::optional -GlobalInstrumentsRegistryTestPeer::FindDoubleGaugeHandleByName( - absl::string_view name) { - return FindInstrument( - GlobalInstrumentsRegistry::GetInstrumentList(), name, - GlobalInstrumentsRegistry::ValueType::kDouble, - GlobalInstrumentsRegistry::InstrumentType::kGauge); -} - absl::optional GlobalInstrumentsRegistryTestPeer::FindCallbackInt64GaugeHandleByName( absl::string_view name) { diff --git a/test/core/util/fake_stats_plugin.h b/test/core/util/fake_stats_plugin.h index 16f7e996890..4b3a4018fa3 100644 --- a/test/core/util/fake_stats_plugin.h +++ b/test/core/util/fake_stats_plugin.h @@ -239,16 +239,6 @@ class FakeStatsPlugin : public StatsPlugin { } break; } - case GlobalInstrumentsRegistry::InstrumentType::kGauge: { - MutexLock lock(&mu_); - if (descriptor.value_type == - GlobalInstrumentsRegistry::ValueType::kInt64) { - int64_gauges_.emplace(descriptor.index, descriptor); - } else { - double_gauges_.emplace(descriptor.index, descriptor); - } - break; - } case GlobalInstrumentsRegistry::InstrumentType::kCallbackGauge: { MutexLock lock(&callback_mu_); if (descriptor.value_type == @@ -347,34 +337,6 @@ class FakeStatsPlugin : public StatsPlugin { if (iter == double_histograms_.end()) return; iter->second.Record(value, label_values, optional_values); } - void SetGauge(GlobalInstrumentsRegistry::GlobalInt64GaugeHandle handle, - int64_t value, absl::Span label_values, - absl::Span optional_values) override { - gpr_log(GPR_INFO, - "FakeStatsPlugin[%p]::RecordGauge(index=%u, value=(uint64)%lu, " - "label_values={%s}, optional_label_values={%s}", - this, handle.index, value, - absl::StrJoin(label_values, ", ").c_str(), - absl::StrJoin(optional_values, ", ").c_str()); - MutexLock lock(&mu_); - auto iter = int64_gauges_.find(handle.index); - if (iter == int64_gauges_.end()) return; - iter->second.Set(value, label_values, optional_values); - } - void SetGauge(GlobalInstrumentsRegistry::GlobalDoubleGaugeHandle handle, - double value, absl::Span label_values, - absl::Span optional_values) override { - gpr_log(GPR_INFO, - "FakeStatsPlugin[%p]::RecordGauge(index=%u, value=(double)%f, " - "label_values={%s}, optional_label_values={%s}", - this, handle.index, value, - absl::StrJoin(label_values, ", ").c_str(), - absl::StrJoin(optional_values, ", ").c_str()); - MutexLock lock(&mu_); - auto iter = double_gauges_.find(handle.index); - if (iter == double_gauges_.end()) return; - iter->second.Set(value, label_values, optional_values); - } void AddCallback(RegisteredMetricCallback* callback) override { gpr_log(GPR_INFO, "FakeStatsPlugin[%p]::AddCallback(%p)", this, callback); callbacks_.insert(callback); @@ -439,28 +401,6 @@ class FakeStatsPlugin : public StatsPlugin { } return iter->second.GetValues(label_values, optional_values); } - absl::optional GetGaugeValue( - GlobalInstrumentsRegistry::GlobalInt64GaugeHandle handle, - absl::Span label_values, - absl::Span optional_values) { - MutexLock lock(&mu_); - auto iter = int64_gauges_.find(handle.index); - if (iter == int64_gauges_.end()) { - return absl::nullopt; - } - return iter->second.GetValue(label_values, optional_values); - } - absl::optional GetGaugeValue( - GlobalInstrumentsRegistry::GlobalDoubleGaugeHandle handle, - absl::Span label_values, - absl::Span optional_values) { - MutexLock lock(&mu_); - auto iter = double_gauges_.find(handle.index); - if (iter == double_gauges_.end()) { - return absl::nullopt; - } - return iter->second.GetValue(label_values, optional_values); - } void TriggerCallbacks() { gpr_log(GPR_INFO, "FakeStatsPlugin[%p]::TriggerCallbacks(): START", this); Reporter reporter(*this); @@ -669,10 +609,6 @@ class FakeStatsPlugin : public StatsPlugin { ABSL_GUARDED_BY(&mu_); absl::flat_hash_map> double_histograms_ ABSL_GUARDED_BY(&mu_); - absl::flat_hash_map> int64_gauges_ - ABSL_GUARDED_BY(&mu_); - absl::flat_hash_map> double_gauges_ - ABSL_GUARDED_BY(&mu_); Mutex callback_mu_; absl::flat_hash_map> int64_callback_gauges_ ABSL_GUARDED_BY(&callback_mu_); @@ -725,10 +661,6 @@ class GlobalInstrumentsRegistryTestPeer { FindUInt64HistogramHandleByName(absl::string_view name); static absl::optional FindDoubleHistogramHandleByName(absl::string_view name); - static absl::optional - FindInt64GaugeHandleByName(absl::string_view name); - static absl::optional - FindDoubleGaugeHandleByName(absl::string_view name); static absl::optional< GlobalInstrumentsRegistry::GlobalCallbackInt64GaugeHandle> FindCallbackInt64GaugeHandleByName(absl::string_view name);