Skip to content

Conversation

@Girgias
Copy link

@Girgias Girgias commented Dec 28, 2025

Description

I changed the signature of _php_error_log() and removed _php_error_log_ex() on php-src@master and thinking of fully removing the exposed API as I can only find one other usage of it outside of dd-trace which is also always calling _php_error_log() with the same mode.

So rather than doing effectively a function indirection call php_log_err() directly (this is a compat macro on master, but my understanding is you support PHP 7.0 still)

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@Girgias Girgias marked this pull request as ready for review December 28, 2025 22:53
@Girgias Girgias requested a review from a team as a code owner December 28, 2025 22:53
realFlowControl added a commit that referenced this pull request Dec 29, 2025
@realFlowControl
Copy link
Member

Thanks @Girgias ❤️
I've triggered our internal GitLab CI job on this PR and will merge once that's green.

@bwoebi
Copy link
Collaborator

bwoebi commented Jan 2, 2026

Hey @Girgias,
This will break on PHP 5 and PHP 7.0. On PHP 7.1+ this is a macro which decays to php_log_err_with_severity, a symbol which did not exist before PHP 7.1.

The problem is that this is a version agnostic zend_extension which we build once - and reuse across all versions (as you can see, it has version checks - and then loads the appropriate ddtrace.so).
(Also, we don't support PHP 5 for ddtrace, but this particular extension is injected regardless of the PHP version, so it just does nothing, but that still requires all symbols to exist.)

If this symbol gets removed on master, the proper fix is using dlsym stuff with disambiguation depending on version number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants