diff --git a/CMakeLists.txt b/CMakeLists.txt index e6fbe5549..745937fc6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -192,7 +192,6 @@ endif() set(DD_PROFILING_SOURCES # cmake-format: sortable - src/daemonize.cc src/ddprof_cmdline.cc src/ddres_list.cc src/ipc.cc diff --git a/include/atomic_shared.hpp b/include/atomic_shared.hpp new file mode 100644 index 000000000..5f4ed6b83 --- /dev/null +++ b/include/atomic_shared.hpp @@ -0,0 +1,57 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once +#include +#include +#include +#include + +#include + +template class AtomicShared : public std::atomic { +public: + static void *operator new(size_t) { + void *const pv = mmap(0, sizeof(T), PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1, 0); + if (pv == MAP_FAILED) + throw std::bad_alloc(); + return pv; + }; + static void operator delete(void *pv) { munmap(pv, sizeof(T)); }; + AtomicShared &operator=(const int b) { + std::atomic::operator=(b); + return *this; + } + + bool value_timedwait(const T &oldval, int timeout) { + // Block until the value is different from oldval. If the timeout is 0, + // check once without blocking. If the value is negative, then block + // indefinitely (may be "expensive" in some sense). + // Doesn't do anything fancy to enforce re-scheduling the thread when + // the condition occurs, nor to decrease sleep overhead. As per the spec, + // doesn't protect against the ABA problem (A changes to B, then back to A, + // before B can be detected in the loop). + // Will perform three "fast checks" before starting to yield to the + // scheduler. This will appear as a hotspot when the caller has to wait + // a lot. + auto start = std::chrono::high_resolution_clock::now(); + auto end = std::chrono::milliseconds(timeout); + if (timeout < 0) { + end = std::chrono::hours::max(); + } + int fast_checks = 3; // hardcoded + do { + if (this->load() != oldval) { + return true; + } else if (fast_checks > 0) { + --fast_checks; + } else { + std::this_thread::yield(); + } + } while (std::chrono::high_resolution_clock::now() - start < end); + return false; + } +}; diff --git a/include/daemonize.hpp b/include/daemonize.hpp index 208fa1061..95e58d773 100644 --- a/include/daemonize.hpp +++ b/include/daemonize.hpp @@ -5,20 +5,108 @@ #pragma once +#include "atomic_shared.hpp" + #include #include +#include + +#include "daemonize.hpp" +#include "ddprof_exit.hpp" +#include "logger.hpp" namespace ddprof { +enum class DaemonizeState { Failure = -1, Invoker = 0, Daemon = 1 }; + struct DaemonizeResult { - pid_t - temp_pid; // -1 on failure, 0 for initial process, > 0 for daemon process - pid_t parent_pid; // pid of process initiating daemonize - pid_t daemon_pid; // pid of daemon process -}; + DaemonizeState state = DaemonizeState::Failure; + pid_t invoker_pid = -1; + pid_t daemon_pid = -1; + + // A reusable barrier between parent/daemon + void barrier() { + switch (state) { + case DaemonizeState::Failure: + break; + case DaemonizeState::Daemon: + sem_daemon->store(true); + sem_invoker->value_timedwait(false, timeout_val); + sem_invoker->store(false); + break; + case DaemonizeState::Invoker: + sem_invoker->store(true); + sem_daemon->value_timedwait(false, timeout_val); + sem_daemon->store(false); + break; + } + }; + + bool is_failure() { return state == DaemonizeState::Failure; } + bool is_daemon() { return state == DaemonizeState::Daemon; } + bool is_invoker() { return state == DaemonizeState::Invoker; } + + // Daemonization function + // cleanup_function is a callable invoked in the context of the intermediate, + // short-lived process that will be killed by daemon process. + bool daemonize(std::function cleanup_function = {}) { + // Initialize + pid_transfer = std::make_unique>(); + sem_invoker = std::make_unique>(); + sem_daemon = std::make_unique>(); + pid_transfer->store(0); + sem_invoker->store(false); + sem_daemon->store(false); -// Daemonization function -// cleanup_function is a callable invoked in the context of the intermediate, -// short-lived process that will be killed by daemon process. -DaemonizeResult daemonize(std::function cleanup_function = {}); -} // namespace ddprof \ No newline at end of file + // Start daemonizing + invoker_pid = getpid(); + pid_t temp_pid = fork(); // "middle"/"child" (temporary) PID + state = DaemonizeState::Failure; + + if (!temp_pid) { // temp PID enter branch + if (fork()) { // temp PID enter branch + if (cleanup_function) { + cleanup_function(); + } + + // Temporary PID exits to force re-init of grandchild (daemon) + throw ddprof::exit(); + std::exit(0); + + } else { // grandchild (daemon) PID enter branch + daemon_pid = getpid(); + + // Tell the invoker my PID, then wait until it gets changed again. + pid_transfer->store(daemon_pid); + if (!pid_transfer->value_timedwait(daemon_pid, timeout_val)) { + return false; + } + state = DaemonizeState::Daemon; + return true; + } + } else if (temp_pid != -1) { // parent PID enter branch + // Try to read the PID of the daemon from shared memory, then notify + if (!pid_transfer->value_timedwait(0, timeout_val)) { + return false; + } + daemon_pid = pid_transfer->exchange(0); // consume + reset + state = DaemonizeState::Invoker; + return true; + } + + // Should only arrive here if the first-level fork failed, but add sink to + if (getpid() != invoker_pid) { + LG_WRN("Extraneous PID (%d) detected", getpid()); + throw ddprof::exit(); + std::exit(-1); + } + return false; + } + +private: + std::unique_ptr> pid_transfer; + std::unique_ptr> sem_invoker; + std::unique_ptr> sem_daemon; + static constexpr size_t timeout_val = 1000; // 10 seconds +}; +} // namespace ddprof diff --git a/include/ddprof_cmdline.hpp b/include/ddprof_cmdline.hpp index aad07b003..6f9755ccc 100644 --- a/include/ddprof_cmdline.hpp +++ b/include/ddprof_cmdline.hpp @@ -8,6 +8,10 @@ #include // size_t #include // uint64_t +#include +#include +#include + typedef struct PerfWatcher PerfWatcher; /**************************** Cmdline Helpers *********************************/ @@ -22,9 +26,13 @@ typedef struct PerfWatcher PerfWatcher; /// Returns index to element that compars to str, otherwise -1 int arg_which(const char *str, char const *const *set, int sz_set); +int arg_which(const char *str, const std::vector &list); bool arg_inset(const char *str, char const *const *set, int sz_set); -bool arg_yesno(const char *str, int mode); +bool is_yes(const char *str); +bool is_yes(const std::string &str); +bool is_no(const char *str); +bool is_no(const std::string &str); bool watcher_from_str(const char *str, PerfWatcher *watcher); diff --git a/include/ddprof_context.hpp b/include/ddprof_context.hpp index e73edb716..bd4a95129 100644 --- a/include/ddprof_context.hpp +++ b/include/ddprof_context.hpp @@ -10,9 +10,12 @@ #include "ddprof_defs.hpp" #include "ddprof_worker_context.hpp" #include "exporter_input.hpp" +#include "logger.hpp" +#include "metric_aggregator.hpp" #include "perf_watcher.hpp" #include +#include // forward declarations typedef struct StackHandler StackHandler; @@ -33,9 +36,9 @@ typedef struct DDProfContext { bool wait_on_socket; bool show_samples; cpu_set_t cpu_affinity; - const char *switch_user; - const char *internal_stats; - const char *tags; + std::string switch_user; + std::string internal_stats; + std::string tags; } params; bool initialized; @@ -45,4 +48,7 @@ typedef struct DDProfContext { int num_watchers; void *callback_ctx; // user state to be used in callback (lib mode) DDProfWorkerContext worker_ctx; + MetricAggregator metrics; + + void release() noexcept; } DDProfContext; diff --git a/include/ddprof_exit.hpp b/include/ddprof_exit.hpp new file mode 100644 index 000000000..5cb2600b2 --- /dev/null +++ b/include/ddprof_exit.hpp @@ -0,0 +1,10 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once + +namespace ddprof { +struct exit : public std::exception {}; +} // namespace ddprof diff --git a/include/ddprof_input.hpp b/include/ddprof_input.hpp index d40cafef8..9f2da7caa 100644 --- a/include/ddprof_input.hpp +++ b/include/ddprof_input.hpp @@ -5,6 +5,8 @@ #pragma once +#include + #include "ddprof_defs.hpp" #include "ddres_def.hpp" #include "exporter_input.hpp" @@ -16,28 +18,29 @@ typedef int16_t watcher_index_t; typedef struct DDProfInput { int nb_parsed_params; // Parameters for interpretation - char *log_mode; - char *log_level; + std::string log_mode; + std::string log_level; // Input parameters - char *show_config; - char *show_samples; - char *affinity; - char *enable; - char *native_enable; - char *agentless; - char *upload_period; - char *fault_info; - char *core_dumps; - char *nice; - char *pid; - char *global; - char *worker_period; - char *internal_stats; - char *tags; - char *url; - char *socket; - char *preset; - char *switch_user; + std::string show_config; + std::string show_samples; + std::string affinity; + std::string enable; + std::string native_enable; + std::string agentless; + std::string upload_period; + std::string fault_info; + std::string core_dumps; + std::string nice; + std::string pid; + std::string global; + std::string worker_period; + std::string internal_stats; + std::string tags; + std::string url; + std::string socket; + std::string metrics_socket; + std::string preset; + std::string switch_user; // Watcher presets PerfWatcher watchers[MAX_TYPE_WATCHER]; int num_watchers; @@ -71,38 +74,39 @@ typedef struct DDProfInput { */ // clang-format off -// A B C D E F G H I -#define OPT_TABLE(XX) \ - 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.) \ - 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.) \ - XX(DD_PROFILING_PPROF_PREFIX, debug_pprof_prefix, O, 'O', 1, input, NULL, "", exp_input.) \ - XX(DD_PROFILING_AGENTLESS, agentless, L, 'L', 1, input, NULL, "", ) \ - XX(DD_TAGS, tags, T, 'T', 1, input, NULL, "", ) \ - XX(DD_PROFILING_ENABLED, enable, d, 'd', 1, input, NULL, "yes", ) \ - XX(DD_PROFILING_NATIVE_ENABLED, native_enable, n, 'n', 1, input, NULL, "", ) \ - XX(DD_PROFILING_UPLOAD_PERIOD, upload_period, u, 'u', 1, input, NULL, "59", ) \ - XX(DD_PROFILING_NATIVE_WORKER_PERIOD, worker_period, w, 'w', 1, input, NULL, "240", ) \ - XX(DD_PROFILING_NATIVE_FAULT_INFO, fault_info, s, 's', 1, input, NULL, "yes", ) \ - XX(DD_PROFILING_NATIVE_CORE_DUMPS, core_dumps, m, 'm', 1, input, NULL, "no", ) \ - XX(DD_PROFILING_NATIVE_NICE, nice, i, 'i', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SHOW_CONFIG, show_config, c, 'c', 1, input, NULL, "no", ) \ - XX(DD_PROFILING_NATIVE_LOG_MODE, log_mode, o, 'o', 1, input, NULL, "stdout", ) \ - XX(DD_PROFILING_NATIVE_LOG_LEVEL, log_level, l, 'l', 1, input, NULL, "error", ) \ - XX(DD_PROFILING_NATIVE_TARGET_PID, pid, p, 'p', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_GLOBAL, global, g, 'g', 1, input, NULL, "", ) \ - XX(DD_PROFILING_INTERNAL_STATS, internal_stats, b, 'b', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SOCKET, socket, z, 'z', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_PRESET, preset, D, 'D', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SHOW_SAMPLES, show_samples, y, 'y', 0, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_CPU_AFFINITY, affinity, a, 'a', 1, input, NULL, "", ) \ - XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) +// A B C D E F G H I +#define OPT_TABLE(XX) \ + 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.) \ + 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.) \ + XX(DD_PROFILING_PPROF_PREFIX, debug_pprof_prefix, O, 'O', 1, input, NULL, "", exp_input.) \ + XX(DD_PROFILING_AGENTLESS, agentless, L, 'L', 1, input, NULL, "", ) \ + XX(DD_TAGS, tags, T, 'T', 1, input, NULL, "", ) \ + XX(DD_PROFILING_ENABLED, enable, d, 'd', 1, input, NULL, "yes", ) \ + XX(DD_PROFILING_NATIVE_ENABLED, native_enable, n, 'n', 1, input, NULL, "", ) \ + XX(DD_PROFILING_UPLOAD_PERIOD, upload_period, u, 'u', 1, input, NULL, "59", ) \ + XX(DD_PROFILING_NATIVE_WORKER_PERIOD, worker_period, w, 'w', 1, input, NULL, "240", ) \ + XX(DD_PROFILING_NATIVE_FAULT_INFO, fault_info, s, 's', 1, input, NULL, "yes", ) \ + XX(DD_PROFILING_NATIVE_CORE_DUMPS, core_dumps, m, 'm', 1, input, NULL, "no", ) \ + XX(DD_PROFILING_NATIVE_NICE, nice, i, 'i', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_SHOW_CONFIG, show_config, c, 'c', 1, input, NULL, "no", ) \ + XX(DD_PROFILING_NATIVE_LOG_MODE, log_mode, o, 'o', 1, input, NULL, "stdout", ) \ + XX(DD_PROFILING_NATIVE_LOG_LEVEL, log_level, l, 'l', 1, input, NULL, "error", ) \ + XX(DD_PROFILING_NATIVE_TARGET_PID, pid, p, 'p', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_GLOBAL, global, g, 'g', 1, input, NULL, "", ) \ + XX(DD_PROFILING_INTERNAL_STATS, internal_stats, b, 'b', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_SOCKET, socket, z, 'z', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_METRICS_SOCKET, metrics_socket, k, 'k', 1, input, NULL, "/var/run/datadog-agent/statsd.sock", ) \ + XX(DD_PROFILING_NATIVE_PRESET, preset, D, 'D', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_SHOW_SAMPLES, show_samples, y, 'y', 0, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_CPU_AFFINITY, affinity, a, 'a', 1, input, NULL, "", ) \ + XX(DD_PROFILING_NATIVE_SWITCH_USER, switch_user, W, 'W', 1, input, NULL, "", ) // clang-format on #define X_ENUM(a, b, c, d, e, f, g, h, i) a, @@ -122,5 +126,3 @@ void ddprof_print_help(); // Print help void ddprof_print_params(const DDProfInput *input); - -void ddprof_input_free(DDProfInput *input); diff --git a/include/ddprof_worker_context.hpp b/include/ddprof_worker_context.hpp index de05c3dca..1051f7ec7 100644 --- a/include/ddprof_worker_context.hpp +++ b/include/ddprof_worker_context.hpp @@ -10,6 +10,7 @@ #include #include +#include typedef struct DDProfExporter DDProfExporter; typedef struct DDProfPProf DDProfPProf; @@ -22,9 +23,9 @@ typedef struct UserTags UserTags; struct DDProfWorkerContext { // Persistent reference to the state shared accross workers PersistentWorkerState *persistent_worker_state; - PEventHdr pevent_hdr; // perf_event buffer holder - DDProfExporter *exp[2]; // wrapper around rust exporter - DDProfPProf *pprof[2]; // wrapper around rust exporter + PEventHdr pevent_hdr; // perf_event buffer holder + std::shared_ptr exp[2]; // wrapper around rust exporter + std::shared_ptr pprof[2]; // wrapper around rust exporter int i_current_pprof; volatile bool exp_error; pthread_t exp_tid; diff --git a/include/ddres_helpers.hpp b/include/ddres_helpers.hpp index 463952703..b801602f1 100644 --- a/include/ddres_helpers.hpp +++ b/include/ddres_helpers.hpp @@ -9,6 +9,8 @@ #include "ddres_list.hpp" #include "logger.hpp" +#include + /// Replacement for variadic macro niladic expansion via `__VA_OPT__`, which /// is unsupported (boo!) in standards-compliant C static analysis tools and /// checkers. @@ -58,7 +60,7 @@ int e = errno; \ LG_ERR(__VA_ARGS__); \ LOG_ERROR_DETAILS(LG_ERR, what); \ - LG_ERR("errno(%d): %s", e, strerror(e)); \ + LG_ERR("errno(%d): %s", e, std::strerror(e)); \ return ddres_error(what); \ } \ } while (0) diff --git a/include/ddres_list.hpp b/include/ddres_list.hpp index bc0ec47bc..412cc8816 100644 --- a/include/ddres_list.hpp +++ b/include/ddres_list.hpp @@ -20,6 +20,7 @@ X(UKNWEXCEPT, "unknown exception caught") #define NATIVE_ERROR_TABLE(X) \ + X(EXIT, "exiting process") \ X(DWFL_LIB_ERROR, "error withing dwfl library") \ X(UW_CACHE_ERROR, "error from unwinding cache") \ X(UW_ERROR, "error from unwinding code") \ diff --git a/include/exporter/ddprof_exporter.hpp b/include/exporter/ddprof_exporter.hpp index cc1b59344..fb4f4ee42 100644 --- a/include/exporter/ddprof_exporter.hpp +++ b/include/exporter/ddprof_exporter.hpp @@ -5,6 +5,8 @@ #pragma once +#include + #include "ddprof_defs.hpp" #include "ddres_def.hpp" #include "exporter_input.hpp" @@ -20,8 +22,8 @@ typedef struct UserTags UserTags; typedef struct DDProfExporter { ExporterInput _input; - char *_url; // url contains path and port - const char *_debug_pprof_prefix; // write pprofs to folder + std::string _url; // url contains path and port + std::string _debug_pprof_prefix; // write pprofs to folder ddog_ProfileExporter *_exporter; bool _agent; bool _export; // debug mode : should we send profiles ? @@ -30,12 +32,12 @@ typedef struct DDProfExporter { } DDProfExporter; DDRes ddprof_exporter_init(const ExporterInput *exporter_input, - DDProfExporter *exporter); + std::shared_ptr &exporter); -DDRes ddprof_exporter_new(const UserTags *user_tags, DDProfExporter *exporter); +DDRes ddprof_exporter_new(const UserTags *user_tags, DDProfExporter &exporter); -DDRes ddprof_exporter_export(const struct ddog_Profile *profile, +DDRes ddprof_exporter_export(const struct ddog_Profile &profile, const ddprof::Tags &additional_tags, - uint32_t profile_seq, DDProfExporter *exporter); + uint32_t profile_seq, DDProfExporter &exporter); -DDRes ddprof_exporter_free(DDProfExporter *exporter); +DDRes ddprof_exporter_free(DDProfExporter &exporter); diff --git a/include/exporter_input.hpp b/include/exporter_input.hpp index b6daa449d..3dbb338bb 100644 --- a/include/exporter_input.hpp +++ b/include/exporter_input.hpp @@ -6,29 +6,26 @@ #pragma once #include "ddres.hpp" -#include "string_view.hpp" #include - -#define USERAGENT_DEFAULT "ddprof" -#define LANGUAGE_DEFAULT "native" -#define FAMILY_DEFAULT "native" +#include +#include 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 *port; // port appended to the host IP (ignored in agentless) - const char - *service; // service to identify the profiles (ex:prof-probe-native) - const char *service_version; // appended to tags (example: 1.2.1) - const char *do_export; // prevent exports if needed (debug flag) - const char *debug_pprof_prefix; // local pprof prefix (debug) - string_view user_agent; // ignored for now (override in shared lib) - string_view language; // appended to the tags (set to native) - string_view family; - string_view profiler_version; + std::string api_key; // Datadog api key + std::string environment; // ex: staging / local / prod + std::string host; // agent host ex:162.184.2.1 + std::string site; // not used for now + std::string port; // port appended to the host IP (ignored in agentless) + std::string + service; // service to identify the profiles (ex:prof-probe-native) + std::string service_version; // appended to tags (example: 1.2.1) + std::string do_export; // prevent exports if needed (debug flag) + std::string debug_pprof_prefix; // local pprof prefix (debug) + std::string_view user_agent = "ddprof"; + std::string_view language = "native"; + std::string_view family = "native"; + std::string_view profiler_version; bool agentless; // Whether or not to actually use API key/intake } ExporterInput; @@ -39,33 +36,12 @@ typedef struct ExporterInput { // Possible improvement : create X table to factorize this code -#define DUP_PARAM(key_name) \ - if (src->key_name) { \ - dest->key_name = strdup(src->key_name); \ - if (!dest->key_name) { \ - return ddres_error(DD_WHAT_ARGUMENT); \ - } \ - } else \ - dest->key_name = NULL; - static inline void exporter_input_dflt(ExporterInput *exp_input) { - exp_input->family = STRING_VIEW_LITERAL(FAMILY_DEFAULT); - exp_input->language = STRING_VIEW_LITERAL(LANGUAGE_DEFAULT); - exp_input->user_agent = STRING_VIEW_LITERAL(USERAGENT_DEFAULT); exp_input->profiler_version = str_version(); } static inline DDRes exporter_input_copy(const ExporterInput *src, ExporterInput *dest) { - DUP_PARAM(api_key); - DUP_PARAM(environment); - DUP_PARAM(host); - DUP_PARAM(site); - DUP_PARAM(port); - DUP_PARAM(service); - DUP_PARAM(service_version); - DUP_PARAM(do_export); - DUP_PARAM(debug_pprof_prefix); dest->user_agent = src->user_agent; dest->language = src->language; dest->family = src->family; @@ -74,14 +50,4 @@ static inline DDRes exporter_input_copy(const ExporterInput *src, } // free the allocated strings -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->port); - free((char *)exporter_input->service); - free((char *)exporter_input->service_version); - free((char *)exporter_input->do_export); - free((char *)exporter_input->debug_pprof_prefix); -} +static inline void exporter_input_free(ExporterInput *exporter_input) {} diff --git a/include/logger_setup.hpp b/include/logger_setup.hpp index 28d4c0314..5d669be9d 100644 --- a/include/logger_setup.hpp +++ b/include/logger_setup.hpp @@ -4,5 +4,7 @@ // Datadog, Inc. #pragma once +#include void setup_logger(const char *log_mode, const char *log_level); +void setup_logger(const std::string &log_mode, const std::string &log_level); diff --git a/include/metric_aggregator.hpp b/include/metric_aggregator.hpp new file mode 100644 index 000000000..d02a3fa16 --- /dev/null +++ b/include/metric_aggregator.hpp @@ -0,0 +1,54 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. This product includes software +// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present +// Datadog, Inc. + +#pragma once + +#include +#include +#include + +#include "ddres.hpp" +#include "statsd.hpp" + +struct MetricAggregator { + std::string base_path = "profiler.native."; + std::string sockpath = "/var/run/datadog-agent/statsd.sock"; + std::unordered_map values; + + void add(const std::string &str, uint64_t val) { + std::string safe_str = str; + std::replace(safe_str.begin(), safe_str.end(), ':', '.'); + if (auto _f = values.find(safe_str); _f == values.end()) + values[safe_str] = 0; + values[safe_str] = values[safe_str] + val; + } + + void clear() { values.clear(); } + + bool send() { + int fd = -1; + if (IsDDResNotOK(statsd_connect(sockpath.c_str(), sockpath.size(), &fd)) || + -1 == fd) { + LG_WRN("Could not connect to socket %s", sockpath.c_str()); + return false; + } + for (const auto &pair : values) { + std::string metric_name = base_path + pair.first; + void *coerced_val = (void *)&pair.second; + if (IsDDResNotOK( + statsd_send(fd, metric_name.c_str(), coerced_val, STAT_GAUGE))) { + LG_ERR("Could not send metric %s on fd %d", metric_name.c_str(), fd); + } else { + PRINT_NFO("Sent metric %s of value %lu", metric_name.c_str(), + pair.second); + } + } + statsd_close(fd); + + // Since we sent the metrics, clear them + clear(); + return true; + } +}; diff --git a/include/perf_watcher.hpp b/include/perf_watcher.hpp index 7fd0a7d38..6d57d89c7 100644 --- a/include/perf_watcher.hpp +++ b/include/perf_watcher.hpp @@ -155,5 +155,8 @@ const char *sample_type_name_from_idx(int idx); const char *sample_type_unit_from_idx(int idx); int sample_type_id_to_count_sample_type_id(int idx); -// Helper functions, mostly for tests +// Other helper functions uint64_t perf_event_default_sample_type(); +unsigned int tracepoint_id_from_event(const char *eventname, + const char *groupname); +const char *get_tracefs_root(); diff --git a/include/pprof/ddprof_pprof.hpp b/include/pprof/ddprof_pprof.hpp index e902d04a0..d0e6781cf 100644 --- a/include/pprof/ddprof_pprof.hpp +++ b/include/pprof/ddprof_pprof.hpp @@ -5,6 +5,8 @@ #pragma once +#include + #include "ddprof_context.hpp" #include "ddprof_defs.hpp" #include "ddres_def.hpp" @@ -23,7 +25,8 @@ struct DDProfPProf { ddprof::Tags _tags; }; -DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext *ctx); +DDRes pprof_create_profile(std::shared_ptr &pprof, + DDProfContext *ctx); /** * Aggregate to the existing profile the provided unwinding output. @@ -35,13 +38,13 @@ DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext *ctx); DDRes pprof_aggregate(const UnwindOutput *uw_output, const SymbolHdr *symbol_hdr, uint64_t value, uint64_t count, const PerfWatcher *watcher, - DDProfPProf *pprof); + DDProfPProf &pprof); -DDRes pprof_reset(DDProfPProf *pprof); +DDRes pprof_reset(DDProfPProf &pprof); -DDRes pprof_write_profile(const DDProfPProf *pprof, int fd); +DDRes pprof_write_profile(const DDProfPProf &pprof, int fd); -DDRes pprof_free_profile(DDProfPProf *pprof); +DDRes pprof_free_profile(DDProfPProf &pprof); void ddprof_print_sample(const UnwindOutput &uw_output, const SymbolHdr &symbol_hdr, uint64_t value, diff --git a/include/tags.hpp b/include/tags.hpp index a08456017..3f6650de9 100644 --- a/include/tags.hpp +++ b/include/tags.hpp @@ -18,6 +18,6 @@ void split(const char *str, Tags &tags, char c = ','); } // namespace ddprof struct UserTags { - UserTags(const char *tag_str, int nproc); + UserTags(const std::string &tag_str, int nproc); ddprof::Tags _tags; }; diff --git a/include/version.hpp b/include/version.hpp index a7e80edce..ba1300d48 100644 --- a/include/version.hpp +++ b/include/version.hpp @@ -5,7 +5,8 @@ #pragma once -#include "string_view.hpp" +#include +#include // Name and versions are defined in build system #ifndef MYNAME @@ -25,7 +26,18 @@ # define VER_REV "custom" #endif -/// Versions are updated in cmake files -string_view str_version(); +#define _S(X) #X +#define S(X) _S(X) + +// Versions are updated in cmake files +constexpr std::string_view str_version() { + if (!*VER_REV) + return S(VER_MAJ) "." S(VER_MIN) "." S(VER_PATCH); + else + return S(VER_MAJ) "." S(VER_MIN) "." S(VER_PATCH) "+" VER_REV; +} void print_version(); + +#undef S +#undef _S diff --git a/src/daemonize.cc b/src/daemonize.cc deleted file mode 100644 index 498f20354..000000000 --- a/src/daemonize.cc +++ /dev/null @@ -1,67 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. This product includes software -// developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present -// Datadog, Inc. - -#include "daemonize.hpp" - -#include -#include -#include - -namespace ddprof { -DaemonizeResult daemonize(std::function cleanup_function) { - int pipefd[2]; - if (pipe2(pipefd, O_CLOEXEC) == -1) { - return {-1, -1, -1}; - } - - pid_t parent_pid = getpid(); - pid_t temp_pid = fork(); // "middle" (temporary) PID - - if (temp_pid == -1) { - return {-1, -1, -1}; - } - - if (!temp_pid) { // If I'm the temp PID enter branch - close(pipefd[0]); - - temp_pid = getpid(); - if (pid_t child_pid = fork(); - child_pid) { // If I'm the temp PID again, enter branch - if (cleanup_function) { - cleanup_function(); - } - - // Block until our child exits or sends us a kill signal - // NOTE, current process is NOT expected to unblock here; rather it - // ends by SIGTERM. Exiting here is an error condition. - waitpid(child_pid, NULL, 0); - exit(1); - } else { - child_pid = getpid(); - if (write(pipefd[1], &child_pid, sizeof(child_pid)) != - sizeof(child_pid)) { - exit(1); - } - close(pipefd[1]); - // If I'm the child PID, then leave and attach profiler - return {temp_pid, parent_pid, child_pid}; - } - } else { - close(pipefd[1]); - - pid_t grandchild_pid; - if (read(pipefd[0], &grandchild_pid, sizeof(grandchild_pid)) != - sizeof(grandchild_pid)) { - return {-1, -1, -1}; - } - - // If I'm the target PID, then now it's time to wait until my - // child, the middle PID, returns. - waitpid(temp_pid, NULL, 0); - return {0, parent_pid, grandchild_pid}; - } -} - -} // namespace ddprof \ No newline at end of file diff --git a/src/ddprof.cc b/src/ddprof.cc index 6be0e1579..acc4802cf 100644 --- a/src/ddprof.cc +++ b/src/ddprof.cc @@ -5,7 +5,8 @@ #include "ddprof.hpp" -#include +#include +#include #include #include @@ -18,6 +19,7 @@ #include "ddprof_cmdline.hpp" #include "ddprof_context.hpp" #include "ddprof_context_lib.hpp" +#include "ddprof_exit.hpp" #include "ddprof_input.hpp" #include "ddprof_stats.hpp" #include "ddprof_worker.hpp" @@ -48,7 +50,7 @@ static void sigsegv_handler(int sig, siginfo_t *si, void *uc) { size_t sz = backtrace(buf, 4096); #endif fprintf(stderr, "ddprof[%d]: <%s> has encountered an error and will exit\n", - getpid(), str_version().ptr); + getpid(), str_version().data()); if (sig == SIGSEGV) printf("[DDPROF] Fault address: %p\n", si->si_addr); #ifdef __GLIBC__ @@ -107,7 +109,7 @@ DDRes ddprof_setup(DDProfContext *ctx) { DDRES_CHECK_FWD(ddprof_stats_init()); DDRES_CHECK_FWD(pevent_enable(pevent_hdr)); - } + } catch (ddprof::exit const &_) { throw; } CatchExcept2DDRes(); return ddres_init(); } @@ -157,7 +159,7 @@ void ddprof_attach_handler(DDProfContext *ctx, LG_NFO("Initiating Profiling"); main_loop_lib(&perf_funs, ctx); if (errno) - LG_WRN("Profiling context no longer valid (%s)", strerror(errno)); + LG_WRN("Profiling context no longer valid (%s)", std::strerror(errno)); else LG_WRN("Profiling context no longer valid"); diff --git a/src/ddprof_cmdline.cc b/src/ddprof_cmdline.cc index 11b0645c6..0dfad510a 100644 --- a/src/ddprof_cmdline.cc +++ b/src/ddprof_cmdline.cc @@ -8,12 +8,8 @@ #include #include #include -#include #include #include -#include -#include -#include #include "ddres_helpers.hpp" #include "event_config.hpp" @@ -21,6 +17,17 @@ #include "perf_archmap.hpp" #include "perf_watcher.hpp" +template +int arg_which(const char *str, const std::string_view (&list)[N]) { + size_t sz = strlen(str); + for (size_t i = 0; i < N; ++i) { + const auto &el = list[i]; + if (sz == el.size() && !strcasecmp(el.data(), str)) + return static_cast(i); + } + return -1; +} + int arg_which(const char *str, char const *const *set, int sz_set) { if (!str || !set) return -1; @@ -31,55 +38,32 @@ int arg_which(const char *str, char const *const *set, int sz_set) { return -1; } +template +bool arg_inlist(const char *str, const std::string_view (&list)[N]) { + return -1 != arg_which(str, list); +} + bool arg_inset(const char *str, char const *const *set, int sz_set) { int ret = arg_which(str, set, sz_set); return !(-1 == ret); } -bool arg_yesno(const char *str, int mode) { - const int sizeOfPatterns = 3; - static const char *yes_set[] = {"yes", "true", "on"}; // mode = 1 - static const char *no_set[] = {"no", "false", "off"}; // mode = 0 - assert(0 == mode || 1 == mode); - char const *const *set = (!mode) ? no_set : yes_set; - if (arg_which(str, set, sizeOfPatterns) != -1) { - return true; - } - return false; +bool is_yes(const char *str) { + constexpr std::string_view list[] = {"1", "on", "yes", + "true", "enable", "enabled"}; + return arg_inlist(str, list); } -unsigned int tracepoint_id_from_event(const char *eventname, - const char *groupname) { - if (!eventname || !*eventname || !groupname || !*groupname) - return 0; - - static char path[4096]; // Arbitrary, but path sizes limits are difficult - static char buf[sizeof("4294967296")]; // For reading 32-bit decimal int - char *buf_copy = buf; - size_t pathsz = - snprintf(path, sizeof(path), "/sys/kernel/tracing/events/%s/%s/id", - groupname, eventname); - if (pathsz >= sizeof(path)) - return 0; - int fd = open(path, O_RDONLY); - if (-1 == fd) - return 0; - - // Read the data in an eintr-safe way - int read_ret = -1; - long trace_id = 0; - do { - read_ret = read(fd, buf, sizeof(buf)); - } while (read_ret == -1 && errno == EINTR); - close(fd); - if (read_ret > 0) - trace_id = strtol(buf, &buf_copy, 10); - if (*buf_copy && *buf_copy != '\n') - return 0; - - return trace_id; +bool is_yes(const std::string &str) { return is_yes(str.c_str()); } + +bool is_no(const char *str) { + constexpr std::string_view list[] = {"0", "off", "no", + "false", "disable", "disabled"}; + return arg_inlist(str, list); } +bool is_no(const std::string &str) { return is_no(str.c_str()); } + // If this returns false, then the passed watcher should be regarded as invalid constexpr uint64_t kIgnoredWatcherID = -1ul; bool watcher_from_str(const char *str, PerfWatcher *watcher) { @@ -103,11 +87,7 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { // If the event doesn't match an ewatcher, it is only valid if a group was // also provided (splitting events on ':' is the responsibility of the // parser) - auto *tmp_tracepoint_watcher = tracepoint_default_watcher(); - if (!tmp_tracepoint_watcher) { - return false; - } - *watcher = *tmp_tracepoint_watcher; + *watcher = *tracepoint_default_watcher(); } else { return false; } @@ -132,10 +112,24 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { watcher->config = tracepoint_id; } + // The output mode isn't set as part of the configuration templates; we + // always default to callgraph mode + watcher->output_mode = EventConfMode::kCallgraph; + if (EventConfMode::kAll <= conf->mode) { + watcher->output_mode = conf->mode; + } + + // Configure some stuff relative to the mode + if (!(EventConfMode::kCallgraph <= watcher->output_mode)) { + watcher->sample_type &= ~PERF_SAMPLE_STACK_USER; + watcher->sample_stack_size = 0; + } + // Configure the sampling strategy. If no valid conf, use template default if (conf->cadence != 0) { if (conf->cad_type == EventConfCadenceType::kPeriod) { watcher->sample_period = conf->cadence; + watcher->options.is_freq = false; } else if (conf->cad_type == EventConfCadenceType::kFrequency) { watcher->sample_frequency = conf->cadence; watcher->options.is_freq = true; @@ -159,13 +153,6 @@ bool watcher_from_str(const char *str, PerfWatcher *watcher) { if (conf->value_scale != 0.0) watcher->value_scale = conf->value_scale; - // The output mode isn't set as part of the configuration templates; we - // always default to callgraph mode - watcher->output_mode = EventConfMode::kCallgraph; - if (EventConfMode::kAll <= conf->mode) { - watcher->output_mode = conf->mode; - } - watcher->tracepoint_event = conf->eventname; watcher->tracepoint_group = conf->groupname; watcher->tracepoint_label = conf->label; diff --git a/src/ddprof_context_lib.cc b/src/ddprof_context_lib.cc index 862214d4f..11d206ddf 100644 --- a/src/ddprof_context_lib.cc +++ b/src/ddprof_context_lib.cc @@ -15,11 +15,21 @@ #include #include +#include +#include #include #include #include #include +void DDProfContext::release() noexcept { + if (params.sockfd >= 0) { + close(params.sockfd); + } + params.sockfd = -1; + LOG_close(); // weird because log context isn't owned by me here? +} + static const PerfWatcher * find_duplicate_event(ddprof::span watchers) { bool seen[DDPROF_PWE_LENGTH] = {}; @@ -135,7 +145,6 @@ static void log_watcher(const PerfWatcher *w, int idx) { /**************************** Argument Processor ***************************/ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { - *ctx = {}; setup_logger(input->log_mode, input->log_level); if (const PerfWatcher *dup_watcher = find_duplicate_event( @@ -160,62 +169,67 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { // Process enable. Note that we want the effect to hit an inner profile. // TODO das210603 do the semantics of this match other profilers? - ctx->params.enable = !arg_yesno(input->enable, 0); // default yes + ctx->params.enable = !is_no(input->enable); // Default yes if (ctx->params.enable) setenv("DD_PROFILING_ENABLED", "true", true); else setenv("DD_PROFILING_ENABLED", "false", true); // Process native profiler enablement override - if (input->native_enable) { - ctx->params.enable = arg_yesno(input->native_enable, 1); + if (!input->native_enable.empty()) { + ctx->params.enable = !is_no(input->native_enable); } // Process enablement for agent mode - ctx->exp_input.agentless = arg_yesno(input->agentless, 1); // default no + ctx->exp_input.agentless = is_yes(input->agentless); // process upload_period - if (input->upload_period) { - double x = strtod(input->upload_period, NULL); - if (x > 0.0) - ctx->params.upload_period = x; + if (!input->upload_period.empty()) { + char *ptr_end; + double val = std::strtod(input->upload_period.c_str(), &ptr_end); + if (ptr_end != input->upload_period.c_str() && val > 0.0) + ctx->params.upload_period = val; } ctx->params.worker_period = 240; - if (input->worker_period) { - char *ptr_period = input->worker_period; - int tmp_period = strtol(input->worker_period, &ptr_period, 10); - if (ptr_period != input->worker_period && tmp_period > 0) - ctx->params.worker_period = tmp_period; + if (!input->worker_period.empty()) { + char *ptr_end; + long val = std::strtol(input->worker_period.c_str(), &ptr_end, 10); + if (ptr_end != input->worker_period.c_str() && val > 0) + ctx->params.worker_period = val; } // Process fault_info - ctx->params.fault_info = arg_yesno(input->fault_info, 1); // default no + ctx->params.fault_info = is_yes(input->fault_info); // Process core_dumps // This probably makes no sense with fault_info enabled, but considering that // there are other dumpable signals, we ignore - ctx->params.core_dumps = arg_yesno(input->core_dumps, 1); // default no + ctx->params.core_dumps = is_yes(input->core_dumps); // default no // Process nice level // default value is -1 : nothing to override ctx->params.nice = -1; - if (input->nice) { - char *ptr_nice = input->nice; - int tmp_nice = strtol(input->nice, &ptr_nice, 10); - if (ptr_nice != input->nice) - ctx->params.nice = tmp_nice; + if (input->nice.c_str()) { + char *ptr_end; + long val = std::strtol(input->nice.c_str(), &ptr_end, 10); + if (ptr_end != input->nice.c_str() && val > 0) + ctx->params.nice = val; } ctx->params.num_cpu = ddprof::nprocessors_conf(); // Adjust target PID - pid_t pid_tmp = 0; - if (input->pid && (pid_tmp = strtol(input->pid, NULL, 10))) - ctx->params.pid = pid_tmp; + if (!input->pid.empty()) { + char *ptr_end; + long val = std::strtol(input->pid.c_str(), &ptr_end, 10); + if (ptr_end != input->pid.c_str() && val > 1) { + ctx->params.pid = val; + } + } // Adjust global mode - ctx->params.global = arg_yesno(input->global, 1); // default no + ctx->params.global = is_yes(input->global); if (ctx->params.global) { if (ctx->params.pid) { LG_WRN("[INPUT] Ignoring PID (%d) in param due to global mode", @@ -225,29 +239,21 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } // Enable or disable the propagation of internal statistics - if (input->internal_stats) { - ctx->params.internal_stats = strdup(input->internal_stats); - if (!ctx->params.internal_stats) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_BADALLOC, - "Unable to allocate string for internal_stats"); - } + if (!input->internal_stats.empty()) { + ctx->params.internal_stats = input->internal_stats; } // Specify export tags - if (input->tags) { - ctx->params.tags = strdup(input->tags); - if (!ctx->params.tags) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_BADALLOC, - "Unable to allocate string for tags"); - } + if (input->tags.c_str()) { + ctx->params.tags = input->tags; } // 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; + if (!input->url.empty()) { + char *host = (char *)input->url.c_str(); + char *delim = strchr(host, ':'); char *port = NULL; + LG_NTC("Processing URL: %s", host); if (delim && delim[1] == '/' && delim[2] == '/') { // A colon was found. // http://hostname:port -> (hostname, port) @@ -258,8 +264,8 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { // Drop the schema *delim = '\0'; - if (!strncasecmp(input->url, "http", 4) || - !strncasecmp(input->url, "https", 5)) { + if (!strncasecmp(input->url.c_str(), "http", 4) || + !strncasecmp(input->url.c_str(), "https", 5)) { *delim = ':'; host = delim + 3; // Navigate after schema } @@ -281,16 +287,12 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { // 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); + input->exp_input.host = host; // For the pretty-printer + ctx->exp_input.host = 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); + input->exp_input.port = port; // Merely for the pretty-printer + ctx->exp_input.port = port; } // Revert the delimiter in case we want to print the URL later @@ -299,11 +301,17 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } } + // If NULL, uses a default + if (!input->metrics_socket.empty()) + ctx->metrics.sockpath = input->metrics_socket; + else + ctx->metrics.sockpath = "/var/run/datadog-agent/statsd.sock"; + ctx->params.sockfd = -1; ctx->params.wait_on_socket = false; - if (input->socket && strlen(input->socket) > 0) { - std::string_view sv{input->socket}; + if (!input->socket.empty()) { int sockfd; + std::string_view sv{input->socket}; auto [ptr, ec] = std::from_chars(sv.data(), sv.end(), sockfd); if (ec == std::errc() && ptr == sv.end()) { ctx->params.sockfd = sockfd; @@ -313,10 +321,13 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { ctx->params.dd_profiling_fd = -1; - const char *preset = input->preset; - if (!preset && ctx->num_watchers == 0) { - // use `default` preset when no preset and no events were given in input + const char *preset; + if (input->preset.empty() && ctx->num_watchers == 0) { preset = "default"; + } else if (input->preset.empty()) { // explicitly: watchers disable preset + preset = NULL; + } else { + preset = input->preset.c_str(); } if (preset) { @@ -325,8 +336,9 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } CPU_ZERO(&ctx->params.cpu_affinity); - if (input->affinity) { - if (!ddprof::parse_cpu_mask(input->affinity, ctx->params.cpu_affinity)) { + if (!input->affinity.empty()) { + if (!ddprof::parse_cpu_mask(input->affinity.c_str(), + ctx->params.cpu_affinity)) { DDRES_RETURN_ERROR_LOG(DD_WHAT_INPUT_PROCESS, "Invalid CPU affinity mask"); } @@ -350,7 +362,7 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { order_watchers({ctx->watchers, static_cast(ctx->num_watchers)}); // Process input printer (do this right before argv/c modification) - if (input->show_config && arg_yesno(input->show_config, 1)) { + if (!input->show_config.empty() && is_yes(input->show_config)) { PRINT_NFO("Printing parameters -->"); ddprof_print_params(input); @@ -360,7 +372,7 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { // Tell the user what mode is being used PRINT_NFO(" Profiling mode: %s", -1 == ctx->params.pid ? "global" - : pid_tmp ? "target" + : ctx->params.pid ? "target" : "wrapper"); // Show watchers @@ -370,31 +382,16 @@ DDRes ddprof_context_set(DDProfInput *input, DDProfContext *ctx) { } } - ctx->params.show_samples = input->show_samples != nullptr; + ctx->params.show_samples = is_yes(input->show_samples); - if (input->switch_user) { - ctx->params.switch_user = strdup(input->switch_user); + if (!input->switch_user.empty()) { + ctx->params.switch_user = input->switch_user; } ctx->initialized = true; return ddres_init(); } -void ddprof_context_free(DDProfContext *ctx) { - if (ctx->initialized) { - exporter_input_free(&ctx->exp_input); - free((char *)ctx->params.internal_stats); - free((char *)ctx->params.tags); - free((char *)ctx->params.switch_user); - if (ctx->params.sockfd != -1) { - close(ctx->params.sockfd); - } - *ctx = {}; - } - - LOG_close(); -} - int ddprof_context_allocation_profiling_watcher_idx(const DDProfContext *ctx) { ddprof::span watchers{ctx->watchers, static_cast(ctx->num_watchers)}; auto it = diff --git a/src/ddprof_input.cc b/src/ddprof_input.cc index e3ece4eb0..f5f4adb48 100644 --- a/src/ddprof_input.cc +++ b/src/ddprof_input.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -17,7 +18,6 @@ #include "version.hpp" /************************ Options Table Helper Macros *************************/ -#define X_FREE(a, b, c, d, e, f, g, h, i) FREE_EXP(i b, f); #define X_LOPT(a, b, c, d, e, f, g, h, i) {#b, e, 0, d}, #define X_DFLT(a, b, c, d, e, f, g, h, i) DFLT_EXP(#a, i b, f, g, h); #define X_OSTR(a, b, c, d, e, f, g, h, i) #c ":" @@ -26,41 +26,30 @@ // TODO das210603 I don't think this needs to be inlined as a macro anymore #define DFLT_EXP(evar, key, targ, func, dfault) \ { \ - char *_buf = NULL; \ - if (!((targ)->key)) { \ + if (((targ)->key).empty()) { \ if (getenv(evar)) \ - _buf = strdup(getenv(evar)); \ + (targ)->key = getenv(evar); \ else if (*(dfault)) \ - _buf = strdup(dfault); \ - (targ)->key = _buf; \ + (targ)->key = dfault; \ } \ } #define CASE_EXP(casechar, targ, key) \ case casechar: \ - if ((targ)->key) \ - free((void *)(targ)->key); \ if (optarg && *optarg) \ - (targ)->key = strdup(optarg); \ + (targ)->key = optarg; \ else \ - (targ)->key = strdup(""); \ + (targ)->key = ""; \ break; -// TODO das210603 I don't think this needs to be inlined as a macro anymore -#define FREE_EXP(key, targ) \ - { \ - free((void *)(targ)->key); \ - (targ)->key = NULL; \ - } - #define X_HLPK(a, b, c, d, e, f, g, h, i) \ " -" #c ", --" #b ", (envvar: " #a ")", // Helpers for expanding the OPT_TABLE here #define X_PRNT(a, b, c, d, e, f, g, h, i) \ { \ - if ((f)->i b) { \ - PRINT_NFO(" " #b ": %s", (f)->i b); \ + if (!((f)->i b).empty()) { \ + PRINT_NFO(" " #b ": %s", ((f)->i b).c_str()); \ } \ } @@ -136,6 +125,7 @@ const char* help_str[DD_KLEN] = { " Enables statsd metrics for " MYNAME ". Value should point to a statsd socket.\n" " Example: /var/run/datadog-agent/statsd.sock\n", [DD_PROFILING_NATIVE_SOCKET] = STR_UNDF, + [DD_PROFILING_NATIVE_METRICS_SOCKET] = STR_UNDF, [DD_PROFILING_NATIVE_PRESET] = " Select a predefined profiling configuration.\n" " Available presets:\n" @@ -278,5 +268,3 @@ DDRes ddprof_input_parse(int argc, char **argv, DDProfInput *input, } void ddprof_print_params(const DDProfInput *input) { OPT_TABLE(X_PRNT); } - -void ddprof_input_free(DDProfInput *input) { OPT_TABLE(X_FREE); } diff --git a/src/ddprof_module_lib.cc b/src/ddprof_module_lib.cc index 3f921f5aa..908049f32 100644 --- a/src/ddprof_module_lib.cc +++ b/src/ddprof_module_lib.cc @@ -12,6 +12,7 @@ #include "logger.hpp" #include "string_format.hpp" +#include #include #include #include diff --git a/src/ddprof_stats.cc b/src/ddprof_stats.cc index 063063146..fb5f092b1 100644 --- a/src/ddprof_stats.cc +++ b/src/ddprof_stats.cc @@ -113,7 +113,7 @@ DDRes ddprof_stats_get(unsigned int stat, long *out) { } DDRes ddprof_stats_send(const char *statsd_socket) { - if (!statsd_socket) { + if (!statsd_socket || !*statsd_socket) { LG_NTC("No statsd socket provided"); return ddres_init(); } diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc index f588e01b5..bf4882cad 100644 --- a/src/ddprof_worker.cc +++ b/src/ddprof_worker.cc @@ -70,7 +70,7 @@ static DDRes report_lost_events(DDProfContext *ctx) { DDRES_CHECK_FWD(pprof_aggregate( &us->output, &us->symbol_hdr, watcher->sample_period, ctx->worker_ctx.lost_events_per_watcher[watcher_idx], watcher, - ctx->worker_ctx.pprof[ctx->worker_ctx.i_current_pprof])); + *ctx->worker_ctx.pprof[ctx->worker_ctx.i_current_pprof])); ctx->worker_ctx.lost_events_per_watcher[watcher_idx] = 0; } } @@ -206,6 +206,18 @@ static DDRes worker_update_stats(ProcStatus *procstat, const DsoHdr *dso_hdr, return ddres_init(); } +void ddprof_metric_aggregate(DDProfContext *ctx, PerfWatcher *watcher, + uint64_t val) { + // Fallthrough sequence to figure out what we call this thing + const char *metric_name = "unnameable_metric"; + if (!watcher->tracepoint_label.empty()) + metric_name = watcher->tracepoint_label.c_str(); + else if (!watcher->desc.empty()) + metric_name = watcher->desc.c_str(); + + ctx->metrics.add(metric_name, val); +} + /************************* perf_event_open() helpers **************************/ /// Entry point for sample aggregation DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, @@ -231,70 +243,80 @@ DDRes ddprof_pr_sample(DDProfContext *ctx, perf_event_sample *sample, if (watcher->config == PERF_COUNT_SW_TASK_CLOCK) ddprof_stats_add(STATS_TARGET_CPU_USAGE, sample->period, NULL); + // Keep track of TSC-based time + auto unwind_ticks = ddprof::get_tsc_cycles(); + + // Depending on the type of watcher, compute a value for sample + uint64_t sample_val = perf_value_from_sample(watcher, sample); + // Attempt to fully unwind if the watcher has a callgraph type - DDRes res = {}; - if (EventConfMode::kCallgraph <= watcher->output_mode) - res = unwindstate__unwind(us); - - /* This test is not 100% accurate: - * Linux kernel does not take into account stack start (ie. end address since - * stack grows down) when capturing the stack, it starts from SP register and - * only limits the range with the requested size and user space end (cf. - * https://elixir.bootlin.com/linux/v5.19.3/source/kernel/events/core.c#L6582). - * Then it tries to copy this range, and stops when it encounters a non-mapped - * address - * (https://elixir.bootlin.com/linux/v5.19.3/source/kernel/events/core.c#L6660). - * This works well for main thread since [stack] is at the top of the process - * user address space (and there is a gap between [vvar] and [stack]), but - * for threads, stack can be allocated anywhere on the heap and if address - * space below allocated stack is mapped, then kernel will happily copy the - * whole range up to the requested sample stack size, therefore always - * returning samples with `dyn_size` equals to the requested sample stack - * size, even if the end of captured stack is not actually part of the stack. - * - * That's why we consider the stack as truncated in input only if it is also - * detected as incomplete during unwinding. - */ - if (sample->size_stack == ctx->watchers[watcher_pos].sample_stack_size && - us->output.is_incomplete) { - ddprof_stats_add(STATS_UNWIND_TRUNCATED_INPUT, 1, nullptr); - } + if (EventConfMode::kCallgraph <= watcher->output_mode) { + DDRes res = unwindstate__unwind(us); + + /* This test is not 100% accurate: + * Linux kernel does not take into account stack start (ie. end address + * since stack grows down) when capturing the stack, it starts from SP + * register and only limits the range with the requested size and user space + * end (cf. + * https://elixir.bootlin.com/linux/v5.19.3/source/kernel/events/core.c#L6582). + * Then it tries to copy this range, and stops when it encounters a + * non-mapped address + * (https://elixir.bootlin.com/linux/v5.19.3/source/kernel/events/core.c#L6660). + * This works well for main thread since [stack] is at the top of the + * process user address space (and there is a gap between [vvar] and + * [stack]), but for threads, stack can be allocated anywhere on the heap + * and if address space below allocated stack is mapped, then kernel will + * happily copy the whole range up to the requested sample stack size, + * therefore always returning samples with `dyn_size` equals to the + * requested sample stack size, even if the end of captured stack is not + * actually part of the stack. + * + * That's why we consider the stack as truncated in input only if it is also + * detected as incomplete during unwinding. + */ + if (sample->size_stack == ctx->watchers[watcher_pos].sample_stack_size && + us->output.is_incomplete) { + ddprof_stats_add(STATS_UNWIND_TRUNCATED_INPUT, 1, nullptr); + } - auto unwind_ticks = ddprof::get_tsc_cycles(); - DDRES_CHECK_FWD( - ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL)); + DDRES_CHECK_FWD( + ddprof_stats_add(STATS_UNWIND_AVG_TIME, unwind_ticks - ticks0, NULL)); - // Aggregate if unwinding went well (todo : fatal error propagation) - if (!IsDDResFatal(res) && EventConfMode::kCallgraph <= watcher->output_mode) { + // Aggregate if unwinding went well (todo : fatal error propagation) + if (!IsDDResFatal(res) && + EventConfMode::kCallgraph <= watcher->output_mode) { #ifndef DDPROF_NATIVE_LIB - // Depending on the type of watcher, compute a value for sample - uint64_t sample_val = perf_value_from_sample(watcher, sample); - - // in lib mode we don't aggregate (protect to avoid link failures) - int i_export = ctx->worker_ctx.i_current_pprof; - DDProfPProf *pprof = ctx->worker_ctx.pprof[i_export]; - DDRES_CHECK_FWD(pprof_aggregate(&us->output, &us->symbol_hdr, sample_val, 1, - watcher, pprof)); - if (ctx->params.show_samples) { - ddprof_print_sample(us->output, us->symbol_hdr, sample->period, *watcher); - } + // in lib mode we don't aggregate (protect to avoid link failures) + int i_export = ctx->worker_ctx.i_current_pprof; + DDProfPProf &pprof = *ctx->worker_ctx.pprof[i_export]; + DDRES_CHECK_FWD(pprof_aggregate(&us->output, &us->symbol_hdr, sample_val, + 1, watcher, pprof)); + if (ctx->params.show_samples) { + ddprof_print_sample(us->output, us->symbol_hdr, sample->period, + *watcher); + } #else - // Call the user's stack handler - if (ctx->stack_handler) { - if (!ctx->stack_handler->apply(&us->output, ctx, - ctx->stack_handler->callback_ctx, - watcher_pos)) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_STACK_HANDLE, - "Stack handler returning errors"); + // Call the user's stack handler + if (ctx->stack_handler) { + if (!ctx->stack_handler->apply(&us->output, ctx, + ctx->stack_handler->callback_ctx, + watcher_pos)) { + DDRES_RETURN_ERROR_LOG(DD_WHAT_STACK_HANDLE, + "Stack handler returning errors"); + } } - } #endif + } } + // If this watcher has a metric type, aggregate now + if (EventConfMode::kMetric <= watcher->output_mode) + ddprof_metric_aggregate(ctx, watcher, sample_val); + + // Finalize DDRES_CHECK_FWD(ddprof_stats_add(STATS_AGGREGATION_AVG_TIME, ddprof::get_tsc_cycles() - unwind_ticks, NULL)); - return {}; } @@ -314,9 +336,9 @@ void *ddprof_worker_export_thread(void *arg) { // gets joined forcefully, we should not resume on same value uint32_t profile_seq = (worker->persistent_worker_state->profile_seq)++; - if (IsDDResFatal(ddprof_exporter_export(worker->pprof[i]->_profile, + if (IsDDResFatal(ddprof_exporter_export(*worker->pprof[i]->_profile, worker->pprof[i]->_tags, profile_seq, - worker->exp[i]))) { + *worker->exp[i]))) { LG_NFO("Failed to export from worker"); worker->exp_error = true; } @@ -329,6 +351,9 @@ void *ddprof_worker_export_thread(void *arg) { DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, [[maybe_unused]] bool synchronous_export) { #ifndef DDPROF_NATIVE_LIB + // Ship stats + ctx->metrics.send(); + // Take the current pprof contents and ship them to the backend. This also // clears the pprof for reuse // Dispatch happens in a thread, with the underlying data structure for @@ -365,7 +390,7 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, // Reset the current, ensuring the timestamp starts when we are about to write // to it DDRES_CHECK_FWD( - pprof_reset(ctx->worker_ctx.pprof[ctx->worker_ctx.i_current_pprof])); + pprof_reset(*ctx->worker_ctx.pprof[ctx->worker_ctx.i_current_pprof])); if (!synchronous_export) { pthread_create(&ctx->worker_ctx.exp_tid, NULL, ddprof_worker_export_thread, @@ -388,10 +413,9 @@ DDRes ddprof_worker_cycle(DDProfContext *ctx, int64_t now, // And emit diagnostic output (if it's enabled) print_diagnostics(ctx->worker_ctx.us->dso_hdr); - if (IsDDResNotOK(ddprof_stats_send(ctx->params.internal_stats))) { + if (IsDDResNotOK(ddprof_stats_send(ctx->params.internal_stats.c_str()))) { LG_WRN("Unable to utilize to statsd socket. Suppressing future stats."); - free((void *)ctx->params.internal_stats); - ctx->params.internal_stats = NULL; + ctx->params.internal_stats = ""; } // Increase the counts of exports @@ -490,29 +514,16 @@ DDRes ddprof_worker_init(DDProfContext *ctx, PersistentWorkerState *persistent_worker_state) { try { DDRES_CHECK_FWD(worker_library_init(ctx, persistent_worker_state)); - ctx->worker_ctx.exp[0] = - (DDProfExporter *)calloc(1, sizeof(DDProfExporter)); - ctx->worker_ctx.exp[1] = - (DDProfExporter *)calloc(1, sizeof(DDProfExporter)); - ctx->worker_ctx.pprof[0] = new DDProfPProf(); - ctx->worker_ctx.pprof[1] = new DDProfPProf(); - if (!ctx->worker_ctx.exp[0] || !ctx->worker_ctx.exp[1]) { - free(ctx->worker_ctx.exp[0]); - free(ctx->worker_ctx.exp[1]); - delete ctx->worker_ctx.pprof[0]; - delete ctx->worker_ctx.pprof[1]; - DDRES_RETURN_ERROR_LOG(DD_WHAT_BADALLOC, "Error creating exporter"); - } DDRES_CHECK_FWD( ddprof_exporter_init(&ctx->exp_input, ctx->worker_ctx.exp[0])); DDRES_CHECK_FWD( ddprof_exporter_init(&ctx->exp_input, ctx->worker_ctx.exp[1])); // warning : depends on unwind init - DDRES_CHECK_FWD( - ddprof_exporter_new(ctx->worker_ctx.user_tags, ctx->worker_ctx.exp[0])); - DDRES_CHECK_FWD( - ddprof_exporter_new(ctx->worker_ctx.user_tags, ctx->worker_ctx.exp[1])); + DDRES_CHECK_FWD(ddprof_exporter_new(ctx->worker_ctx.user_tags, + *ctx->worker_ctx.exp[0])); + DDRES_CHECK_FWD(ddprof_exporter_new(ctx->worker_ctx.user_tags, + *ctx->worker_ctx.exp[1])); DDRES_CHECK_FWD(pprof_create_profile(ctx->worker_ctx.pprof[0], ctx)); DDRES_CHECK_FWD(pprof_create_profile(ctx->worker_ctx.pprof[1], ctx)); @@ -538,16 +549,8 @@ DDRes ddprof_worker_free(DDProfContext *ctx) { DDRES_CHECK_FWD(worker_library_free(ctx)); for (int i = 0; i < 2; i++) { - if (ctx->worker_ctx.exp[i]) { - DDRES_CHECK_FWD(ddprof_exporter_free(ctx->worker_ctx.exp[i])); - free(ctx->worker_ctx.exp[i]); - ctx->worker_ctx.exp[i] = nullptr; - } - if (ctx->worker_ctx.pprof[i]) { - DDRES_CHECK_FWD(pprof_free_profile(ctx->worker_ctx.pprof[i])); - delete ctx->worker_ctx.pprof[i]; - ctx->worker_ctx.pprof[i] = nullptr; - } + DDRES_CHECK_FWD(ddprof_exporter_free(*ctx->worker_ctx.exp[i])); + DDRES_CHECK_FWD(pprof_free_profile(*ctx->worker_ctx.pprof[i])); } } CatchExcept2DDRes(); diff --git a/src/dso_hdr.cc b/src/dso_hdr.cc index bec88c7db..a3bfac9b1 100644 --- a/src/dso_hdr.cc +++ b/src/dso_hdr.cc @@ -15,6 +15,7 @@ #include #include +#include #include #include #include diff --git a/src/dwfl_symbol_lookup.cc b/src/dwfl_symbol_lookup.cc index 10269691d..979a31c4a 100644 --- a/src/dwfl_symbol_lookup.cc +++ b/src/dwfl_symbol_lookup.cc @@ -14,6 +14,7 @@ #include #include +#include #include // #define DEBUG diff --git a/src/exe/main.cc b/src/exe/main.cc index 828b0f766..bd4489437 100644 --- a/src/exe/main.cc +++ b/src/exe/main.cc @@ -9,6 +9,7 @@ #include "ddprof_context.hpp" #include "ddprof_context_lib.hpp" #include "ddprof_cpumask.hpp" +#include "ddprof_exit.hpp" #include "ddprof_input.hpp" #include "ddres.hpp" #include "defer.hpp" @@ -20,8 +21,9 @@ #include #include +#include #include -#include +#include #include #include #include @@ -158,14 +160,11 @@ static DDRes get_library_path(TempFileHolder &libdd_profiling_path, // Parse input and initialize context static InputResult parse_input(int *argc, char ***argv, DDProfContext *ctx) { - //---- Inititiate structs - *ctx = {}; // Set temporary logger for argument parsing LOG_open(LOG_STDERR, NULL); LOG_setlevel(LL_WARNING); DDProfInput input = {}; - defer { ddprof_input_free(&input); }; bool continue_exec; DDRes res = ddprof_input_parse(*argc, *argv, &input, &continue_exec); if (IsDDResNotOK(res) || !continue_exec) { @@ -178,9 +177,10 @@ static InputResult parse_input(int *argc, char ***argv, DDProfContext *ctx) { // cmdline args have been processed. Set the ctx if (IsDDResNotOK(ddprof_context_set(&input, ctx))) { LG_ERR("Error setting up profiling context, exiting"); - ddprof_context_free(ctx); + ctx->release(); return InputResult::kError; } + // Adjust input parameters for execvp() (we do this even if unnecessary) *argv += input.nb_parsed_params; *argc -= input.nb_parsed_params; @@ -210,7 +210,7 @@ static InputResult parse_input(int *argc, char ***argv, DDProfContext *ctx) { } static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { - auto defer_context_free = make_defer([ctx] { ddprof_context_free(ctx); }); + auto defer_context_free = make_defer([ctx] { ctx->release(); }); is_profiler = false; @@ -221,8 +221,8 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { const bool in_wrapper_mode = ctx->params.pid == 0; TempFileHolder dd_profiling_lib_holder, dd_loader_lib_holder; + ddprof::DaemonizeResult daemonize_res; - pid_t temp_pid = 0; if (in_wrapper_mode) { // If no PID was specified earlier, we autodaemonize and target current pid @@ -259,14 +259,18 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { } ctx->params.pid = getpid(); - auto daemonize_res = ddprof::daemonize([ctx] { ddprof_context_free(ctx); }); + daemonize_res.daemonize({[ctx] { + ctx->release(); + throw ddprof::exit(); + }}); - if (daemonize_res.temp_pid == -1) { + if (daemonize_res.is_failure()) { return -1; } - temp_pid = daemonize_res.temp_pid; - if (!temp_pid) { + if (daemonize_res.is_invoker()) { + daemonize_res.barrier(); // Waits until the grandchild has finalized + // non-daemon process: return control to caller defer_child_socket_close.reset(); defer_context_free.release(); @@ -313,7 +317,7 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { 0) { LG_ERR("Failed to set profiler CPU affinity to 0x%s: %s", ddprof::cpu_mask_to_string(ctx->params.cpu_affinity).c_str(), - strerror(errno)); + std::strerror(errno)); return -1; } } @@ -329,8 +333,8 @@ static int start_profiler_internal(DDProfContext *ctx, bool &is_profiler) { // If we have a temp PID, then it's waiting for us to send it a signal // after we finish instrumenting. This will end that process, which in // turn will unblock the target from calling exec. - if (temp_pid) { - kill(temp_pid, SIGTERM); + if (daemonize_res.is_daemon()) { + daemonize_res.barrier(); } if (ctx->params.sockfd != -1 && ctx->params.wait_on_socket) { @@ -405,9 +409,9 @@ static void start_profiler(DDProfContext *ctx) { bool is_profiler = false; // ownership of context is passed to start_profiler_internal - int res = start_profiler_internal(ctx, is_profiler); + start_profiler_internal(ctx, is_profiler); if (is_profiler) { - exit(res); + throw ddprof::exit(); } // In wrapper mode (ie. ctx->params.pid == 0), whatever happened to ddprof, // continue and start user process @@ -416,7 +420,7 @@ static void start_profiler(DDProfContext *ctx) { /**************************** Program Entry Point *****************************/ int main(int argc, char *argv[]) { - DDProfContext ctx; + DDProfContext ctx = {}; // parse inputs and populate context switch (parse_input(&argc, &argv, &ctx)) { @@ -425,24 +429,26 @@ int main(int argc, char *argv[]) { case InputResult::kSuccess: break; case InputResult::kError: - [[fallthrough]]; + ctx.release(); default: return -1; } { - defer { ddprof_context_free(&ctx); }; + defer { ctx.release(); }; /****************************************************************************\ | Run the Profiler | \****************************************************************************/ // Ownership of context is passed to start_profiler // This function does not return in the context of profiler process // It only returns in the context of target process (ie. in non-PID mode) - start_profiler(&ctx); + try { + start_profiler(&ctx); + } catch (ddprof::exit const &_) { return -1; } - if (ctx.params.switch_user) { - if (!IsDDResOK(become_user(ctx.params.switch_user))) { - LG_ERR("Failed to switch to user %s", ctx.params.switch_user); + if (!ctx.params.switch_user.empty()) { + if (!IsDDResOK(become_user(ctx.params.switch_user.c_str()))) { + LG_ERR("Failed to switch to user %s", ctx.params.switch_user.c_str()); return -1; } } @@ -459,7 +465,7 @@ int main(int argc, char *argv[]) { LG_ERR("%s: permission denied", argv[0]); break; default: - LG_ERR("%s: failed to execute (%s)", argv[0], strerror(errno)); + LG_ERR("%s: failed to execute (%s)", argv[0], std::strerror(errno)); break; } } diff --git a/src/exporter/ddprof_exporter.cc b/src/exporter/ddprof_exporter.cc index 74246c124..45f38a138 100644 --- a/src/exporter/ddprof_exporter.cc +++ b/src/exporter/ddprof_exporter.cc @@ -13,7 +13,8 @@ #include #include -#include +#include +#include #include #include #include @@ -26,17 +27,6 @@ static const int k_timeout_ms = 10000; 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; -} - static DDRes create_pprof_file(ddog_Timespec start, const char *dbg_pprof_prefix, int *fd) { char time_start[128] = {}; @@ -58,7 +48,7 @@ static DDRes write_profile(const ddog_EncodedProfile *encoded_profile, int fd) { if (write(fd, buffer->ptr, buffer->len) == 0) { DDRES_RETURN_ERROR_LOG(DD_WHAT_EXPORTER, "Failed to write byte buffer to stdout! %s\n", - strerror(errno)); + std::strerror(errno)); } return ddres_init(); } @@ -74,15 +64,15 @@ static DDRes write_pprof_file(const ddog_EncodedProfile *encoded_profile, } DDRes ddprof_exporter_init(const ExporterInput *exporter_input, - DDProfExporter *exporter) { - memset(exporter, 0, sizeof(DDProfExporter)); + std::shared_ptr &exporter) { + exporter = std::make_shared(); DDRES_CHECK_FWD(exporter_input_copy(exporter_input, &exporter->_input)); // if we have an API key we assume we are heading for intake (slightly // fragile #TODO add a parameter) - if (exporter_input->agentless && exporter_input->api_key && - strlen(exporter_input->api_key) >= k_size_api_key) { + if (exporter_input->agentless && + exporter_input->api_key.size() >= k_size_api_key) { LG_NTC("[EXPORTER] Targeting intake instead of agent (API Key available)"); exporter->_agent = false; } else { @@ -92,29 +82,29 @@ DDRes ddprof_exporter_init(const ExporterInput *exporter_input, if (exporter->_agent) { exporter->_url = - alloc_url_agent("http://", exporter_input->host, exporter_input->port); + "http://" + exporter_input->host + ":" + exporter_input->port; } else { // site is the usual option for intake - if (exporter->_input.site) { + if (!exporter->_input.site.empty()) { // warning : should not contain intake.profile. (prepended in // libdatadog_profiling) - exporter->_url = strdup(exporter_input->site); + exporter->_url = exporter_input->site; } else { LG_WRN( "[EXPORTER] Agentless - Attempting to use host (%s) instead of empty " "site", - exporter_input->host); - exporter->_url = strdup(exporter_input->host); + exporter_input->host.c_str()); + exporter->_url = exporter_input->host; } } - if (!exporter->_url) { + if (exporter->_url.empty()) { DDRES_RETURN_ERROR_LOG(DD_WHAT_EXPORTER, "Failed to write url"); } - LG_NTC("[EXPORTER] URL %s", exporter->_url); + LG_NTC("[EXPORTER] URL %s", exporter->_url.c_str()); // Debug process : capture pprof to a folder exporter->_debug_pprof_prefix = exporter->_input.debug_pprof_prefix; - exporter->_export = arg_yesno(exporter->_input.do_export, 1); + exporter->_export = is_yes(exporter->_input.do_export); exporter->_last_pprof_size = -1; @@ -140,27 +130,23 @@ static DDRes fill_stable_tags(const UserTags *user_tags, // language is guaranteed to be filled DDRES_CHECK_FWD( - add_single_tag(tags_exporter, "language", - std::string_view(exporter->_input.language.ptr, - exporter->_input.language.len))); + add_single_tag(tags_exporter, "language", exporter->_input.language)); - if (exporter->_input.environment) + if (!exporter->_input.environment.empty()) DDRES_CHECK_FWD( add_single_tag(tags_exporter, "env", exporter->_input.environment)); - if (exporter->_input.service_version) + if (!exporter->_input.service_version.empty()) DDRES_CHECK_FWD(add_single_tag(tags_exporter, "version", exporter->_input.service_version)); - if (exporter->_input.service) + if (!exporter->_input.service.empty()) DDRES_CHECK_FWD( add_single_tag(tags_exporter, "service", exporter->_input.service)); - if (exporter->_input.profiler_version.len) - DDRES_CHECK_FWD(add_single_tag( - tags_exporter, "profiler_version", - std::string_view(exporter->_input.profiler_version.ptr, - exporter->_input.profiler_version.len))); + if (!exporter->_input.profiler_version.empty()) + DDRES_CHECK_FWD(add_single_tag(tags_exporter, "profiler_version", + exporter->_input.profiler_version)); for (auto &el : user_tags->_tags) { DDRES_CHECK_FWD(add_single_tag(tags_exporter, el.first, el.second)); @@ -168,24 +154,24 @@ static DDRes fill_stable_tags(const UserTags *user_tags, return ddres_init(); } -DDRes ddprof_exporter_new(const UserTags *user_tags, DDProfExporter *exporter) { +DDRes ddprof_exporter_new(const UserTags *user_tags, DDProfExporter &exporter) { ddog_Vec_tag tags_exporter = ddog_Vec_tag_new(); - fill_stable_tags(user_tags, exporter, tags_exporter); + fill_stable_tags(user_tags, &exporter, tags_exporter); - ddog_CharSlice base_url = to_CharSlice(exporter->_url); + ddog_CharSlice base_url = to_CharSlice(exporter._url.c_str()); ddog_Endpoint endpoint; - if (exporter->_agent) { + if (exporter._agent) { endpoint = ddog_Endpoint_agent(base_url); } else { - ddog_CharSlice api_key = to_CharSlice(exporter->_input.api_key); + ddog_CharSlice api_key = to_CharSlice(exporter._input.api_key); endpoint = ddog_Endpoint_agentless(base_url, api_key); } ddog_NewProfileExporterResult new_exporter = ddog_ProfileExporter_new( - to_CharSlice(exporter->_input.family), &tags_exporter, endpoint); + to_CharSlice(exporter._input.family), &tags_exporter, endpoint); if (new_exporter.tag == DDOG_NEW_PROFILE_EXPORTER_RESULT_OK) { - exporter->_exporter = new_exporter.ok; + exporter._exporter = new_exporter.ok; } else { DDRES_RETURN_ERROR_LOG(DD_WHAT_EXPORTER, "Failure creating exporter - %s", new_exporter.err.ptr); @@ -235,12 +221,12 @@ static DDRes fill_cycle_tags(const ddprof::Tags &additional_tags, return ddres_init(); } -DDRes ddprof_exporter_export(const ddog_Profile *profile, +DDRes ddprof_exporter_export(const ddog_Profile &profile, const ddprof::Tags &additional_tags, - uint32_t profile_seq, DDProfExporter *exporter) { + uint32_t profile_seq, DDProfExporter &exporter) { DDRes res = ddres_init(); ddog_SerializeResult serialized_result = - ddog_Profile_serialize(profile, nullptr, nullptr); + ddog_Profile_serialize(&profile, nullptr, nullptr); defer { ddog_SerializeResult_drop(serialized_result); }; if (serialized_result.tag != DDOG_SERIALIZE_RESULT_OK) { DDRES_RETURN_ERROR_LOG(DD_WHAT_EXPORTER, "Failed to serialize"); @@ -248,8 +234,8 @@ DDRes ddprof_exporter_export(const ddog_Profile *profile, ddog_EncodedProfile *encoded_profile = &serialized_result.ok; - if (exporter->_debug_pprof_prefix) { - write_pprof_file(encoded_profile, exporter->_debug_pprof_prefix); + if (!exporter._debug_pprof_prefix.empty()) { + write_pprof_file(encoded_profile, exporter._debug_pprof_prefix.c_str()); } ddog_Timespec start = encoded_profile->start; @@ -260,9 +246,9 @@ DDRes ddprof_exporter_export(const ddog_Profile *profile, .len = encoded_profile->buffer.len, }; - exporter->_last_pprof_size = profile_data.len; + exporter._last_pprof_size = profile_data.len; - if (exporter->_export) { + if (exporter._export) { ddog_Vec_tag ffi_additional_tags = ddog_Vec_tag_new(); defer { ddog_Vec_tag_drop(ffi_additional_tags); }; DDRES_CHECK_FWD( @@ -278,19 +264,20 @@ DDRes ddprof_exporter_export(const ddog_Profile *profile, ddog_Slice_file files = {.ptr = files_, .len = std::size(files_)}; ddog_Request *request = - ddog_ProfileExporter_build(exporter->_exporter, start, end, files, + ddog_ProfileExporter_build(exporter._exporter, start, end, files, &ffi_additional_tags, k_timeout_ms); if (request) { ddog_SendResult result = - ddog_ProfileExporter_send(exporter->_exporter, request, nullptr); + ddog_ProfileExporter_send(exporter._exporter, request, nullptr); defer { ddog_SendResult_drop(result); }; if (result.tag == DDOG_SEND_RESULT_ERR) { - LG_WRN("Failure to establish connection, check url %s", exporter->_url); + LG_WRN("Failure to establish connection, check url %s", + exporter._url.c_str()); LG_WRN("Failure to send profiles (%.*s)", (int)result.err.len, result.err.ptr); // Free error buffer (prefer this API to the free API) - if (exporter->_nb_consecutive_errors++ >= + if (exporter._nb_consecutive_errors++ >= K_NB_CONSECUTIVE_ERRORS_ALLOWED) { // this will shut down profiler res = ddres_error(DD_WHAT_EXPORTER); @@ -299,7 +286,7 @@ DDRes ddprof_exporter_export(const ddog_Profile *profile, } } else { // success establishing connection - exporter->_nb_consecutive_errors = 0; + exporter._nb_consecutive_errors = 0; res = check_send_response_code(result.http_response.code); } } else { @@ -310,12 +297,9 @@ DDRes ddprof_exporter_export(const ddog_Profile *profile, return res; } -DDRes ddprof_exporter_free(DDProfExporter *exporter) { - if (exporter->_exporter) - ddog_ProfileExporter_delete(exporter->_exporter); - exporter->_exporter = nullptr; - exporter_input_free(&exporter->_input); - free(exporter->_url); - exporter->_url = nullptr; +DDRes ddprof_exporter_free(DDProfExporter &exporter) { + if (exporter._exporter) + ddog_ProfileExporter_delete(exporter._exporter); + exporter._exporter = nullptr; return ddres_init(); } diff --git a/src/lib/allocation_tracker.cc b/src/lib/allocation_tracker.cc index 63ef0e026..370eb43a5 100644 --- a/src/lib/allocation_tracker.cc +++ b/src/lib/allocation_tracker.cc @@ -281,7 +281,7 @@ DDRes AllocationTracker::push_sample(uint64_t allocated_size, if (write(_pevent.fd, &count, sizeof(count)) != sizeof(count)) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFRB, "Error writing to memory allocation eventfd (%s)", - strerror(errno)); + std::strerror(errno)); } } return {}; diff --git a/src/lib/dd_profiling.cc b/src/lib/dd_profiling.cc index cb5493444..160559442 100644 --- a/src/lib/dd_profiling.cc +++ b/src/lib/dd_profiling.cc @@ -104,7 +104,7 @@ struct ProfilerAutoStart { // and library will not be loaded. bool autostart = false; const char *autostart_env = getenv(k_profiler_auto_start_env_variable); - if (autostart_env && arg_yesno(autostart_env, 1)) { + if (autostart_env && is_yes(autostart_env)) { autostart = true; } else { // if library is preloaded, autostart profiling since there is no way @@ -132,7 +132,7 @@ struct ProfilerAutoStart { ProfilerAutoStart g_autostart; -int exec_ddprof(pid_t target_pid, pid_t parent_pid, int sock_fd) { +int exec_ddprof(pid_t target_pid, int sock_fd) { char ddprof_str[] = "ddprof"; char pid_buf[32]; @@ -151,8 +151,6 @@ int exec_ddprof(pid_t target_pid, pid_t parent_pid, int sock_fd) { // would trigger a fork bomb unsetenv("LD_PRELOAD"); - kill(parent_pid, SIGTERM); - if (const char *ddprof_exe = getenv(k_profiler_ddprof_exe_env_variable); ddprof_exe) { execve(ddprof_exe, argv, environ); @@ -218,19 +216,17 @@ int ddprof_start_profiling_internal() { auto defer_parent_socket_close = make_defer([&sockfds]() { close(sockfds[kParentIdx]); }); - auto daemonize_res = ddprof::daemonize(); - if (daemonize_res.temp_pid == -1) { + ddprof::DaemonizeResult daemonize_res; + if (!daemonize_res.daemonize(nullptr)) { return -1; } - if (daemonize_res.temp_pid) { - // executed by daemonized process - + if (daemonize_res.is_daemon()) { // close parent socket end defer_parent_socket_close.reset(); defer_child_socket_close.release(); - exec_ddprof(target_pid, daemonize_res.temp_pid, sockfds[kChildIdx]); + exec_ddprof(target_pid, sockfds[kChildIdx]); exit(1); } @@ -275,7 +271,8 @@ int ddprof_start_profiling_internal() { int ddprof_start_profiling() { try { return ddprof_start_profiling_internal(); - } catch (...) {} + } catch (ddprof::exit const &_) { return 0; } catch (...) { + } return -1; } diff --git a/src/logger.cc b/src/logger.cc index 1608ea89c..0ba2e84c0 100644 --- a/src/logger.cc +++ b/src/logger.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/src/logger_setup.cc b/src/logger_setup.cc index d5483c06c..f15bddeae 100644 --- a/src/logger_setup.cc +++ b/src/logger_setup.cc @@ -55,4 +55,8 @@ void setup_logger(const char *log_mode, const char *log_level) { LOG_setlevel(LL_ERROR); break; } -} \ No newline at end of file +} + +void setup_logger(const std::string &log_mode, const std::string &log_level) { + return setup_logger(log_mode.c_str(), log_level.c_str()); +} diff --git a/src/perf.cc b/src/perf.cc index 931dd126c..371a14177 100644 --- a/src/perf.cc +++ b/src/perf.cc @@ -90,9 +90,11 @@ perf_event_attr perf_config_from_watcher(const PerfWatcher *watcher, attr.sample_type = watcher->sample_type; attr.sample_stack_user = watcher->sample_stack_size; - // If use_kernel is requested false --> exclude_kernel == true - if (watcher->options.use_kernel == PerfWatcherUseKernel::kTry) - attr.exclude_kernel = true; + // NB inverted (kTry is handled at a higher level) + if (watcher->options.use_kernel == PerfWatcherUseKernel::kOff) + attr.exclude_kernel = 1; + else + attr.exclude_kernel = 0; // Extras (metadata for tracking process state) if (extras) { diff --git a/src/perf_mainloop.cc b/src/perf_mainloop.cc index 0a6ac216a..1335cbc63 100644 --- a/src/perf_mainloop.cc +++ b/src/perf_mainloop.cc @@ -6,6 +6,7 @@ #include "perf_mainloop.hpp" #include "ddprof_context_lib.hpp" +#include "ddprof_exit.hpp" #include "ddprof_worker.hpp" #include "ddres.hpp" #include "defer.hpp" @@ -39,23 +40,27 @@ static inline int64_t now_nanos() { return (tv.tv_sec * 1000000 + tv.tv_usec) * 1000; } -static void handle_signal(int) { - g_termination_requested = true; +#include +static void handle_signal(int signo) { + g_termination_requested = true; // for all signals - // forwarding signal to child - if (g_child_pid) { - kill(g_child_pid, SIGTERM); + // If we receive a terminal request, forward a SIGTERM to the child. + switch (signo) { + case SIGTERM: + if (g_child_pid) { + kill(g_child_pid, SIGTERM); + } + throw ddprof::exit(); } } static DDRes install_signal_handler() { sigset_t sigset; - struct sigaction sa; + struct sigaction sa = {}; DDRES_CHECK_ERRNO(sigemptyset(&sigset), DD_WHAT_MAINLOOP_INIT, "sigemptyset failed"); sa.sa_handler = &handle_signal; sa.sa_mask = sigset; - sa.sa_flags = SA_RESTART; DDRES_CHECK_ERRNO(sigaction(SIGTERM, &sa, NULL), DD_WHAT_MAINLOOP_INIT, "Setting SIGTERM handler failed"); DDRES_CHECK_ERRNO(sigaction(SIGINT, &sa, NULL), DD_WHAT_MAINLOOP_INIT, @@ -318,11 +323,8 @@ DDRes main_loop(const WorkerAttr *attr, DDProfContext *ctx) { } if (is_worker) { worker(ctx, attr, persistent_worker_state); - // Ensure worker does not return, - // because we don't want to free resources (perf_event fds,...) that are - // shared between processes. Only free the context. - ddprof_context_free(ctx); - exit(0); + // ctx->release(); + throw ddprof::exit(); } return {}; } diff --git a/src/perf_ringbuffer.cc b/src/perf_ringbuffer.cc index a4e000356..d91b41d0f 100644 --- a/src/perf_ringbuffer.cc +++ b/src/perf_ringbuffer.cc @@ -8,7 +8,8 @@ #include "logger.hpp" #include "mpscringbuffer.hpp" -#include +#include +#include bool rb_init(RingBuffer *rb, void *base, size_t size, RingBufferType ring_buffer_type) { diff --git a/src/perf_watcher.cc b/src/perf_watcher.cc index cad7562bc..d9da26279 100644 --- a/src/perf_watcher.cc +++ b/src/perf_watcher.cc @@ -5,10 +5,15 @@ #include "perf_watcher.hpp" +#include "logger.hpp" #include "perf.hpp" +#include #include #include +#include +#include +#include #define BASE_STYPES \ (PERF_SAMPLE_STACK_USER | PERF_SAMPLE_REGS_USER | PERF_SAMPLE_TID | \ @@ -103,3 +108,56 @@ const PerfWatcher *tracepoint_default_watcher() { bool watcher_has_tracepoint(const PerfWatcher *watcher) { return DDPROF_PWT_TRACEPOINT == watcher->sample_type_id; } + +const char *get_tracefs_root() { + static const char *tracepoint_root = NULL; + constexpr std::array candidate_paths = { + "/sys/kernel/tracing/events/", "/sys/kernel/debug/tracing/events"}; + + if (!tracepoint_root) { + struct stat sa; + for (auto &candidate : candidate_paths) { + if (!stat(candidate.data(), &sa)) + return tracepoint_root = candidate.data(); + } + // If none of them worked, then it failed. + // Log the message. + LG_WRN("Could not determine tracefs root"); + } + return tracepoint_root; +} + +unsigned int tracepoint_id_from_event(const char *eventname, + const char *groupname) { + if (!eventname || !*eventname || !groupname || !*groupname) + return 0; + + static char path[4096]; // Arbitrary, but path sizes limits are difficult + static char buf[sizeof("4294967296")]; // For reading 32-bit decimal int + const char *tracefs_root = get_tracefs_root(); + if (!tracefs_root) { + return 0; + } + char *buf_copy = buf; + size_t pathsz = snprintf(path, sizeof(path), "%s/%s/%s/id", tracefs_root, + groupname, eventname); + if (pathsz >= sizeof(path)) + return 0; + int fd = open(path, O_RDONLY); + if (-1 == fd) + return 0; + + // Read the data in an eintr-safe way + int read_ret = -1; + long trace_id = 0; + do { + read_ret = read(fd, buf, sizeof(buf)); + } while (read_ret == -1 && errno == EINTR); + close(fd); + if (read_ret > 0) + trace_id = strtol(buf, &buf_copy, 10); + if (*buf_copy && *buf_copy != '\n') + return 0; + + return trace_id; +} diff --git a/src/pevent_lib.cc b/src/pevent_lib.cc index 07997aff5..e869a267f 100644 --- a/src/pevent_lib.cc +++ b/src/pevent_lib.cc @@ -14,7 +14,7 @@ #include "user_override.hpp" #include -#include +#include #include #include #include @@ -34,7 +34,7 @@ static DDRes pevent_create(PEventHdr *pevent_hdr, int watcher_idx, } void pevent_init(PEventHdr *pevent_hdr) { - memset(pevent_hdr, 0, sizeof(PEventHdr)); + *pevent_hdr = {}; pevent_hdr->max_size = MAX_NB_PERF_EVENT_OPEN; for (size_t k = 0; k < pevent_hdr->max_size; ++k) { pevent_hdr->pes[k].fd = -1; @@ -95,7 +95,7 @@ static DDRes pevent_register_cpu_0(const PerfWatcher *watcher, int watcher_idx, display_system_config(); DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error calling perfopen on watcher %d.0 (%s)", - watcher_idx, strerror(errno)); + watcher_idx, std::strerror(errno)); } return ddres_init(); @@ -120,7 +120,7 @@ static DDRes pevent_open_all_cpus(const PerfWatcher *watcher, int watcher_idx, if (fd == -1) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error calling perfopen on watcher %d.%d (%s)", - watcher_idx, cpu_idx, strerror(errno)); + watcher_idx, cpu_idx, std::strerror(errno)); } pevent_set_info(fd, pes[template_pevent_idx].attr_idx, pes[pevent_idx]); } @@ -152,7 +152,7 @@ DDRes pevent_mmap_event(PEvent *event) { if (!region) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFMMAP, "Could not mmap memory for watcher #%d: %s", - event->watcher_pos, strerror(errno)); + event->watcher_pos, std::strerror(errno)); } if (!rb_init(&event->rb, region, event->ring_buffer_size, event->ring_buffer_type)) { @@ -248,17 +248,17 @@ DDRes pevent_munmap(PEventHdr *pevent_hdr) { DDRes pevent_close_event(PEvent *event) { if (event->fd != -1) { if (close(event->fd) == -1) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, - "Error when closing fd=%d (watcher #%d) (%s)", - event->fd, event->watcher_pos, strerror(errno)); + DDRES_RETURN_ERROR_LOG( + DD_WHAT_PERFOPEN, "Error when closing fd=%d (watcher #%d) (%s)", + event->fd, event->watcher_pos, std::strerror(errno)); } event->fd = -1; } if (event->custom_event && event->mapfd != -1) { if (close(event->mapfd) == -1) { - DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, - "Error when closing mapfd=%d (watcher #%d) (%s)", - event->mapfd, event->watcher_pos, strerror(errno)); + DDRES_RETURN_ERROR_LOG( + DD_WHAT_PERFOPEN, "Error when closing mapfd=%d (watcher #%d) (%s)", + event->mapfd, event->watcher_pos, std::strerror(errno)); } } return {}; diff --git a/src/pprof/ddprof_pprof.cc b/src/pprof/ddprof_pprof.cc index d81f3b3f5..4b573221b 100644 --- a/src/pprof/ddprof_pprof.cc +++ b/src/pprof/ddprof_pprof.cc @@ -20,10 +20,12 @@ #define PPROF_MAX_LABELS 5 -DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext *ctx) { +DDRes pprof_create_profile(std::shared_ptr &pprof, + DDProfContext *ctx) { PerfWatcher *watchers = ctx->watchers; size_t num_watchers = ctx->num_watchers; ddog_ValueType perf_value_type[DDPROF_PWT_LENGTH]; + pprof = std::make_shared(); // Figure out which sample_type_ids are used by active watchers // We also record the watcher with the lowest valid sample_type id, since that @@ -126,12 +128,12 @@ DDRes pprof_create_profile(DDProfPProf *pprof, DDProfContext *ctx) { return ddres_init(); } -DDRes pprof_free_profile(DDProfPProf *pprof) { - if (pprof->_profile) { - ddog_Profile_free(pprof->_profile); +DDRes pprof_free_profile(DDProfPProf &pprof) { + if (pprof._profile) { + ddog_Profile_free(pprof._profile); } - pprof->_profile = NULL; - pprof->_nb_values = 0; + pprof._profile = NULL; + pprof._nb_values = 0; return ddres_init(); } @@ -172,11 +174,11 @@ static void write_line(const ddprof::Symbol &symbol, ddog_Line *ffi_line) { DDRes pprof_aggregate(const UnwindOutput *uw_output, const SymbolHdr *symbol_hdr, uint64_t value, uint64_t count, const PerfWatcher *watcher, - DDProfPProf *pprof) { + DDProfPProf &pprof) { const ddprof::SymbolTable &symbol_table = symbol_hdr->_symbol_table; const ddprof::MapInfoTable &mapinfo_table = symbol_hdr->_mapinfo_table; - ddog_Profile *profile = pprof->_profile; + ddog_Profile *profile = pprof._profile; int64_t values[DDPROF_PWT_LENGTH] = {}; values[watcher->pprof_sample_idx] = value * count; @@ -243,7 +245,7 @@ DDRes pprof_aggregate(const UnwindOutput *uw_output, } ddog_Sample sample = { .locations = {.ptr = locations_buff, .len = cur_loc}, - .values = {.ptr = values, .len = pprof->_nb_values}, + .values = {.ptr = values, .len = pprof._nb_values}, .labels = {.ptr = labels, .len = labels_num}, }; @@ -255,8 +257,8 @@ DDRes pprof_aggregate(const UnwindOutput *uw_output, return ddres_init(); } -DDRes pprof_reset(DDProfPProf *pprof) { - if (!ddog_Profile_reset(pprof->_profile, nullptr)) { +DDRes pprof_reset(DDProfPProf &pprof) { + if (!ddog_Profile_reset(pprof._profile, nullptr)) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PPROF, "Unable to reset profile"); } return ddres_init(); diff --git a/src/ringbuffer_utils.cc b/src/ringbuffer_utils.cc index 5ddbbf41e..e8475ed72 100644 --- a/src/ringbuffer_utils.cc +++ b/src/ringbuffer_utils.cc @@ -44,18 +44,18 @@ DDRes ring_buffer_create(size_t buffer_size_page_order, if (pevent->mapfd == -1) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error calling memfd_create on watcher %d (%s)", - pevent->watcher_pos, strerror(errno)); + pevent->watcher_pos, std::strerror(errno)); } if (ftruncate(pevent->mapfd, buffer_size) == -1) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error calling ftruncate on watcher %d (%s)", - pevent->watcher_pos, strerror(errno)); + pevent->watcher_pos, std::strerror(errno)); } pevent->fd = eventfd(0, 0); if (pevent->fd == -1) { DDRES_RETURN_ERROR_LOG(DD_WHAT_PERFOPEN, "Error calling evenfd on watcher %d (%s)", - pevent->watcher_pos, strerror(errno)); + pevent->watcher_pos, std::strerror(errno)); } pevent->custom_event = custom_event; pevent->ring_buffer_type = ring_buffer_type; diff --git a/src/statsd.cc b/src/statsd.cc index ed7d3431a..dcfd9f7b3 100644 --- a/src/statsd.cc +++ b/src/statsd.cc @@ -8,7 +8,8 @@ #include "ddres.hpp" #include -#include +#include +#include #include #include #include @@ -24,7 +25,7 @@ DDRes statsd_listen(const char *path, size_t sz_path, int *fd) { int socktype = SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK; if (-1 == (fd_sock = socket(AF_UNIX, socktype, 0))) { DDRES_RETURN_WARN_LOG(DD_WHAT_STATSD, "[STATSD] Creating UDS failed (%s)", - strerror(errno)); + std::strerror(errno)); } // Attempt to bind to the given path. This is necessary for datagram-type @@ -40,7 +41,7 @@ DDRes statsd_listen(const char *path, size_t sz_path, int *fd) { if (bind(fd_sock, (struct sockaddr *)&addr_bind, sizeof(addr_bind))) { close(fd_sock); DDRES_RETURN_WARN_LOG(DD_WHAT_STATSD, "Binding UDS failed (%s)", - strerror(errno)); + std::strerror(errno)); } *fd = fd_sock; @@ -71,7 +72,7 @@ DDRes statsd_connect(const char *path, size_t sz_path, int *fd) { close(fd_sock); DDRES_RETURN_WARN_LOG(DD_WHAT_STATSD, "[STATSD] Connecting to host failed (%s)", - strerror(errno)); + std::strerror(errno)); } // If we're here, then the connection has been fully established diff --git a/src/tags.cc b/src/tags.cc index 983adb43e..0f06475a3 100644 --- a/src/tags.cc +++ b/src/tags.cc @@ -57,7 +57,7 @@ static Tag split_kv(const char *str, size_t end_pos, char c = ':') { } void split(const char *str, Tags &tags, char c) { - if (!str) { + if (!str || !*str) { // nothing to do return; } @@ -83,8 +83,8 @@ void split(const char *str, Tags &tags, char c) { } // namespace ddprof -UserTags::UserTags(const char *tag_str, int nproc) { - ddprof::split(tag_str, _tags); +UserTags::UserTags(const std::string &tag_str, int nproc) { + ddprof::split(tag_str.c_str(), _tags); _tags.push_back({"number_of_cpu_cores", std::to_string(nproc)}); _tags.push_back({"number_hw_concurent_threads", std::to_string(ddprof::get_nb_hw_thread())}); diff --git a/src/tempfile.cc b/src/tempfile.cc index fe2d28fca..2765816dd 100644 --- a/src/tempfile.cc +++ b/src/tempfile.cc @@ -9,6 +9,7 @@ #include "defer.hpp" #include "sha1.h" +#include #include #include #include diff --git a/src/user_override.cc b/src/user_override.cc index 56683ed91..8485ab804 100644 --- a/src/user_override.cc +++ b/src/user_override.cc @@ -12,9 +12,9 @@ #include "logger.hpp" #include +#include #include #include -#include #include #include #include @@ -83,11 +83,11 @@ DDRes user_override(uid_t uid, gid_t gid, UIDInfo *old_uids) { DumpableRestorer dumpable_restorer; if (gid != static_cast(-1)) { DDRES_CHECK_INT(setresgid(gid, gid, -1), DD_WHAT_USERID, - "Unable to set gid %d (%s)", gid, strerror(errno)); + "Unable to set gid %d (%s)", gid, std::strerror(errno)); } if (uid != static_cast(-1)) { DDRES_CHECK_INT(setresuid(uid, uid, -1), DD_WHAT_USERID, - "Unable to set uid %d (%s)", uid, strerror(errno)); + "Unable to set uid %d (%s)", uid, std::strerror(errno)); } return {}; diff --git a/src/version.cc b/src/version.cc index 0f89c19ef..568d25a78 100644 --- a/src/version.cc +++ b/src/version.cc @@ -4,23 +4,7 @@ // Datadog, Inc. #include "version.hpp" -#include "string_view.hpp" -#include +#include -string_view str_version() { - static char profiler_version[1024] = {0}; - string_view version_str; - if (*VER_REV) - version_str.len = snprintf(profiler_version, 1024, "%d.%d.%d+%s", VER_MAJ, - VER_MIN, VER_PATCH, VER_REV); - else - version_str.len = snprintf(profiler_version, 1024, "%d.%d.%d", VER_MAJ, - VER_MIN, VER_PATCH); - - version_str.ptr = profiler_version; - - return version_str; -} - -void print_version() { printf(MYNAME " %s\n", str_version().ptr); } +void print_version() { printf(MYNAME " %s\n", str_version().data()); } diff --git a/test/ddprof_exporter-ut.cc b/test/ddprof_exporter-ut.cc index bb48552ef..df3420c8f 100644 --- a/test/ddprof_exporter-ut.cc +++ b/test/ddprof_exporter-ut.cc @@ -85,10 +85,10 @@ void fill_mock_exporter_input(ExporterInput &exporter_input, exporter_input.service_version = "42"; exporter_input.do_export = "yes"; exporter_input.debug_pprof_prefix = "some_prefix"; - exporter_input.user_agent = STRING_VIEW_LITERAL("DDPROF_MOCK"); - exporter_input.language = STRING_VIEW_LITERAL("NATIVE"); - exporter_input.family = STRING_VIEW_LITERAL("SANCHEZ"); - exporter_input.profiler_version = STRING_VIEW_LITERAL("1.1.2"); + exporter_input.user_agent = "DDPROF_MOCK"; + exporter_input.language = "NATIVE"; + exporter_input.family = "SANCHEZ"; + exporter_input.profiler_version = "1.1.2"; } TEST(DDProfExporter, url) { @@ -98,26 +98,26 @@ TEST(DDProfExporter, url) { // Test the site / host / port / API logic // If API key --> use site fill_mock_exporter_input(exporter_input, url, true); - DDProfExporter exporter; - DDRes res = ddprof_exporter_init(&exporter_input, &exporter); + std::shared_ptr exporter; + DDRes res = ddprof_exporter_init(&exporter_input, exporter); EXPECT_TRUE(IsDDResOK(res)); - EXPECT_EQ(strcmp(exporter._url, "datadog_is_cool.com"), 0); - ddprof_exporter_free(&exporter); + EXPECT_TRUE(exporter->_url == url.first); + ddprof_exporter_free(*exporter); // Default to host if site not found // To be discussed : should we fail here ? - exporter_input.site = nullptr; - res = ddprof_exporter_init(&exporter_input, &exporter); + exporter_input.site = ""; + res = ddprof_exporter_init(&exporter_input, exporter); EXPECT_TRUE(IsDDResOK(res)); - EXPECT_EQ(strcmp(exporter._url, "25.04.1988.0"), 0); - ddprof_exporter_free(&exporter); + EXPECT_TRUE(exporter->_url == "25.04.1988.0"); + ddprof_exporter_free(*exporter); // If no API key --> expect host fill_mock_exporter_input(exporter_input, url, false); - res = ddprof_exporter_init(&exporter_input, &exporter); + 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); + EXPECT_TRUE(exporter->_url == "http://25.04.1988.0:1234"); + ddprof_exporter_free(*exporter); } TEST(DDProfExporter, simple) { @@ -129,13 +129,13 @@ TEST(DDProfExporter, simple) { { // setup input parameters fill_mock_exporter_input(exporter_input, url, false); } - DDProfPProf pprofs; - DDProfExporter exporter; - DDRes res = ddprof_exporter_init(&exporter_input, &exporter); + std::shared_ptr pprofs; + std::shared_ptr exporter; + DDRes res = ddprof_exporter_init(&exporter_input, exporter); EXPECT_TRUE(IsDDResOK(res)); { // override folder to write debug pprofs // You can view content using : pprof -raw ./test/data/ddprof_ - exporter._debug_pprof_prefix = UNIT_TEST_DATA "/ddprof_"; + exporter->_debug_pprof_prefix = UNIT_TEST_DATA "/ddprof_"; } { // Aggregate pprofs @@ -150,30 +150,30 @@ TEST(DDProfExporter, simple) { ctx.watchers[0] = *ewatcher_from_str("sCPU"); ctx.num_watchers = 1; - res = pprof_create_profile(&pprofs, &ctx); + res = pprof_create_profile(pprofs, &ctx); EXPECT_TRUE(IsDDResOK(res)); res = pprof_aggregate(&mock_output, &symbol_hdr, 1000, 1, &ctx.watchers[0], - &pprofs); + *pprofs); EXPECT_TRUE(IsDDResOK(res)); } { - UserTags user_tags(nullptr, 4); + UserTags user_tags("", 4); - res = ddprof_exporter_new(&user_tags, &exporter); + res = ddprof_exporter_new(&user_tags, *exporter); EXPECT_TRUE(IsDDResOK(res)); if (get_url_from_env(K_RECEPTOR_ENV_ADDR)) { // receptor is defined Tags empty_tags; - res = ddprof_exporter_export(pprofs._profile, empty_tags, 0, &exporter); + res = ddprof_exporter_export(*pprofs->_profile, empty_tags, 0, *exporter); // We should not be able to send profiles (usually 404) EXPECT_FALSE(IsDDResOK(res)); } } - res = ddprof_exporter_free(&exporter); + res = ddprof_exporter_free(*exporter); EXPECT_TRUE(IsDDResOK(res)); - res = pprof_free_profile(&pprofs); + res = pprof_free_profile(*pprofs); EXPECT_TRUE(IsDDResOK(res)); } diff --git a/test/ddprof_input-ut.cc b/test/ddprof_input-ut.cc index 5f64b5254..172b15f86 100644 --- a/test/ddprof_input-ut.cc +++ b/test/ddprof_input-ut.cc @@ -12,7 +12,6 @@ #include "loghandle.hpp" #include "perf_watcher.hpp" #include "span.hpp" -#include "string_view.hpp" #include #include @@ -28,7 +27,6 @@ class InputTest : public ::testing::Test { bool s_version_called = false; void print_version() { s_version_called = true; } -string_view str_version() { return STRING_VIEW_LITERAL("1.2.3"); } TEST_F(InputTest, default_values) { DDProfInput input; @@ -36,12 +34,11 @@ TEST_F(InputTest, default_values) { EXPECT_TRUE(IsDDResOK(res)); const char *env_serv = getenv("DD_SERVICE"); if (env_serv != NULL) { - EXPECT_EQ(strcmp(input.exp_input.service, env_serv), 0); + EXPECT_TRUE(input.exp_input.service == env_serv); } else { - EXPECT_EQ(strcmp(input.exp_input.service, "myservice"), 0); + EXPECT_TRUE(input.exp_input.service == "myservice"); } - EXPECT_EQ(strcmp(input.log_mode, "stdout"), 0); - ddprof_input_free(&input); + EXPECT_TRUE(input.log_mode == "stdout"); } TEST_F(InputTest, version_called) { @@ -54,7 +51,6 @@ TEST_F(InputTest, version_called) { EXPECT_TRUE(s_version_called); EXPECT_FALSE(contine_exec); EXPECT_EQ(input.nb_parsed_params, 2); - ddprof_input_free(&input); } TEST_F(InputTest, single_param) { @@ -69,10 +65,9 @@ TEST_F(InputTest, single_param) { ddprof_input_parse(argc, (char **)input_values, &input, &contine_exec); EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - EXPECT_EQ(strcmp(input.core_dumps, "yes"), 0); + EXPECT_TRUE(input.core_dumps == "yes"); EXPECT_EQ(input.nb_parsed_params, 3); ddprof_print_params(&input); - ddprof_input_free(&input); } TEST_F(InputTest, no_params) { @@ -85,7 +80,6 @@ TEST_F(InputTest, no_params) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); EXPECT_EQ(input.nb_parsed_params, 1); - ddprof_input_free(&input); } TEST_F(InputTest, dump_fixed) { @@ -97,7 +91,6 @@ TEST_F(InputTest, dump_fixed) { ddprof_input_parse(argc, (char **)input_values, &input, &contine_exec); EXPECT_FALSE(IsDDResOK(res)); EXPECT_FALSE(contine_exec); - ddprof_input_free(&input); } TEST_F(InputTest, event_from_env) { @@ -119,7 +112,6 @@ TEST_F(InputTest, event_from_env) { EXPECT_NE(nullptr, watcher); EXPECT_EQ(watcher->config, input.watchers[0].config); EXPECT_EQ(input.watchers[0].sample_period, 1000); - ddprof_input_free(&input); } { DDProfInput input; @@ -133,7 +125,6 @@ TEST_F(InputTest, event_from_env) { EXPECT_TRUE(contine_exec); EXPECT_EQ(input.nb_parsed_params, 1); EXPECT_EQ(input.num_watchers, 0); - ddprof_input_free(&input); } { DDProfInput input; @@ -153,7 +144,6 @@ TEST_F(InputTest, event_from_env) { EXPECT_EQ(input.watchers[0].config, watcher->config); EXPECT_EQ(input.watchers[0].type, watcher->type); EXPECT_EQ(input.watchers[0].sample_period, 1000); - ddprof_input_free(&input); } { DDProfInput input; @@ -181,8 +171,6 @@ TEST_F(InputTest, event_from_env) { EXPECT_EQ(input.watchers[0].sample_period, 1000); EXPECT_EQ(input.watchers[1].sample_period, 123); EXPECT_EQ(input.watchers[2].sample_period, 456); - - ddprof_input_free(&input); } } @@ -200,12 +188,11 @@ TEST_F(InputTest, duplicate_events) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_FALSE(IsDDResOK(res)); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // Duplicate events (except tracepoints) are disallowed @@ -219,12 +206,11 @@ TEST_F(InputTest, duplicate_events) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_FALSE(IsDDResOK(res)); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } } @@ -240,7 +226,7 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); @@ -259,8 +245,7 @@ TEST_F(InputTest, presets) { }), watchers.end()); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // Default preset for PID mode should be CPU @@ -273,15 +258,14 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); EXPECT_EQ(ctx.num_watchers, 1); EXPECT_EQ(ctx.watchers[0].ddprof_event_type, DDPROF_PWE_sCPU); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // Check cpu_only preset @@ -294,7 +278,7 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); @@ -303,8 +287,7 @@ TEST_F(InputTest, presets) { EXPECT_EQ(ctx.num_watchers, 1); EXPECT_EQ(ctx.watchers[0].ddprof_event_type, DDPROF_PWE_sCPU); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // Check alloc_only preset @@ -318,7 +301,7 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); @@ -326,8 +309,7 @@ TEST_F(InputTest, presets) { EXPECT_EQ(ctx.watchers[1].ddprof_event_type, DDPROF_PWE_sALLOC); EXPECT_EQ(ctx.watchers[0].ddprof_event_type, DDPROF_PWE_sDUM); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // Default preset should not be loaded if an event is given in input @@ -340,15 +322,14 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); EXPECT_EQ(ctx.num_watchers, 1); EXPECT_EQ(ctx.watchers[0].ddprof_event_type, DDPROF_PWE_sCPU); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } { // If preset is explicit given in input, then another event with the same @@ -363,7 +344,7 @@ TEST_F(InputTest, presets) { EXPECT_TRUE(IsDDResOK(res)); EXPECT_TRUE(contine_exec); - DDProfContext ctx; + DDProfContext ctx = {}; res = ddprof_context_set(&input, &ctx); EXPECT_TRUE(IsDDResOK(res)); @@ -372,7 +353,6 @@ TEST_F(InputTest, presets) { EXPECT_EQ(ctx.watchers[0].sample_frequency, 1234); EXPECT_EQ(ctx.watchers[1].ddprof_event_type, DDPROF_PWE_sALLOC); - ddprof_input_free(&input); - ddprof_context_free(&ctx); + ctx.release(); } } diff --git a/test/ddprof_pprof-ut.cc b/test/ddprof_pprof-ut.cc index 21d06bf3b..7f4d9dda4 100644 --- a/test/ddprof_pprof-ut.cc +++ b/test/ddprof_pprof-ut.cc @@ -24,14 +24,14 @@ namespace ddprof { DwflSymbolLookup::DwflSymbolLookup() : _lookup_setting(K_CACHE_ON) {} TEST(DDProfPProf, init_profiles) { - DDProfPProf pprof; + std::shared_ptr pprof; DDProfContext ctx = {}; ctx.watchers[0] = *ewatcher_from_str("sCPU"); ctx.num_watchers = 1; - DDRes res = pprof_create_profile(&pprof, &ctx); + DDRes res = pprof_create_profile(pprof, &ctx); EXPECT_TRUE(IsDDResOK(res)); - res = pprof_free_profile(&pprof); + res = pprof_free_profile(*pprof); EXPECT_TRUE(IsDDResOK(res)); } @@ -68,20 +68,20 @@ TEST(DDProfPProf, aggregate) { MapInfoTable &mapinfo_table = symbol_hdr._mapinfo_table; fill_unwind_symbols(table, mapinfo_table, mock_output); - DDProfPProf pprof; + std::shared_ptr pprof; DDProfContext ctx = {}; ctx.watchers[0] = *ewatcher_from_str("sCPU"); ctx.num_watchers = 1; - DDRes res = pprof_create_profile(&pprof, &ctx); + DDRes res = pprof_create_profile(pprof, &ctx); EXPECT_TRUE(IsDDResOK(res)); res = pprof_aggregate(&mock_output, &symbol_hdr, 1000, 1, &ctx.watchers[0], - &pprof); + *pprof); EXPECT_TRUE(IsDDResOK(res)); - test_pprof(&pprof); + test_pprof(&*pprof); - res = pprof_free_profile(&pprof); + res = pprof_free_profile(*pprof); EXPECT_TRUE(IsDDResOK(res)); } diff --git a/test/ddprofcmdline-ut.cc b/test/ddprofcmdline-ut.cc index 1b7ae4b40..1e4cf0c51 100644 --- a/test/ddprofcmdline-ut.cc +++ b/test/ddprofcmdline-ut.cc @@ -22,10 +22,10 @@ TEST(CmdLineTst, ArgWhich) { TEST(CmdLineTst, ArgYesNo) { const char *yesStr = "YeS"; const char *noStr = "nO"; - ASSERT_TRUE(arg_yesno(yesStr, 1)); - ASSERT_FALSE(arg_yesno(noStr, 1)); - ASSERT_TRUE(arg_yesno(noStr, 0)); - ASSERT_FALSE(arg_yesno(yesStr, 0)); + ASSERT_TRUE(is_yes(yesStr)); + ASSERT_FALSE(is_yes(noStr)); + ASSERT_TRUE(is_no(noStr)); + ASSERT_FALSE(is_no(yesStr)); } TEST(CmdLineTst, PartialFilled) { diff --git a/test/self_unwind/self_unwind.cc b/test/self_unwind/self_unwind.cc index 575cfd62b..5a992e10c 100644 --- a/test/self_unwind/self_unwind.cc +++ b/test/self_unwind/self_unwind.cc @@ -78,12 +78,12 @@ int main(int argc, char *const argv[]) { &input, &continue_exec))) { std::cerr << "unable to init input " << std::endl; ret = -1; - goto CLEANUP_INPUT; + goto CLEANUP; } if (!continue_exec) { std::cerr << "Bad arguments... EXIT" << std::endl; ret = -1; - goto CLEANUP_INPUT; + goto CLEANUP; } if (IsDDResNotOK(ddprof_context_set(&input, &ctx))) { @@ -102,9 +102,7 @@ int main(int argc, char *const argv[]) { ret = compare_to_ref(exe_name, symbol_map, data_directory); CLEANUP: - ddprof_context_free(&ctx); -CLEANUP_INPUT: - ddprof_input_free(&input); + ctx.release(); return ret; } diff --git a/test/tags-ut.cc b/test/tags-ut.cc index be501876a..635526cf5 100644 --- a/test/tags-ut.cc +++ b/test/tags-ut.cc @@ -51,7 +51,7 @@ TEST(Tags, more_tags) { TEST(Tags, user_tags) { LogHandle handle; - UserTags user_tags(nullptr, 8); + UserTags user_tags("", 8); std::for_each(user_tags._tags.begin(), user_tags._tags.end(), [](Tag const &el) { LG_DBG("Tag = %s:%s", el.first.c_str(), el.second.c_str()); diff --git a/test/version-ut.cc b/test/version-ut.cc index 3f3445f98..e1f5e4559 100644 --- a/test/version-ut.cc +++ b/test/version-ut.cc @@ -12,6 +12,5 @@ TEST(VersionTest, VersionStr) { std::string expectedStr; expectedStr += std::to_string(VER_MAJ) + "." + std::to_string(VER_MIN) + "." + std::to_string(VER_PATCH); - std::string apiStr(str_version().ptr, str_version().len); - EXPECT_TRUE(apiStr.find(expectedStr) != std::string::npos); + EXPECT_TRUE(str_version().find(expectedStr) != std::string::npos); }