Skip to content

Commit 8cb578e

Browse files
authored
feat(profiling): add experimental outside-in stack sampling method [backport #8471 to 2.7] (#8635)
Backport of #8471 to 2.7 This allows users to optionally use a new stack sampling technique. Briefly, * Avoids issues in Python 3.11 and later where the C struct backing Python frame objects may contain invalid pointers (causing segfaults within standard API calls) * Addresses a bias in the "normal" stack sampler where we had a tendency to sample threads only at the points where they transitioned off-GIL * Hopefully removes some serialization overhead, although in the first few versions the overall consumption of this technique may be higher. There are a few tests around the new code, those will be added to CI in a later update. ## 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)
1 parent 9f7ba62 commit 8cb578e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+1333
-582
lines changed

.gitignore

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ ddtrace/profiling/collector/_traceback.c
1313
ddtrace/profiling/collector/stack.c
1414
ddtrace/profiling/exporter/pprof.c
1515
ddtrace/profiling/_build.c
16-
ddtrace/internal/datadog/profiling/ddup.cpp
17-
ddtrace/internal/datadog/profiling/_ddup.cpp
16+
ddtrace/internal/datadog/profiling/ddup/_ddup.cpp
1817
ddtrace/internal/_encoding.c
1918
ddtrace/internal/_rand.c
2019
ddtrace/internal/_tagset.c
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
include(CheckIPOSupported)
2+
3+
function(add_ddup_config target)
4+
target_compile_options(${target} PRIVATE
5+
"$<$<CONFIG:Debug>:-Og;-ggdb3>"
6+
"$<$<CONFIG:Release>:-Os>"
7+
-ffunction-sections -fno-semantic-interposition -Wall -Werror -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast
8+
)
9+
target_link_options(${target} PRIVATE
10+
"$<$<CONFIG:Release>:-s>"
11+
-Wl,--as-needed -Wl,-Bsymbolic-functions -Wl,--gc-sections
12+
)
13+
set_property(TARGET ${target} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
14+
check_ipo_supported(RESULT result)
15+
if (result)
16+
set_property(TARGET ${target} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
17+
endif()
18+
19+
# Propagate sanitizers
20+
if (SANITIZE_OPTIONS)
21+
# Some sanitizers (or the analysis--such as symbolization--tooling thereof) work better with frame
22+
# pointers, so we include it here.
23+
target_compile_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS} -fno-omit-frame-pointer)
24+
target_link_options(${target} PRIVATE -fsanitize=${SANITIZE_OPTIONS})
25+
endif()
26+
27+
# If DO_FANALYZER is specified and we're using gcc, then we can use -fanalyzer
28+
if (DO_FANALYZER AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
29+
target_compile_options(${target} PRIVATE -fanalyzer)
30+
endif()
31+
endfunction()

ddtrace/internal/datadog/profiling/cmake/FindCppcheck.cmake

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
# Only add this project if it doesn't already exist
2+
if (TARGET cppcheck_project)
3+
return()
4+
endif()
5+
16
include(ExternalProject)
27

38
# Build cppcheck from sources
@@ -17,10 +22,20 @@ endif()
1722

1823
# This function will add a cppcheck target for a given directory
1924
# unless DO_CPPCHECK is set to false, then it will add a target that does nothing
20-
function(add_cppcheck_target NAME DIRECTORY)
25+
function(add_cppcheck_target)
26+
# Parse additional arguments as lists
27+
set(options)
28+
set(oneValueArgs TARGET)
29+
set(multiValueArgs INCLUDE SRC)
30+
cmake_parse_arguments(PARSE_ARGV 0 ARG "${options}" "${oneValueArgs}" "${multiValueArgs}")
31+
32+
# Automatically generate the cppcheck target name
33+
set(NAME "cppcheck_dd_${ARGV0}")
34+
2135
if (DO_CPPCHECK)
22-
add_custom_target("${NAME}"
23-
COMMAND ${CPPCHECK_EXECUTABLE}
36+
# Initialize command variable
37+
set(cppcheck_cmd
38+
${CPPCHECK_EXECUTABLE}
2439
--enable=all
2540
--addon=threadsafety.py
2641
--addon=misc
@@ -31,18 +46,31 @@ function(add_cppcheck_target NAME DIRECTORY)
3146
--suppress=missingIncludeSystem
3247
--inline-suppr
3348
--error-exitcode=1
34-
-I ${CMAKE_SOURCE_DIR}/include
35-
-I ${Datadog_INCLUDE_DIRS}
36-
${CMAKE_SOURCE_DIR}/src
37-
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
38-
COMMENT "Running cppcheck"
3949
)
40-
add_dependencies(cppcheck_runner "${NAME}")
50+
51+
# Append include directories to the command
52+
foreach(INCLUDE_DIR ${ARG_INCLUDE})
53+
list(APPEND cppcheck_cmd -I ${INCLUDE_DIR})
54+
endforeach()
55+
56+
# Append source directories/files to the command
57+
foreach(SRC_FILE ${ARG_SRC})
58+
list(APPEND cppcheck_cmd ${SRC_FILE})
59+
endforeach()
60+
61+
# Define the custom target with the constructed command
62+
add_custom_target(${NAME}
63+
COMMAND ${cppcheck_cmd}
64+
COMMENT "Running cppcheck on ${ARGV0}"
65+
)
66+
67+
# Make the cppcheck target a dependent of the specified target
68+
add_dependencies(${ARGV0} ${NAME})
4169
else()
42-
# Need to make this function work, but kinda do nothing
43-
add_custom_target("${NAME}"
44-
COMMAND echo "building ${NAME} without cppcheck"
45-
COMMENT "cppcheck is disabled"
70+
# Define a do-nothing target if cppcheck is disabled
71+
add_custom_target(${NAME}
72+
COMMAND echo "Cppcheck target ${NAME} is disabled."
73+
COMMENT "cppcheck is disabled for ${ARGV0}"
4674
)
4775
endif()
4876
endfunction()

ddtrace/internal/datadog/profiling/cmake/FindLibdatadog.cmake

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
1+
# Only add this project if Datadog::Profiling is not already defined
2+
if(TARGET Datadog::Profiling)
3+
return()
4+
endif()
5+
6+
include(ExternalProject)
17
set(TAG_LIBDATADOG
28
"v5.0.0"
39
CACHE STRING "libdatadog github tag")
410

11+
set(Datadog_BUILD_DIR ${CMAKE_BINARY_DIR}/libdatadog)
512
set(Datadog_ROOT ${Datadog_BUILD_DIR}/libdatadog-${TAG_LIBDATADOG})
613

714
message(STATUS "${CMAKE_CURRENT_LIST_DIR}/tools/fetch_libdatadog.sh ${TAG_LIBDATADOG} ${Datadog_ROOT}")
@@ -15,7 +22,7 @@ set(Datadog_DIR "${Datadog_ROOT}/cmake")
1522

1623
# Prefer static library to shared library
1724
set(CMAKE_FIND_LIBRARY_SUFFIXES_BACKUP ${CMAKE_FIND_LIBRARY_SUFFIXES})
18-
set(CMAKE_FIND_LIBRARY_SUFFIXES .a .so)
25+
set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
1926

2027
find_package(Datadog REQUIRED)
2128

ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt

Lines changed: 21 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,19 @@ project(dd_wrapper
44
LANGUAGES CXX
55
)
66

7-
# Infer some basic properties about the build. This is necessary because multiple
8-
# extensions reuse this cmake file, so we need its artifacts to go in a consistent
9-
# place
10-
get_filename_component(dd_wrapper_BUILD_PARENT "${CMAKE_BINARY_DIR}/../.." ABSOLUTE)
11-
set(dd_wrapper_BUILD_DIR "${dd_wrapper_BUILD_PARENT}/ddtrace.internal.datadog.profiling")
7+
# Build in a predictable location. This is needed for setup.py
8+
get_filename_component(dd_wrapper_BUILD_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../ddtrace.internal.datadog.profiling" ABSOLUTE)
129

1310
# Custom modules are in the parent directory
1411
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
1512

16-
# Setup state for FindLibdatadog
17-
set(Datadog_BUILD_DIR "${dd_wrapper_BUILD_PARENT}/libdatadog")
18-
1913
# Includes
20-
include(CMakePackageConfigHelpers) # For generating the config file
2114
include(FetchContent)
2215
include(ExternalProject)
2316
include(FindLibdatadog)
17+
include(AnalysisFunc)
2418
include(FindCppcheck)
2519

26-
# Send some of these vars back up to the parent
27-
if (DD_WRAPPER_IS_SUBPROJECT)
28-
set(Datadog_INCLUDE_DIRS "${Datadog_INCLUDE_DIRS}" PARENT_SCOPE)
29-
set(dd_wrapper_BUILD_DIR "${dd_wrapper_BUILD_PARENT}/ddtrace.internal.datadog.profiling" PARENT_SCOPE)
30-
endif()
31-
32-
add_compile_options(
33-
"$<$<CONFIG:Debug>:-Og;-ggdb3>"
34-
"$<$<CONFIG:Release>:-Os -s>"
35-
-fno-semantic-interposition -Wall -Werror -Wextra -Wshadow -Wnon-virtual-dtor -Wold-style-cast
36-
)
37-
3820
# Library sources
3921
add_library(dd_wrapper SHARED
4022
src/uploader_builder.cpp
@@ -45,19 +27,17 @@ add_library(dd_wrapper SHARED
4527
src/interface.cpp
4628
)
4729

30+
# Add common configuration flags
31+
add_ddup_config(dd_wrapper)
32+
4833
# At present, C++17 is chosen as the minimum standard. This conflicts with the
4934
# manylinux2014 standard upon which we must rely. We'll have to statically link
5035
# libstdc++ to avoid this conflict, but need to remain mindful of symbol visibility
5136
# and overall binary size.
5237
target_compile_features(dd_wrapper PUBLIC cxx_std_17)
5338

54-
# Set some linker options
55-
target_compile_options(dd_wrapper PRIVATE -fno-semantic-interposition)
56-
target_compile_options(dd_wrapper PRIVATE -ffunction-sections -fdata-sections)
39+
# Statically link libstdc++ to avoid manylinux2014 conflicts
5740
target_link_options(dd_wrapper PRIVATE -static-libstdc++)
58-
target_link_options(dd_wrapper PRIVATE -Wl,-Bsymbolic-functions)
59-
target_link_options(dd_wrapper PRIVATE -Wl,--gc-sections)
60-
set_property(TARGET dd_wrapper PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
6141

6242
target_include_directories(dd_wrapper PRIVATE
6343
include
@@ -68,35 +48,22 @@ target_link_libraries(dd_wrapper PRIVATE
6848
)
6949
set_target_properties(dd_wrapper PROPERTIES POSITION_INDEPENDENT_CODE ON)
7050

71-
set_target_properties(dd_wrapper
72-
PROPERTIES
73-
LIBRARY_OUTPUT_DIRECTORY "${dd_wrapper_BUILD_DIR}"
74-
)
75-
76-
# If DO_FANALYZER is specified and we're using gcc, then we can use -fanalyzer
77-
if (DO_FANALYZER AND CMAKE_CXX_COMPILER_ID MATCHES "GNU")
78-
target_compile_options(dd_wrapper PRIVATE -fanalyzer)
79-
endif()
80-
81-
# If DO_CPPCHECK is specified, then we can use cppcheck
82-
add_cppcheck_target(cppcheck_dd_wrapper ${CMAKE_CURRENT_SOURCE_DIR})
83-
84-
# Propagate sanitizers
85-
if (SANITIZE_OPTIONS)
86-
# Some sanitizers (or the analysis--such as symbolization--tooling thereof) work better with frame
87-
# pointers, so we include it here.
88-
target_compile_options(dd_wrapper PRIVATE -fsanitize=${SANITIZE_OPTIONS} -fno-omit-frame-pointer)
89-
target_link_options(dd_wrapper PRIVATE -fsanitize=${SANITIZE_OPTIONS})
51+
# If LIB_INSTALL_DIR is set, install the library.
52+
# Install one directory up--both ddup and stackv2 are set to the same relative level.
53+
if (LIB_INSTALL_DIR)
54+
install(TARGETS dd_wrapper
55+
LIBRARY DESTINATION ${LIB_INSTALL_DIR}/..
56+
ARCHIVE DESTINATION ${LIB_INSTALL_DIR}/..
57+
RUNTIME DESTINATION ${LIB_INSTALL_DIR}/..
58+
)
9059
endif()
9160

92-
# If dd_wrapper_INSTALL_DIR is specified by the parent, use it
93-
if (NOT dd_wrapper_INSTALL_DIR)
94-
# If not, then just keep it in the build directory
95-
set(dd_wrapper_INSTALL_DIR "${CMAKE_BINARY_DIR}/lib")
96-
endif()
97-
message(STATUS "dd_wrapper_INSTALL_DIR: ${dd_wrapper_INSTALL_DIR}")
98-
install(TARGETS dd_wrapper
99-
DESTINATION ${dd_wrapper_INSTALL_DIR}
61+
# Configure cppcheck
62+
add_cppcheck_target(dd_wrapper
63+
DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
64+
INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/include
65+
${Datadog_INCLUDE_DIRS}
66+
SRC ${CMAKE_CURRENT_SOURCE_DIR}/src
10067
)
10168

10269
# Add the tests

ddtrace/internal/datadog/profiling/dd_wrapper/include/interface.hpp

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <stddef.h>
44
#include <stdint.h>
5+
#include <string_view>
56

67
// Forward decl of the return pointer
78
namespace Datadog {
@@ -12,21 +13,21 @@ class Sample;
1213
extern "C"
1314
{
1415
#endif
15-
void ddup_config_env(const char* dd_env);
16-
void ddup_config_service(const char* service);
17-
void ddup_config_version(const char* version);
18-
void ddup_config_runtime(const char* runtime);
19-
void ddup_config_runtime_version(const char* runtime_version);
20-
void ddup_config_profiler_version(const char* profiler_version);
21-
void ddup_config_url(const char* url);
16+
void ddup_config_env(std::string_view dd_env);
17+
void ddup_config_service(std::string_view service);
18+
void ddup_config_version(std::string_view version);
19+
void ddup_config_runtime(std::string_view runtime);
20+
void ddup_config_runtime_version(std::string_view runtime_version);
21+
void ddup_config_profiler_version(std::string_view profiler_version);
22+
void ddup_config_url(std::string_view url);
2223
void ddup_config_max_nframes(int max_nframes);
2324

24-
void ddup_config_user_tag(const char* key, const char* val);
25+
void ddup_config_user_tag(std::string_view key, std::string_view val);
2526
void ddup_config_sample_type(unsigned int type);
2627

2728
bool ddup_is_initialized();
2829
void ddup_init();
29-
void ddup_set_runtime_id(const char* id, size_t sz);
30+
void ddup_set_runtime_id(std::string_view id);
3031
bool ddup_upload();
3132

3233
// Proxy functions to the underlying sample
@@ -37,22 +38,22 @@ extern "C"
3738
void ddup_push_release(Datadog::Sample* sample, int64_t release_time, int64_t count);
3839
void ddup_push_alloc(Datadog::Sample* sample, uint64_t size, uint64_t count);
3940
void ddup_push_heap(Datadog::Sample* sample, uint64_t size);
40-
void ddup_push_lock_name(Datadog::Sample* sample, const char* lock_name);
41+
void ddup_push_lock_name(Datadog::Sample* sample, std::string_view lock_name);
4142
void ddup_push_threadinfo(Datadog::Sample* sample,
4243
int64_t thread_id,
4344
int64_t thread_native_id,
44-
const char* thread_name);
45+
std::string_view thread_name);
4546
void ddup_push_task_id(Datadog::Sample* sample, int64_t task_id);
46-
void ddup_push_task_name(Datadog::Sample* sample, const char* task_name);
47+
void ddup_push_task_name(Datadog::Sample* sample, std::string_view task_name);
4748
void ddup_push_span_id(Datadog::Sample* sample, int64_t span_id);
4849
void ddup_push_local_root_span_id(Datadog::Sample* sample, int64_t local_root_span_id);
49-
void ddup_push_trace_type(Datadog::Sample* sample, const char* trace_type);
50-
void ddup_push_trace_resource_container(Datadog::Sample* sample, const char* trace_resource_container);
51-
void ddup_push_exceptioninfo(Datadog::Sample* sample, const char* exception_type, int64_t count);
52-
void ddup_push_class_name(Datadog::Sample* sample, const char* class_name);
50+
void ddup_push_trace_type(Datadog::Sample* sample, std::string_view trace_type);
51+
void ddup_push_trace_resource_container(Datadog::Sample* sample, std::string_view trace_resource_container);
52+
void ddup_push_exceptioninfo(Datadog::Sample* sample, std::string_view exception_type, int64_t count);
53+
void ddup_push_class_name(Datadog::Sample* sample, std::string_view class_name);
5354
void ddup_push_frame(Datadog::Sample* sample,
54-
const char* _name,
55-
const char* _filename,
55+
std::string_view _name,
56+
std::string_view _filename,
5657
uint64_t address,
5758
int64_t line);
5859
void ddup_flush_sample(Datadog::Sample* sample);

ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ class Sample
3939
std::vector<int64_t> values = {};
4040

4141
public:
42-
// Initialization and stuff
43-
void start_sample();
44-
4542
// Helpers
4643
bool push_label(ExportLabelKey key, std::string_view val);
4744
bool push_label(ExportLabelKey key, int64_t val);

0 commit comments

Comments
 (0)