libdatadog 28.0.2: integrate libdatadog sample types#504
Draft
libdatadog 28.0.2: integrate libdatadog sample types#504
Conversation
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).
Benchmark results for collatzParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
Benchmark results for BadBoggleSolver_runParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
r1viollet
commented
Mar 5, 2026
include/perf_watcher.hpp
Outdated
|
|
||
| // Predefined sample type mappings — defined in perf_watcher.cc. | ||
| // Generic sample counting (hardware events, misc software events). | ||
| extern const WatcherSampleTypes k_stype_tracepoint; |
Collaborator
Author
There was a problem hiding this comment.
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>
47c35b8 to
4326981
Compare
Collaborator
Author
|
tracepoints do not work... fixing this |
r1viollet
commented
Mar 6, 2026
| DDPROF_PWT_LENGTH * static_cast<int>(kNbEventAggregationModes); | ||
| // Maps a ddog_prof_SampleType integer value to the kebab-case name used in | ||
| // debug log output (must match what simple_malloc-ut.sh greps for). | ||
| constexpr const char *sample_type_name(uint32_t raw_type) { |
Collaborator
Author
There was a problem hiding this comment.
we could align with static assert values
- 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>
r1viollet
commented
Mar 6, 2026
| .type = PERF_TYPE_TRACEPOINT, | ||
| .sample_period = 1, | ||
| .sample_type_id = DDPROF_PWT_TRACEPOINT, | ||
| .sample_type_info = k_stype_tracepoint, |
Collaborator
Author
There was a problem hiding this comment.
tracepoints needs the libdatadog change
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nsavoire
reviewed
Mar 6, 2026
|
|
||
| // CPU: wall/cpu nanoseconds in sum mode, sample count in live mode. | ||
| inline constexpr WatcherSampleTypes k_stype_cpu = { | ||
| {k_stype_val_cpu_time, k_stype_val_cpu_samples}, |
Collaborator
There was a problem hiding this comment.
cpu time should not have an associated "live" sample type
nsavoire
reviewed
Mar 6, 2026
| inline constexpr uint32_t k_stype_val_inuse_objects = 27; // INUSE_OBJECTS | ||
|
|
||
| // Generic sample counting (hardware events, misc software events). | ||
| // count_types use the sentinel (k_stype_val_sample) because tracepoints |
Collaborator
There was a problem hiding this comment.
Using k_stype_val_sample as sentinel feels off
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
needs following PR: DataDog/libdatadog#1676
Motivation
What inspired you to submit this pull request? In order to motivate your point, it may be valuable to explain why this change is useful or what problem is being solved.
Additional Notes
This is the section to put technical guidance/constraints, call out potential regressions, cite sources, and in general offer some exposition on the how of the PR. This section doesn't need to be incredibly detailed, but it makes life easier for reviewers!
How to test the change?
Describe here in detail how the change can be validated. This is a great section to call out specific tests you've added or improved, or to acknowledge code sections which are particularly difficult to test.