From 634cea31ca59949afc6dcafeb90a599cedd4f11a Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Wed, 2 Oct 2024 14:53:01 -0700 Subject: [PATCH 1/4] feat!: Implement consistency for DD_SERVICE - Set a default service name value when none is provided by DD_SERVICE or configuration Issues: APMAPI-709 --- doc/maintainers.md | 1 - include/datadog/error.h | 2 +- src/datadog/platform_util.cpp | 20 ++++++++++++++++++++ src/datadog/platform_util.h | 2 ++ src/datadog/tracer_config.cpp | 3 ++- test/test_tracer_config.cpp | 11 ++++++++--- 6 files changed, 33 insertions(+), 6 deletions(-) diff --git a/doc/maintainers.md b/doc/maintainers.md index 8db4cd3a..d36ee126 100644 --- a/doc/maintainers.md +++ b/doc/maintainers.md @@ -385,7 +385,6 @@ david@ein:~/src/dd-trace-cpp/src/datadog$ sed -n 's/^\s*\(\w\+\)\s*=\s*[0-9]\+,\ 1 SPAN_SAMPLING_RULES_MAX_PER_SECOND_WRONG_TYPE 1 SPAN_SAMPLING_RULES_INVALID_JSON 1 SPAN_SAMPLING_RULES_FILE_IO - 1 SERVICE_NAME_REQUIRED 1 RULE_WRONG_TYPE 1 RULE_TAG_WRONG_TYPE 1 RULE_PROPERTY_WRONG_TYPE diff --git a/include/datadog/error.h b/include/datadog/error.h index fdc0dc33..bb7b88c6 100644 --- a/include/datadog/error.h +++ b/include/datadog/error.h @@ -24,7 +24,7 @@ struct Error { // versions. enum Code { OTHER = 1, - SERVICE_NAME_REQUIRED = 2, + // SERVICE_NAME_REQUIRED = 2, MESSAGEPACK_ENCODE_FAILURE = 3, CURL_REQUEST_FAILURE = 4, DATADOG_AGENT_NULL_HTTP_CLIENT = 5, diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index 600f4309..dee779ec 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -189,6 +189,26 @@ int get_process_id() { #endif } +std::string get_process_name() { +#if defined(__APPLE__) || defined(__FreeBSD__) + return getprogname(); +#elif defined(__linux__) || defined(__unix__) + return program_invocation_short_name; +#elif defined(_MSC_VER) + TCHAR szExeFileName[MAX_PATH]; + GetModuleFileName(NULL, szExeFileName, MAX_PATH); +#ifdef UNICODE + std::wstring wStr(szExeFileName); + std::string path = std::string(wStr.begin(), wStr.end()); +#else + std::string path = std::string(szExeFileName); +#endif + return path.substr(path.find_last_of("/\\") + 1); +#else + return "unknown-service"; +#endif +} + int at_fork_in_child(void (*on_fork)()) { #if defined(_MSC_VER) // Windows does not have `fork`, and so this is not relevant there. diff --git a/src/datadog/platform_util.h b/src/datadog/platform_util.h index 7e1cfb75..cfadfecd 100644 --- a/src/datadog/platform_util.h +++ b/src/datadog/platform_util.h @@ -26,6 +26,8 @@ std::string get_hostname(); int get_process_id(); +std::string get_process_name(); + int at_fork_in_child(void (*on_fork)()); } // namespace tracing diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index 6cf0b280..ce6e8574 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -15,6 +15,7 @@ #include "datadog_agent.h" #include "json.hpp" #include "parse_util.h" +#include "platform_util.h" #include "string_util.h" namespace datadog { @@ -263,7 +264,7 @@ Expected finalize_config(const TracerConfig &user_config, pick(env_config->service, user_config.service, ""); if (final_config.defaults.service.empty()) { - return Error{Error::SERVICE_NAME_REQUIRED, "Service name is required."}; + final_config.defaults.service = get_process_name(); } final_config.metadata[ConfigName::SERVICE_NAME] = ConfigMetadata( diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 1050e216..940e2d55 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -145,16 +145,21 @@ class SomewhatSecureTemporaryFile : public std::fstream { TEST_CASE("TracerConfig::defaults") { TracerConfig config; - SECTION("service is required") { + SECTION("service is not required") { SECTION("empty") { auto finalized = finalize_config(config); - REQUIRE(!finalized); - REQUIRE(finalized.error().code == Error::SERVICE_NAME_REQUIRED); + REQUIRE(finalized); +#ifdef _MSC_VER + REQUIRE(finalized->defaults.service == "tests.exe"); +#else + REQUIRE(finalized->defaults.service == "tests"); +#endif } SECTION("nonempty") { config.service = "testsvc"; auto finalized = finalize_config(config); REQUIRE(finalized); + REQUIRE(finalized->defaults.service == "testsvc"); } } From c312c99043b8c372a1b5fc6efd4f72b9a1ff1a48 Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Thu, 26 Sep 2024 14:54:34 -0700 Subject: [PATCH 2/4] test: Remove the parametric app's default service and environment configuration so we can test default behaviors Issues: APMAPI-708 , APMAPI-709 --- test/system-tests/main.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/system-tests/main.cpp b/test/system-tests/main.cpp index 85e4c012..516d8ac2 100644 --- a/test/system-tests/main.cpp +++ b/test/system-tests/main.cpp @@ -77,8 +77,6 @@ int main(int argc, char* argv[]) { datadog::tracing::TracerConfig config; config.logger = logger; config.agent.event_scheduler = event_scheduler; - config.service = "cpp-parametric-test"; - config.environment = "staging"; config.name = "http.request"; auto finalized_config = datadog::tracing::finalize_config(config); From 2cdefb46c957f87bd4dd4e41d43a6bbb6ca39d72 Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Mon, 7 Oct 2024 07:00:43 -0700 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Damien Mehala --- src/datadog/platform_util.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index dee779ec..39728168 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -191,17 +191,20 @@ int get_process_id() { std::string get_process_name() { #if defined(__APPLE__) || defined(__FreeBSD__) - return getprogname(); + char* process_name = getprogname(); + return (process_name != nullptr) ? process_name : "unknown-service"; #elif defined(__linux__) || defined(__unix__) return program_invocation_short_name; #elif defined(_MSC_VER) - TCHAR szExeFileName[MAX_PATH]; - GetModuleFileName(NULL, szExeFileName, MAX_PATH); + TCHAR exe_name[MAX_PATH]; + if (GetModuleFileName(NULL, exe_name, MAX_PATH) <= 0) { + return "unknown-service"; + } #ifdef UNICODE - std::wstring wStr(szExeFileName); + std::wstring wStr(exe_name); std::string path = std::string(wStr.begin(), wStr.end()); #else - std::string path = std::string(szExeFileName); + std::string path = std::string(exe_name); #endif return path.substr(path.find_last_of("/\\") + 1); #else From bb5065c9ee91fc836b0a327744ff405185f56047 Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Mon, 7 Oct 2024 10:06:54 -0400 Subject: [PATCH 4/4] Fix formatting after incorporating code review feedback --- src/datadog/platform_util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/datadog/platform_util.cpp b/src/datadog/platform_util.cpp index 39728168..a270686b 100644 --- a/src/datadog/platform_util.cpp +++ b/src/datadog/platform_util.cpp @@ -199,7 +199,7 @@ std::string get_process_name() { TCHAR exe_name[MAX_PATH]; if (GetModuleFileName(NULL, exe_name, MAX_PATH) <= 0) { return "unknown-service"; - } + } #ifdef UNICODE std::wstring wStr(exe_name); std::string path = std::string(wStr.begin(), wStr.end());