From a12beeed48e515e659560a9fb45ee93aca58f0eb Mon Sep 17 00:00:00 2001 From: David Sanchez <838104+sanchda@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:35:22 -0500 Subject: [PATCH] chore(profiling): add non-running tests for stack v2 (#8576) This PR creates a harness for independently building and running the stack v2 code. I did it this way for a few reasons * Don't want to add unnecessary overhead to our normal test process * Want to run the build for these native components under multiple static analysis tools * Want to run some of the tests for these components under sanitizers? * But also want to do exhaustive testing of the interfaces, since we've had a few regressions here This PR includes both the tests and the fixups they demand. A separate PR will add a CI job to execute these (or will add it to a pre-existing job, TBD). There IS some dead code in here, since we do provide the recipe for getting and running Infer. During testing, I found that I couldn't reliably get infer to find stdlib headers, which completely broke the quality of its output, so it won't actually be used. At the same time, I think we might want to use it someday, and the setup is a little fiddly, so I'm including it here. I'm recommending a backport to 2.7 for this because the latest 2.7 releases make this feature available, but the implementation is subtly (and sometimes not-so-subtly) broken without the fixups here. Since this feature is so isolated (I'm the only one backporting anything related!), this feels safe...ish. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] If change touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: sanchda Co-authored-by: Gabriele N. Tornetta (cherry picked from commit eb635dd99bc05d1394d3c97d74e9816021426282) --- .../profiling/cmake/AnalysisFunc.cmake | 15 +- .../profiling/cmake/FindClangtidy.cmake | 19 ++ .../profiling/cmake/FindCppcheck.cmake | 29 ++- .../datadog/profiling/cmake/FindInfer.cmake | 66 ++++++ .../profiling/cmake/tools/fetch_infer.sh | 76 +++++++ .../profiling/cmake/tools/infer_checksums.txt | 1 + .../profiling/dd_wrapper/CMakeLists.txt | 5 + .../dd_wrapper/include/interface.hpp | 6 +- .../dd_wrapper/include/libdatadog_helpers.hpp | 11 + .../profiling/dd_wrapper/include/profile.hpp | 2 +- .../profiling/dd_wrapper/include/sample.hpp | 8 +- .../dd_wrapper/include/sample_manager.hpp | 1 + .../profiling/dd_wrapper/include/uploader.hpp | 7 +- .../dd_wrapper/include/uploader_builder.hpp | 4 +- .../profiling/dd_wrapper/src/interface.cpp | 94 ++++----- .../profiling/dd_wrapper/src/profile.cpp | 69 +++---- .../profiling/dd_wrapper/src/sample.cpp | 118 +++++------ .../dd_wrapper/src/sample_manager.cpp | 29 +-- .../profiling/dd_wrapper/src/uploader.cpp | 74 ++++--- .../dd_wrapper/src/uploader_builder.cpp | 64 +++--- .../profiling/dd_wrapper/test/CMakeLists.txt | 2 +- .../datadog/profiling/dd_wrapper/test/api.cpp | 10 + .../dd_wrapper/test/initialization.cpp | 12 -- .../profiling/dd_wrapper/test/test_utils.hpp | 2 + .../datadog/profiling/ddup/CMakeLists.txt | 12 +- .../datadog/profiling/ddup/__init__.py | 136 ++++++------- .../internal/datadog/profiling/ddup/_ddup.pyi | 8 +- .../internal/datadog/profiling/ddup/_ddup.pyx | 94 ++++++--- .../datadog/profiling/ddup/test/interface.py | 190 ++++++++++++++++++ ddtrace/internal/datadog/profiling/runner.sh | 0 .../datadog/profiling/setup_custom.sh | 84 ++++++-- .../datadog/profiling/stack_v2/CMakeLists.txt | 4 + ddtrace/profiling/profiler.py | 2 +- pyproject.toml | 1 + 34 files changed, 879 insertions(+), 376 deletions(-) create mode 100644 ddtrace/internal/datadog/profiling/cmake/FindClangtidy.cmake create mode 100644 ddtrace/internal/datadog/profiling/cmake/FindInfer.cmake create mode 100755 ddtrace/internal/datadog/profiling/cmake/tools/fetch_infer.sh create mode 100644 ddtrace/internal/datadog/profiling/cmake/tools/infer_checksums.txt create mode 100644 ddtrace/internal/datadog/profiling/ddup/test/interface.py mode change 100755 => 100644 ddtrace/internal/datadog/profiling/runner.sh diff --git a/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake b/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake index da2584a08d1..75e067dad66 100644 --- a/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake +++ b/ddtrace/internal/datadog/profiling/cmake/AnalysisFunc.cmake @@ -10,7 +10,8 @@ function(add_ddup_config target) "$<$:-s>" -Wl,--as-needed -Wl,-Bsymbolic-functions -Wl,--gc-sections ) - set_property(TARGET ${target} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) + + # If we can IPO, then do so check_ipo_supported(RESULT result) if (result) set_property(TARGET ${target} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE) @@ -22,6 +23,18 @@ function(add_ddup_config target) # pointers, so we include it here. target_compile_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS} -fno-omit-frame-pointer) target_link_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS}) + + # We need to do a little bit of work in order to ensure the dynamic *san libraries can be linked + # Procedure adapted from datadog/ddprof :) + execute_process( + COMMAND ${CMAKE_CXX_COMPILER} -print-file-name= + OUTPUT_VARIABLE LIBSAN_LIB_PATH + OUTPUT_STRIP_TRAILING_WHITESPACE COMMAND_ERROR_IS_FATAL ANY + ) + set_target_properties(${target} PROPERTIES + INSTALL_RPATH ${LIBSAN_LIB_PATH} + BUILD_RPATH ${LIBSAN_LIB_PATH} + ) endif() # If DO_FANALYZER is specified and we're using gcc, then we can use -fanalyzer diff --git a/ddtrace/internal/datadog/profiling/cmake/FindClangtidy.cmake b/ddtrace/internal/datadog/profiling/cmake/FindClangtidy.cmake new file mode 100644 index 00000000000..c607a15bd10 --- /dev/null +++ b/ddtrace/internal/datadog/profiling/cmake/FindClangtidy.cmake @@ -0,0 +1,19 @@ +# Enable clang-tidy by default +option(DO_CLANGTIDY "Enable clang-tidy" OFF) + +# Default executable for clang-tidy +if (NOT DEFINED CLANGTIDY_CMD) + find_program(CLANGTIDY_CMD NAMES clang-tidy) +endif() + +function(add_clangtidy_target TARGET) + if (NOT DO_CLANGTIDY) + return() + endif() + + if (CLANGTIDY_CMD) + set_target_properties(${TARGET} PROPERTIES CXX_CLANG_TIDY "${CLANGTIDY_CMD};--checks=bugprone-*,clang-analyzer-*,cppcoreguidelines-*,modernize-*,performance-*,readability-*,-modernize-use-trailing-return-type,-performance-avoid-endl") + else() + message(FATAL_ERROR "clang-tidy requested but executable not found") + endif() +endfunction() diff --git a/ddtrace/internal/datadog/profiling/cmake/FindCppcheck.cmake b/ddtrace/internal/datadog/profiling/cmake/FindCppcheck.cmake index b215e1dfad2..b7f87a0a73c 100644 --- a/ddtrace/internal/datadog/profiling/cmake/FindCppcheck.cmake +++ b/ddtrace/internal/datadog/profiling/cmake/FindCppcheck.cmake @@ -3,16 +3,21 @@ if (TARGET cppcheck_project) return() endif() +# Set the default value for the cppcheck option +option(DO_CPPCHECK "Enable cppcheck" OFF) + include(ExternalProject) # Build cppcheck from sources if (DO_CPPCHECK) - ExternalProject_Add(cppcheck_project - GIT_REPOSITORY https://github.com/danmar/cppcheck.git - GIT_TAG "2.13.3" - CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/cppcheck - ) - set(CPPCHECK_EXECUTABLE ${CMAKE_BINARY_DIR}/cppcheck/bin/cppcheck) + if (NOT CPPCHECK_EXECUTABLE OR NOT EXISTS "${CPPCHECK_EXECUTABLE}") + ExternalProject_Add(cppcheck_project + GIT_REPOSITORY https://github.com/danmar/cppcheck.git + GIT_TAG "2.13.3" + CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/cppcheck + ) + set(CPPCHECK_EXECUTABLE ${CMAKE_BINARY_DIR}/cppcheck/bin/cppcheck) + endif() # The function we use to register targets for cppcheck would require us to run separate # commands for each target, which is annoying. Instead we'll consolidate all the targets @@ -30,7 +35,7 @@ function(add_cppcheck_target) cmake_parse_arguments(PARSE_ARGV 0 ARG "${options}" "${oneValueArgs}" "${multiValueArgs}") # Automatically generate the cppcheck target name - set(NAME "cppcheck_dd_${ARGV0}") + set(NAME "cppcheck_${ARGV0}") if (DO_CPPCHECK) # Initialize command variable @@ -50,6 +55,7 @@ function(add_cppcheck_target) # Append include directories to the command foreach(INCLUDE_DIR ${ARG_INCLUDE}) + message(STATUS "Adding include directory to cppcheck: ${INCLUDE_DIR}") list(APPEND cppcheck_cmd -I ${INCLUDE_DIR}) endforeach() @@ -73,4 +79,13 @@ function(add_cppcheck_target) COMMENT "cppcheck is disabled for ${ARGV0}" ) endif() + + # Create a standard target to run everything + if (NOT TARGET cppcheck) + add_custom_target(cppcheck COMMENT "Runs cppcheck on all projects") + endif() + + if (DO_CPPCHECK) + add_dependencies(cppcheck ${NAME}) + endif() endfunction() diff --git a/ddtrace/internal/datadog/profiling/cmake/FindInfer.cmake b/ddtrace/internal/datadog/profiling/cmake/FindInfer.cmake new file mode 100644 index 00000000000..351875ad6a2 --- /dev/null +++ b/ddtrace/internal/datadog/profiling/cmake/FindInfer.cmake @@ -0,0 +1,66 @@ +# Only add this project if Infer is not defined +if(TARGET Infer) + return() +endif() + +include(ExternalProject) +# Only grab infer dependencies if we need to +if (DO_INFER) + if (NOT Infer_EXECUTABLE OR NOT EXISTS "${Infer_EXECUTABLE}") + set(TAG_INFER + "v1.1.0" + CACHE STRING "infer github tag") + + set(Infer_BUILD_DIR ${CMAKE_BINARY_DIR}/infer) + set(Infer_ROOT ${Infer_BUILD_DIR}/infer-${TAG_LIBDATADOG}) + + message(STATUS "${CMAKE_CURRENT_LIST_DIR}/tools/fetch_infer.sh ${TAG_INFER} ${Infer_ROOT}") + execute_process( + COMMAND "${CMAKE_CURRENT_LIST_DIR}/tools/fetch_infer.sh" ${TAG_INFER} ${Infer_ROOT} + WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR} + COMMAND_ERROR_IS_FATAL ANY + ) + + set(Infer_DIR "${Infer_ROOT}") + set(Infer_EXECUTABLE "${Infer_DIR}/bin/infer") + endif() +endif() + +# Add a target for using infer. It does nothing if DO_INFER is not set +function(add_infer_target TARGET) + # Automatically generate the infer target name + set(NAME "infer_dd_${TARGET}") + + if (NOT TARGET ${TARGET}) + message(FATAL_ERROR "Target ${TARGET} does not exist") + return() + endif() + + if (DO_INFER) + # Initialize command variable + set(infer_cmd + ${Infer_EXECUTABLE} + --compilation-database ${CMAKE_CURRENT_BINARY_DIR}/compile_commands.json + ) + + # Define the custom target with the constructed command + add_custom_target(${NAME} + COMMAND ${infer_cmd} + COMMENT "Running infer on ${TARGET}" + ) + + # infer can't seem to find stdlib headers, so we have to add them to the target so they are included in the compile_commands.json + foreach(inc ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES}) + target_include_directories(${TARGET} PRIVATE ${inc}) + endforeach() + + # Make the infer target a dependent of the specified target + add_dependencies(${TARGET} ${NAME}) + else() + # Define a do-nothing target if infer is disabled + add_custom_target(${NAME} + COMMAND echo "infer target ${NAME} is disabled." + COMMENT "infer is disabled for ${TARGET}" + ) + endif() +endfunction() diff --git a/ddtrace/internal/datadog/profiling/cmake/tools/fetch_infer.sh b/ddtrace/internal/datadog/profiling/cmake/tools/fetch_infer.sh new file mode 100755 index 00000000000..658b9198c05 --- /dev/null +++ b/ddtrace/internal/datadog/profiling/cmake/tools/fetch_infer.sh @@ -0,0 +1,76 @@ +#!/bin/bash +# http://redsymbol.net/articles/unofficial-bash-strict-mode/ +set -euox pipefail +IFS=$'\n\t' + +usage() { + echo "Usage :" + echo "$0 " + echo "" + echo "Example" + echo " $0 v0.7.0-rc.1 ./vendor" +} + +if [ $# != 2 ] || [ "$1" == "-h" ]; then + usage + exit 1 +fi + +SCRIPTPATH=$(readlink -f "$0") +SCRIPTDIR=$(dirname "$SCRIPTPATH") + +MARCH=$(uname -m) + +TAG_INFER=$1 +TARGET_EXTRACT=$2 + +CHECKSUM_FILE=${SCRIPTDIR}/infer_checksums.txt + +# https://github.com/facebook/infer/releases/download/v1.1.0/infer-linux64-v1.1.0.tar.xz +TAR_INFER=infer-linux64-${TAG_INFER}.tar.xz +GITHUB_URL_INFER=https://github.com/facebook/infer/releases/download/${TAG_INFER}/${TAR_INFER} + +SHA256_INFER="blank" +while IFS=' ' read -r checksum filename; do + if [ "$filename" == "$TAR_INFER" ]; then + SHA256_INFER="$checksum $filename" + break + fi +done < "$CHECKSUM_FILE" + +if [ "$SHA256_INFER" == "blank" ]; then + echo "Could not find checksum for ${TAR_INFER} in ${CHECKSUM_FILE}" + exit 1 +else + echo "Using infer sha256: ${SHA256_INFER}" +fi + +mkdir -p "$TARGET_EXTRACT" || true +cd "$TARGET_EXTRACT" + +if [[ -e "${TAR_INFER}" ]]; then + already_present=1 +else + already_present=0 + echo "Downloading infer ${GITHUB_URL_INFER}..." + if command -v curl > /dev/null 2>&1; then + curl -fsSLO "${GITHUB_URL_INFER}" + elif command -v wget > /dev/null 2>&1; then + wget -q -O "${GITHUB_URL_INFER##*/}" "${GITHUB_URL_INFER}" + else + echo "Error: neither curl nor wget is available." >&2 + exit 1 + fi +fi + +echo "Checking infer sha256" +if ! echo "${SHA256_INFER}" | sha256sum -c -; then + echo "Error validating infer SHA256" + echo "Please clear $TARGET_EXTRACT before restarting" + exit 1 +fi + +if [[ $already_present -eq 0 ]]; then + echo "Extracting ${TAR_INFER}" + tar xf "${TAR_INFER}" --strip-components=1 --no-same-owner +fi diff --git a/ddtrace/internal/datadog/profiling/cmake/tools/infer_checksums.txt b/ddtrace/internal/datadog/profiling/cmake/tools/infer_checksums.txt new file mode 100644 index 00000000000..5d5d41c2c4a --- /dev/null +++ b/ddtrace/internal/datadog/profiling/cmake/tools/infer_checksums.txt @@ -0,0 +1 @@ +5f5d453814422e93e2a70998d8946b09a2721628ff427f67ff0123dea87461d4 infer-linux64-v1.1.0.tar.xz diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt index 9c2f58dc89b..e4c3de7bc0f 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt @@ -15,7 +15,9 @@ include(FetchContent) include(ExternalProject) include(FindLibdatadog) include(AnalysisFunc) +include(FindClangtidy) include(FindCppcheck) +include(FindInfer) # Library sources add_library(dd_wrapper SHARED @@ -59,6 +61,9 @@ add_cppcheck_target(dd_wrapper SRC ${CMAKE_CURRENT_SOURCE_DIR}/src ) +add_infer_target(dd_wrapper) +add_clangtidy_target(dd_wrapper) + # Add the tests if (BUILD_TESTING) enable_testing() diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/interface.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/interface.hpp index 0018a4b5e15..34a985d1a16 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/interface.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/interface.hpp @@ -27,7 +27,7 @@ extern "C" bool ddup_is_initialized(); void ddup_init(); - void ddup_set_runtime_id(std::string_view id); + void ddup_set_runtime_id(std::string_view runtime_id); bool ddup_upload(); // Proxy functions to the underlying sample @@ -36,8 +36,8 @@ extern "C" void ddup_push_cputime(Datadog::Sample* sample, int64_t cputime, int64_t count); void ddup_push_acquire(Datadog::Sample* sample, int64_t acquire_time, int64_t count); void ddup_push_release(Datadog::Sample* sample, int64_t release_time, int64_t count); - void ddup_push_alloc(Datadog::Sample* sample, uint64_t size, uint64_t count); - void ddup_push_heap(Datadog::Sample* sample, uint64_t size); + void ddup_push_alloc(Datadog::Sample* sample, int64_t size, int64_t count); + void ddup_push_heap(Datadog::Sample* sample, int64_t size); void ddup_push_lock_name(Datadog::Sample* sample, std::string_view lock_name); void ddup_push_threadinfo(Datadog::Sample* sample, int64_t thread_id, diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/libdatadog_helpers.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/libdatadog_helpers.hpp index bc22798f1b9..d1e9e6d36ab 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/libdatadog_helpers.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/libdatadog_helpers.hpp @@ -4,6 +4,7 @@ #include #include #include +#include extern "C" { @@ -126,6 +127,16 @@ add_tag(ddog_Vec_Tag& tags, const ExportTagKey key, std::string_view val, std::s return add_tag(tags, key_sv, val, errmsg); } +inline std::variant +get_newexporter_result(const ddog_prof_Exporter_NewResult& res) +{ + if (res.tag == DDOG_PROF_EXPORTER_NEW_RESULT_OK) { + return res.ok; // NOLINT (cppcoreguidelines-pro-type-union-access) + } else { + return res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + } +} + // Keep macros from propagating #undef X_STR #undef X_ENUM diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp index 8e0ab35c595..3af8251f5dc 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp @@ -70,7 +70,7 @@ class Profile void profile_release(); // String table manipulation - std::string_view insert_or_get(std::string_view sv); + std::string_view insert_or_get(std::string_view str); // constref getters const ValueIndex& val(); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp index c70b831847b..0a7b8750b6e 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp @@ -4,7 +4,6 @@ #include "profile.hpp" #include "types.hpp" -#include #include #include #include @@ -32,8 +31,7 @@ class Sample uint64_t samples = 0; // Storage for labels - std::array(ExportLabelKey::Length_)> labels{}; - size_t cur_label = 0; + std::vector labels{}; // Storage for values std::vector values = {}; @@ -50,8 +48,8 @@ class Sample bool push_cputime(int64_t cputime, int64_t count); bool push_acquire(int64_t acquire_time, int64_t count); bool push_release(int64_t lock_time, int64_t count); - bool push_alloc(uint64_t size, uint64_t count); - bool push_heap(uint64_t size); + bool push_alloc(int64_t size, int64_t count); + bool push_heap(int64_t size); // Adds metadata to sample bool push_lock_name(std::string_view lock_name); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample_manager.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample_manager.hpp index 28c1e81da7f..ec6e388b96e 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample_manager.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/sample_manager.hpp @@ -27,6 +27,7 @@ class SampleManager // Sampling entrypoint (this could also be called `build_ptr()`) static Sample* start_sample(); + static void drop_sample(Sample* sample); // Handles state management after forks static void postfork_child(); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp index be57ccaa335..c333e604de3 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp @@ -18,12 +18,17 @@ struct DdogProfExporterDeleter void operator()(ddog_prof_Exporter* ptr) const; }; +struct DdogCancellationTokenDeleter +{ + void operator()(ddog_CancellationToken* ptr) const; +}; + class Uploader { private: static inline std::mutex upload_lock{}; std::string errmsg; - static inline ddog_CancellationToken* cancel{ nullptr }; + static inline std::unique_ptr cancel; std::string runtime_id; std::string url; std::unique_ptr ddog_exporter; diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp index e1ad741abd4..f9f2e6ceba5 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp @@ -12,7 +12,7 @@ namespace Datadog { class UploaderBuilder { - using ExporterTagset = std::unordered_map; + using ExporterTagset = std::unordered_map; static inline std::mutex tag_mutex{}; // Building parameters @@ -31,7 +31,7 @@ class UploaderBuilder static constexpr std::string_view family{ "python" }; public: - static void set_env(std::string_view dd_env_); + static void set_env(std::string_view _dd_env); static void set_service(std::string_view _service); static void set_version(std::string_view _version); static void set_runtime(std::string_view _runtime); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/interface.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/interface.cpp index 21405dff2e8..c0ccae883ee 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/interface.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/interface.cpp @@ -11,7 +11,8 @@ #include // State -bool is_ddup_initialized = false; +bool is_ddup_initialized = false; // NOLINT (cppcoreguidelines-avoid-non-const-global-variables) +std::once_flag ddup_init_flag; // NOLINT (cppcoreguidelines-avoid-non-const-global-variables) // When a fork is detected, we need to reinitialize this state. // This handler will be called in the single thread of the child process after the fork @@ -40,81 +41,80 @@ ddup_prefork() // Configuration void -ddup_config_env(std::string_view dd_env) +ddup_config_env(std::string_view dd_env) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_env(dd_env); } void -ddup_config_service(std::string_view service) +ddup_config_service(std::string_view service) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_service(service); } void -ddup_config_version(std::string_view version) +ddup_config_version(std::string_view version) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_version(version); } void -ddup_config_runtime(std::string_view runtime) +ddup_config_runtime(std::string_view runtime) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_runtime(runtime); } void -ddup_config_runtime_version(std::string_view runtime_version) +ddup_config_runtime_version(std::string_view runtime_version) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_runtime_version(runtime_version); } void -ddup_config_profiler_version(std::string_view profiler_version) +ddup_config_profiler_version(std::string_view profiler_version) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_profiler_version(profiler_version); } void -ddup_config_url(std::string_view url) +ddup_config_url(std::string_view url) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_url(url); } void -ddup_config_user_tag(std::string_view key, std::string_view val) +ddup_config_user_tag(std::string_view key, std::string_view val) // cppcheck-suppress unusedFunction { Datadog::UploaderBuilder::set_tag(key, val); } void -ddup_set_runtime_id(std::string_view id) +ddup_set_runtime_id(std::string_view runtime_id) // cppcheck-suppress unusedFunction { - Datadog::UploaderBuilder::set_runtime_id(id); + Datadog::UploaderBuilder::set_runtime_id(runtime_id); } void -ddup_config_sample_type(unsigned int _type) +ddup_config_sample_type(unsigned int _type) // cppcheck-suppress unusedFunction { Datadog::SampleManager::add_type(_type); } void -ddup_config_max_nframes(int max_nframes) +ddup_config_max_nframes(int max_nframes) // cppcheck-suppress unusedFunction { Datadog::SampleManager::set_max_nframes(max_nframes); } bool -ddup_is_initialized() +ddup_is_initialized() // cppcheck-suppress unusedFunction { return is_ddup_initialized; } -std::atomic initialized_count{ 0 }; void -ddup_init() +ddup_init() // cppcheck-suppress unusedFunction { - const static bool initialized = []() { + std::call_once(ddup_init_flag, []() { // Perform any one-time startup operations Datadog::SampleManager::init(); @@ -125,119 +125,120 @@ ddup_init() // Set the global initialization flag is_ddup_initialized = true; return true; - }(); - - initialized_count.fetch_add(static_cast(initialized)); - if (initialized_count > 1) { - std::cerr << "ddup_init() called " << initialized_count << " times" << std::endl; - } + }); } Datadog::Sample* -ddup_start_sample() +ddup_start_sample() // cppcheck-suppress unusedFunction { return Datadog::SampleManager::start_sample(); } void -ddup_push_walltime(Datadog::Sample* sample, int64_t walltime, int64_t count) +ddup_push_walltime(Datadog::Sample* sample, int64_t walltime, int64_t count) // cppcheck-suppress unusedFunction { sample->push_walltime(walltime, count); } void -ddup_push_cputime(Datadog::Sample* sample, int64_t cputime, int64_t count) +ddup_push_cputime(Datadog::Sample* sample, int64_t cputime, int64_t count) // cppcheck-suppress unusedFunction { sample->push_cputime(cputime, count); } void -ddup_push_acquire(Datadog::Sample* sample, int64_t acquire_time, int64_t count) +ddup_push_acquire(Datadog::Sample* sample, int64_t acquire_time, int64_t count) // cppcheck-suppress unusedFunction { sample->push_acquire(acquire_time, count); } void -ddup_push_release(Datadog::Sample* sample, int64_t release_time, int64_t count) +ddup_push_release(Datadog::Sample* sample, int64_t release_time, int64_t count) // cppcheck-suppress unusedFunction { sample->push_release(release_time, count); } void -ddup_push_alloc(Datadog::Sample* sample, uint64_t size, uint64_t count) +ddup_push_alloc(Datadog::Sample* sample, int64_t size, int64_t count) // cppcheck-suppress unusedFunction { sample->push_alloc(size, count); } void -ddup_push_heap(Datadog::Sample* sample, uint64_t size) +ddup_push_heap(Datadog::Sample* sample, int64_t size) // cppcheck-suppress unusedFunction { sample->push_heap(size); } void -ddup_push_lock_name(Datadog::Sample* sample, std::string_view lock_name) +ddup_push_lock_name(Datadog::Sample* sample, std::string_view lock_name) // cppcheck-suppress unusedFunction { sample->push_lock_name(lock_name); } void -ddup_push_threadinfo(Datadog::Sample* sample, int64_t thread_id, int64_t thread_native_id, std::string_view thread_name) +ddup_push_threadinfo(Datadog::Sample* sample, + int64_t thread_id, + int64_t thread_native_id, + std::string_view thread_name) // cppcheck-suppress unusedFunction { sample->push_threadinfo(thread_id, thread_native_id, thread_name); } void -ddup_push_task_id(Datadog::Sample* sample, int64_t task_id) +ddup_push_task_id(Datadog::Sample* sample, int64_t task_id) // cppcheck-suppress unusedFunction { sample->push_task_id(task_id); } void -ddup_push_task_name(Datadog::Sample* sample, std::string_view task_name) +ddup_push_task_name(Datadog::Sample* sample, std::string_view task_name) // cppcheck-suppress unusedFunction { sample->push_task_name(task_name); } void -ddup_push_span_id(Datadog::Sample* sample, int64_t span_id) +ddup_push_span_id(Datadog::Sample* sample, int64_t span_id) // cppcheck-suppress unusedFunction { sample->push_span_id(span_id); } void -ddup_push_local_root_span_id(Datadog::Sample* sample, int64_t local_root_span_id) +ddup_push_local_root_span_id(Datadog::Sample* sample, int64_t local_root_span_id) // cppcheck-suppress unusedFunction { sample->push_local_root_span_id(local_root_span_id); } void -ddup_push_trace_type(Datadog::Sample* sample, std::string_view trace_type) +ddup_push_trace_type(Datadog::Sample* sample, std::string_view trace_type) // cppcheck-suppress unusedFunction { sample->push_trace_type(trace_type); } void -ddup_push_trace_resource_container(Datadog::Sample* sample, std::string_view trace_resource_container) +ddup_push_trace_resource_container(Datadog::Sample* sample, + std::string_view trace_resource_container) // cppcheck-suppress unusedFunction { sample->push_trace_resource_container(trace_resource_container); } void -ddup_push_exceptioninfo(Datadog::Sample* sample, std::string_view exception_type, int64_t count) +ddup_push_exceptioninfo(Datadog::Sample* sample, + std::string_view exception_type, + int64_t count) // cppcheck-suppress unusedFunction { sample->push_exceptioninfo(exception_type, count); } void -ddup_push_class_name(Datadog::Sample* sample, std::string_view class_name) +ddup_push_class_name(Datadog::Sample* sample, std::string_view class_name) // cppcheck-suppress unusedFunction { sample->push_class_name(class_name); } void -ddup_push_frame(Datadog::Sample* sample, +ddup_push_frame(Datadog::Sample* sample, // cppcheck-suppress unusedFunction std::string_view _name, std::string_view _filename, uint64_t address, @@ -247,20 +248,19 @@ ddup_push_frame(Datadog::Sample* sample, } void -ddup_flush_sample(Datadog::Sample* sample) +ddup_flush_sample(Datadog::Sample* sample) // cppcheck-suppress unusedFunction { sample->flush_sample(); } void -ddup_drop_sample(Datadog::Sample* sample) +ddup_drop_sample(Datadog::Sample* sample) // cppcheck-suppress unusedFunction { - // After a sample is dropped, the user should no longer use it - delete sample; + Datadog::SampleManager::drop_sample(sample); } bool -ddup_upload() +ddup_upload() // cppcheck-suppress unusedFunction { if (!is_ddup_initialized) { std::cerr << "ddup_upload() called before ddup_init()" << std::endl; @@ -283,7 +283,7 @@ ddup_upload() Datadog::Sample::profile_release(); Datadog::Sample::profile_clear_state(); } - void operator()(std::string& err) { std::cerr << "Failed to create uploader: " << err << std::endl; } + void operator()(const std::string& err) { std::cerr << "Failed to create uploader: " << err << std::endl; } } visitor; std::visit(visitor, uploader); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp index a62cd812e41..086e90ee495 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp @@ -19,22 +19,21 @@ make_profile(const ddog_prof_Slice_ValueType& sample_types, { // Private helper function for creating a ddog_prof_Profile from arguments ddog_prof_Profile_NewResult res = ddog_prof_Profile_new(sample_types, period, nullptr); - if (res.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { - const std::string errmsg = Datadog::err_to_msg(&res.err, "Error initializing profile"); + if (res.tag != DDOG_PROF_PROFILE_NEW_RESULT_OK) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + const std::string errmsg = Datadog::err_to_msg(&err, "Error initializing profile"); std::cerr << errmsg << std::endl; - ddog_Error_drop(&res.err); + ddog_Error_drop(&err); return false; } - profile = res.ok; + profile = res.ok; // NOLINT (cppcoreguidelines-pro-type-union-access) return true; } } -using namespace Datadog; - bool -Profile::cycle_buffers() +Datadog::Profile::cycle_buffers() { const std::lock_guard lock(profile_mtx); @@ -42,17 +41,18 @@ Profile::cycle_buffers() // Clear the profile before using it auto res = ddog_prof_Profile_reset(&cur_profile, nullptr); - if (!res.ok) { - const std::string errmsg = err_to_msg(&res.err, "Error resetting profile"); + if (!res.ok) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + const std::string errmsg = err_to_msg(&err, "Error resetting profile"); std::cout << "Could not drop profile:" << errmsg << std::endl; - ddog_Error_drop(&res.err); + ddog_Error_drop(&err); return false; } return true; } void -Profile::setup_samplers() +Datadog::Profile::setup_samplers() { // TODO propagate error if no valid samplers are defined samplers.clear(); @@ -63,30 +63,30 @@ Profile::setup_samplers() }; // Check which samplers were enabled by the user - if (type_mask & SampleType::CPU) { + if (0U != (type_mask & SampleType::CPU)) { val_idx.cpu_time = get_value_idx("cpu-time", "nanoseconds"); val_idx.cpu_count = get_value_idx("cpu-samples", "count"); } - if (type_mask & SampleType::Wall) { + if (0U != (type_mask & SampleType::Wall)) { val_idx.wall_time = get_value_idx("wall-time", "nanoseconds"); val_idx.wall_count = get_value_idx("wall-samples", "count"); } - if (type_mask & SampleType::Exception) { + if (0U != (type_mask & SampleType::Exception)) { val_idx.exception_count = get_value_idx("exception-samples", "count"); } - if (type_mask & SampleType::LockAcquire) { + if (0U != (type_mask & SampleType::LockAcquire)) { val_idx.lock_acquire_time = get_value_idx("lock-acquire-wait", "nanoseconds"); val_idx.lock_acquire_count = get_value_idx("lock-acquire", "count"); } - if (type_mask & SampleType::LockRelease) { + if (0U != (type_mask & SampleType::LockRelease)) { val_idx.lock_release_time = get_value_idx("lock-release-hold", "nanoseconds"); val_idx.lock_release_count = get_value_idx("lock-release", "count"); } - if (type_mask & SampleType::Allocation) { + if (0U != (type_mask & SampleType::Allocation)) { val_idx.alloc_space = get_value_idx("alloc-space", "bytes"); val_idx.alloc_count = get_value_idx("alloc-samples", "count"); } - if (type_mask & SampleType::Heap) { + if (0U != (type_mask & SampleType::Heap)) { val_idx.heap_space = get_value_idx("heap-space", "bytes"); } @@ -98,13 +98,13 @@ Profile::setup_samplers() } size_t -Profile::get_sample_type_length() +Datadog::Profile::get_sample_type_length() { return samplers.size(); } ddog_prof_Profile& -Profile::profile_borrow() +Datadog::Profile::profile_borrow() { // We could wrap this in an object for better RAII, but since this // sequence is only used in a single place, we'll hold off on that sidequest. @@ -113,13 +113,13 @@ Profile::profile_borrow() } void -Profile::profile_release() +Datadog::Profile::profile_release() { profile_mtx.unlock(); } void -Profile::one_time_init(SampleType type, unsigned int _max_nframes) +Datadog::Profile::one_time_init(SampleType type, unsigned int _max_nframes) { // In contemporary dd-trace-py, it is expected that the initialization path is in // a single thread, and done only once. @@ -163,43 +163,44 @@ Profile::one_time_init(SampleType type, unsigned int _max_nframes) } std::string_view -Profile::insert_or_get(std::string_view sv) +Datadog::Profile::insert_or_get(std::string_view str) { const std::lock_guard lock(string_table_mtx); // Serialize access - auto it = strings.find(sv); - if (it != strings.end()) { - return *it; + auto str_it = strings.find(str); + if (str_it != strings.end()) { + return *str_it; } - string_storage.emplace_back(sv); + string_storage.emplace_back(str); strings.insert(string_storage.back()); return string_storage.back(); } -const ValueIndex& -Profile::val() +const Datadog::ValueIndex& +Datadog::Profile::val() { return val_idx; } bool -Profile::collect(const ddog_prof_Sample& sample) +Datadog::Profile::collect(const ddog_prof_Sample& sample) { // TODO this should propagate some kind of timestamp for timeline support const std::lock_guard lock(profile_mtx); auto res = ddog_prof_Profile_add(&cur_profile, sample, 0); - if (!res.ok) { - const std::string errmsg = err_to_msg(&res.err, "Error adding sample to profile"); + if (!res.ok) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + const std::string errmsg = err_to_msg(&err, "Error adding sample to profile"); std::cerr << errmsg << std::endl; - ddog_Error_drop(&res.err); + ddog_Error_drop(&err); return false; } return true; } void -Profile::postfork_child() +Datadog::Profile::postfork_child() { profile_mtx.unlock(); cycle_buffers(); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp index 40f5792696b..d2a28562505 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample.cpp @@ -2,9 +2,7 @@ #include -using namespace Datadog; - -Sample::Sample(SampleType _type_mask, unsigned int _max_nframes) +Datadog::Sample::Sample(SampleType _type_mask, unsigned int _max_nframes) : max_nframes{ _max_nframes } , type_mask{ _type_mask } { @@ -17,13 +15,13 @@ Sample::Sample(SampleType _type_mask, unsigned int _max_nframes) } void -Sample::profile_clear_state() +Datadog::Sample::profile_clear_state() { profile_state.cycle_buffers(); } void -Sample::push_frame_impl(std::string_view name, std::string_view filename, uint64_t address, int64_t line) +Datadog::Sample::push_frame_impl(std::string_view name, std::string_view filename, uint64_t address, int64_t line) { static const ddog_prof_Mapping null_mapping = { 0, 0, 0, to_slice(""), to_slice("") }; name = profile_state.insert_or_get(name); @@ -45,7 +43,7 @@ Sample::push_frame_impl(std::string_view name, std::string_view filename, uint64 } void -Sample::push_frame(std::string_view name, std::string_view filename, uint64_t address, int64_t line) +Datadog::Sample::push_frame(std::string_view name, std::string_view filename, uint64_t address, int64_t line) { if (locations.size() <= max_nframes) { @@ -56,12 +54,8 @@ Sample::push_frame(std::string_view name, std::string_view filename, uint64_t ad } bool -Sample::push_label(const ExportLabelKey key, std::string_view val) +Datadog::Sample::push_label(const ExportLabelKey key, std::string_view val) { - if (cur_label >= labels.size()) { - return false; - } - // Get the sv for the key const std::string_view key_sv = to_string(key); @@ -74,20 +68,15 @@ Sample::push_label(const ExportLabelKey key, std::string_view val) // Otherwise, persist the val string and add the label val = profile_state.insert_or_get(val); - labels[cur_label].key = to_slice(key_sv); - labels[cur_label].str = to_slice(val); - cur_label++; + auto& label = labels.emplace_back(); + label.key = to_slice(key_sv); + label.str = to_slice(val); return true; } bool -Sample::push_label(const ExportLabelKey key, int64_t val) +Datadog::Sample::push_label(const ExportLabelKey key, int64_t val) { - if (cur_label >= labels.size()) { - std::cout << "Bad push_label (num)" << std::endl; - return false; - } - // Get the sv for the key. If there is no key, then there // is no label. Right now this is OK. // TODO make this not OK @@ -96,26 +85,25 @@ Sample::push_label(const ExportLabelKey key, int64_t val) return true; } - labels[cur_label].key = to_slice(key_sv); - labels[cur_label].str = to_slice(""); - labels[cur_label].num = val; - labels[cur_label].num_unit = to_slice(""); - cur_label++; + auto& label = labels.emplace_back(); + label.key = to_slice(key_sv); + label.str = to_slice(""); + label.num = val; + label.num_unit = to_slice(""); return true; } void -Sample::clear_buffers() +Datadog::Sample::clear_buffers() { std::fill(values.begin(), values.end(), 0); - std::fill(std::begin(labels), std::end(labels), ddog_prof_Label{}); + labels.clear(); locations.clear(); - cur_label = 0; dropped_frames = 0; } bool -Sample::flush_sample() +Datadog::Sample::flush_sample() { if (dropped_frames > 0) { const std::string name = @@ -126,7 +114,7 @@ Sample::flush_sample() const ddog_prof_Sample sample = { .locations = { locations.data(), locations.size() }, .values = { values.data(), values.size() }, - .labels = { labels.data(), cur_label }, + .labels = { labels.data(), labels.size() }, }; const bool ret = profile_state.collect(sample); @@ -135,11 +123,11 @@ Sample::flush_sample() } bool -Sample::push_cputime(int64_t cputime, int64_t count) +Datadog::Sample::push_cputime(int64_t cputime, int64_t count) { // NB all push-type operations return bool for semantic uniformity, // even if they can't error. This should promote generic code. - if (type_mask & SampleType::CPU) { + if (0U != (type_mask & SampleType::CPU)) { values[profile_state.val().cpu_time] += cputime * count; values[profile_state.val().cpu_count] += count; return true; @@ -149,9 +137,9 @@ Sample::push_cputime(int64_t cputime, int64_t count) } bool -Sample::push_walltime(int64_t walltime, int64_t count) +Datadog::Sample::push_walltime(int64_t walltime, int64_t count) { - if (type_mask & SampleType::Wall) { + if (0U != (type_mask & SampleType::Wall)) { values[profile_state.val().wall_time] += walltime * count; values[profile_state.val().wall_count] += count; return true; @@ -161,9 +149,9 @@ Sample::push_walltime(int64_t walltime, int64_t count) } bool -Sample::push_exceptioninfo(std::string_view exception_type, int64_t count) +Datadog::Sample::push_exceptioninfo(std::string_view exception_type, int64_t count) { - if (type_mask & SampleType::Exception) { + if (0U != (type_mask & SampleType::Exception)) { push_label(ExportLabelKey::exception_type, exception_type); values[profile_state.val().exception_count] += count; return true; @@ -173,9 +161,9 @@ Sample::push_exceptioninfo(std::string_view exception_type, int64_t count) } bool -Sample::push_acquire(int64_t acquire_time, int64_t count) +Datadog::Sample::push_acquire(int64_t acquire_time, int64_t count) // NOLINT (bugprone-easily-swappable-parameters) { - if (type_mask & SampleType::LockAcquire) { + if (0U != (type_mask & SampleType::LockAcquire)) { values[profile_state.val().lock_acquire_time] += acquire_time; values[profile_state.val().lock_acquire_count] += count; return true; @@ -185,9 +173,9 @@ Sample::push_acquire(int64_t acquire_time, int64_t count) } bool -Sample::push_release(int64_t lock_time, int64_t count) +Datadog::Sample::push_release(int64_t lock_time, int64_t count) // NOLINT (bugprone-easily-swappable-parameters) { - if (type_mask & SampleType::LockRelease) { + if (0U != (type_mask & SampleType::LockRelease)) { values[profile_state.val().lock_release_time] += lock_time; values[profile_state.val().lock_release_count] += count; return true; @@ -197,9 +185,14 @@ Sample::push_release(int64_t lock_time, int64_t count) } bool -Sample::push_alloc(uint64_t size, uint64_t count) +Datadog::Sample::push_alloc(int64_t size, int64_t count) // NOLINT (bugprone-easily-swappable-parameters) { - if (type_mask & SampleType::Allocation) { + if (size < 0 || count < 0) { + std::cout << "bad push alloc (params)" << std::endl; + return false; + } + + if (0U != (type_mask & SampleType::Allocation)) { values[profile_state.val().alloc_space] += size; values[profile_state.val().alloc_count] += count; return true; @@ -209,9 +202,14 @@ Sample::push_alloc(uint64_t size, uint64_t count) } bool -Sample::push_heap(uint64_t size) +Datadog::Sample::push_heap(int64_t size) { - if (type_mask & SampleType::Heap) { + if (size < 0) { + std::cout << "bad push heap (params)" << std::endl; + return false; + } + + if (0U != (type_mask & SampleType::Heap)) { values[profile_state.val().heap_space] += size; return true; } @@ -220,14 +218,14 @@ Sample::push_heap(uint64_t size) } bool -Sample::push_lock_name(std::string_view lock_name) +Datadog::Sample::push_lock_name(std::string_view lock_name) { push_label(ExportLabelKey::lock_name, lock_name); return true; } bool -Sample::push_threadinfo(int64_t thread_id, int64_t thread_native_id, std::string_view thread_name) +Datadog::Sample::push_threadinfo(int64_t thread_id, int64_t thread_native_id, std::string_view thread_name) { std::string temp_string; if (thread_name.empty()) { @@ -244,7 +242,7 @@ Sample::push_threadinfo(int64_t thread_id, int64_t thread_native_id, std::string } bool -Sample::push_task_id(int64_t task_id) +Datadog::Sample::push_task_id(int64_t task_id) { if (!push_label(ExportLabelKey::task_id, task_id)) { std::cout << "bad push" << std::endl; @@ -253,7 +251,7 @@ Sample::push_task_id(int64_t task_id) return true; } bool -Sample::push_task_name(std::string_view task_name) +Datadog::Sample::push_task_name(std::string_view task_name) { if (!push_label(ExportLabelKey::task_name, task_name)) { std::cout << "bad push" << std::endl; @@ -263,9 +261,12 @@ Sample::push_task_name(std::string_view task_name) } bool -Sample::push_span_id(uint64_t span_id) +Datadog::Sample::push_span_id(uint64_t span_id) { - const int64_t recoded_id = reinterpret_cast(span_id); + // The pprof container expects a signed 64-bit integer for numeric labels, whereas + // the emitted ID is unsigned (full 64-bit range). We type-pun to int64_t here. + const int64_t recoded_id = + reinterpret_cast(span_id); // NOLINT (cppcoreguidelines-pro-type-reinterpret-cast) if (!push_label(ExportLabelKey::span_id, recoded_id)) { std::cout << "bad push" << std::endl; return false; @@ -274,9 +275,10 @@ Sample::push_span_id(uint64_t span_id) } bool -Sample::push_local_root_span_id(uint64_t local_root_span_id) +Datadog::Sample::push_local_root_span_id(uint64_t local_root_span_id) { - const int64_t recoded_id = reinterpret_cast(local_root_span_id); + const int64_t recoded_id = + reinterpret_cast(local_root_span_id); // NOLINT (cppcoreguidelines-pro-type-reinterpret-cast) if (!push_label(ExportLabelKey::local_root_span_id, recoded_id)) { std::cout << "bad push" << std::endl; return false; @@ -285,7 +287,7 @@ Sample::push_local_root_span_id(uint64_t local_root_span_id) } bool -Sample::push_trace_type(std::string_view trace_type) +Datadog::Sample::push_trace_type(std::string_view trace_type) { if (!push_label(ExportLabelKey::trace_type, trace_type)) { std::cout << "bad push" << std::endl; @@ -295,7 +297,7 @@ Sample::push_trace_type(std::string_view trace_type) } bool -Sample::push_trace_resource_container(std::string_view trace_resource_container) +Datadog::Sample::push_trace_resource_container(std::string_view trace_resource_container) { if (!push_label(ExportLabelKey::trace_resource_container, trace_resource_container)) { std::cout << "bad push" << std::endl; @@ -305,7 +307,7 @@ Sample::push_trace_resource_container(std::string_view trace_resource_container) } bool -Sample::push_class_name(std::string_view class_name) +Datadog::Sample::push_class_name(std::string_view class_name) { if (!push_label(ExportLabelKey::class_name, class_name)) { std::cout << "bad push" << std::endl; @@ -315,19 +317,19 @@ Sample::push_class_name(std::string_view class_name) } ddog_prof_Profile& -Sample::profile_borrow() +Datadog::Sample::profile_borrow() { return profile_state.profile_borrow(); } void -Sample::profile_release() +Datadog::Sample::profile_release() { profile_state.profile_release(); } void -Sample::postfork_child() +Datadog::Sample::postfork_child() { profile_state.postfork_child(); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample_manager.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample_manager.cpp index 87fbf25d8e1..72137b9a5d0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample_manager.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/sample_manager.cpp @@ -1,16 +1,14 @@ #include "sample_manager.hpp" #include "types.hpp" -using namespace Datadog; - void -SampleManager::add_type(unsigned int type) +Datadog::SampleManager::add_type(unsigned int type) { type_mask = static_cast((type_mask | type) & SampleType::All); } void -SampleManager::set_max_nframes(unsigned int _max_nframes) +Datadog::SampleManager::set_max_nframes(unsigned int _max_nframes) { if (_max_nframes > 0) { max_nframes = _max_nframes; @@ -18,26 +16,31 @@ SampleManager::set_max_nframes(unsigned int _max_nframes) // If the user has requested more than we're allowed to give, reduce the limit and warn the user. if (max_nframes > g_backend_max_nframes) { - std::cerr << "Requested limit of " << max_nframes << " will be reduced to " << g_backend_max_nframes - << std::endl; + // We don't emit an error here for now. max_nframes = g_backend_max_nframes; } } -Sample* -SampleManager::start_sample() +Datadog::Sample* +Datadog::SampleManager::start_sample() +{ + return new Datadog::Sample(type_mask, max_nframes); // NOLINT(cppcoreguidelines-owning-memory) +} + +void +Datadog::SampleManager::drop_sample(Datadog::Sample* sample) { - return new Sample(type_mask, max_nframes); + delete sample; // NOLINT(cppcoreguidelines-owning-memory) } void -SampleManager::postfork_child() +Datadog::SampleManager::postfork_child() { - Sample::postfork_child(); + Datadog::Sample::postfork_child(); } void -SampleManager::init() +Datadog::SampleManager::init() { - Sample::profile_state.one_time_init(type_mask, max_nframes); + Datadog::Sample::profile_state.one_time_init(type_mask, max_nframes); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp index ef3b2a2a926..46e7e21b9f0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp @@ -6,26 +6,38 @@ using namespace Datadog; void DdogProfExporterDeleter::operator()(ddog_prof_Exporter* ptr) const { + // According to the rust docs, the `cancel()` call is synchronous + // https://docs.rs/tokio-util/latest/tokio_util/sync/struct.CancellationToken.html#method.cancel ddog_prof_Exporter_drop(ptr); } -Uploader::Uploader(std::string_view _url, ddog_prof_Exporter* _ddog_exporter) +void +DdogCancellationTokenDeleter::operator()(ddog_CancellationToken* ptr) const +{ + if (ptr != nullptr) { + ddog_CancellationToken_cancel(ptr); + ddog_CancellationToken_drop(ptr); + } +} + +Datadog::Uploader::Uploader(std::string_view _url, ddog_prof_Exporter* _ddog_exporter) : url{ _url } , ddog_exporter{ _ddog_exporter } {} bool -Uploader::upload(ddog_prof_Profile& profile) +Datadog::Uploader::upload(ddog_prof_Profile& profile) { // Serialize the profile ddog_prof_Profile_SerializeResult result = ddog_prof_Profile_serialize(&profile, nullptr, nullptr, nullptr); - if (result.tag != DDOG_PROF_PROFILE_SERIALIZE_RESULT_OK) { - errmsg = err_to_msg(&result.err, "Error serializing pprof"); + if (result.tag != DDOG_PROF_PROFILE_SERIALIZE_RESULT_OK) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = result.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + errmsg = err_to_msg(&err, "Error serializing pprof"); std::cerr << errmsg << std::endl; - ddog_Error_drop(&result.err); + ddog_Error_drop(&err); return false; } - ddog_prof_EncodedProfile* encoded = &result.ok; + ddog_prof_EncodedProfile* encoded = &result.ok; // NOLINT (cppcoreguidelines-pro-type-union-access) // If we have any custom tags, set them now ddog_Vec_Tag tags = ddog_Vec_Tag_new(); @@ -48,10 +60,12 @@ Uploader::upload(ddog_prof_Profile& profile) max_timeout_ms); ddog_prof_EncodedProfile_drop(encoded); - if (build_res.tag == DDOG_PROF_EXPORTER_REQUEST_BUILD_RESULT_ERR) { - errmsg = err_to_msg(&build_res.err, "Error building request"); + if (build_res.tag == + DDOG_PROF_EXPORTER_REQUEST_BUILD_RESULT_ERR) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = build_res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + errmsg = err_to_msg(&err, "Error building request"); std::cerr << errmsg << std::endl; - ddog_Error_drop(&build_res.err); + ddog_Error_drop(&err); ddog_Vec_Tag_drop(tags); return false; } @@ -61,9 +75,11 @@ Uploader::upload(ddog_prof_Profile& profile) // Create a new cancellation token. Maybe we can get away without doing this, but // since we're recreating the uploader fresh every time anyway, we recreate one more thing. - // NB `cancel_inflight()` already drops the old token. - cancel = ddog_CancellationToken_new(); - auto* cancel_for_request = ddog_CancellationToken_clone(cancel); + // NB wrapping this in a unique_ptr to easily add RAII semantics; maybe should just wrap it in a + // class instead + cancel.reset(ddog_CancellationToken_new()); + std::unique_ptr cancel_for_request; + cancel_for_request.reset(ddog_CancellationToken_clone(cancel.get())); // The upload operation sets up some global state in libdatadog (the tokio runtime), so // we ensure exclusivity here. @@ -71,12 +87,14 @@ Uploader::upload(ddog_prof_Profile& profile) const std::lock_guard lock_guard(upload_lock); // Build and check the response object - ddog_prof_Exporter_Request* req = build_res.ok; - ddog_prof_Exporter_SendResult res = ddog_prof_Exporter_send(ddog_exporter.get(), &req, cancel_for_request); - if (res.tag == DDOG_PROF_EXPORTER_SEND_RESULT_ERR) { - errmsg = err_to_msg(&res.err, "Error uploading"); + ddog_prof_Exporter_Request* req = build_res.ok; // NOLINT (cppcoreguidelines-pro-type-union-access) + ddog_prof_Exporter_SendResult res = + ddog_prof_Exporter_send(ddog_exporter.get(), &req, cancel_for_request.get()); + if (res.tag == DDOG_PROF_EXPORTER_SEND_RESULT_ERR) { // NOLINT (cppcoreguidelines-pro-type-union-access) + auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access) + errmsg = err_to_msg(&err, "Error uploading"); std::cerr << errmsg << std::endl; - ddog_Error_drop(&res.err); + ddog_Error_drop(&err); ddog_Vec_Tag_drop(tags); return false; } @@ -85,50 +103,42 @@ Uploader::upload(ddog_prof_Profile& profile) // Cleanup ddog_Vec_Tag_drop(tags); - ddog_CancellationToken_drop(cancel_for_request); - return true; } void -Uploader::lock() +Datadog::Uploader::lock() { upload_lock.lock(); } void -Uploader::unlock() +Datadog::Uploader::unlock() { upload_lock.unlock(); } void -Uploader::cancel_inflight() +Datadog::Uploader::cancel_inflight() { - // According to the rust docs, the `cancel()` call is synchronous - // https://docs.rs/tokio-util/latest/tokio_util/sync/struct.CancellationToken.html#method.cancel - if (cancel) { - ddog_CancellationToken_cancel(cancel); - ddog_CancellationToken_drop(cancel); - } - cancel = nullptr; + cancel.reset(); } void -Uploader::prefork() +Datadog::Uploader::prefork() { lock(); cancel_inflight(); } void -Uploader::postfork_parent() +Datadog::Uploader::postfork_parent() { unlock(); } void -Uploader::postfork_child() +Datadog::Uploader::postfork_child() { unlock(); } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp index 435ce37d1b4..af2fc8a33a4 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp @@ -1,77 +1,74 @@ #include "uploader_builder.hpp" #include "libdatadog_helpers.hpp" -#include "types.hpp" -#include #include #include +#include #include #include #include -using namespace Datadog; - void -UploaderBuilder::set_env(std::string_view _dd_env) +Datadog::UploaderBuilder::set_env(std::string_view _dd_env) { if (!_dd_env.empty()) { dd_env = _dd_env; } } void -UploaderBuilder::set_service(std::string_view _service) +Datadog::UploaderBuilder::set_service(std::string_view _service) { if (!_service.empty()) { service = _service; } } void -UploaderBuilder::set_version(std::string_view _version) +Datadog::UploaderBuilder::set_version(std::string_view _version) { if (!_version.empty()) { version = _version; } } void -UploaderBuilder::set_runtime(std::string_view _runtime) +Datadog::UploaderBuilder::set_runtime(std::string_view _runtime) { if (!_runtime.empty()) { runtime = _runtime; } } void -UploaderBuilder::set_runtime_version(std::string_view _runtime_version) +Datadog::UploaderBuilder::set_runtime_version(std::string_view _runtime_version) { if (!_runtime_version.empty()) { runtime_version = _runtime_version; } } void -UploaderBuilder::set_profiler_version(std::string_view _profiler_version) +Datadog::UploaderBuilder::set_profiler_version(std::string_view _profiler_version) { if (!_profiler_version.empty()) { profiler_version = _profiler_version; } } void -UploaderBuilder::set_url(std::string_view _url) +Datadog::UploaderBuilder::set_url(std::string_view _url) { if (!_url.empty()) { url = _url; } } void -UploaderBuilder::set_tag(std::string_view _key, std::string_view _val) +Datadog::UploaderBuilder::set_tag(std::string_view _key, std::string_view _val) { if (!_key.empty() && !_val.empty()) { const std::lock_guard lock(tag_mutex); - user_tags[_key] = _val; + user_tags[std::string(_key)] = std::string(_val); } } void -UploaderBuilder::set_runtime_id(std::string_view _runtime_id) +Datadog::UploaderBuilder::set_runtime_id(std::string_view _runtime_id) { if (!_runtime_id.empty()) { runtime_id = _runtime_id; @@ -81,20 +78,23 @@ UploaderBuilder::set_runtime_id(std::string_view _runtime_id) std::string join(const std::vector& vec, const std::string& delim) { - return std::accumulate( - vec.begin(), vec.end(), std::string(), [&delim](const std::string& a, const std::string& b) -> std::string { - // if a and b are empty, it doesn't matter what we return, as long as there's no delimiter - if (a.empty()) { - return b; - } else if (b.empty()) { - return a; - } - return a + delim + b; - }); + return std::accumulate(vec.begin(), + vec.end(), + std::string(), + [&delim](const std::string& left, const std::string& right) -> std::string { + // If the left and right operands are empty, we don't want to add a delimiter + if (left.empty()) { + return right; + } + if (right.empty()) { + return left; + } + return left + delim + right; + }); } -std::variant -UploaderBuilder::build() +std::variant +Datadog::UploaderBuilder::build() { // Setup the ddog_Exporter ddog_Vec_Tag tags = ddog_Vec_Tag_new(); @@ -140,14 +140,16 @@ UploaderBuilder::build() to_slice("dd-trace-py"), to_slice(profiler_version), to_slice(family), &tags, ddog_Endpoint_agent(to_slice(url))); ddog_Vec_Tag_drop(tags); + auto ddog_exporter_result = Datadog::get_newexporter_result(res); ddog_prof_Exporter* ddog_exporter = nullptr; - if (res.tag == DDOG_PROF_EXPORTER_NEW_RESULT_OK) { - ddog_exporter = res.ok; + if (std::holds_alternative(ddog_exporter_result)) { + ddog_exporter = std::get(ddog_exporter_result); } else { - std::string errmsg = err_to_msg(&res.err, "Error initializing exporter"); - ddog_Error_drop(&res.err); // errmsg contains a copy of res.err, OK to drop + auto& err = std::get(ddog_exporter_result); + std::string errmsg = Datadog::err_to_msg(&err, "Error initializing exporter"); + ddog_Error_drop(&err); // errmsg contains a copy of err.message return errmsg; } - return Uploader{ url, ddog_exporter }; + return Datadog::Uploader{ url, ddog_exporter }; } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt index 6163229a5df..8b285b9ceca 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/CMakeLists.txt @@ -19,7 +19,7 @@ function(dd_wrapper_add_test name) gtest_main dd_wrapper ) - add_ddup_config(dd_wrapper) + add_ddup_config(${name}) gtest_discover_tests(${name}) endfunction() diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/api.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/test/api.cpp index e6921e60e21..816fb4c0c74 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/api.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/api.cpp @@ -16,6 +16,8 @@ single_sample_noframe() auto h = ddup_start_sample(); ddup_push_walltime(h, 1.0, 1); ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; // Upload. It'll fail, but whatever ddup_upload(); @@ -38,6 +40,8 @@ single_oneframe_sample() ddup_push_walltime(h, 1.0, 1); ddup_push_frame(h, "my_test_frame", "my_test_file", 1, 1); ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; // Upload. It'll fail, but whatever ddup_upload(); @@ -68,6 +72,8 @@ single_manyframes_sample() ddup_push_frame(h, name.c_str(), file.c_str(), 1, 1); } ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; // Upload. It'll fail, but whatever ddup_upload(); @@ -98,6 +104,8 @@ single_toomanyframes_sample() ddup_push_frame(h, name.c_str(), file.c_str(), 1, 1); } ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; // Upload. It'll fail, but whatever ddup_upload(); @@ -139,6 +147,8 @@ lotsa_frames_lotsa_samples() ddup_push_frame(h, name.c_str(), file.c_str(), 1, 1); } ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; } // Upload. It'll fail, but whatever diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/initialization.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/test/initialization.cpp index edab82b729f..6b5a6aabec7 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/initialization.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/initialization.cpp @@ -30,18 +30,6 @@ TEST(DD_WrapperTest, TestInitWithEmpty) EXPECT_EXIT(empty_init(), ::testing::ExitedWithCode(0), ""); } -void -null_init() -{ - configure(nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, 0); - std::exit(0); -} - -TEST(DD_WrapperTest, TestInitWithNull) -{ - EXPECT_EXIT(null_init(), ::testing::ExitedWithCode(0), ""); -} - void minimal_init() { diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/test/test_utils.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/test/test_utils.hpp index fae6e8864d3..e5e28459428 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/test/test_utils.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/test/test_utils.hpp @@ -122,6 +122,8 @@ send_sample(unsigned int id) ddup_push_frame(h, file.c_str(), func.c_str(), i, 0); } ddup_flush_sample(h); + ddup_drop_sample(h); + h = nullptr; } // This emulates a single sampler, which periodically (100hz) sends a sample diff --git a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt index 9a59d9344a0..b6fc8c6ff86 100644 --- a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt @@ -5,7 +5,7 @@ cmake_minimum_required(VERSION 3.19) # setup.py to build the wheel. Otherwise, we have to propagate a lot of state all around. # Thus, we use the same name as the Python package (i.e., the caller sets EXTENSION_NAME) # ddup is used by a default for standalone/test builds. -set(EXTENSION_NAME "_ddup" CACHE STRING "Name of the extension") +set(EXTENSION_NAME "_ddup.so" CACHE STRING "Name of the extension") project(${EXTENSION_NAME}) message(STATUS "Building extension: ${EXTENSION_NAME}") @@ -31,6 +31,16 @@ set(ENV{PY_MAJOR_VERSION} ${PY_MAJOR_VERSION}) set(ENV{PY_MINOR_VERSION} ${PY_MINOR_VERSION}) set(ENV{PY_MICRO_VERSION} ${PY_MICRO_VERSION}) +# if PYTHON_EXECUTABLE is unset or empty, but Python3_EXECUTABLE is set, use that +if (NOT PYTHON_EXECUTABLE AND Python3_EXECUTABLE) + set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE}) +endif() + +# If we still don't have a Python executable, we can't continue +if (NOT PYTHON_EXECUTABLE) + message(FATAL_ERROR "Python executable not found") +endif() + # Cythonize the .pyx file set(DDUP_CPP_SRC ${CMAKE_CURRENT_BINARY_DIR}/_ddup.cpp) add_custom_command( diff --git a/ddtrace/internal/datadog/profiling/ddup/__init__.py b/ddtrace/internal/datadog/profiling/ddup/__init__.py index 1de52209fe1..32bd273c5a4 100644 --- a/ddtrace/internal/datadog/profiling/ddup/__init__.py +++ b/ddtrace/internal/datadog/profiling/ddup/__init__.py @@ -1,99 +1,95 @@ -from typing import Dict # noqa:F401 -from typing import Optional # noqa:F401 - from .utils import sanitize_string # noqa: F401 -is_available = False - +try: + from ._ddup import * # noqa: F403, F401 -def not_implemented(func): - def wrapper(*args, **kwargs): - raise NotImplementedError("{} is not implemented on this platform".format(func.__name__)) + is_available = True +except Exception as e: + from typing import Dict # noqa:F401 + from typing import Optional # noqa:F401 -@not_implemented -def init( - env, # type: Optional[str] - service, # type: Optional[str] - version, # type: Optional[str] - tags, # type: Optional[Dict[str, str]] - max_nframes, # type: Optional[int] - url, # type: Optional[str] -): - pass + from ddtrace.internal.logger import get_logger + LOG = get_logger(__name__) + LOG.debug("Failed to import _ddup: %s", e) -@not_implemented -def upload(): # type: () -> None - pass + is_available = False + # Decorator for not-implemented + def not_implemented(func): + def wrapper(*args, **kwargs): + raise NotImplementedError("{} is not implemented on this platform".format(func.__name__)) -class SampleHandle: @not_implemented - def push_cputime(self, value, count): # type: (int, int) -> None + def init( + env, # type: Optional[str] + service, # type: Optional[str] + version, # type: Optional[str] + tags, # type: Optional[Dict[str, str]] + max_nframes, # type: Optional[int] + url, # type: Optional[str] + ): pass @not_implemented - def push_walltime(self, value, count): # type: (int, int) -> None + def upload(): # type: () -> None pass - @not_implemented - def push_acquire(self, value, count): # type: (int, int) -> None - pass + class SampleHandle: + @not_implemented + def push_cputime(self, value, count): # type: (int, int) -> None + pass - @not_implemented - def push_release(self, value, count): # type: (int, int) -> None - pass + @not_implemented + def push_walltime(self, value, count): # type: (int, int) -> None + pass - @not_implemented - def push_alloc(self, value, count): # type: (int, int) -> None - pass + @not_implemented + def push_acquire(self, value, count): # type: (int, int) -> None + pass - @not_implemented - def push_heap(self, value): # type: (int) -> None - pass + @not_implemented + def push_release(self, value, count): # type: (int, int) -> None + pass - @not_implemented - def push_lock_name(self, lock_name): # type: (str) -> None - pass + @not_implemented + def push_alloc(self, value, count): # type: (int, int) -> None + pass - @not_implemented - def push_frame(self, name, filename, address, line): # type: (str, str, int, int) -> None - pass + @not_implemented + def push_heap(self, value): # type: (int) -> None + pass - @not_implemented - def push_threadinfo(self, thread_id, thread_native_id, thread_name): # type: (int, int, Optional[str]) -> None - pass + @not_implemented + def push_lock_name(self, lock_name): # type: (str) -> None + pass - @not_implemented - def push_taskinfo(self, task_id, task_name): # type: (int, str) -> None - pass + @not_implemented + def push_frame(self, name, filename, address, line): # type: (str, str, int, int) -> None + pass - @not_implemented - def push_exceptioninfo(self, exc_type, count): # type: (type, int) -> None - pass + @not_implemented + def push_threadinfo(self, thread_id, thread_native_id, thread_name): # type: (int, int, Optional[str]) -> None + pass - @not_implemented - def push_class_name(self, class_name): # type: (str) -> None - pass + @not_implemented + def push_taskinfo(self, task_id, task_name): # type: (int, str) -> None + pass - @not_implemented - def push_span(self, span, endpoint_collection_enabled): # type: (Optional[Span], bool) -> None - pass + @not_implemented + def push_exceptioninfo(self, exc_type, count): # type: (type, int) -> None + pass - @not_implemented - def flush_sample(self): # type: () -> None - pass + @not_implemented + def push_class_name(self, class_name): # type: (str) -> None + pass + @not_implemented + def push_span(self, span, endpoint_collection_enabled): # type: (Optional[Span], bool) -> None + pass -try: - from ._ddup import * # noqa: F403, F401 - - is_available = True - -except Exception as e: - from ddtrace.internal.logger import get_logger - - LOG = get_logger(__name__) - LOG.debug("Failed to import _ddup: %s", e) + @not_implemented + def flush_sample(self): # type: () -> None + pass diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi index a843c4d0576..53c415fbaf6 100644 --- a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi +++ b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi @@ -1,7 +1,6 @@ from typing import Dict from typing import Optional from typing import Union - from ddtrace._trace.span import Span StringType = Union[str, bytes, None] @@ -12,7 +11,7 @@ def init( version: StringType, tags: Optional[Dict[Union[str, bytes], Union[str, bytes]]], max_nframes: Optional[int], - url: StringType, + url: Optional[str], ) -> None: ... def upload() -> None: ... @@ -26,9 +25,8 @@ class SampleHandle: def push_lock_name(self, lock_name: StringType) -> None: ... def push_frame(self, name: StringType, filename: StringType, address: int, line: int) -> None: ... def push_threadinfo(self, thread_id: int, thread_native_id: int, thread_name: StringType) -> None: ... - def push_task_id(self, task_id: int) -> None: ... - def push_task_name(self, task_name: StringType) -> None: ... - def push_exceptioninfo(self, exc_type: type, count: int) -> None: ... + def push_taskinfo(self, task_id: int, task_name: StringType) -> None: ... + def push_exceptioninfo(self, exc_type: Union[None, bytes, str, type], count: int) -> None: ... def push_class_name(self, class_name: StringType) -> None: ... def push_span(self, span: Optional[Span], endpoint_collection_enabled: bool) -> None: ... def flush_sample(self) -> None: ... diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx index cfb48e5d97f..bdfab1aaa9f 100644 --- a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx +++ b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx @@ -7,23 +7,21 @@ from typing import Optional from typing import Union import ddtrace -from ddtrace.internal.runtime import get_runtime_id from ddtrace.internal.compat import ensure_binary from ddtrace.internal.constants import DEFAULT_SERVICE_NAME +from ddtrace.internal.datadog.profiling.ddup.utils import sanitize_string +from ddtrace.internal.runtime import get_runtime_id from ddtrace._trace.span import Span -from .utils import sanitize_string StringType = Union[str, bytes, None] -def ensure_binary_or_empty(s: StringType) -> bytes: - try: - return ensure_binary(s) - except Exception: - pass - return b"" - +cdef extern from "stdint.h": + ctypedef unsigned long long uint64_t + ctypedef long long int64_t + cdef uint64_t UINT64_MAX + cdef int64_t INT64_MAX cdef extern from "" namespace "std" nogil: cdef cppclass string_view: @@ -34,8 +32,6 @@ cdef extern from "sample.hpp" namespace "Datadog": pass cdef extern from "interface.hpp": - ctypedef signed int int64_t - ctypedef unsigned int uint64_t void ddup_config_env(string_view env) void ddup_config_service(string_view service) void ddup_config_version(string_view version) @@ -55,8 +51,8 @@ cdef extern from "interface.hpp": void ddup_push_cputime(Sample *sample, int64_t cputime, int64_t count) void ddup_push_acquire(Sample *sample, int64_t acquire_time, int64_t count) void ddup_push_release(Sample *sample, int64_t release_time, int64_t count) - void ddup_push_alloc(Sample *sample, uint64_t size, uint64_t count) - void ddup_push_heap(Sample *sample, uint64_t size) + void ddup_push_alloc(Sample *sample, int64_t size, int64_t count) + void ddup_push_heap(Sample *sample, int64_t size) void ddup_push_lock_name(Sample *sample, string_view lock_name) void ddup_push_threadinfo(Sample *sample, int64_t thread_id, int64_t thread_native_id, string_view thread_name) void ddup_push_task_id(Sample *sample, int64_t task_id) @@ -99,6 +95,35 @@ cdef call_ddup_config_user_tag(bytes key, bytes val): ddup_config_user_tag(string_view(key, len(key)), string_view(val, len(val))) +# Conversion functions +def ensure_binary_or_empty(s: StringType) -> bytes: + try: + return ensure_binary(s) + except Exception: + pass + return b"" + + +cdef uint64_t clamp_to_uint64_unsigned(value): + # This clamps a Python int to the nonnegative range of an unsigned 64-bit integer. + # The name is redundant, but consistent with the other clamping function. + if value < 0: + return 0 + if value > UINT64_MAX: + return UINT64_MAX + return value + + +cdef int64_t clamp_to_int64_unsigned(value): + # This clamps a Python int to the nonnegative range of a signed 64-bit integer. + if value < 0: + return 0 + if value > INT64_MAX: + return INT64_MAX + return value + + +# Public API def init( service: StringType = None, env: StringType = None, @@ -126,7 +151,7 @@ def init( call_ddup_config_profiler_version(ensure_binary_or_empty(ddtrace.__version__)) if max_nframes is not None: - ddup_config_max_nframes(max_nframes) # call_* only needed for string-type args + ddup_config_max_nframes(clamp_to_int64_unsigned(max_nframes)) if tags is not None: for key, val in tags.items(): if key and val: @@ -154,34 +179,34 @@ cdef class SampleHandle: def push_cputime(self, value: int, count: int) -> None: if self.ptr is not NULL: - ddup_push_cputime(self.ptr, value, count) + ddup_push_cputime(self.ptr, clamp_to_int64_unsigned(value), clamp_to_int64_unsigned(count)) def push_walltime(self, value: int, count: int) -> None: if self.ptr is not NULL: - ddup_push_walltime(self.ptr, value, count) + ddup_push_walltime(self.ptr, clamp_to_int64_unsigned(value), clamp_to_int64_unsigned(count)) def push_acquire(self, value: int, count: int) -> None: if self.ptr is not NULL: - ddup_push_acquire(self.ptr, value, count) + ddup_push_acquire(self.ptr, clamp_to_int64_unsigned(value), clamp_to_int64_unsigned(count)) def push_release(self, value: int, count: int) -> None: if self.ptr is not NULL: - ddup_push_release(self.ptr, value, count) + ddup_push_release(self.ptr, clamp_to_int64_unsigned(value), clamp_to_int64_unsigned(count)) def push_alloc(self, value: int, count: int) -> None: if self.ptr is not NULL: - ddup_push_alloc(self.ptr, value, count) + ddup_push_alloc(self.ptr, clamp_to_int64_unsigned(value), clamp_to_int64_unsigned(count)) def push_heap(self, value: int) -> None: if self.ptr is not NULL: - ddup_push_heap(self.ptr, value) + ddup_push_heap(self.ptr, clamp_to_int64_unsigned(value)) def push_lock_name(self, lock_name: StringType) -> None: if self.ptr is not NULL: lock_name_bytes = ensure_binary_or_empty(lock_name) ddup_push_lock_name(self.ptr, string_view(lock_name_bytes, len(lock_name_bytes))) - def push_frame(self, name: StringType, filename: StringType, int address, int line) -> None: + def push_frame(self, name: StringType, filename: StringType, address: int, line: int) -> None: if self.ptr is not NULL: # Customers report `name` and `filename` may be unexpected objects, so sanitize. name_bytes = ensure_binary_or_empty(sanitize_string(name)) @@ -190,8 +215,8 @@ cdef class SampleHandle: self.ptr, string_view(name_bytes, len(name_bytes)), string_view(filename_bytes, len(filename_bytes)), - address, - line + clamp_to_uint64_unsigned(address), + clamp_to_int64_unsigned(line), ) def push_threadinfo(self, thread_id: int, thread_native_id: int, thread_name: StringType) -> None: @@ -201,14 +226,14 @@ cdef class SampleHandle: thread_name_bytes = ensure_binary_or_empty(thread_name) ddup_push_threadinfo( self.ptr, - thread_id, - thread_native_id, + clamp_to_int64_unsigned(thread_id), + clamp_to_int64_unsigned(thread_native_id), string_view(thread_name_bytes, len(thread_name_bytes)) ) def push_task_id(self, task_id: int) -> None: if self.ptr is not NULL: - ddup_push_task_id(self.ptr, task_id) + ddup_push_task_id(self.ptr, clamp_to_int64_unsigned(task_id)) def push_task_name(self, task_name: StringType) -> None: if self.ptr is not NULL: @@ -216,11 +241,18 @@ cdef class SampleHandle: task_name_bytes = ensure_binary_or_empty(task_name) ddup_push_task_name(self.ptr, string_view(task_name_bytes, len(task_name_bytes))) - def push_exceptioninfo(self, exc_type: type, count: int) -> None: + def push_exceptioninfo(self, exc_type: Union[None, bytes, str, type], count: int) -> None: if self.ptr is not NULL: - if exc_type is not None: + exc_name = None + if exc_type is type: exc_name = ensure_binary_or_empty(exc_type.__module__ + "." + exc_type.__name__) - ddup_push_exceptioninfo(self.ptr, string_view(exc_name, len(exc_name)), count) + else: + exc_name = ensure_binary_or_empty(exc_type) + ddup_push_exceptioninfo( + self.ptr, + string_view(exc_name, len(exc_name)), + clamp_to_int64_unsigned(count) + ) def push_class_name(self, class_name: StringType) -> None: if self.ptr is not NULL: @@ -233,11 +265,11 @@ cdef class SampleHandle: if not span: return if span.span_id: - ddup_push_span_id(self.ptr, span.span_id) + ddup_push_span_id(self.ptr, clamp_to_uint64_unsigned(span.span_id)) if not span._local_root: return if span._local_root.span_id: - ddup_push_local_root_span_id(self.ptr, span._local_root.span_id) + ddup_push_local_root_span_id(self.ptr, clamp_to_uint64_unsigned(span._local_root.span_id)) if span._local_root.span_type: span_type_bytes = ensure_binary_or_empty(span._local_root.span_type) ddup_push_trace_type(self.ptr, string_view(span_type_bytes, len(span_type_bytes))) diff --git a/ddtrace/internal/datadog/profiling/ddup/test/interface.py b/ddtrace/internal/datadog/profiling/ddup/test/interface.py new file mode 100644 index 00000000000..f4ee385fbd6 --- /dev/null +++ b/ddtrace/internal/datadog/profiling/ddup/test/interface.py @@ -0,0 +1,190 @@ +# mypy: ignore-errors +from itertools import product +import os +from pathlib import Path +import sys +from unittest.mock import MagicMock + + +sys.modules["ddtrace"] = MagicMock() +sys.modules["ddtrace"].__version__ = "0.0.0" +sys.modules["ddtrace.internal"] = MagicMock() +sys.modules["ddtrace.internal.compat"] = MagicMock() +sys.modules["ddtrace.internal.constants"] = MagicMock() +sys.modules["ddtrace.internal.datadog"] = MagicMock() +sys.modules["ddtrace.internal.datadog.profiling"] = MagicMock() +sys.modules["ddtrace.internal.datadog.profiling.ddup"] = MagicMock() +sys.modules["ddtrace.internal.datadog.profiling.ddup.utils"] = MagicMock() +sys.modules["ddtrace.internal.runtime"] = MagicMock() +sys.modules["ddtrace.internal.runtime.compat"] = MagicMock() +sys.modules["ddtrace.internal.runtime.constants"] = MagicMock() +sys.modules["ddtrace._trace"] = MagicMock() +sys.modules["ddtrace._trace.span"] = MagicMock() + + +# Setup the Span object +# This is terrible not-quite-copypasta, but this is just a quick-and-dirty harness. +# This will get replaced in the next iteration +class Span(object): + def __init__( + self, + span_id=None, # type: Optional[int] + service=None, # type: Union[None, str, bytes] + span_type=None, # type: Union[None, str, bytes] + _local_root=None, # type: Optional[Span] + ): + self.span_id = span_id + self.service = service + self.span_type = span_type + self._local_root = _local_root + + +sys.modules["ddtrace._trace.span"].Span = Span + + +# The location of the ddup folder as it is in the dd-trace-py library +ddup_path = Path(__file__).parent.resolve() / ".." / ".." / "build" / "ddup" +sys.path.insert(0, str(ddup_path)) +import _ddup # noqa + + +# Function for running a test in a fork +# Returns true if there were no exceptions, else false +def run_test(test: callable) -> bool: + pid = os.fork() + if pid == 0: + try: + test() + os._exit(0) + except Exception as e: + print(e) + raise e + os._exit(1) + else: + _, status = os.waitpid(pid, 0) + return os.WIFEXITED(status) and os.WEXITSTATUS(status) == 0 + + +# Initialization tests; we run every single type combination +def InitTest(name, tags, value): + def test(): + _ddup.init( + service=name, + env=name, + version=name, + tags=tags, + max_nframes=value, + url=name, + ) + + return test + + +# 3 * 5 * 13 = 195 tests +InitTypeTests = [ + InitTest(name, tags, value) + for name, tags, value in product( + [None, b"name", "name"], + [None, {"tag": "value"}, {"tag": b"value"}, {b"tag": "value"}, {b"tag": b"value"}], + [ + 0, + -1, + 1, + 256, + 2**30 + 1, + 2**48 - 1, + 2**48, + 2**48 + 1, + 2**62 - 1, + 2**62, + 2**62 + 1, + 2**64 - 1, + 2**64, + 2**64 + 1, + ], # values + ) +] + +InitNormal = InitTest("name", {"tag": "value"}, 10) + + +# Test all sample interfaces +def SampleTestSimple(): + InitNormal() + h = _ddup.SampleHandle() + h.push_walltime(1, 1) + h.push_cputime(1, 1) + h.push_acquire(1, 1) + h.push_release(1, 1) + h.push_alloc(1, 1) + h.push_heap(1) + h.flush_sample() + + +def SampleTest(value, name, exc_type, lineno, span, endpoint): + def test(): + try: + InitNormal() + h = _ddup.SampleHandle() + h.push_walltime(value, 1) + h.push_cputime(value, 1) + h.push_acquire(value, 1) + h.push_release(value, 1) + h.push_alloc(value, 1) + h.push_heap(value) + h.push_threadinfo(value, value, name) + h.push_task_id(value) + h.push_task_name(name) + h.push_exceptioninfo(exc_type, value) + h.push_span(span, endpoint) + h.push_frame(name, name, value, lineno) + h.flush_sample() + except Exception as e: + # Just print the exception, including the line and stuff + print("Exception in test:") + print(e) + raise e + + return test + + +def MakeSpan(span_id, service, span_type, add_local_root): + return Span(span_id, service, span_type, Span(span_id, service, span_type) if add_local_root else None) + + +# 13 * 3 * 3 * 5 * 2 * 3 = 7020 tests +SampleTypeTests = [ + SampleTest(value, exc_type, name, lineno, MakeSpan(value, name, name, add_local_root), endpoint) + for value, exc_type, name, lineno, add_local_root, endpoint in product( + [ + 0, + -1, + 1, + 256, + 2**30 + 1, + 2**48 - 1, + 2**48, + 2**48 + 1, + 2**62 - 1, + 2**62, + 2**62 + 1, + 2**64 - 1, + 2**64, + 2**64 + 1, + ], # values + [None, Exception, ValueError], # exc_type + [None, b"name", "name"], # names + [-1, 0, 1, 256, 1000], # lineno + [False, True], # add_local_root + [None, False, True], # endpoint + ) +] + + +# Run the init tests +for test in InitTypeTests: + assert run_test(test) +assert run_test(InitNormal) +assert run_test(SampleTestSimple) +for test in SampleTypeTests: + assert run_test(test) diff --git a/ddtrace/internal/datadog/profiling/runner.sh b/ddtrace/internal/datadog/profiling/runner.sh old mode 100755 new mode 100644 diff --git a/ddtrace/internal/datadog/profiling/setup_custom.sh b/ddtrace/internal/datadog/profiling/setup_custom.sh index 0c41aed8517..142d444db23 100755 --- a/ddtrace/internal/datadog/profiling/setup_custom.sh +++ b/ddtrace/internal/datadog/profiling/setup_custom.sh @@ -14,28 +14,23 @@ highest_clang="" highest_clangxx="" # Function to find the highest version of compilers +# Note that the product of this check is ignored if the user passes CC/CXX find_highest_compiler_version() { local base_name=$1 local highest_var_name=$2 - local highest_version=0 # Try to find the latest versions of both GCC and Clang # The range 5-20 is arbitrary (GCC 5 was released in 2015, and 20 is just a # a high number since Clang is on version 17) - # TODO provide a passthrough in CI where we want to use a pinned or specified - # version. Not a big deal since this script is currently only used for local - # development. for version in {20..5}; do if command -v "${base_name}-${version}" &> /dev/null; then - if [ $version -gt $highest_version ]; then - highest_version=$version - eval "$highest_var_name=${base_name}-${version}" - fi + eval "$highest_var_name=${base_name}-${version}" + return fi done # Check for the base version if no numbered version was found - if [ $highest_version -eq 0 ] && command -v "$base_name" &> /dev/null; then + if command -v "$base_name" &> /dev/null; then eval "$highest_var_name=$base_name" fi } @@ -46,6 +41,9 @@ find_highest_compiler_version g++ highest_gxx find_highest_compiler_version clang highest_clang find_highest_compiler_version clang++ highest_clangxx +# Get the highest clang_tidy from the $highest_clangxx variable +CLANGTIDY_CMD=${highest_clangxx/clang++/clang-tidy} + ### Build setup # Targets to target dirs declare -A target_dirs @@ -55,6 +53,9 @@ target_dirs["dd_wrapper"]="dd_wrapper" # Compiler options declare -A compiler_args +compiler_args["address"]="-DSANITIZE_OPTIONS=address" +compiler_args["leak"]="-DSANITIZE_OPTIONS=leak" +compiler_args["undefined"]="-DSANITIZE_OPTIONS=undefined" compiler_args["safety"]="-DSANITIZE_OPTIONS=address,leak,undefined" compiler_args["thread"]="-DSANITIZE_OPTIONS=thread" compiler_args["numerical"]="-DSANITIZE_OPTIONS=integer,nullability,signed-integer-overflow,bounds,float-divide-by-zero" @@ -62,6 +63,9 @@ compiler_args["dataflow"]="-DSANITIZE_OPTIONS=dataflow" compiler_args["memory"]="-DSANITIZE_OPTIONS=memory" compiler_args["fanalyzer"]="-DDO_FANALYZE=ON" compiler_args["cppcheck"]="-DDO_CPPCHECK=ON" +compiler_args["infer"]="-DDO_INFER=ON" +compiler_args["clangtidy"]="-DDO_CLANGTIDY=ON" +compiler_args["clangtidy_cmd"]="-DCLANGTIDY_CMD=${CLANGTIDY_CMD}" # Initial cmake args cmake_args=( @@ -78,8 +82,12 @@ targets=("dd_wrapper") # Helper functions for finding the compiler(s) set_clang() { - export CC=$highest_clang - export CXX=$highest_clangxx + if [ -z "${CC:-}" ]; then + export CC=$highest_clang + fi + if [ -z "${CXX:-}" ]; then + export CXX=$highest_clangxx + fi cmake_args+=( -DCMAKE_C_COMPILER=$CC -DCMAKE_CXX_COMPILER=$CXX @@ -87,8 +95,13 @@ set_clang() { } set_gcc() { - export CC=$highest_gcc - export CXX=$highest_gxx + # Only set CC or CXX if they're not set + if [ -z "${CC:-}" ]; then + export CC=$highest_gcc + fi + if [ -z "${CXX:-}" ]; then + export CXX=$highest_gxx + fi cmake_args+=( -DCMAKE_C_COMPILER=$CC -DCMAKE_CXX_COMPILER=$CXX @@ -99,17 +112,17 @@ set_gcc() { run_cmake() { target=$1 dir=${target_dirs[$target]} + build=${BUILD_DIR}/${dir} if [ -z "$dir" ]; then echo "No directory specified for cmake" exit 1 fi - # Enter the directory and create the build directory - pushd $dir || { echo "Failed to change to $dir"; exit 1; } - mkdir -p $BUILD_DIR && cd $BUILD_DIR || { echo "Failed to create build directory for $dir"; exit 1; } + # Make sure we have the build directory + mkdir -p ${build} && pushd ${build} || { echo "Failed to create build directory for $dir"; exit 1; } # Run cmake - cmake "${cmake_args[@]}" .. || { echo "cmake failed"; exit 1; } + cmake "${cmake_args[@]}" -S=$MY_DIR/$dir || { echo "cmake failed"; exit 1; } cmake --build . || { echo "build failed"; exit 1; } if [[ " ${cmake_args[*]} " =~ " -DDO_CPPCHECK=ON " ]]; then echo "--------------------------------------------------------------------- Running CPPCHECK" @@ -117,7 +130,6 @@ run_cmake() { fi if [[ " ${cmake_args[*]} " =~ " -DBUILD_TESTING=ON " ]]; then echo "--------------------------------------------------------------------- Running Tests" -# make test || { echo "tests failed"; exit 1; } ctest --output-on-failure || { echo "tests failed!"; exit 1; } fi @@ -130,12 +142,17 @@ print_help() { echo "Usage: ${MY_NAME} [options] [build_mode] [target]" echo "Options (one of)" echo " -h, --help Show this help message and exit" + echo " -a, --address Clang + " ${compile_args["address"]} + echo " -l, --leak Clang + " ${compile_args["leak"]} + echo " -u, --undefined Clang + " ${compile_args["undefined"]} echo " -s, --safety Clang + " ${compile_args["safety"]} echo " -t, --thread Clang + " ${compile_args["thread"]} echo " -n, --numerical Clang + " ${compile_args["numerical"]} - echo " -d, --dataflow Clang + " ${compile_args["dataflow"]} + echo " -d, --dataflow Clang + " ${compile_args["dataflow"]} # Requires custom libstdc++ to work echo " -m --memory Clang + " ${compile_args["memory"]} echo " -C --cppcheck Clang + " ${compile_args["cppcheck"]} + echo " -I --infer Clang + " ${compile_args["infer"]} + echo " -T --clangtidy Clang + " ${compile_args["clangtidy"]} echo " -f, --fanalyze GCC + " ${compile_args["fanalyzer"]} echo " -c, --clang Clang (alone)" echo " -g, --gcc GCC (alone)" @@ -173,9 +190,21 @@ add_compiler_args() { print_help exit 0 ;; + -a|--address) + cmake_args+=(${compiler_args["address"]}) + set_clang + ;; + -l|--leak) + cmake_args+=(${compiler_args["leak"]}) + set_clang + ;; + -u|--undefined) + cmake_args+=(${compiler_args["undefined"]}) + set_clang + ;; -s|--safety) cmake_args+=(${compiler_args["safety"]}) - set_clang + set_gcc ;; -t|--thread) cmake_args+=(${compiler_args["thread"]}) @@ -196,6 +225,21 @@ add_compiler_args() { -C|--cppcheck) cmake_args+=(${compiler_args["cppcheck"]}) set_clang + if command -v cppcheck &> /dev/null; then + cmake_args+=(-DCPPCHECK_EXECUTABLE=$(which cppcheck)) + fi + ;; + -I|--infer) + cmake_args+=(${compiler_args["infer"]}) + set_clang + if command -v infer &> /dev/null; then + cmake_args+=(-DInfer_EXECUTABLE=$(which infer)) + fi + ;; + -T|--clangtidy) + cmake_args+=(${compiler_args["clangtidy"]}) + cmake_args+=(${compiler_args["clangtidy_cmd"]}) + set_clang ;; -f|--fanalyze) cmake_args+=(${compiler_args["fanalyzer"]}) diff --git a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt index 2a021fb49b8..7a57a86fd43 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt @@ -48,6 +48,10 @@ add_ddup_config(${EXTENSION_NAME}) add_cppcheck_target(${EXTENSION_NAME} DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/include + ${CMAKE_CURRENT_SOURCE_DIR}/include/vendored + ${CMAKE_CURRENT_SOURCE_DIR}/include/util + ${CMAKE_CURRENT_SOURCE_DIR}/.. + ${echion_SOURCE_DIR} SRC ${CMAKE_CURRENT_SOURCE_DIR}/src ) diff --git a/ddtrace/profiling/profiler.py b/ddtrace/profiling/profiler.py index e7382106f34..db261b3b140 100644 --- a/ddtrace/profiling/profiler.py +++ b/ddtrace/profiling/profiler.py @@ -207,7 +207,7 @@ def _build_default_exporters(self): env=self.env, service=self.service, version=self.version, - tags=self.tags, + tags=self.tags, # type: ignore max_nframes=config.max_frames, url=endpoint, ) diff --git a/pyproject.toml b/pyproject.toml index 7c20d07c52b..82857ecd05e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -170,6 +170,7 @@ exclude_dirs = [ "ddtrace/commands/ddtrace_run.py", "ddtrace/ext/git.py", "ddtrace/ext/test.py", + "ddtrace/internal/datadog/profiling/ddup/test/interface.py", "ddtrace/internal/module.py", "ddtrace/internal/processor/stats.py", "ddtrace/internal/rate_limiter.py",