diff --git a/libs/config/README.md b/libs/config/README.md index e4836a6..98efffa 100644 --- a/libs/config/README.md +++ b/libs/config/README.md @@ -3,10 +3,12 @@ ## Overview The `Config` class serves as the configuration object for the Registry class, which manages how metrics are sent to -SpectatorD. The `Config` constructor takes two parameters. The first parameter is required and is a `WriterConfig` +SpectatorD. The `Config` constructor takes three parameters. The first parameter is required and is a `WriterConfig` object. This object defines how metrics will be sent to `SpectatorD`. The second parameter `extraTags` is an unordered map of strings allowing you to provide additional tags to all of your metrics. Extra tags are additional -key-value pairs attached to all metrics and will be merged with environment-derived tags. +key-value pairs attached to all metrics and will be merged with environment-derived tags; on a key collision the +environment-derived value wins. The third parameter `includeEnvTags` defaults to `true` and controls whether the +environment-derived tags are added (see [Environment Variables](#environment-variables)). ## Usage @@ -32,10 +34,23 @@ Config config(WriterConfig(WriterTypes::Memory), tags); ## Environment Variables -If the following environment variables are set and not empty, there key and value will also be read and applied to -your tags -- `TITUS_CONTAINER_NAME`: Set the `nf.container` tag automatically -- `TITUS_PROCESS_NAME`: Set the `nf.process` tag automatically +If the following environment variables are set and not empty or whitespace-only, their values are added to your tags. +They take precedence over the same keys passed in `extraTags`. + +- `NETFLIX_CONTAINER_NAME`, falling back to `TITUS_CONTAINER_NAME`: sets the `nf.container` tag. +- `NETFLIX_PROCESS_NAME`, falling back to the deprecated `TITUS_PROCESS_NAME`: sets the `nf.process` tag. Set it + separately for each process, for example in the container's entrypoint: `NETFLIX_PROCESS_NAME=processA ./processA`. + A container that runs several processes must give each one its own name. Use a stable, low-cardinality name made of + the characters `-._A-Za-z0-9^~`; any other character is replaced with `_`. + +To leave out both tags, pass `false` as the third parameter, `includeEnvTags`. Do this in a process that reports +metrics on behalf of other containers or processes, such as a relay or an agent like atlas-system-agent's pod monitor, +so that its own container and process names do not overwrite the tags of the metrics it reports. Tags passed in +`extraTags` are still added. + +```cpp +Config config(WriterConfig(WriterTypes::UDP), {}, false); +``` ### Warning diff --git a/libs/config/config.cpp b/libs/config/config.cpp index f86934b..6967fb2 100644 --- a/libs/config/config.cpp +++ b/libs/config/config.cpp @@ -6,6 +6,8 @@ #include #include +#include +#include namespace spectator { @@ -13,49 +15,57 @@ struct ConfigConstants { static constexpr auto Container = "nf.container"; static constexpr auto Process = "nf.process"; - static constexpr auto EnvVarContainer = "TITUS_CONTAINER_NAME"; - static constexpr auto EnvVarProcess = "TITUS_PROCESS_NAME"; + static constexpr auto EnvVarContainer = "NETFLIX_CONTAINER_NAME"; + static constexpr auto EnvVarContainerFallback = "TITUS_CONTAINER_NAME"; + static constexpr auto EnvVarProcess = "NETFLIX_PROCESS_NAME"; + // Deprecated: read only when NETFLIX_PROCESS_NAME is unset or blank. Set NETFLIX_PROCESS_NAME instead. + static constexpr auto EnvVarProcessFallback = "TITUS_PROCESS_NAME"; }; -std::unordered_map CalculateTags( - const std::unordered_map& tags) +// Sets tags[tag] from the first of env_vars that is set and not empty or whitespace-only. The value is stored as-is +// (not trimmed). +static void AddFirstNonEmpty(std::unordered_map& tags, const char* tag, + std::initializer_list env_vars) { - std::unordered_map valid_tags; - - for (const auto& [fst, snd] : tags) + for (const char* env_var : env_vars) { - if (IsEmptyOrWhitespace(fst) == false && IsEmptyOrWhitespace(snd) == false) + const char* value = std::getenv(env_var); + if (value != nullptr && IsEmptyOrWhitespace(value) == false) { - valid_tags[fst] = snd; + tags[tag] = value; + return; } } +} - const char* container_name = std::getenv(ConfigConstants::EnvVarContainer); - const char* process_name = std::getenv(ConfigConstants::EnvVarProcess); +std::unordered_map CalculateTags( + const std::unordered_map& tags, bool includeEnvTags) +{ + std::unordered_map valid_tags; - if (container_name != nullptr) + for (const auto& [fst, snd] : tags) { - std::string container_str(container_name); - if (IsEmptyOrWhitespace(container_str) == false) + if (IsEmptyOrWhitespace(fst) == false && IsEmptyOrWhitespace(snd) == false) { - valid_tags[ConfigConstants::Container] = container_str; + valid_tags[fst] = snd; } } - if (process_name != nullptr) + // Added after the passed-in tags, so the environment values win on a key collision. + if (includeEnvTags == true) { - std::string process_str(process_name); - if (IsEmptyOrWhitespace(process_str) == false) - { - valid_tags[ConfigConstants::Process] = process_str; - } + AddFirstNonEmpty(valid_tags, ConfigConstants::Container, + {ConfigConstants::EnvVarContainer, ConfigConstants::EnvVarContainerFallback}); + AddFirstNonEmpty(valid_tags, ConfigConstants::Process, + {ConfigConstants::EnvVarProcess, ConfigConstants::EnvVarProcessFallback}); } return valid_tags; } -Config::Config(const WriterConfig& writerConfig, const std::unordered_map& extraTags) - : m_extraTags(CalculateTags(extraTags)), m_writerConfig(writerConfig) +Config::Config(const WriterConfig& writerConfig, const std::unordered_map& extraTags, + bool includeEnvTags) + : m_extraTags(CalculateTags(extraTags, includeEnvTags)), m_writerConfig(writerConfig) { Logger::info("Config initialized with writer type: {}, buffer size: {}, location: {}", WriterTypeToString(m_writerConfig.GetType()), m_writerConfig.GetBufferSize(), m_writerConfig.GetLocation()); diff --git a/libs/config/config.h b/libs/config/config.h index 382cc27..14b5dcd 100644 --- a/libs/config/config.h +++ b/libs/config/config.h @@ -10,7 +10,12 @@ namespace spectator { class Config { public: - Config(const WriterConfig& writerConfig, const std::unordered_map& extraTags = {}); + // When includeEnvTags is true (the default), nf.container and nf.process are added from the environment and win + // over extraTags (see README.md). Pass false in a process that reports metrics on behalf of other containers or + // processes, such as a relay or an agent like atlas-system-agent's pod monitor, so its own identity does not + // overwrite their per-meter tags. + Config(const WriterConfig& writerConfig, const std::unordered_map& extraTags = {}, + bool includeEnvTags = true); ~Config() = default; Config(const Config& other) = default; diff --git a/libs/config/test_config.cpp b/libs/config/test_config.cpp index 2724e09..71bfc5d 100644 --- a/libs/config/test_config.cpp +++ b/libs/config/test_config.cpp @@ -43,14 +43,18 @@ class ConfigTest : public testing::Test { protected: // Create guards for each environment variable + EnvironmentVariableGuard netflixContainerGuard{"NETFLIX_CONTAINER_NAME"}; EnvironmentVariableGuard containerGuard{"TITUS_CONTAINER_NAME"}; - EnvironmentVariableGuard processGuard{"TITUS_PROCESS_NAME"}; + EnvironmentVariableGuard processGuard{"NETFLIX_PROCESS_NAME"}; + EnvironmentVariableGuard legacyProcessGuard{"TITUS_PROCESS_NAME"}; void SetUp() override { // Ensure environment variables are unset before each test + netflixContainerGuard.unsetValue(); containerGuard.unsetValue(); processGuard.unsetValue(); + legacyProcessGuard.unsetValue(); } }; @@ -217,4 +221,159 @@ TEST_F(ConfigTest, MergingTags) EXPECT_EQ(config.GetExtraTags().at("custom"), "value"); EXPECT_EQ(config.GetExtraTags().at("env"), "test"); } +} + +// Test the nf.container sources: NETFLIX_CONTAINER_NAME first, TITUS_CONTAINER_NAME as the fallback +TEST_F(ConfigTest, ContainerNameSources) +{ + WriterConfig writerConfig(WriterTypes::Memory); + + // NETFLIX_CONTAINER_NAME wins over TITUS_CONTAINER_NAME + { + netflixContainerGuard.setValue("app"); + containerGuard.setValue("main"); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.container"), "app"); + } + + // An empty or whitespace-only NETFLIX_CONTAINER_NAME falls back to TITUS_CONTAINER_NAME + for (const char* blank : {"", " "}) + { + netflixContainerGuard.setValue(blank); + containerGuard.setValue("main"); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.container"), "main"); + } + + // NETFLIX_CONTAINER_NAME alone + { + netflixContainerGuard.setValue("app"); + containerGuard.unsetValue(); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.container"), "app"); + } + + // Neither set: no nf.container + { + netflixContainerGuard.unsetValue(); + containerGuard.unsetValue(); + + Config config(writerConfig); + + EXPECT_FALSE(config.GetExtraTags().count("nf.container")); + } + + // NETFLIX_CONTAINER_NAME wins over a passed-in nf.container + { + netflixContainerGuard.setValue("app"); + + std::unordered_map tags = {{"nf.container", "passed-in"}}; + + Config config(writerConfig, tags); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.container"), "app"); + } +} + +// Test the nf.process sources: NETFLIX_PROCESS_NAME first, the deprecated TITUS_PROCESS_NAME as the fallback +TEST_F(ConfigTest, ProcessNameSources) +{ + WriterConfig writerConfig(WriterTypes::Memory); + + // NETFLIX_PROCESS_NAME wins over TITUS_PROCESS_NAME + { + processGuard.setValue("processA"); + legacyProcessGuard.setValue("legacy-process"); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.process"), "processA"); + } + + // TITUS_PROCESS_NAME is used when NETFLIX_PROCESS_NAME is unset + { + processGuard.unsetValue(); + legacyProcessGuard.setValue("legacy-process"); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.process"), "legacy-process"); + } + + // TITUS_PROCESS_NAME is used when NETFLIX_PROCESS_NAME is empty or whitespace-only + for (const char* blank : {"", " "}) + { + processGuard.setValue(blank); + legacyProcessGuard.setValue("legacy-process"); + + Config config(writerConfig); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.process"), "legacy-process"); + } + + // Empty values are ignored: no nf.process + { + processGuard.setValue(""); + legacyProcessGuard.setValue(""); + + Config config(writerConfig); + + EXPECT_FALSE(config.GetExtraTags().count("nf.process")); + } + + // NETFLIX_PROCESS_NAME wins over a passed-in nf.process + { + processGuard.setValue("processA"); + legacyProcessGuard.unsetValue(); + + std::unordered_map tags = {{"nf.process", "passed-in"}}; + + Config config(writerConfig, tags); + + EXPECT_EQ(config.GetExtraTags().size(), 1); + EXPECT_EQ(config.GetExtraTags().at("nf.process"), "processA"); + } +} + +// Test that includeEnvTags = false leaves out the environment-derived tags and keeps the passed-in ones +TEST_F(ConfigTest, IncludeEnvTagsFalse) +{ + WriterConfig writerConfig(WriterTypes::Memory); + netflixContainerGuard.setValue("app"); + containerGuard.setValue("main"); + processGuard.setValue("processA"); + legacyProcessGuard.setValue("legacy-process"); + + // No environment-derived tags + { + Config config(writerConfig, {}, false); + + EXPECT_TRUE(config.GetExtraTags().empty()); + } + + // Passed-in tags are kept, including nf.container and nf.process + { + std::unordered_map tags = { + {"custom", "value"}, {"nf.container", "passed-in"}, {"nf.process", "passed-in"}}; + + Config config(writerConfig, tags, false); + + EXPECT_EQ(config.GetExtraTags().size(), 3); + EXPECT_EQ(config.GetExtraTags().at("custom"), "value"); + EXPECT_EQ(config.GetExtraTags().at("nf.container"), "passed-in"); + EXPECT_EQ(config.GetExtraTags().at("nf.process"), "passed-in"); + } } \ No newline at end of file diff --git a/spectator/test_registry.cpp b/spectator/test_registry.cpp index a21dcfe..265a16d 100644 --- a/spectator/test_registry.cpp +++ b/spectator/test_registry.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -8,6 +9,23 @@ using namespace spectator; +// Config adds nf.container and nf.process from these environment variables. Clear them before the tests run, so the +// exact-output checks below also pass on hosts where the platform sets them. Registered from a global initializer +// because this test uses gtest_main. +class ClearEnvTagsEnvironment : public testing::Environment +{ + public: + void SetUp() override + { + unsetenv("NETFLIX_CONTAINER_NAME"); + unsetenv("TITUS_CONTAINER_NAME"); + unsetenv("NETFLIX_PROCESS_NAME"); + unsetenv("TITUS_PROCESS_NAME"); + } +}; + +testing::Environment* const clearEnvTagsEnvironment = testing::AddGlobalTestEnvironment(new ClearEnvTagsEnvironment); + TEST(RegistryTest, Close) { auto config = Config(WriterConfig(WriterTypes::Memory));