Integrate dd-win-prof profiling feature into dd-sdk-cpp by r1viollet · Pull Request #185 · DataDog/dd-sdk-cpp

r1viollet · 2026-03-16T16:25:10Z

Description

CMake: DD_ENABLE_PROFILER option, FetchContent for dd-win-prof
Public API: Profiling::Register() following Rum/Logging patterns
RUM context: RumContextSnapshot + callback wiring to dd-win-prof
Stub UploadThread_PrepareReport (dd-win-prof uploads independently)

Inspired from @aforsythe 's branch & co-authored using claude-4.6

Testing

e2e tests are ongoing
leaving as draft until I can add a test for the profiling integration

Follow up

Config needs to be unified
Profiling needs to work with tokens
e2e testing

Related PR

dd-win-prof PR

@aforsythe

- CMake: DD_ENABLE_PROFILER option, FetchContent for dd-win-prof - Public API: Profiling::Register() following Rum/Logging patterns - RUM context: RumContextSnapshot + callback wiring to dd-win-prof - Stub UploadThread_PrepareReport (dd-win-prof uploads independently) Inspired from @aforsythe 's branch & co-authored using claude-4.6

datadog-datadog-prod-us1-2 · 2026-03-16T16:25:22Z

✅ Code Quality ✅ Code Vulnerabilities ✅ Library Vulnerabilities ✅ Secrets

🎉 All green!

🛠️ No new code quality issues
🛡️ No new code vulnerabilities
📚 No new vulnerable libraries detected
🔑 No new secrets detected

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

🔗 Commit SHA: 23f5263 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

Automatically wire profiling callback if RUM & profiling are active co-authored using claude-4.6

r1viollet · 2026-03-17T10:35:15Z

+    FetchContent_Declare(
+        dd-win-prof
+        GIT_REPOSITORY https://github.com/DataDog/dd-win-prof.git
+        GIT_TAG        74b9c6f


this could be a param from CI

we can add this in later versions

r1viollet · 2026-03-17T10:53:40Z

+
+  // Wire RUM context changes → profiling: capture a weak_ptr so the callback
+  // doesn't extend the profiling feature's lifetime
+  std::weak_ptr<Profiling> weak_profiling = profiling_impl;


Do we feel this is OK ?

@awforsythe I would need your advice on these parts

While the direct coupling of these two features isn't the cleanest approach, I think it's fine for now - it's an internal detail that can be changed without much disruption (AFAIK), and this code is only exercised in an opt-in, explicitly-experimental case, so I think it's relatively harmless to leave as-is for this PR.

As of a couple of recently-merged PRs (#180 and #181), the SDK now has a message bus that allows features to receive asynchronous notifications for things like RUM context changes, so the actual design would likely involve implementing MakeMessageHandler for Profiling, returning a function that handles ContextChangedMessage and takes the appropriate action when ctx.rum has changed.

That would obviate the need for SetContextChangeCallback (and direct coupling of features via the Core) entirely; but it's not pressing for this PR.

- remove interning of profiling config - add more granular change detection on context changes

r1viollet · 2026-03-17T16:56:45Z

+
+Create a `.env` file in this directory (git-ignored):
+
+```


for now we have some redundant config

- Skip SetRumSession when session hasn't changed - Skip SetRumView when view hasn't changed - Call ClearRumContext on session end (application_id == Zero) - Cache UUID string representations to avoid repeated allocations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…rofiling-integration

r1viollet · 2026-03-17T22:28:14Z

      // command; write the relevant UUIDs to the global RumFeatureContext, so that
      // other features can enrich their events with RUM data
-      ctx.rum = rum->_application_snapshot.ToFeatureContext();
+      const RumFeatureContext new_context =


this could be optimized further, for now there would be allocation on large view names

- cleanup const cast - allow restart after stop

fuzzybinary · 2026-03-19T21:25:17Z

  DD_SITE_AP1,
  DD_SITE_AP2,
  DD_SITE_US1_FED,
+  DD_SITE_STAGING,


I am not sure we want to commit staging as client usable option....

I'm fine with leaving this out, we still need a good way for us to test, what would be the suggestion ?

I think I still had issues with getting the data in staging, even with this (the profiling data was fine though).
If you can deliver something that gets the data to staging, I'm happy to remove this part of the code.

Instead of adding a new site enum and calling config.SetSite(datadog::Site::staging) etc.; I think you could just do config.Internal_UseCustomEndpoint("https://browser-intake-datad0g.com") to circumvent site-based URL resolution entirely. Would that work for your needs?

fuzzybinary · 2026-03-19T21:26:49Z

+#ifdef DD_ENABLE_PROFILER
+#include "datadog/impl/features/profiling/profiling.hpp"


I'll let you and Alex hash this out but I'd rather have this be DD_ENABLE_PROFILING only because DD_ENABLE_PROFILER sounds like we'll enabling profiling the SDK, not the enduser's application.

I agree, simply because we're calling the feature Profiling, so using that name verbatim feels clearer.

awforsythe · 2026-04-08T18:23:01Z

It's been a while since the original base commit, and since then I've made a bunch of changes in main that have resulted in somewhat gnarly merge conflicts. Namely:

feat: Add context thread, enqueue all RUM and Logging operations for async processing #169 added a context thread, changed the contract between features and core, and made access and mutation of CoreContext asynchronous
feat: Include RUM context in crash reports #182 made the CrashReporting feature use RumFeatureContext, and added its own comparison operators to that type
chore: Reorganize impl/ to better reflect architectural boundaries #211 (in aforsythe/chore/reorganize-impl, not yet merged to main) reorganizes the source tree, requiring include-path fixups and making datadog/src/impl/profiling/ the correct path for a profiling feature module

I didn't want to leave this PR in the lurch, so I made a copy of the branch (as aforsythe/rebase/profiling-integration), rebased it onto aforsythe/chore/reorganize-impl, resolved all the conflicts, and made sure it was building OK on Windows with -DDD_ENABLE_PROFILER=ON.

aforsythe/chore/reorganize-impl...aforsythe/rebase/profiling-integration

I don't have spare time at the moment to address any of the review notes, check that the smoke test is working, or implement the longer-term fix that's now possible thanks to the addition of a message bus (i.e. updating Profiling to handle ContextChangedMessage instead of having the Core directly wire Rum and Profiling together via a callback), but I'd be happy to take over some or all of those changes in the near future if needed.

In the meantime, let know if you'd like me to force-push my rebase back to origin/r1viollet/profiling-integration and help get this PR merged as-is.

awforsythe · 2026-04-14T12:35:34Z

As we discussed in Slack: I'll open a new PR with my rebased changes. (It may be a day or two.)

Simplify RUM x Profiling setup

1677e63

Automatically wire profiling callback if RUM & profiling are active co-authored using claude-4.6

r1viollet force-pushed the r1viollet/profiling-integration branch from 9c24469 to a1eea38 Compare March 17, 2026 10:34

r1viollet commented Mar 17, 2026

View reviewed changes

RUM x Profiling: add an example

18a7494

r1viollet force-pushed the r1viollet/profiling-integration branch from a1eea38 to 18a7494 Compare March 17, 2026 10:42

r1viollet commented Mar 17, 2026

View reviewed changes

Comment thread src/datadog/impl/features/profiling/profiling.cpp Outdated

r1viollet commented Mar 17, 2026

View reviewed changes

Comment thread src/datadog/impl/features/profiling/profiling.cpp Outdated

r1viollet commented Mar 17, 2026

View reviewed changes

Comment thread src/datadog/impl/core/feature_types/rum.hpp Outdated

RUM x Profiling: profiling integration

c5f59c0

- remove interning of profiling config - add more granular change detection on context changes

r1viollet force-pushed the r1viollet/profiling-integration branch from 044e1ed to c5f59c0 Compare March 17, 2026 16:50

r1viollet commented Mar 17, 2026

View reviewed changes

r1viollet marked this pull request as ready for review March 17, 2026 17:01

r1viollet requested review from a team as code owners March 17, 2026 17:01

r1viollet and others added 2 commits March 17, 2026 18:07

Add a staging endpoint

9517679

joepeeples previously approved these changes Mar 17, 2026

View reviewed changes

r1viollet added 2 commits March 17, 2026 23:14

RUM x Profiling: Handle end of sessions

327faa3

Merge branch 'main' of github.com:DataDog/dd-sdk-cpp into r1viollet/p…

fd53e04

…rofiling-integration

r1viollet dismissed joepeeples’s stale review via fd53e04 March 17, 2026 22:23

r1viollet commented Mar 17, 2026

View reviewed changes

RUM x Profiling: minor adjustments

23f5263

- cleanup const cast - allow restart after stop

fuzzybinary reviewed Mar 19, 2026

View reviewed changes

fuzzybinary requested a review from awforsythe March 26, 2026 19:49

awforsythe mentioned this pull request Mar 26, 2026

feat: Add dd-win-prof as a dependency, built on Windows when DD_ENABLE_PROFILER=ON #157

Closed

awforsythe mentioned this pull request Apr 20, 2026

Integrate dd-win-prof as a Feature (rebased) #228

Draft

		#ifdef DD_ENABLE_PROFILER
		#include "datadog/impl/features/profiling/profiling.hpp"

Uh oh!

Conversation

r1viollet commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Follow up

Related PR

Uh oh!

datadog-datadog-prod-us1-2 Bot commented Mar 16, 2026 • edited by datadog-prod-us1-5 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awforsythe commented Apr 8, 2026

Uh oh!

awforsythe commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

r1viollet commented Mar 16, 2026 •

edited

Loading

datadog-datadog-prod-us1-2 Bot commented Mar 16, 2026 •

edited by datadog-prod-us1-5 Bot

Loading