diff --git a/src/core/lib/channel/metrics.cc b/src/core/lib/channel/metrics.cc index 07c91048b32..3d89cc8fa7d 100644 --- a/src/core/lib/channel/metrics.cc +++ b/src/core/lib/channel/metrics.cc @@ -16,20 +16,16 @@ #include "src/core/lib/channel/metrics.h" -#include "absl/algorithm/container.h" - namespace grpc_core { -std::vector - GlobalInstrumentsRegistry::instruments_; - GlobalInstrumentsRegistry::GlobalUInt64CounterHandle GlobalInstrumentsRegistry::RegisterUInt64Counter( absl::string_view name, absl::string_view description, absl::string_view unit, std::vector label_keys, std::vector optional_label_keys) { - uint32_t index = instruments_.size(); - instruments_.push_back( + auto& instruments = gInstruments(); + uint32_t index = instruments.size(); + instruments.push_back( {.value_type = ValueType::kUInt64, .instrument_type = InstrumentType::kCounter, .index = index, @@ -48,8 +44,9 @@ GlobalInstrumentsRegistry::RegisterDoubleCounter( absl::string_view name, absl::string_view description, absl::string_view unit, std::vector label_keys, std::vector optional_label_keys) { - uint32_t index = instruments_.size(); - instruments_.push_back( + auto& instruments = gInstruments(); + uint32_t index = instruments.size(); + instruments.push_back( {.value_type = ValueType::kDouble, .instrument_type = InstrumentType::kCounter, .index = index, @@ -68,8 +65,9 @@ GlobalInstrumentsRegistry::RegisterUInt64Histogram( absl::string_view name, absl::string_view description, absl::string_view unit, std::vector label_keys, std::vector optional_label_keys) { - uint32_t index = instruments_.size(); - instruments_.push_back( + auto& instruments = gInstruments(); + uint32_t index = instruments.size(); + instruments.push_back( {.value_type = ValueType::kUInt64, .instrument_type = InstrumentType::kHistogram, .index = index, @@ -88,8 +86,9 @@ GlobalInstrumentsRegistry::RegisterDoubleHistogram( absl::string_view name, absl::string_view description, absl::string_view unit, std::vector label_keys, std::vector optional_label_keys) { - uint32_t index = instruments_.size(); - instruments_.push_back( + auto& instruments = gInstruments(); + uint32_t index = instruments.size(); + instruments.push_back( {.value_type = ValueType::kDouble, .instrument_type = InstrumentType::kHistogram, .index = index, @@ -103,9 +102,11 @@ GlobalInstrumentsRegistry::RegisterDoubleHistogram( return handle; } -const std::vector& -GlobalInstrumentsRegistry::instruments() { - return instruments_; +void GlobalInstrumentsRegistry::ForEach( + absl::AnyInvocable f) { + for (const auto& instrument : gInstruments()) { + f(instrument); + } } std::atomic GlobalStatsPluginRegistry::self_{ @@ -118,14 +119,15 @@ void GlobalStatsPluginRegistry::RegisterStatsPlugin( } GlobalStatsPluginRegistry::StatsPluginGroup -GlobalStatsPluginRegistry::GetStatsPluginsForTarget(absl::string_view target) { +GlobalStatsPluginRegistry::GetStatsPluginsForChannel( + const StatsPlugin::Scope& scope) { MutexLock lock(&mutex_); StatsPluginGroup group; - absl::c_for_each(plugins_, [&group, target](const auto& plugin) { - if (plugin->IsEnabledForTarget(target)) { + for (const auto& plugin : plugins_) { + if (plugin->IsEnabledForChannel(scope)) { group.push_back(plugin); } - }); + } return group; } diff --git a/src/core/lib/channel/metrics.h b/src/core/lib/channel/metrics.h index b1fc9393f2a..606f8a73044 100644 --- a/src/core/lib/channel/metrics.h +++ b/src/core/lib/channel/metrics.h @@ -31,7 +31,10 @@ namespace grpc_core { -// Static only. +// A global instruments registry whose API is designed to be used to register +// instruments (Counter and Histogram) as part of program startup, before the +// execution of the main function. Using this API after the main function begins +// may result into those instruments missing in StatsPlugins. class GlobalInstrumentsRegistry { public: enum class ValueType { @@ -61,10 +64,10 @@ class GlobalInstrumentsRegistry { // runs or between different versions. uint32_t index; }; - struct GlobalUInt64CounterHandle : GlobalHandle {}; - struct GlobalDoubleCounterHandle : GlobalHandle {}; - struct GlobalUInt64HistogramHandle : GlobalHandle {}; - struct GlobalDoubleHistogramHandle : GlobalHandle {}; + struct GlobalUInt64CounterHandle : public GlobalHandle {}; + struct GlobalDoubleCounterHandle : public GlobalHandle {}; + struct GlobalUInt64HistogramHandle : public GlobalHandle {}; + struct GlobalDoubleHistogramHandle : public GlobalHandle {}; // Creates instrument in the GlobalInstrumentsRegistry. static GlobalUInt64CounterHandle RegisterUInt64Counter( @@ -83,21 +86,31 @@ class GlobalInstrumentsRegistry { absl::string_view name, absl::string_view description, absl::string_view unit, std::vector label_keys, std::vector optional_label_keys); - // Getter functions for StatsPlugins to query registered counters/histograms. - static const std::vector& instruments(); + static void ForEach( + absl::AnyInvocable f); GlobalInstrumentsRegistry() = delete; private: - static std::vector instruments_; + // Uses the Construct-on-First-Use idiom to avoid the static initialization + // order fiasco. + static std::vector& gInstruments() { + static std::vector instruments_; + return instruments_; + } }; -// Interface. +// The StatsPlugin interface. class StatsPlugin { public: virtual ~StatsPlugin() = default; - virtual bool IsEnabledForTarget(absl::string_view target) = 0; - virtual bool IsEnabledForServer(ChannelArgs& args) = 0; + + struct Scope { + absl::string_view target; + absl::string_view authority; + }; + virtual bool IsEnabledForChannel(const Scope& scope) const = 0; + virtual bool IsEnabledForServer(const ChannelArgs& args) const = 0; virtual void AddCounter( GlobalInstrumentsRegistry::GlobalUInt64CounterHandle handle, @@ -122,6 +135,7 @@ class StatsPlugin { // AsyncInstrument* GetObservableCounter( // absl::string_view name, absl::string_view description, // absl::string_view unit); + // TODO(yijiem): This is an optimization for the StatsPlugin to create its own // representation of the label_values and use it multiple times. We would // change AddCounter and RecordHistogram to take RefCountedPtr @@ -179,7 +193,7 @@ class GlobalStatsPluginRegistry { void RegisterStatsPlugin(std::shared_ptr plugin); // The following two functions can be invoked to get a StatsPluginGroup for // a specified scope. - StatsPluginGroup GetStatsPluginsForTarget(absl::string_view target); + StatsPluginGroup GetStatsPluginsForChannel(const StatsPlugin::Scope& scope); // TODO(yijiem): Implement this. // StatsPluginsGroup GetStatsPluginsForServer(ChannelArgs& args); diff --git a/test/core/channel/metrics_test.cc b/test/core/channel/metrics_test.cc index 566fc384521..f993f63e711 100644 --- a/test/core/channel/metrics_test.cc +++ b/test/core/channel/metrics_test.cc @@ -64,19 +64,29 @@ std::string MakeKeyAttributes( Append(MixUp(optional_label_keys, optional_values)); } +class ChannelScope { + public: + ChannelScope(absl::string_view target, absl::string_view authority) { + scope_.target = target; + scope_.authority = authority; + } + + absl::string_view target() const { return scope_.target; } + absl::string_view authority() const { return scope_.authority; } + + private: + StatsPlugin::Scope scope_; +}; + // TODO(yijiem): Move this to test/core/util/fake_stats_plugin.h class FakeStatsPlugin : public StatsPlugin { public: - bool IsEnabledForTarget(absl::string_view target) override { - return target_selector_(target); + bool IsEnabledForChannel(const Scope& scope) const override { + return channel_filter_(ChannelScope(scope.target, scope.authority)); } - bool IsEnabledForServer(ChannelArgs& args) override { return false; } - - void SetTargetSelector( - absl::AnyInvocable - target_selector) { - target_selector_ = std::move(target_selector); + bool IsEnabledForServer(const ChannelArgs& args) const override { + return false; } void AddCounter(GlobalInstrumentsRegistry::GlobalUInt64CounterHandle handle, @@ -156,29 +166,31 @@ class FakeStatsPlugin : public StatsPlugin { friend class FakeStatsPluginBuilder; explicit FakeStatsPlugin( - absl::AnyInvocable - target_selector) - : target_selector_(std::move(target_selector)) { - for (const auto& descriptor : GlobalInstrumentsRegistry::instruments()) { - if (descriptor.instrument_type == - GlobalInstrumentsRegistry::InstrumentType::kCounter) { - if (descriptor.value_type == - GlobalInstrumentsRegistry::ValueType::kUInt64) { - uint64_counters_.emplace(descriptor.index, descriptor); - } else { - double_counters_.emplace(descriptor.index, descriptor); - } - } else { - EXPECT_EQ(descriptor.instrument_type, - GlobalInstrumentsRegistry::InstrumentType::kHistogram); - if (descriptor.value_type == - GlobalInstrumentsRegistry::ValueType::kUInt64) { - uint64_histograms_.emplace(descriptor.index, descriptor); - } else { - double_histograms_.emplace(descriptor.index, descriptor); - } - } - } + absl::AnyInvocable + channel_filter) + : channel_filter_(std::move(channel_filter)) { + GlobalInstrumentsRegistry::ForEach( + [this](const GlobalInstrumentsRegistry::GlobalInstrumentDescriptor& + descriptor) { + if (descriptor.instrument_type == + GlobalInstrumentsRegistry::InstrumentType::kCounter) { + if (descriptor.value_type == + GlobalInstrumentsRegistry::ValueType::kUInt64) { + uint64_counters_.emplace(descriptor.index, descriptor); + } else { + double_counters_.emplace(descriptor.index, descriptor); + } + } else { + EXPECT_EQ(descriptor.instrument_type, + GlobalInstrumentsRegistry::InstrumentType::kHistogram); + if (descriptor.value_type == + GlobalInstrumentsRegistry::ValueType::kUInt64) { + uint64_histograms_.emplace(descriptor.index, descriptor); + } else { + double_histograms_.emplace(descriptor.index, descriptor); + } + } + }); } template @@ -261,7 +273,7 @@ class FakeStatsPlugin : public StatsPlugin { std::vector> storage_; }; - absl::AnyInvocable target_selector_; + absl::AnyInvocable channel_filter_; // Instruments. absl::flat_hash_map> uint64_counters_; absl::flat_hash_map> double_counters_; @@ -272,41 +284,42 @@ class FakeStatsPlugin : public StatsPlugin { // TODO(yijiem): Move this to test/core/util/fake_stats_plugin.h class FakeStatsPluginBuilder { public: - FakeStatsPluginBuilder& SetTargetSelector( - absl::AnyInvocable - target_selector) { - target_selector_ = std::move(target_selector); + FakeStatsPluginBuilder& SetChannelFilter( + absl::AnyInvocable + channel_filter) { + channel_filter_ = std::move(channel_filter); return *this; } std::shared_ptr BuildAndRegister() { auto f = std::shared_ptr( - new FakeStatsPlugin(std::move(target_selector_))); + new FakeStatsPlugin(std::move(channel_filter_))); GlobalStatsPluginRegistry::Get().RegisterStatsPlugin(f); return f; } private: - absl::AnyInvocable target_selector_; + absl::AnyInvocable channel_filter_; }; class MetricsTest : public testing::Test { public: void SetUp() override { - plugin1_ = + plugin1_ = FakeStatsPluginBuilder() + .SetChannelFilter([](const ChannelScope& scope) { + return absl::EndsWith(scope.target(), + "domain1.domain2.domain3.domain4"); + }) + .BuildAndRegister(); + plugin2_ = FakeStatsPluginBuilder() - .SetTargetSelector([](absl::string_view target) { - return absl::EndsWith(target, "domain1.domain2.domain3.domain4"); + .SetChannelFilter([](const ChannelScope& scope) { + return absl::EndsWith(scope.target(), "domain2.domain3.domain4"); }) .BuildAndRegister(); - plugin2_ = FakeStatsPluginBuilder() - .SetTargetSelector([](absl::string_view target) { - return absl::EndsWith(target, "domain2.domain3.domain4"); - }) - .BuildAndRegister(); plugin3_ = FakeStatsPluginBuilder() - .SetTargetSelector([](absl::string_view target) { - return absl::EndsWith(target, "domain3.domain4"); + .SetChannelFilter([](const ChannelScope& scope) { + return absl::EndsWith(scope.target(), "domain3.domain4"); }) .BuildAndRegister(); } @@ -362,15 +375,15 @@ GlobalInstrumentsRegistry::GlobalDoubleHistogramHandle TEST_F(MetricsTest, UInt64Counter) { GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain1.domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain1.domain2.domain3.domain4"}) .AddCounter(uint64_counter_handle_, 1, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain2.domain3.domain4"}) .AddCounter(uint64_counter_handle_, 2, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain3.domain4"}) .AddCounter(uint64_counter_handle_, 3, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); EXPECT_EQ(plugin1_->GetCounterValue( @@ -389,17 +402,17 @@ TEST_F(MetricsTest, UInt64Counter) { TEST_F(MetricsTest, DoubleCounter) { GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain1.domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain1.domain2.domain3.domain4"}) .AddCounter(double_counter_handle_, 1.23, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain2.domain3.domain4"}) .AddCounter(double_counter_handle_, 2.34, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain3.domain4"}) .AddCounter(double_counter_handle_, 3.45, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); @@ -419,17 +432,17 @@ TEST_F(MetricsTest, DoubleCounter) { TEST_F(MetricsTest, UInt64Histogram) { GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain1.domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain1.domain2.domain3.domain4"}) .RecordHistogram(uint64_histogram_handle_, 1, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain2.domain3.domain4"}) .RecordHistogram(uint64_histogram_handle_, 2, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain3.domain4"}) .RecordHistogram(uint64_histogram_handle_, 3, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); @@ -449,17 +462,17 @@ TEST_F(MetricsTest, UInt64Histogram) { TEST_F(MetricsTest, DoubleHistogram) { GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain1.domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain1.domain2.domain3.domain4"}) .RecordHistogram(double_histogram_handle_, 1.23, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain2.domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain2.domain3.domain4"}) .RecordHistogram(double_histogram_handle_, 2.34, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"}); GlobalStatsPluginRegistry::Get() - .GetStatsPluginsForTarget("domain3.domain4") + .GetStatsPluginsForChannel({.target = "domain3.domain4"}) .RecordHistogram(double_histogram_handle_, 3.45, {"label_value_1", "label_value_2"}, {"optional_label_value_1", "optional_label_value_2"});