Skip to content

libdatadog 29.0.0: integrate libdatadog sample types#504

Open
r1viollet wants to merge 8 commits intomainfrom
r1viollet/libdatadog_28.0.2
Open

libdatadog 29.0.0: integrate libdatadog sample types#504
r1viollet wants to merge 8 commits intomainfrom
r1viollet/libdatadog_28.0.2

Conversation

@r1viollet
Copy link
Collaborator

@r1viollet r1viollet commented Mar 5, 2026

What does this PR do?

Upgrades libdatadog to v29.0.0 and integrates the new ddog_prof_SampleType enum for all pprof sample type declarations.

Previously, ddprof passed sample type names as free-form strings directly to libdatadog. Starting with v28, libdatadog introduced a ddog_prof_SampleType enum to replace those strings, with the string mapping handled internally by the library.

Motivation

Allow to release ddprof

How to test the change?

Run with --preset cpu_live_heap --debug_pprof_prefix /tmp/test\ and inspect with pprof -raw`: all 6 expected columns.

Run with tracepoints:

DDPROF=/home/r1viollet/dd/ddprof/build_gcc_alpine-linux-1.2.5_AlpRel/ddprof && sudo $DDPROF -l notice -S test-r1 -e "event=tlb:tlb_flush" --debug_pprof_prefix /tmp/test /usr/local/bin/BadBoggleSolver_run 10

API changes from v26.0.0:
- ddog_prof_Endpoint_agent/agentless gained a use_system_resolver bool.
  ddprof is an out-of-process profiler that does not fork during export,
  so use_system_resolver=true (system /etc/resolv.conf) is safe and correct.
- ddog_prof_ValueType (name+unit strings) replaced by ddog_prof_SampleType
  enum; ddog_prof_Period.type_ renamed to .sample_type accordingly.

The ValueType->SampleType mapping is expressed as a constexpr lookup table
(k_ddog_sample_types) whose rows directly mirror PROFILE_TYPE_TABLE in
perf_watcher.hpp. A static_assert on DDPROF_PWT_LENGTH enforces that adding
a new profile type requires updating the mapping explicitly.

Also add Datadog_LOCAL_ROOT cmake override to Findlibdatadog.cmake to allow
testing local libdatadog builds without a GitHub release (skips the fetch
script when set).
@pr-commenter
Copy link

pr-commenter bot commented Mar 5, 2026

Benchmark results for collatz

Parameters

Baseline Candidate
config baseline candidate
profiler-version ddprof 0.23.0+0c700352.103448873 ddprof 0.23.0+18c47295.103465578

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean execution_time
scenario:ddprof -S bench-collatz --preset cpu_only collatz_runner.sh same

@pr-commenter
Copy link

pr-commenter bot commented Mar 5, 2026

Benchmark results for BadBoggleSolver_run

Parameters

Baseline Candidate
config baseline candidate
profiler-version ddprof 0.23.0+0c700352.103448873 ddprof 0.23.0+18c47295.103465578

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean execution_time
scenario:ddprof -S bench-bad-boggle-solver BadBoggleSolver_run work 1000 unsure
[-16.575ms; -3.960ms] or [-0.830%; -0.198%]


// Predefined sample type mappings — defined in perf_watcher.cc.
// Generic sample counting (hardware events, misc software events).
extern const WatcherSampleTypes k_stype_tracepoint;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly strange, adjusting

…mapping

Remove the intermediate DDPROF_PWT_* profile type taxonomy and
k_ddog_sample_types lookup table, replacing them with WatcherSampleTypes
structs that directly store ddog_prof_SampleType integer values per
aggregation mode.

Key changes:
- Remove PROFILE_TYPE_TABLE macro and DDPROF_SAMPLE_TYPES enum
- Remove 5 helper functions (sample_type_name_from_idx, etc.)
- Add WatcherSampleTypes{sample_types[], count_types[]} to PerfWatcher
- Add pprof_active bool to replace the sDUM special-casing
- Replace ProfSampleTypes/get_active_ids/compute_pprof_values with
  a simple SlotRegistry that deduplicates ddog_prof_SampleType slots
- Use uint32_t storage to avoid libdatadog header dependency in
  perf_watcher.cc; static_asserts in ddprof_pprof.cc verify values

All 53 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@r1viollet r1viollet force-pushed the r1viollet/libdatadog_28.0.2 branch from 47c35b8 to 4326981 Compare March 5, 2026 17:48
@r1viollet
Copy link
Collaborator Author

tracepoints do not work... fixing this

r1viollet and others added 2 commits March 6, 2026 13:22
- Fix tracepoint count sentinel: k_stype_tracepoint count_types now use
  k_stype_val_sample (sentinel = no companion) instead of k_stype_val_tracepoint.
  Previously both pprof_index and pprof_count_index resolved to the same slot,
  causing the count to overwrite the primary value in pprof_aggregate.
- Fix watcher_has_tracepoint to check k_stype_val_tracepoint instead of
  k_stype_val_sample
- Fix debug pprof filename extension from .pprof.lz4 to .pprof.zst (the
  serialized bytes from libdatadog are zstd-compressed)
- Add static_assert for DDOG_PROF_SAMPLE_TYPE_TRACEPOINT == 38
- Use named k_stype_val_* constants in sample_type_name() switch cases
- Add static_assert per sample type string to catch future libdatadog
  renames at compile time

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the outdated DDPROF_PWT_* table with the new WatcherSampleTypes
model. Document all three watcher kinds (cpu, alloc, tracepoint), the
primary/count companion pattern, the sentinel mechanism, and the
compile-time static_assert safety net.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@r1viollet r1viollet marked this pull request as ready for review March 19, 2026 10:51
@r1viollet r1viollet changed the title libdatadog 28.0.2: integrate libdatadog sample types libdatadog 29.0.0: integrate libdatadog sample types Mar 19, 2026
This avoids forward declaring profile types
The documentation can be regenated from source if needed
@r1viollet r1viollet requested a review from nsavoire March 19, 2026 11:31
inline constexpr uint32_t k_stype_none = UINT32_MAX;

// Tracepoints: one event = one sample, no count companion, no live mode.
// clang-format off
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsavoire this part looks OK ? wdyt ?

Comment on lines +61 to +77
// Verify that sample_type_name() returns the strings the backend expects.
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_CPU_TIME)) == "cpu-time");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_CPU_SAMPLES)) == "cpu-samples");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_ALLOC_SPACE)) == "alloc-space");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_ALLOC_SAMPLES)) == "alloc-samples");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_INUSE_SPACE)) == "inuse-space");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_INUSE_OBJECTS)) == "inuse-objects");
static_assert(std::string_view(sample_type_name(
DDOG_PROF_SAMPLE_TYPE_TRACEPOINT)) == "tracepoint");
static_assert(std::string_view(
sample_type_name(DDOG_PROF_SAMPLE_TYPE_SAMPLE)) == "sample");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure these are useful: sample_type_name function is very simple

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these change, we would BE would not see the relevant profile types.

X(sEMU, "Emu. Faults", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS, 99, DDPROF_PWT_TRACEPOINT, IS_FREQ) \
X(sDUM, "Dummy", PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY, 1, DDPROF_PWT_NOCOUNT, {}) \
X(sALLOC, "Allocations", kDDPROF_TYPE_CUSTOM, kDDPROF_COUNT_ALLOCATIONS, 524288, DDPROF_PWT_ALLOC_SPACE, SKIP_FRAMES)
X(hCPU, "CPU Cycles", PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES, 99, k_stype_tracepoint, true, IS_FREQ) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could replace pprof_active with a dummy sample type (similarly to what was done before):

inline constexpr WatcherSampleTypes k_stype_dummy = {
    {k_stype_none,   k_stype_none},
    {k_stype_none, k_stype_none}};

inline constexpr bool is_pprof_active(const WatcherSampleTypes &st) {
  return st.sample_types[0] != k_stype_none;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants