Profiling: fix UB and simplify SystemSettings #3578

morrisonlevi · 2026-01-15T05:28:59Z

Description

Currently we're using MaybeUninit and doing various memory operations on SYSTEM_SETTINGS. With a tweak made in #3577, we can simplify this.

Additionally, this fixes likely undefined behavior on ZTS and possibly even NTS builds as well. There was a mutable reference taken to the SystemSettings in rinit. Note that we did not actually mutate it, so in practice we were likely fine, but this would have been instant undefined behavior simply to materialize the mutable reference if any other references existed, which in ZTS would have been quite likely (other workers could also be in rinit).

Note that there shouldn't be any meaningful changes with regards to the change in the URI default, but there is one thing in particular we should watch out for: the default uri may have changed if we have some bad logic somewhere. I'll call it out with a CR comment.

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

morrisonlevi · 2026-01-15T05:33:16Z

profiling/src/lib.rs

            git_repository_url: None,
            tags: vec![],
-            system_settings: ptr::NonNull::from(INITIAL_SYSTEM_SETTINGS.deref()),
+            system_settings: SystemSettings::get(),


Here's an example of a potential behavior divergence. Before, this would have used INITIAL_SYSTEM_SETTINGS which would have done AgentEndpoint::default(), but now, it will have the value SystemSettings uses, which could just be the socket depending on when exactly it was called. More precisely, AgentEndpoint::default() checks that the apm.socket exists before using it, and if it doesn't exist, it falls back to the URI.

Now, this shouldn't really matter in any scenarios, because we really should be changing these to real values ASAP. It's something to specifically scrutinize in the PR.

morrisonlevi · 2026-01-15T05:34:31Z

profiling/src/lib.rs

+    // Initialize the lazy lock holding the env var for new origin detection.
+    _ = std::sync::LazyLock::force(&libdd_common::entity_id::DD_EXTERNAL_ENV);


This is unrelated to this PR, but it's a safer place for it. Maybe I should make it its own PR but it's 10:35pm for me right now when I noticed this.

datadog-official · 2026-01-15T05:36:56Z

⚠️ Tests

✨ Fix all issues with Cursor

⚠️ Warnings

🧪 1022 Tests failed

testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Fix with Cursor)

testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Fix with Cursor)
testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69694cca00000000d6acdefa1a140e42
tid: 69694cca00000000
hexProcessTraceId: d6acdefa1a140e42
hexProcessSpanId: ac80597416dbd897
processTraceId: 15468983985871523394
processSpanId: 12430033326677153943
View all

ℹ️ Info

❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 4352c47 | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

codecov-commenter · 2026-01-15T05:39:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.92%. Comparing base (ed3089e) to head (4352c47).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3578      +/-   ##
==========================================
- Coverage   62.02%   61.92%   -0.11%     
==========================================
  Files         140      140              
  Lines       13309    13309              
  Branches     1762     1762              
==========================================
- Hits         8255     8241      -14     
- Misses       4265     4278      +13     
- Partials      789      790       +1

see 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed3089e...4352c47. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pr-commenter · 2026-01-15T05:43:29Z

Benchmarks [ profiler ]

Benchmark execution time: 2026-01-15 20:27:30

Comparing candidate commit 4352c47 in PR branch levi/const-system-settings with baseline commit ed3089e in branch master.

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

pr-commenter · 2026-01-15T06:36:51Z

Benchmarks [ tracer ]

Benchmark execution time: 2026-01-15 06:34:50

Comparing candidate commit 8afb20c in PR branch levi/const-system-settings with baseline commit 4e3890b in branch levi/const-agent-endpoint.

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

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

🟥 execution_time [+5.235µs; +6.385µs] or [+4.962%; +6.052%]

morrisonlevi · 2026-01-15T17:03:53Z

The key reason the stale URI won't be an issue is becauseProfiler::init, which is what leads to using the agent URI, is not called until after config::first_rinit which is where the configured URI is actually set. So the stale config should not be read and should be inert. Additionally, prior to that moment, the config should act as-if it is disabled, so there shouldn't really be samples or other things taken.

profiling/src/profiling/mod.rs

danielsn · 2026-01-15T17:23:58Z

profiling/src/config.rs

-        let addr = unsafe { (*ptr::addr_of_mut!(SYSTEM_SETTINGS)).assume_init_mut() };
-        ptr::NonNull::from(addr)
+    /// Returns the "current" system settings, which are always memory-safe
+    /// but may point to "initial" values rather than the configured ones,


should there be an "is_initial" field on the settings for debugging purposes?

I've pushed a commit with state tracking, I'll play a little bit with it but don't have time for a lot of experimentation right now.

This also serves to document some AtomicPtr operations and orderings.

realFlowControl · 2026-01-16T13:15:25Z

profiling/src/config.rs

+#[derive(Copy, Clone, Debug, Default)]
+pub enum SystemSettingsState {
+    /// Indicates the system settings are not aware of the configuration at
+    /// the moment.
+    #[default]
+    ConfigUnaware,
+
+    /// Indicates the system settings _are_ aware of configuration at the
+    /// moment.
+    ConfigAware,
+
+    /// Expressly disabled, such as a child process post-fork (forks are not
+    /// currently profiled, except certain forks made by SAPIs).
+    Disabled,
+}
+


I like that 👍

morrisonlevi added the profiling Relates to the Continuous Profiler label Jan 15, 2026

morrisonlevi commented Jan 15, 2026

View reviewed changes

realFlowControl mentioned this pull request Jan 15, 2026

Profiling: refactor AgentEndpoint so it can be const #3577

Merged

2 tasks

Base automatically changed from levi/const-agent-endpoint to master January 15, 2026 12:58

refactor(profiling): simplify SystemSettings

32db8b9

morrisonlevi force-pushed the levi/const-system-settings branch from 8afb20c to 32db8b9 Compare January 15, 2026 13:04

github-actions bot added the tracing label Jan 15, 2026

morrisonlevi marked this pull request as ready for review January 15, 2026 16:29

morrisonlevi requested a review from a team as a code owner January 15, 2026 16:29

morrisonlevi marked this pull request as draft January 15, 2026 16:30

fix(profiling): UB with SystemSettings in rinit

cfe23dd

morrisonlevi changed the title ~~Profiling: simplify SystemSettings~~ Profiling: fix UB and simplify SystemSettings Jan 15, 2026

morrisonlevi added 🐛 bug Something isn't working and removed tracing labels Jan 15, 2026

fix test to not use mut as well

1d51ce0

github-actions bot added the tracing label Jan 15, 2026

morrisonlevi marked this pull request as ready for review January 15, 2026 17:20

danielsn reviewed Jan 15, 2026

View reviewed changes

extract Profiler::is_timeline_enabled helper

f6261b9

This also serves to document some AtomicPtr operations and orderings.

morrisonlevi force-pushed the levi/const-system-settings branch from a7b55fc to b6efbb8 Compare January 15, 2026 20:06

experiment with SystemSettingsState tracking

4352c47

morrisonlevi force-pushed the levi/const-system-settings branch from b6efbb8 to 4352c47 Compare January 15, 2026 20:11

morrisonlevi mentioned this pull request Jan 16, 2026

Profiling: fix SystemSettings initialization #3579

Merged

2 tasks

realFlowControl reviewed Jan 16, 2026

View reviewed changes

realFlowControl approved these changes Jan 16, 2026

View reviewed changes

morrisonlevi merged commit 48c4254 into master Jan 16, 2026
2001 of 2007 checks passed

morrisonlevi deleted the levi/const-system-settings branch January 16, 2026 21:07

github-actions bot added this to the 1.16.0 milestone Jan 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profiling: fix UB and simplify SystemSettings #3578

Profiling: fix UB and simplify SystemSettings #3578

Uh oh!

morrisonlevi commented Jan 15, 2026 •

edited

Loading

Uh oh!

morrisonlevi Jan 15, 2026

Uh oh!

morrisonlevi Jan 15, 2026 •

edited

Loading

Uh oh!

datadog-official bot commented Jan 15, 2026 •

edited by datadog-datadog-prod-us1 bot

Loading

Uh oh!

codecov-commenter commented Jan 15, 2026 •

edited

Loading

Uh oh!

pr-commenter bot commented Jan 15, 2026 •

edited

Loading

Uh oh!

pr-commenter bot commented Jan 15, 2026

Uh oh!

morrisonlevi commented Jan 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

danielsn Jan 15, 2026

Uh oh!

morrisonlevi Jan 15, 2026

Uh oh!

realFlowControl Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		// Initialize the lazy lock holding the env var for new origin detection.
		_ = std::sync::LazyLock::force(&libdd_common::entity_id::DD_EXTERNAL_ENV);

Profiling: fix UB and simplify SystemSettings #3578

Profiling: fix UB and simplify SystemSettings #3578

Uh oh!

Conversation

morrisonlevi commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer checklist

Uh oh!

morrisonlevi Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

morrisonlevi Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datadog-official bot commented Jan 15, 2026 • edited by datadog-datadog-prod-us1 bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

ℹ️ Info

Uh oh!

codecov-commenter commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ profiler ]

Uh oh!

pr-commenter bot commented Jan 15, 2026

Benchmarks [ tracer ]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

Uh oh!

morrisonlevi commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

danielsn Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

morrisonlevi Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

realFlowControl Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

morrisonlevi commented Jan 15, 2026 •

edited

Loading

morrisonlevi Jan 15, 2026 •

edited

Loading

datadog-official bot commented Jan 15, 2026 •

edited by datadog-datadog-prod-us1 bot

Loading

codecov-commenter commented Jan 15, 2026 •

edited

Loading

pr-commenter bot commented Jan 15, 2026 •

edited

Loading

morrisonlevi commented Jan 15, 2026 •

edited

Loading