Skip to content

[PROF-13732] Enable testing libdatadog-based features on macOS #5351

Merged
ivoanjo merged 30 commits intomasterfrom
ivoanjo/prof-13732-improve-macos-testing
Mar 16, 2026
Merged

[PROF-13732] Enable testing libdatadog-based features on macOS #5351
ivoanjo merged 30 commits intomasterfrom
ivoanjo/prof-13732-improve-macos-testing

Conversation

@ivoanjo
Copy link
Copy Markdown
Member

@ivoanjo ivoanjo commented Feb 12, 2026

What does this PR do?

With the upgrade to libdatadog v27 in #5274, we finally have libdatadog builds on rubygems.org. This PR adjusts our testing logic to:

  1. Assume that if you're on macOS libdatadog is now available (and fail if it isn't)
  2. Enable testing with libdatadog in CI

I ended up also de-duplicating some of our helper code, hopefully simplifying how we do this kind of integration.

Motivation:

Make sure libdatadog features are working and keep working on macOS.

Change log entry

Yes. Enable libdatadog-based features on macOS

Additional Notes:

This PR is on top of #5274, so we can only merge it once that one's in good shape.

I don't have a macOS machine available myself, so please let me know if I missed a spot!

How to test the change?

The existing tests are now executed and not skipped on macOS so that should be the validation :)

@ivoanjo ivoanjo requested review from a team as code owners February 12, 2026 12:51
@ivoanjo ivoanjo requested review from vpellan and removed request for a team February 12, 2026 12:51
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Feb 12, 2026

Benchmarks

Benchmark execution time: 2026-03-16 15:36:22

Comparing candidate commit 9baf8ac in PR branch ivoanjo/prof-13732-improve-macos-testing with baseline commit 1fc7720 in branch master.

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Feb 12, 2026

TODO for myself : Since this branch is not (yet) targeting master, most of CI checks are not running. Once #5274 is merged and this branch I should check that the extra tests are successfully running on our macOS CI jobs.

Copy link
Copy Markdown
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Super exciting!

@gyuheon0h gyuheon0h force-pushed the ivoanjo/libdatadog-26-ruby branch 2 times, most recently from 29113a3 to 30c93b0 Compare February 24, 2026 12:39
@gyuheon0h gyuheon0h force-pushed the ivoanjo/libdatadog-26-ruby branch 4 times, most recently from 913e09d to 9330399 Compare March 2, 2026 18:47
Base automatically changed from ivoanjo/libdatadog-26-ruby to master March 2, 2026 21:34
ivoanjo added 8 commits March 3, 2026 10:08
Use of libdatadog will make this helper more and more common so let's
put it in a central location.
I chose to keep the `skip_if_libdatadog_not_supported` as I like seeing
the skip on the rspec output but this is very debatable so if you're
unconvinced I don't mind too much going the other way.
Now that we do have separate libdatadog testing, it's time to decouple
this from profiling!
@ivoanjo ivoanjo force-pushed the ivoanjo/prof-13732-improve-macos-testing branch from 3a574e5 to 897c78c Compare March 3, 2026 10:09
@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 3, 2026

Update: Rebased this on top of current master (to pull in latest libdatadog v28)

ivoanjo added 2 commits March 3, 2026 10:29
This avoids the previous approach of manually keeping them in sync (and
we were missing the new `libdatadog_extconf_helpers_spec.rb`).

I've also removed any duplicates.
I think it's safe to assume folks on macOS on arm64 are not running 10
year old compilers.
@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 3, 2026

Now that we have this up-and-running in CI I've found a new more things that need work (which is why "Test macOS" is failing) so I'm moving this to draft while I work on them

@ivoanjo ivoanjo marked this pull request as draft March 3, 2026 14:35
ivoanjo added 4 commits March 3, 2026 16:10
Fix

```
../../../../ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c:245:14: error: unused function 'rescued_after_gvl_running_from_postponed_job' [-Werror,-Wunused-function]
  245 | static VALUE rescued_after_gvl_running_from_postponed_job(VALUE self_instance);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c:1499:14: error: unused function 'handle_sampling_failure_rescued_after_gvl_running_from_postponed_job' [-Werror,-Wunused-function]
 1499 | static VALUE handle_sampling_failure_rescued_after_gvl_running_from_postponed_job(VALUE self_instance, VALUE exception) {
      |
```
BIG thanks to @y9v for helping me debug this!
@ivoanjo ivoanjo force-pushed the ivoanjo/prof-13732-improve-macos-testing branch from 2830ed2 to 0474675 Compare March 3, 2026 16:18
@github-actions github-actions Bot added the core Involves Datadog core libraries label Mar 3, 2026
ivoanjo added 4 commits March 3, 2026 16:24
Fix

```
../../../../ext/datadog_profiling_native_extension/collectors_thread_context.c:293:14: error: unused function '_native_on_gvl_waiting' [-Werror,-Wunused-function]
  293 | static VALUE _native_on_gvl_waiting(DDTRACE_UNUSED VALUE self, VALUE thread);
      |              ^~~~~~~~~~~~~~~~~~~~~~
../../../../ext/datadog_profiling_native_extension/collectors_thread_context.c:294:14: error: unused function '_native_gvl_waiting_at_for' [-Werror,-Wunused-function]
  294 | static VALUE _native_gvl_waiting_at_for(DDTRACE_UNUSED VALUE self, VALUE thread);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/datadog_profiling_native_extension/collectors_thread_context.c:295:14: error: unused function '_native_on_gvl_running' [-Werror,-Wunused-function]
  295 | static VALUE _native_on_gvl_running(DDTRACE_UNUSED VALUE self, VALUE thread);
      |              ^~~~~~~~~~~~~~~~~~~~~~
../../../../ext/datadog_profiling_native_extension/collectors_thread_context.c:296:14: error: unused function '_native_sample_after_gvl_running' [-Werror,-Wunused-function]
  296 | static VALUE _native_sample_after_gvl_running(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE thread, VALUE allow_exception);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/datadog_profiling_native_extension/collectors_thread_context.c:297:14: error: unused function '_native_apply_delta_to_cpu_time_at_previous_sample_ns' [-Werror,-Wunused-function]
  297 | static VALUE _native_apply_delta_to_cpu_time_at_previous_sample_ns(DDTRACE_UNUSED VALUE self, VALUE collector_instance, VALUE thread, VALUE delta_ns);
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
This isn't working because of the wrong DYLIB issue + I suspect the
rpath on the receiver is not correct, so for now we're disabling it
and maybe we'll timebox a fix later.
@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 4, 2026

So right now the macOS tests are passing by using the hacks in extconf.rb but we might wait until the actual upstream libdatadog fix lands in DataDog/libdatadog#1646 so we can remove them.

@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 13, 2026

Update: Libdatadog v29 is now on rubygems.org; once master gets moved to it I expect we'll finally be ready to land this PR

ivoanjo added a commit that referenced this pull request Mar 16, 2026
**What does this PR do?**

This PR bumps the libdatadog dependency from version 28.0.2.1.0 to
29.0.0.1.0.

This new version brings:

* macOS build fixes needed to unblock #5351
* libdatadog support for the
  [OTel process context](open-telemetry/opentelemetry-specification#4719)
  (I plan to submit a PR with some integration for testing this
  separately -- commit is already waiting)

**Motivation:**

Adopt latest libdatadog.

**Change log entry**

Yes. Upgrade libdatadog dependency to version 29.0.0

**Additional Notes:**

N/A

**How to test the change?**

Green CI is good, as usual.
@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 16, 2026

We're finally good to go! macOS testing all green!

@ivoanjo ivoanjo marked this pull request as ready for review March 16, 2026 15:40
@ivoanjo
Copy link
Copy Markdown
Member Author

ivoanjo commented Mar 16, 2026

I'm going to go ahead and merge this one: this PR hasn't substantially changed from when it was initially reviewed, the only difference was merging in master which brought in libdatadog 29 which fixed all the issues we saw before.

@ivoanjo ivoanjo merged commit 0c664fd into master Mar 16, 2026
631 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-13732-improve-macos-testing branch March 16, 2026 16:36
@github-actions github-actions Bot added this to the 2.30.0 milestone Mar 16, 2026
ivoanjo added a commit that referenced this pull request Mar 16, 2026
This was changed in #5351
which was concurrently worked on with this PR.
@Strech Strech mentioned this pull request Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Involves Datadog core libraries profiling Involves Datadog profiling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants