Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(profiling): issues when preloading and non-root php-fpm user are used #1975

Merged
merged 20 commits into from
Mar 22, 2023

Conversation

morrisonlevi
Copy link
Collaborator

@morrisonlevi morrisonlevi commented Mar 16, 2023

Description

Fixes #1919. An issue occurred when preloading and a non-root user for php-fpm were combined, and eventually php-fpm would hang when the Profiler's channels were full. The full channel was worked around in a previous commit that landed in dd-trace-php v0.85.0, but this PR addresses the core issue so that profiles will actually get sent. Sampling will no longer occur when preloading is happening.

This adds a CFG_PRELOAD define for C and cfg(php_preload) for Rust, which match. They are made by the build.rs script, and are based on PHP version (PHP 7.4+ has preloading).

Bumps the profiling version to v0.15.0.

In CI, this changes the cargo build and cargo test type of jobs to use devtoolset-7 if it's present. This was required in a previous version of the PR which used stdatomic.h, but the newer toolchain is likely preferred anyway, so I've kept it even though it's no longer required.

Readiness checklist

  • Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.

@morrisonlevi morrisonlevi added 🐛 bug Something isn't working cat:app-crash profiling Relates to the Continuous Profiler labels Mar 16, 2023
@morrisonlevi morrisonlevi added this to the 0.86.0 milestone Mar 16, 2023
@morrisonlevi morrisonlevi changed the title fix(profiling): hang while preloading and non-root php-fpm user are used fix(profiling): issues when preloading and non-root php-fpm user are used Mar 16, 2023
@morrisonlevi morrisonlevi force-pushed the levi/preload branch 2 times, most recently from e45fc33 to 7197b03 Compare March 16, 2023 22:50
profiling/src/php_ffi.c Outdated Show resolved Hide resolved
profiling/build.rs Outdated Show resolved Hide resolved
@morrisonlevi morrisonlevi marked this pull request as ready for review March 20, 2023 20:21
@morrisonlevi morrisonlevi requested review from a team as code owners March 20, 2023 20:21
@realFlowControl realFlowControl merged commit a6794d0 into master Mar 22, 2023
@realFlowControl realFlowControl deleted the levi/preload branch March 22, 2023 08:24
@Anilm3 Anilm3 mentioned this pull request Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working cat:app-crash profiling Relates to the Continuous Profiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] FPM shows warnings about rust errors when activating profiler
3 participants