diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 2965f617062a..77b45e0fdfc7 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -240,19 +240,6 @@ envoy_cc_library( ], ) -envoy_cc_library( - name = "stats_utility_lib", - srcs = ["stats_utility.cc"], - hdrs = ["stats_utility.h"], - deps = [ - "//source/common/stats:histogram_lib", - "//source/common/stats:stats_lib", - "//source/common/stats:stats_matcher_lib", - "//source/common/stats:tag_producer_lib", - "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", - ], -) - envoy_cc_library( name = "subscription_base_interface", hdrs = ["subscription_base.h"], diff --git a/source/common/config/stats_utility.h b/source/common/config/stats_utility.h deleted file mode 100644 index 409bd1ddc0c8..000000000000 --- a/source/common/config/stats_utility.h +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include "envoy/config/bootstrap/v3/bootstrap.pb.h" -#include "envoy/stats/histogram.h" -#include "envoy/stats/scope.h" -#include "envoy/stats/stats_macros.h" -#include "envoy/stats/stats_matcher.h" -#include "envoy/stats/tag_producer.h" - -namespace Envoy { -namespace Config { - -class StatsUtility { -public: - /** - * Create TagProducer instance. Check all tag names for conflicts to avoid - * unexpected tag name overwriting. - * @param bootstrap bootstrap proto. - * @param cli_tags tags that are provided by the cli - * @throws EnvoyException when the conflict of tag names is found. - */ - static Stats::TagProducerPtr - createTagProducer(const envoy::config::bootstrap::v3::Bootstrap& bootstrap, - const Stats::TagVector& cli_tags); -}; - -} // namespace Config -} // namespace Envoy diff --git a/source/common/stats/tag_producer_impl.cc b/source/common/stats/tag_producer_impl.cc index 621d3e001934..e01fd800f668 100644 --- a/source/common/stats/tag_producer_impl.cc +++ b/source/common/stats/tag_producer_impl.cc @@ -11,13 +11,19 @@ namespace Envoy { namespace Stats { -TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& config) - : TagProducerImpl(config, {}) {} +absl::StatusOr +TagProducerImpl::createTagProducer(const envoy::config::metrics::v3::StatsConfig& config, + const Stats::TagVector& cli_tags) { + absl::Status creation_status; + Stats::TagProducerPtr ret(new Stats::TagProducerImpl(config, cli_tags, creation_status)); + RETURN_IF_NOT_OK(creation_status); + return ret; +} TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& config, - const Stats::TagVector& cli_tags) { + const Stats::TagVector& cli_tags, absl::Status& creation_status) { reserveResources(config); - addDefaultExtractors(config); + creation_status = addDefaultExtractors(config); for (const auto& cli_tag : cli_tags) { addExtractor(std::make_unique(cli_tag.name_, cli_tag.value_)); @@ -34,14 +40,18 @@ TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& envoy::config::metrics::v3::TagSpecifier::TagValueCase::kRegex) { if (tag_specifier.regex().empty()) { - if (addExtractorsMatching(name) == 0) { - throwEnvoyExceptionOrPanic(fmt::format( - "No regex specified for tag specifier and no default regex for name: '{}'", name)); + creation_status = addExtractorsMatching(name); + if (!creation_status.ok()) { + return; } } else { - addExtractor(THROW_OR_RETURN_VALUE( - TagExtractorImplBase::createTagExtractor(name, tag_specifier.regex()), - TagExtractorPtr)); + auto extractor_or_error = + TagExtractorImplBase::createTagExtractor(name, tag_specifier.regex()); + if (!extractor_or_error.ok()) { + creation_status = extractor_or_error.status(); + return; + } + addExtractor(std::move(extractor_or_error.value())); } } else if (tag_specifier.tag_value_case() == envoy::config::metrics::v3::TagSpecifier::TagValueCase::kFixedValue) { @@ -51,14 +61,16 @@ TagProducerImpl::TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& } } -int TagProducerImpl::addExtractorsMatching(absl::string_view name) { +absl::Status TagProducerImpl::addExtractorsMatching(absl::string_view name) { int num_found = 0; for (const auto& desc : Config::TagNames::get().descriptorVec()) { if (desc.name_ == name) { - addExtractor(THROW_OR_RETURN_VALUE( - TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, - desc.negative_match_, desc.re_type_), - TagExtractorPtr)); + auto extractor_or_error = TagExtractorImplBase::createTagExtractor( + desc.name_, desc.regex_, desc.substr_, desc.negative_match_, desc.re_type_); + if (!extractor_or_error.ok()) { + return extractor_or_error.status(); + } + addExtractor(std::move(extractor_or_error.value())); ++num_found; } } @@ -68,7 +80,11 @@ int TagProducerImpl::addExtractorsMatching(absl::string_view name) { ++num_found; } } - return num_found; + if (num_found == 0) { + return absl::InvalidArgumentError(fmt::format( + "No regex specified for tag specifier and no default regex for name: '{}'", name)); + } + return absl::OkStatus(); } void TagProducerImpl::addExtractor(TagExtractorPtr extractor) { @@ -136,18 +152,20 @@ void TagProducerImpl::reserveResources(const envoy::config::metrics::v3::StatsCo tag_extractors_without_prefix_.reserve(config.stats_tags().size()); } -void TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v3::StatsConfig& config) { +absl::Status +TagProducerImpl::addDefaultExtractors(const envoy::config::metrics::v3::StatsConfig& config) { if (!config.has_use_all_default_tags() || config.use_all_default_tags().value()) { for (const auto& desc : Config::TagNames::get().descriptorVec()) { - addExtractor(THROW_OR_RETURN_VALUE( - TagExtractorImplBase::createTagExtractor(desc.name_, desc.regex_, desc.substr_, - desc.negative_match_, desc.re_type_), - TagExtractorPtr)); + auto extractor_or_error = TagExtractorImplBase::createTagExtractor( + desc.name_, desc.regex_, desc.substr_, desc.negative_match_, desc.re_type_); + RETURN_IF_STATUS_NOT_OK(extractor_or_error); + addExtractor(std::move(extractor_or_error.value())); } for (const auto& desc : Config::TagNames::get().tokenizedDescriptorVec()) { addExtractor(std::make_unique(desc.name_, desc.pattern_)); } } + return absl::OkStatus(); } } // namespace Stats diff --git a/source/common/stats/tag_producer_impl.h b/source/common/stats/tag_producer_impl.h index e253dd002dd5..2b78bcccaa23 100644 --- a/source/common/stats/tag_producer_impl.h +++ b/source/common/stats/tag_producer_impl.h @@ -28,10 +28,16 @@ namespace Stats { */ class TagProducerImpl : public TagProducer { public: - TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& config, - const Stats::TagVector& cli_tags); - - TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& config); + /** + * Create TagProducer instance. Check all tag names for conflicts to avoid + * unexpected tag name overwriting. + * @param config stats config. + * @param cli_tags tags that are provided by the cli + * @returns a tag producer or an error pointer if a conflict of tag names is found. + */ + static absl::StatusOr + createTagProducer(const envoy::config::metrics::v3::StatsConfig& config, + const Stats::TagVector& cli_tags); TagProducerImpl() = default; @@ -46,6 +52,9 @@ class TagProducerImpl : public TagProducer { const TagVector& fixedTags() const override { return fixed_tags_; } private: + TagProducerImpl(const envoy::config::metrics::v3::StatsConfig& config, + const Stats::TagVector& cli_tags, absl::Status& creation_status); + friend class DefaultTagRegexTester; /** @@ -60,9 +69,9 @@ class TagProducerImpl : public TagProducer { * more than one TagExtractor can be used to generate a given tag. The default * extractors are specified in common/config/well_known_names.cc. * @param name absl::string_view the extractor to add. - * @return int the number of matching extractors. + * @return Status ok if at least 1 matching extractor was found. */ - int addExtractorsMatching(absl::string_view name); + absl::Status addExtractorsMatching(absl::string_view name); /** * Roughly estimate the size of the vectors. @@ -74,8 +83,9 @@ class TagProducerImpl : public TagProducer { * Adds all default extractors from well_known_names.cc into the collection. * * @param config const envoy::config::metrics::v2::StatsConfig& the config. + * @return a status indicating if the default extractors were successfully added. */ - void addDefaultExtractors(const envoy::config::metrics::v3::StatsConfig& config); + absl::Status addDefaultExtractors(const envoy::config::metrics::v3::StatsConfig& config); /** * Iterates over every tag extractor that might possibly match stat_name, calling diff --git a/source/server/BUILD b/source/server/BUILD index 6fa2605b1efd..9b6baa6bdd9d 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -442,7 +442,6 @@ envoy_cc_library( "//source/common/common:mutex_tracer_lib", "//source/common/common:perf_tracing_lib", "//source/common/common:utility_lib", - "//source/common/config:stats_utility_lib", "//source/common/config:utility_lib", "//source/common/config:xds_resource_lib", "//source/common/grpc:async_client_manager_lib", @@ -461,6 +460,7 @@ envoy_cc_library( "//source/common/secret:secret_manager_impl_lib", "//source/common/signal:fatal_error_handler_lib", "//source/common/singleton:manager_impl_lib", + "//source/common/stats:tag_producer_lib", "//source/common/stats:thread_local_store_lib", "//source/common/tls:context_lib", "//source/common/upstream:cluster_manager_lib", diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 79065df65da0..03bbd16faea6 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -5,7 +5,6 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.h" #include "source/common/common/utility.h" -#include "source/common/config/stats_utility.h" #include "source/common/config/utility.h" #include "source/common/config/well_known_names.h" #include "source/common/event/real_time_system.h" @@ -13,6 +12,7 @@ #include "source/common/local_info/local_info_impl.h" #include "source/common/protobuf/utility.h" #include "source/common/singleton/manager_impl.h" +#include "source/common/stats/tag_producer_impl.h" #include "source/common/tls/context_manager_impl.h" #include "source/common/version/version.h" #include "source/server/admin/admin_factory_context.h" @@ -101,7 +101,9 @@ void ValidationInstance::initialize(const Options& options, regex_engine_ = createRegexEngine( bootstrap_, messageValidationContext().staticValidationVisitor(), serverFactoryContext()); - Config::StatsUtility::createTagProducer(bootstrap_, options_.statsTags()); + auto producer_or_error = + Stats::TagProducerImpl::createTagProducer(bootstrap_.stats_config(), options_.statsTags()); + THROW_IF_STATUS_NOT_OK(producer_or_error, throw); if (!bootstrap_.node().user_agent_build_version().has_version()) { *bootstrap_.mutable_node()->mutable_user_agent_build_version() = VersionInfo::buildVersion(); } diff --git a/source/server/server.cc b/source/server/server.cc index 081fcf9b9c98..254cbc086046 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -29,7 +29,6 @@ #include "source/common/common/enum_to_int.h" #include "source/common/common/mutex_tracer_impl.h" #include "source/common/common/utility.h" -#include "source/common/config/stats_utility.h" #include "source/common/config/utility.h" #include "source/common/config/well_known_names.h" #include "source/common/config/xds_resource.h" @@ -48,6 +47,7 @@ #include "source/common/signal/fatal_error_handler.h" #include "source/common/singleton/manager_impl.h" #include "source/common/stats/stats_matcher_impl.h" +#include "source/common/stats/tag_producer_impl.h" #include "source/common/stats/thread_local_store.h" #include "source/common/stats/timespan_impl.h" #include "source/common/tls/context_manager_impl.h" @@ -497,8 +497,10 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar // Needs to happen as early as possible in the instantiation to preempt the objects that require // stats. - stats_store_.setTagProducer( - Config::StatsUtility::createTagProducer(bootstrap_, options_.statsTags())); + auto producer_or_error = + Stats::TagProducerImpl::createTagProducer(bootstrap_.stats_config(), options_.statsTags()); + RETURN_IF_STATUS_NOT_OK(producer_or_error); + stats_store_.setTagProducer(std::move(producer_or_error.value())); stats_store_.setStatsMatcher(std::make_unique( bootstrap_.stats_config(), stats_store_.symbolTable(), server_contexts_)); stats_store_.setHistogramSettings( diff --git a/test/common/config/BUILD b/test/common/config/BUILD index 2aef53b1499b..08fb0d1208eb 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -126,7 +126,6 @@ envoy_cc_test( external_deps = ["abseil_optional"], deps = [ "//source/common/config:api_version_lib", - "//source/common/config:stats_utility_lib", "//source/common/config:utility_lib", "//source/common/config:well_known_names", "//source/common/stats:stats_lib", diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index 1b6565af0fff..72d7864f1731 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -8,7 +8,6 @@ #include "source/common/common/fmt.h" #include "source/common/config/api_version.h" -#include "source/common/config/stats_utility.h" #include "source/common/config/utility.h" #include "source/common/config/well_known_names.h" #include "source/common/protobuf/protobuf.h" @@ -57,26 +56,6 @@ TEST(UtilityTest, ConfigSourceInitFetchTimeout) { EXPECT_EQ(654, Utility::configSourceInitialFetchTimeout(config_source).count()); } -TEST(UtilityTest, createTagProducer) { - envoy::config::bootstrap::v3::Bootstrap bootstrap; - auto producer = StatsUtility::createTagProducer(bootstrap, {}); - ASSERT_TRUE(producer != nullptr); - Stats::TagVector tags; - auto extracted_name = producer->produceTags("http.config_test.rq_total", tags); - ASSERT_EQ(extracted_name, "http.rq_total"); - ASSERT_EQ(tags.size(), 1); -} - -TEST(UtilityTest, createTagProducerWithDefaultTgs) { - envoy::config::bootstrap::v3::Bootstrap bootstrap; - auto producer = StatsUtility::createTagProducer(bootstrap, {{"foo", "bar"}}); - ASSERT_TRUE(producer != nullptr); - Stats::TagVector tags; - auto extracted_name = producer->produceTags("http.config_test.rq_total", tags); - EXPECT_EQ(extracted_name, "http.rq_total"); - EXPECT_EQ(tags.size(), 2); -} - TEST(UtilityTest, CheckFilesystemSubscriptionBackingPath) { Api::ApiPtr api = Api::createApiForTest(); diff --git a/test/common/stats/tag_extractor_impl_speed_test.cc b/test/common/stats/tag_extractor_impl_speed_test.cc index 712d32ec2652..f6e1ed73ee9c 100644 --- a/test/common/stats/tag_extractor_impl_speed_test.cc +++ b/test/common/stats/tag_extractor_impl_speed_test.cc @@ -90,7 +90,9 @@ const std::vector params = { // NOLINTNEXTLINE(readability-identifier-naming) void BM_ExtractTags(benchmark::State& state) { - TagProducerImpl tag_extractors{envoy::config::metrics::v3::StatsConfig()}; + const Stats::TagVector tags; + auto tag_extractors = + TagProducerImpl::createTagProducer(envoy::config::metrics::v3::StatsConfig(), tags).value(); const auto idx = state.range(0); const auto& p = params[idx]; absl::string_view str = std::get<0>(p); @@ -99,7 +101,7 @@ void BM_ExtractTags(benchmark::State& state) { for (auto _ : state) { UNREFERENCED_PARAMETER(_); TagVector tags; - tag_extractors.produceTags(str, tags); + tag_extractors->produceTags(str, tags); RELEASE_ASSERT(tags.size() == tags_size, absl::StrCat("tags.size()=", tags.size(), " tags_size==", tags_size)); } diff --git a/test/common/stats/tag_extractor_impl_test.cc b/test/common/stats/tag_extractor_impl_test.cc index 5888322b84d2..944cc38b94fb 100644 --- a/test/common/stats/tag_extractor_impl_test.cc +++ b/test/common/stats/tag_extractor_impl_test.cc @@ -87,14 +87,17 @@ TEST(TagExtractorTest, BadRegex) { class DefaultTagRegexTester { public: - DefaultTagRegexTester() : tag_extractors_(envoy::config::metrics::v3::StatsConfig()) {} + DefaultTagRegexTester() + : tag_extractors_(std::move( + TagProducerImpl::createTagProducer(envoy::config::metrics::v3::StatsConfig(), tags_) + .value())) {} void testRegex(const std::string& stat_name, const std::string& expected_tag_extracted_name, const TagVector& expected_tags) { // Test forward iteration through the regexes TagVector tags; - const std::string tag_extracted_name = tag_extractors_.produceTags(stat_name, tags); + const std::string tag_extracted_name = tag_extractors_->produceTags(stat_name, tags); auto cmp = [](const Tag& lhs, const Tag& rhs) { return lhs.name_ == rhs.name_ && lhs.value_ == rhs.value_; @@ -138,10 +141,11 @@ class DefaultTagRegexTester { // version does not add in tag_extractors_.default_tags_ into tags. That doesn't matter // for this test, however. std::list extractors; // Note push-front is used to reverse order. - tag_extractors_.forEachExtractorMatching(metric_name, - [&extractors](const TagExtractorPtr& tag_extractor) { - extractors.push_front(tag_extractor.get()); - }); + reinterpret_cast(tag_extractors_.get()) + ->forEachExtractorMatching(metric_name, + [&extractors](const TagExtractorPtr& tag_extractor) { + extractors.push_front(tag_extractor.get()); + }); IntervalSetImpl remove_characters; TagExtractionContext tag_extraction_context(metric_name); @@ -151,8 +155,9 @@ class DefaultTagRegexTester { return StringUtil::removeCharacters(metric_name, remove_characters); } + const Stats::TagVector tags_; SymbolTableImpl symbol_table_; - TagProducerImpl tag_extractors_; + TagProducerPtr tag_extractors_; }; TEST(TagExtractorTest, DefaultTagExtractors) { diff --git a/test/common/stats/tag_producer_impl_test.cc b/test/common/stats/tag_producer_impl_test.cc index de0856791298..58fc0c8c6b4d 100644 --- a/test/common/stats/tag_producer_impl_test.cc +++ b/test/common/stats/tag_producer_impl_test.cc @@ -36,34 +36,36 @@ class TagProducerTest : public testing::Test { TEST_F(TagProducerTest, CheckConstructor) { // Should pass there were no tag name conflict. addSpecifier("test.x", "xxx"); - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {})); - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {{"test.y", "yyy"}})); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, {}).status().ok()); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, {{"test.y", "yyy"}}).status().ok()); // Should not raise an error when duplicate tag names between cli and config. - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {{"test.x", "yyy"}})); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, {{"test.x", "yyy"}}).status().ok()); // Should not raise an error when duplicate tag names are specified. addSpecifier("test.x", "yyy"); - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {{"test.y", "yyy"}})); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, {{"test.y", "yyy"}}).status().ok()); // Should not raise an error when a cli tag names conflicts with Envoy's default tag names. - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {{Config::TagNames::get().CLUSTER_NAME, "yyy"}})); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, + {{Config::TagNames::get().CLUSTER_NAME, "yyy"}}) + .status() + .ok()); // Also should raise an error when user defined tag name conflicts with Envoy's default tag names. stats_config_.clear_stats_tags(); stats_config_.mutable_use_all_default_tags()->set_value(true); auto& custom_tag_extractor = *stats_config_.mutable_stats_tags()->Add(); custom_tag_extractor.set_tag_name(Config::TagNames::get().CLUSTER_NAME); - EXPECT_NO_THROW(TagProducerImpl(stats_config_, {})); + EXPECT_TRUE(TagProducerImpl::createTagProducer(stats_config_, {}).status().ok()); // Non-default custom name without regex should throw stats_config_.mutable_use_all_default_tags()->set_value(true); stats_config_.clear_stats_tags(); custom_tag_extractor = *stats_config_.mutable_stats_tags()->Add(); custom_tag_extractor.set_tag_name("test_extractor"); - EXPECT_THROW_WITH_MESSAGE( - TagProducerImpl(stats_config_, {}), EnvoyException, - "No regex specified for tag specifier and no default regex for name: 'test_extractor'"); + EXPECT_EQ(TagProducerImpl::createTagProducer(stats_config_, {}).status().message(), + "No regex specified for tag specifier and no default regex for name: 'test_extractor'"); // Also empty regex should throw stats_config_.mutable_use_all_default_tags()->set_value(true); @@ -71,19 +73,18 @@ TEST_F(TagProducerTest, CheckConstructor) { custom_tag_extractor = *stats_config_.mutable_stats_tags()->Add(); custom_tag_extractor.set_tag_name("test_extractor"); custom_tag_extractor.set_regex(""); - EXPECT_THROW_WITH_MESSAGE( - TagProducerImpl(stats_config_, {}), EnvoyException, - "No regex specified for tag specifier and no default regex for name: 'test_extractor'"); + EXPECT_EQ(TagProducerImpl::createTagProducer(stats_config_, {}).status().message(), + "No regex specified for tag specifier and no default regex for name: 'test_extractor'"); } TEST_F(TagProducerTest, DuplicateConfigTagBehavior) { addSpecifier(tag_name_values_.RESPONSE_CODE, "\\.(response_code=(\\d{3}));"); { - TagProducerImpl producer{stats_config_}; + auto producer = TagProducerImpl::createTagProducer(stats_config_, {}).value(); TagVector tags; std::string extracted_name; EXPECT_LOG_CONTAINS("warn", "Skipping duplicate tag", - extracted_name = producer.produceTags( + extracted_name = producer->produceTags( "cluster.xds-grpc.response_code=300;upstream_rq_200", tags)); EXPECT_TRUE(extracted_name == "cluster.;upstream_rq_200" || extracted_name == "cluster.response_code=300;upstream_rq") @@ -100,12 +101,14 @@ TEST_F(TagProducerTest, DuplicateConfigTagBehavior) { TEST_F(TagProducerTest, DuplicateConfigCliTagBehavior) { addSpecifier(tag_name_values_.RESPONSE_CODE, "\\.(response_code=(\\d{3}));"); { - TagProducerImpl producer{stats_config_, {{tag_name_values_.RESPONSE_CODE, "fixed"}}}; + auto producer = TagProducerImpl::createTagProducer(stats_config_, + {{tag_name_values_.RESPONSE_CODE, "fixed"}}) + .value(); TagVector tags; const std::string stat_name = "cluster.xds-grpc.response_code=300;upstream_rq_200"; std::string extracted_name; EXPECT_LOG_CONTAINS("warn", "Skipping duplicate tag", - extracted_name = producer.produceTags(stat_name, tags)); + extracted_name = producer->produceTags(stat_name, tags)); EXPECT_TRUE(extracted_name == "cluster.;upstream_rq_200" || extracted_name == "cluster.response_code=300;upstream_rq" || extracted_name == stat_name) @@ -121,9 +124,9 @@ TEST_F(TagProducerTest, DuplicateConfigCliTagBehavior) { TEST_F(TagProducerTest, Fixed) { const TagVector tag_config{{"my-tag", "fixed"}}; - TagProducerImpl producer{stats_config_, tag_config}; + auto producer(TagProducerImpl::createTagProducer(stats_config_, tag_config).value()); TagVector tags; - EXPECT_EQ("stat-name", producer.produceTags("stat-name", tags)); + EXPECT_EQ("stat-name", producer->produceTags("stat-name", tags)); checkTags(tag_config, tags); } @@ -138,13 +141,34 @@ TEST_F(TagProducerTest, FixedTags) { // This one isn't a fixed value so it won't be included. addSpecifier("regex", "value"); - TagProducerImpl producer{stats_config_, tag_config}; - const auto& tags = producer.fixedTags(); + auto producer = TagProducerImpl::createTagProducer(stats_config_, tag_config).value(); + const auto& tags = producer->fixedTags(); EXPECT_THAT(tags, testing::UnorderedElementsAreArray(TagVector{ {"my-tag", "fixed"}, {"tag2", "value2"}, })); } +TEST(UtilityTest, createTagProducer) { + envoy::config::bootstrap::v3::Bootstrap bootstrap; + auto producer = TagProducerImpl::createTagProducer(bootstrap.stats_config(), {}).value(); + ASSERT_TRUE(producer != nullptr); + Stats::TagVector tags; + auto extracted_name = producer->produceTags("http.config_test.rq_total", tags); + ASSERT_EQ(extracted_name, "http.rq_total"); + ASSERT_EQ(tags.size(), 1); +} + +TEST(UtilityTest, createTagProducerWithDefaultTgs) { + envoy::config::bootstrap::v3::Bootstrap bootstrap; + auto producer = + TagProducerImpl::createTagProducer(bootstrap.stats_config(), {{"foo", "bar"}}).value(); + ASSERT_TRUE(producer != nullptr); + Stats::TagVector tags; + auto extracted_name = producer->produceTags("http.config_test.rq_total", tags); + EXPECT_EQ(extracted_name, "http.rq_total"); + EXPECT_EQ(tags.size(), 2); +} + } // namespace Stats } // namespace Envoy diff --git a/test/common/stats/thread_local_store_speed_test.cc b/test/common/stats/thread_local_store_speed_test.cc index 9aacf260feb3..558a44eb76ea 100644 --- a/test/common/stats/thread_local_store_speed_test.cc +++ b/test/common/stats/thread_local_store_speed_test.cc @@ -28,7 +28,8 @@ class ThreadLocalStorePerf { ThreadLocalStorePerf() : heap_alloc_(symbol_table_), store_(heap_alloc_), api_(Api::createApiForTest(store_, time_system_)) { - store_.setTagProducer(std::make_unique(stats_config_)); + const Stats::TagVector tags; + store_.setTagProducer(Stats::TagProducerImpl::createTagProducer(stats_config_, tags).value()); Stats::TestUtil::forEachSampleStat(1000, true, [this](absl::string_view name) { stat_names_.push_back(std::make_unique(name, symbol_table_)); diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index e153a0303496..656608efdc68 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -788,7 +788,8 @@ TEST_F(StatsThreadLocalStoreTest, ExtractAndAppendTagsFixedValue) { tag_specifier->set_tag_name("foo"); tag_specifier->set_fixed_value("bar"); - store_->setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags_vector; + store_->setTagProducer(TagProducerImpl::createTagProducer(stats_config, tags_vector).value()); StatNamePool pool(symbol_table_); StatNameTagVector tags{{pool.add("a"), pool.add("b")}}; @@ -810,7 +811,8 @@ TEST_F(StatsThreadLocalStoreTest, ExtractAndAppendTagsRegexValueNoMatch) { tag_specifier->set_tag_name("foo"); tag_specifier->set_regex("bar"); - store_->setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags_vector; + store_->setTagProducer(TagProducerImpl::createTagProducer(stats_config, tags_vector).value()); StatNamePool pool(symbol_table_); StatNameTagVector tags{{pool.add("a"), pool.add("b")}}; @@ -829,7 +831,8 @@ TEST_F(StatsThreadLocalStoreTest, ExtractAndAppendTagsRegexValueWithMatch) { tag_specifier->set_tag_name("foo_tag"); tag_specifier->set_regex("^foo.(.+)"); - store_->setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags_vector; + store_->setTagProducer(TagProducerImpl::createTagProducer(stats_config, tags_vector).value()); StatNamePool pool(symbol_table_); StatNameTagVector tags{{pool.add("a"), pool.add("b")}}; @@ -846,7 +849,8 @@ TEST_F(StatsThreadLocalStoreTest, ExtractAndAppendTagsRegexBuiltinExpression) { store_->initializeThreading(main_thread_dispatcher_, tls_); envoy::config::metrics::v3::StatsConfig stats_config; - store_->setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags_vector; + store_->setTagProducer(TagProducerImpl::createTagProducer(stats_config, tags_vector).value()); StatNamePool pool(symbol_table_); StatNameTagVector tags{{pool.add("a"), pool.add("b")}}; @@ -1508,7 +1512,8 @@ class StatsThreadLocalStoreTestNoFixture : public testing::Test { // Use a tag producer that will produce tags. envoy::config::metrics::v3::StatsConfig stats_config; - store_.setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags_vector; + store_.setTagProducer(TagProducerImpl::createTagProducer(stats_config, tags_vector).value()); } ~StatsThreadLocalStoreTestNoFixture() override { diff --git a/test/server/admin/prometheus_stats_test.cc b/test/server/admin/prometheus_stats_test.cc index 077402c76b42..fcc28d77207a 100644 --- a/test/server/admin/prometheus_stats_test.cc +++ b/test/server/admin/prometheus_stats_test.cc @@ -325,7 +325,8 @@ TEST_F(PrometheusStatsFormatterTest, DifferentNamedScopeSameStat) { Stats::CustomStatNamespacesImpl custom_namespaces; Stats::ThreadLocalStoreImpl store(alloc_); envoy::config::metrics::v3::StatsConfig stats_config; - store.setTagProducer(std::make_unique(stats_config)); + const Stats::TagVector tags; + store.setTagProducer(Stats::TagProducerImpl::createTagProducer(stats_config, tags).value()); Stats::StatName name = pool_.add("default.total_match_count"); Stats::ScopeSharedPtr scope1 = store.rootScope()->createScope("cluster.a"); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index 94a0fa79bc04..6a3fada14af5 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -117,7 +117,6 @@ paths: - source/common/formatter/substitution_format_utility.cc - source/common/formatter/substitution_format_string.h - source/common/stats/tag_extractor_impl.cc - - source/common/stats/tag_producer_impl.cc - source/common/http/http2/codec_impl.cc - source/common/http/hash_policy.cc - source/common/http/conn_manager_utility.cc