From eee9a3cfe89d9a7d7b2c65c3a27c7f90a9ecb705 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 23 Mar 2023 11:25:54 +0100 Subject: [PATCH 1/3] Enable UDS settings Ensure UDS settings are allowed by the exporter --- cmake/Format.cmake | 3 +- include/ddprof_input.hpp | 1 - include/exporter_input.hpp | 6 +-- src/ddprof_context_lib.cc | 57 -------------------------- src/ddprof_input.cc | 1 - src/exporter/ddprof_exporter.cc | 71 ++++++++++++++++++++++++++------- test/ddprof_exporter-ut.cc | 16 ++++++-- 7 files changed, 74 insertions(+), 81 deletions(-) diff --git a/cmake/Format.cmake b/cmake/Format.cmake index 52635d3ce..f37748fee 100644 --- a/cmake/Format.cmake +++ b/cmake/Format.cmake @@ -6,4 +6,5 @@ add_custom_target(format COMMAND ${CMAKE_SOURCE_DIR}/tools/style-check.sh) add_custom_target(format-apply COMMAND ${CMAKE_SOURCE_DIR}/tools/style-check.sh apply) -add_custom_target(help-generate COMMAND ${CMAKE_SOURCE_DIR}/tools/help_generate.sh -b ${CMAKE_BINARY_DIR}) +add_custom_target(help-generate COMMAND ${CMAKE_SOURCE_DIR}/tools/help_generate.sh -b + ${CMAKE_BINARY_DIR}) diff --git a/include/ddprof_input.hpp b/include/ddprof_input.hpp index d40cafef8..7f22a34f2 100644 --- a/include/ddprof_input.hpp +++ b/include/ddprof_input.hpp @@ -76,7 +76,6 @@ typedef struct DDProfInput { XX(DD_API_KEY, api_key, A, 'A', 1, input, NULL, "", exp_input.) \ XX(DD_ENV, environment, E, 'E', 1, input, NULL, "", exp_input.) \ XX(DD_AGENT_HOST, host, H, 'H', 1, input, NULL, "localhost", exp_input.) \ - XX(DD_SITE, site, I, 'I', 1, input, NULL, "", exp_input.) \ XX(DD_TRACE_AGENT_PORT, port, P, 'P', 1, input, NULL, "8126", exp_input.) \ XX(DD_TRACE_AGENT_URL, url, U, 'U', 1, input, NULL, "", ) \ XX(DD_SERVICE, service, S, 'S', 1, input, NULL, "myservice", exp_input.) \ diff --git a/include/exporter_input.hpp b/include/exporter_input.hpp index b6daa449d..b1f6eb12f 100644 --- a/include/exporter_input.hpp +++ b/include/exporter_input.hpp @@ -18,7 +18,7 @@ typedef struct ExporterInput { const char *api_key; // Datadog api key const char *environment; // ex: staging / local / prod const char *host; // agent host ex:162.184.2.1 - const char *site; // not used for now + const char *url; // url (can contain port and schema) const char *port; // port appended to the host IP (ignored in agentless) const char *service; // service to identify the profiles (ex:prof-probe-native) @@ -60,7 +60,7 @@ static inline DDRes exporter_input_copy(const ExporterInput *src, DUP_PARAM(api_key); DUP_PARAM(environment); DUP_PARAM(host); - DUP_PARAM(site); + DUP_PARAM(url); DUP_PARAM(port); DUP_PARAM(service); DUP_PARAM(service_version); @@ -78,7 +78,7 @@ static inline void exporter_input_free(ExporterInput *exporter_input) { free((char *)exporter_input->api_key); free((char *)exporter_input->environment); free((char *)exporter_input->host); - free((char *)exporter_input->site); + free((char *)exporter_input->url); free((char *)exporter_input->port); free((char *)exporter_input->service); free((char *)exporter_input->service_version); diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 862214d4f..7e1c8c312 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -242,63 +242,6 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } } - // URL-based host/port override - if (input->url && *input->url) { - LG_NTC("Processing URL: %s", input->url); - char *delim = strchr(input->url, ':'); - char *host = input->url; - char *port = NULL; - if (delim && delim[1] == '/' && delim[2] == '/') { - // A colon was found. - // http://hostname:port -> (hostname, port) - // ftp://hostname:port -> error - // hostname:port -> (hostname, port) - // hostname: -> (hostname, default_port) - // hostname -> (hostname, default_port) - - // Drop the schema - *delim = '\0'; - if (!strncasecmp(input->url, "http", 4) || - !strncasecmp(input->url, "https", 5)) { - *delim = ':'; - host = delim + 3; // Navigate after schema - } - delim = strchr(host, ':'); - } - - if (delim) { - // Check to see if there is another colon for the port - // We're going to treat this as the port. This is slightly problematic, - // since an invalid port is going to invalidate the default and then throw - // an error later, but for now let's just do what the user told us even if - // it isn't what they wanted. :) - *delim = '\0'; - port = delim + 1; - } - - // Modify the input structure to reflect the values from the URL. This - // overwrites an otherwise immutable parameter, which is slightly - // unfortunate, but this way it harmonizes with the downstream movement of - // host/port and the input arg pretty-printer. - if (host) { - free((char *)input->exp_input.host); - free((char *)ctx->exp_input.host); - input->exp_input.host = strdup(host); // For the pretty-printer - ctx->exp_input.host = strdup(host); - } - if (port) { - free((char *)input->exp_input.port); - free((char *)ctx->exp_input.port); - input->exp_input.port = strdup(port); // Merely for the pretty-printer - ctx->exp_input.port = strdup(port); - } - - // Revert the delimiter in case we want to print the URL later - if (delim) { - *delim = ':'; - } - } - ctx->params.sockfd = -1; ctx->params.wait_on_socket = false; if (input->socket && strlen(input->socket) > 0) { diff --git a/src/ddprof_input.cc b/src/ddprof_input.cc index e3ece4eb0..4aefcf698 100644 --- a/src/ddprof_input.cc +++ b/src/ddprof_input.cc @@ -75,7 +75,6 @@ const char* help_str[DD_KLEN] = { " The name of the environment to use in the Datadog UI.\n", [DD_AGENT_HOST] = " The hostname of the agent. Port should also be specified.\n", - [DD_SITE] = STR_UNDF, [DD_TRACE_AGENT_PORT] = " The communication port for the Datadog agent or backend system.\n", [DD_TRACE_AGENT_URL] = diff --git a/src/exporter/ddprof_exporter.cc b/src/exporter/ddprof_exporter.cc index 74246c124..53653dd21 100644 --- a/src/exporter/ddprof_exporter.cc +++ b/src/exporter/ddprof_exporter.cc @@ -28,13 +28,22 @@ static const int k_size_api_key = 32; static char *alloc_url_agent(const char *protocol, const char *host, const char *port) { - size_t expected_size = snprintf(NULL, 0, "%s%s:%s", protocol, host, port); - char *url = (char *)malloc(expected_size + 1); - if (!url) // Early exit on alloc failure - return NULL; - - snprintf(url, expected_size + 1, "%s%s:%s", protocol, host, port); - return url; + if (port) { + size_t expected_size = snprintf(NULL, 0, "%s%s:%s", protocol, host, port); + char *url = (char *)malloc(expected_size + 1); + if (!url) // Early exit on alloc failure + return NULL; + + snprintf(url, expected_size + 1, "%s%s:%s", protocol, host, port); + return url; + } else { + size_t expected_size = snprintf(NULL, 0, "%s%s", protocol, host); + char *url = (char *)malloc(expected_size + 1); + if (!url) // Early exit on alloc failure + return NULL; + snprintf(url, expected_size + 1, "%s%s", protocol, host); + return url; + } } static DDRes create_pprof_file(ddog_Timespec start, @@ -73,6 +82,21 @@ static DDRes write_pprof_file(const ddog_EncodedProfile *encoded_profile, return {}; } +bool contains_port(const char *url) { + const char *port_ptr = strrchr(url, ':'); + if (port_ptr != NULL) { + // Check if the characters after the ':' are digits + for (const char *p = port_ptr + 1; *p != '\0'; p++) { + if (!isdigit(*p)) { + return false; + } + } + return true; + } else { + return false; + } +} + DDRes ddprof_exporter_init(const ExporterInput *exporter_input, DDProfExporter *exporter) { memset(exporter, 0, sizeof(DDProfExporter)); @@ -91,20 +115,39 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input, } if (exporter->_agent) { - exporter->_url = - alloc_url_agent("http://", exporter_input->host, exporter_input->port); + const char *port_str = exporter_input->port; + + if (exporter_input->url) { + // uds -> no port + if (!strncasecmp(exporter_input->url, "unix", 4)) { + port_str = nullptr; + } + // already port -> no port + if (contains_port(exporter_input->url)) { + port_str = nullptr; + } + if (strstr(exporter_input->url, "://") != NULL) { + exporter->_url = alloc_url_agent("", exporter_input->url, port_str); + } else { + exporter->_url = + alloc_url_agent("http://", exporter_input->url, port_str); + } + } else { + exporter->_url = alloc_url_agent("http://", exporter_input->host, + exporter_input->port); + } } else { - // site is the usual option for intake - if (exporter->_input.site) { + if (exporter->_input.url) { // warning : should not contain intake.profile. (prepended in // libdatadog_profiling) - exporter->_url = strdup(exporter_input->site); + exporter->_url = strdup(exporter_input->url); } else { LG_WRN( "[EXPORTER] Agentless - Attempting to use host (%s) instead of empty " - "site", + "url", exporter_input->host); - exporter->_url = strdup(exporter_input->host); + exporter->_url = alloc_url_agent("http://", exporter_input->host, + exporter_input->port); } } if (!exporter->_url) { diff --git a/test/ddprof_exporter-ut.cc b/test/ddprof_exporter-ut.cc index bb48552ef..3e3e94d71 100644 --- a/test/ddprof_exporter-ut.cc +++ b/test/ddprof_exporter-ut.cc @@ -79,7 +79,7 @@ void fill_mock_exporter_input(ExporterInput &exporter_input, exporter_input.agentless = "yes"; exporter_input.environment = "unit-test"; exporter_input.host = url.first.c_str(); - exporter_input.site = "datadog_is_cool.com"; + exporter_input.url = "datadog_is_cool.com"; exporter_input.port = url.second.c_str(); exporter_input.service = MYNAME; exporter_input.service_version = "42"; @@ -104,20 +104,28 @@ TEST(DDProfExporter, url) { EXPECT_EQ(strcmp(exporter._url, "datadog_is_cool.com"), 0); ddprof_exporter_free(&exporter); - // Default to host if site not found // To be discussed : should we fail here ? - exporter_input.site = nullptr; + exporter_input.url = nullptr; res = ddprof_exporter_init(&exporter_input, &exporter); EXPECT_TRUE(IsDDResOK(res)); - EXPECT_EQ(strcmp(exporter._url, "25.04.1988.0"), 0); + EXPECT_EQ(strcmp(exporter._url, "http://25.04.1988.0:1234"), 0); ddprof_exporter_free(&exporter); // If no API key --> expect host fill_mock_exporter_input(exporter_input, url, false); + exporter_input.url = nullptr; res = ddprof_exporter_init(&exporter_input, &exporter); EXPECT_TRUE(IsDDResOK(res)); EXPECT_EQ(strcmp(exporter._url, "http://25.04.1988.0:1234"), 0); ddprof_exporter_free(&exporter); + + // UDS --> expect UDS + fill_mock_exporter_input(exporter_input, url, false); + exporter_input.url = "unix://some/uds/socket.sock"; + res = ddprof_exporter_init(&exporter_input, &exporter); + EXPECT_TRUE(IsDDResOK(res)); + EXPECT_EQ(strcmp(exporter._url, "unix://some/uds/socket.sock"), 0); + ddprof_exporter_free(&exporter); } TEST(DDProfExporter, simple) { From dee4815f535a3894262f993123e528a670316fe2 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 23 Mar 2023 11:46:47 +0100 Subject: [PATCH 2/3] Minor - remove url parameter from the ddprof input Parameter is now written directly to the exporter. --- include/ddprof_input.hpp | 3 +-- src/ddprof_context_lib.cc | 5 ++--- src/exporter/ddprof_exporter.cc | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/ddprof_input.hpp b/include/ddprof_input.hpp index 7f22a34f2..5a0e8a8f7 100644 --- a/include/ddprof_input.hpp +++ b/include/ddprof_input.hpp @@ -34,7 +34,6 @@ typedef struct DDProfInput { char *worker_period; char *internal_stats; char *tags; - char *url; char *socket; char *preset; char *switch_user; @@ -77,7 +76,7 @@ typedef struct DDProfInput { XX(DD_ENV, environment, E, 'E', 1, input, NULL, "", exp_input.) \ XX(DD_AGENT_HOST, host, H, 'H', 1, input, NULL, "localhost", exp_input.) \ XX(DD_TRACE_AGENT_PORT, port, P, 'P', 1, input, NULL, "8126", exp_input.) \ - XX(DD_TRACE_AGENT_URL, url, U, 'U', 1, input, NULL, "", ) \ + XX(DD_TRACE_AGENT_URL, url, U, 'U', 1, input, NULL, "", exp_input.) \ XX(DD_SERVICE, service, S, 'S', 1, input, NULL, "myservice", exp_input.) \ XX(DD_VERSION, service_version, V, 'V', 1, input, NULL, "", exp_input.) \ XX(DD_PROFILING_EXPORT, do_export, X, 'X', 1, input, NULL, "yes", exp_input.) \ diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 7e1c8c312..697be345d 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -152,9 +152,6 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { ctx->watchers[nwatchers] = input->watchers[nwatchers]; } ctx->num_watchers = nwatchers; - - DDRES_CHECK_FWD(exporter_input_copy(&input->exp_input, &ctx->exp_input)); - // Set defaults ctx->params.upload_period = 60.0; @@ -275,6 +272,8 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } } + DDRES_CHECK_FWD(exporter_input_copy(&input->exp_input, &ctx->exp_input)); + ddprof::span watchers{ctx->watchers, static_cast(ctx->num_watchers)}; if (std::find_if(watchers.begin(), watchers.end(), [](const auto &watcher) { return watcher.type < PERF_TYPE_MAX; diff --git a/src/exporter/ddprof_exporter.cc b/src/exporter/ddprof_exporter.cc index 53653dd21..e1d07c156 100644 --- a/src/exporter/ddprof_exporter.cc +++ b/src/exporter/ddprof_exporter.cc @@ -113,7 +113,7 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input, exporter->_agent = true; LG_NTC("[EXPORTER] Targeting agent mode (no API key)"); } - + if (exporter->_agent) { const char *port_str = exporter_input->port; From dedc1e1021fdf07e46c1d9ea954d88f48b089564 Mon Sep 17 00:00:00 2001 From: r1viollet Date: Thu, 23 Mar 2023 12:11:13 +0100 Subject: [PATCH 3/3] Minor add some comments around the url setting flow --- src/exporter/ddprof_exporter.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/exporter/ddprof_exporter.cc b/src/exporter/ddprof_exporter.cc index e1d07c156..d4500fff8 100644 --- a/src/exporter/ddprof_exporter.cc +++ b/src/exporter/ddprof_exporter.cc @@ -113,7 +113,7 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input, exporter->_agent = true; LG_NTC("[EXPORTER] Targeting agent mode (no API key)"); } - + if (exporter->_agent) { const char *port_str = exporter_input->port; @@ -123,20 +123,24 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input, port_str = nullptr; } // already port -> no port - if (contains_port(exporter_input->url)) { + else if (contains_port(exporter_input->url)) { port_str = nullptr; } + // check if schema is already available if (strstr(exporter_input->url, "://") != NULL) { exporter->_url = alloc_url_agent("", exporter_input->url, port_str); } else { + // not available, assume http exporter->_url = alloc_url_agent("http://", exporter_input->url, port_str); } } else { + // no url, use default host and port settings exporter->_url = alloc_url_agent("http://", exporter_input->host, exporter_input->port); } } else { + // agentless mode if (exporter->_input.url) { // warning : should not contain intake.profile. (prepended in // libdatadog_profiling)