Skip to content

Conversation

@ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Oct 24, 2025

What does this PR do?

This PR fixes the profiler's "code provenance" metadata to include the paths to gem executables (if any).

This ensures that, when e.g. starting puma via bin/puma or bundle exec puma, we still identify the puma executable as belonging to the puma gem.

Motivation:

The "code provenance" is used to power the "Only My Code" feature that shows up in multiple places in the profiler UX, and so we want it to be as accurate as possible.

Change log entry

Yes. Fix profiler not identifying executables with gems they belong to

Additional Notes:

N/A

How to test the change?

This change includes test coverage. It can also be tested easily by running

DD_PROFILING_ENABLED=true DD_SERVICE=test-provenance bundle exec ddprofrb exec pry -e "sleep 1; exit"

and checking that bin/pry is correctly categorized as belonging to the pry gem in the Datadog UX.

Here's how this looked before:

image image

and now:

image

This will allow us to add more than one extra path next.
…belong to

**What does this PR do?**

This PR fixes the profiler's "code provenance" metadata to include
the paths to gem executables (if any).

This ensures that, when e.g. starting `puma` via `bin/puma` or
`bundle exec puma`, we still identify the puma executable as
belonging to the `puma` gem.

**Motivation:**

The "code provenance" is used to power the "Only My Code" feature
that shows up in multiple places in the profiler UX, and so we want
it to be as accurate as possible.

**Additional Notes:**

N/A

**How to test the change?**

This change includes test coverage. It can also be tested easily
by running

```bash
DD_PROFILING_ENABLED=true DD_SERVICE=test-provenance bundle exec ddprofrb exec pry -e "sleep 1; exit"
```

and checking that `bin/pry` is correctly categorized as belonging
to the `pry` gem in the Datadog UX.
@ivoanjo ivoanjo requested review from a team as code owners October 24, 2025 09:29
@github-actions github-actions bot added the profiling Involves Datadog profiling label Oct 24, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
Patch Coverage: 100.00%
Total Coverage: 98.44% (-0.00%)

View detailed report

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 43bb3d3 | Docs | Was this helpful? Give us feedback!

@pr-commenter
Copy link

pr-commenter bot commented Oct 24, 2025

Benchmarks

Benchmark execution time: 2025-10-24 09:55:05

Comparing candidate commit 43bb3d3 in PR branch ivoanjo/prof-12841-code-provenance-execs with baseline commit c6a509f in branch master.

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

@ivoanjo ivoanjo merged commit 534a27f into master Oct 24, 2025
558 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-12841-code-provenance-execs branch October 24, 2025 16:16
@github-actions github-actions bot added this to the 2.23.0 milestone Oct 24, 2025
@Strech Strech mentioned this pull request Dec 11, 2025
ivoanjo added a commit that referenced this pull request Dec 15, 2025
**What does this PR do?**

This PR fixes a regression introduced in #4999: In that PR we added a
call to `Bundler.bin_path` but did not realize that bundler not only
returns the path **BUT CRUCIALLY** it makes sure the path exists
by going as far as trying to create it if it doesn't.

If the directory does not exist, and the application directory is
read-only (or the app does not have enough permissions), the
application will fail with:

> There was an error while trying to write to (directory). It is likely
> that you need to grant write permissions for that path.
> (Bundler::PermissionError)

Fixes #5137

**Motivation:**

Fix a regression introduced in #4999 that shipped in v2.23.0.

**Additional Notes:**

The bundler's `bin_path` method has had a similar
behavior/implementation for 15+ years:

* master -> https://github.com/ruby/rubygems/blob/570c3419c7f111d8665710f4e2cb15ca878e606d/bundler/lib/bundler.rb#L119-L126
* Jul 10, 2010 -> ruby/rubygems@f799489

So we should be fine in replicating its behavior + having a
test that checks we're matching it.

Worst case, we get `CodeProvenance` slightly off, which
compared to "broke the app", seems better.

I've also added a `rescue` to the method so hopefully
we don't run into more surprises here.

(Note that while we could've kept calling `Bundler.bin_path`,
wrapping it with the `rescue`, this would still produce the
side-effect of creating the folder and it seems weird to
me to end up in this situation of "turning on the profiler
causes bin/ to be created". This is why I chose to reimplement
the behavior, minus folder creation)

**How to test the change?**

I've updated the test coverage.

Furthermore, the repro mentioned in #5137 also works to
show it's fixed:

```bash
$ rm -rf bin/
$ chmod -w .
$ bundle exec rake compile && DD_PROFILING_ENABLED=true bundle exec ruby -rdatadog/profiling/preload -e "puts 'hello'"
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

profiling Involves Datadog profiling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants