dd-trace-php icon indicating copy to clipboard operation
dd-trace-php copied to clipboard

[Bug] OpenTracing compatibility with php 8.1

Open greg0ire opened this issue 2 years ago • 8 comments

Bug description

When attempting to use this library with a recent version of opentracing/opentracing in order to support PHP 8.1, it crashes with

PHP Fatal error: Declaration of DDTrace\OpenTracer\Tracer::getScopeManager() must be compatible with OpenTracing\Tracer::getScopeManager(): OpenTracing\ScopeManager in /opt/datadog-php/dd-trace-sources/src/DDTrace/OpenTracer/Tracer.php

Note that to support PHP 8.1, it is required to use opentracing 1.0.2 at least.

PHP version

8.1.2

Tracer version

0.68.1

Installed extensions

[PHP Modules]
amqp
bcmath
calendar
Core
ctype
curl
date
ddtrace
dom
exif
FFI
fileinfo
filter
ftp
gd
gettext
hash
iconv
igbinary
intl
json
libxml
mbstring
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
pdo_pgsql
pgsql
Phar
posix
readline
redis
Reflection
session
shmop
SimpleXML
soap
sockets
sodium
SPL
standard
sysvmsg
sysvsem
sysvshm
tokenizer
xml
xmlreader
xmlrpc
xmlwriter
xsl
yaml
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache
ddtrace

OS info

PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

Diagnostics and configuration

Output of phpinfo() (ddtrace >= 0.47.0)

No easy way to do this, sorry.

Upgrading info

From PHP 7.4.12 to 8.1.3

Since this library supports php 5.4, this does not seem very easy to fix to say the least, but maybe you already have ways to deal with such situations?

greg0ire avatar Mar 02 '22 17:03 greg0ire

php 5.4 went EOL more than six years ago, maybe it's time to drop support

hauke725 avatar Mar 03 '22 09:03 hauke725

Either that or maintain 2 branches in parallel. I suppose Datadog is supposed to support a very, very wide range of PHP versions.

greg0ire avatar Mar 03 '22 10:03 greg0ire

You have to use the wrapper DDTrace\OpenTracer1 instead of DDTrace\OpenTracer. Class names don't change, only the namespace that you use.

labbati avatar Mar 03 '22 11:03 labbati

Should we close this then?

greg0ire avatar Mar 03 '22 13:03 greg0ire

I am leaving it here because this caused a crash and our solution (two namespaces) must be improved. We are thinking about it. One possibility (still in discussion) is to move the open tracing wrapper with different release lines for the different versions and PHP version enforced in the composer file. What do you think about it @greg0ire ?

labbati avatar Mar 08 '22 10:03 labbati

It would be great to have the right signatures transparently, depending on the version of PHP. Having different release lines (and different branches?) sounds like the most sensible solution. It might even be possible to use Rector to generate one branch from the other.

greg0ire avatar Mar 08 '22 16:03 greg0ire

Removing sev-1 as the mitigation has been provided.

labbati avatar Jul 05 '22 09:07 labbati

Here is an example of PHPStan downgrading its codebase with Rector: https://github.com/phpstan/phpstan-src/blob/f1fd385433480f87bdda1d7d8ff6887e25f003ba/build/rector-downgrade.php

greg0ire avatar Jul 05 '22 10:07 greg0ire