DataDog / dd-trace-php

Datadog PHP Clients
https://docs.datadoghq.com/tracing/setup/php
Other
496 stars 155 forks source link

Tracer is changing $this parameter in call_user_func_array #374

Closed alexsegura closed 5 years ago

alexsegura commented 5 years ago

Hi,

I'm using Debian & PHP 7.2 FPM

The tracer is screwing up some Guzzle calls. When I uninstall the tracer, the error goes away.

I got the error URI must be a string or UriInterface

#0 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(145): GuzzleHttp\Psr7\uri_for(Object(GuzzleHttp\Psr7\Request))
#1 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(117): GuzzleHttp\Client->buildUri(Object(GuzzleHttp\Psr7\Request), Array)
#2 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(131): GuzzleHttp\Client->requestAsync('transfer', Object(GuzzleHttp\Psr7\Request), Array)
#3 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(89): GuzzleHttp\Client->request('transfer', Object(GuzzleHttp\Psr7\Request), Array)
#4 [internal function]: GuzzleHttp\Client->__call('transfer', Array)
#5 /opt/datadog-php/dd-trace-sources/src/DDTrace/Util/CodeTracer.php(50): call_user_func_array(Array, Array)
#6 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(125): Mailjet\Request->DDTrace\Util\{closure}(Object(GuzzleHttp\Psr7\Request), Array)
#7 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(131): GuzzleHttp\Client->requestAsync('post', Object(GuzzleHttp\Psr7\Uri), Array)
#8 /var/www/demo/releases/224/vendor/guzzlehttp/guzzle/src/Client.php(89): GuzzleHttp\Client->request('post', 'https://api.mai...', Array)
#9 [internal function]: GuzzleHttp\Client->__call('post', Array)
#10 /var/www/demo/releases/224/vendor/mailjet/mailjet-apiv3-php/src/Mailjet/Request.php(89): call_user_func_array(Array, Array)
#11 /var/www/demo/releases/224/vendor/mailjet/mailjet-apiv3-php/src/Mailjet/Client.php(120): Mailjet\Request->call(true)
#12 /var/www/demo/releases/224/vendor/mailjet/mailjet-apiv3-php/src/Mailjet/Client.php(162): Mailjet\Client->_call('POST', 'send', '', Array)
#13 /var/www/demo/releases/224/vendor/mailjet/mailjet-swiftmailer/SwiftMailer/MailjetTransport.php(137): Mailjet\Client->post(Array, Array)
#14 /var/www/demo/releases/224/vendor/swiftmailer/swiftmailer/lib/classes/Swift/MemorySpool.php(91): Mailjet\MailjetSwiftMailer\SwiftMailer\MailjetTransport->send(Object(Swift_Message), Array)
#15 /var/www/demo/releases/224/vendor/symfony/swiftmailer-bundle/EventListener/EmailSenderListener.php(61): Swift_MemorySpool->flushQueue(Object(Mailjet\MailjetSwiftMailer\SwiftMailer\MailjetTransport))
#16 /var/www/demo/releases/224/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php(212): Symfony\Bundle\SwiftmailerBundle\EventListener\EmailSenderListener->onTerminate(Object(Symfony\Component\HttpKernel\Event\PostResponseEvent), 'kernel.terminat...', Object(Symfony\Component\EventDispatcher\EventDispatcher))
#17 /var/www/demo/releases/224/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/EventDispatcher.php(44): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(Array, 'kernel.terminat...', Object(Symfony\Component\HttpKernel\Event\PostResponseEvent))
#18 [internal function]: Symfony\Component\EventDispatcher\EventDispatcher->dispatch('kernel.terminat...', Object(Symfony\Component\HttpKernel\Event\PostResponseEvent))
#19 /opt/datadog-php/dd-trace-sources/src/DDTrace/Util/TryCatchFinally.php(31): call_user_func_array(Array, Array)
#20 /opt/datadog-php/dd-trace-sources/src/DDTrace/Integrations/Symfony/V4/SymfonyBundle.php(143): DDTrace\Util\TryCatchFinally::executePublicMethod(Object(DDTrace\Scope), Object(Symfony\Component\EventDispatcher\EventDispatcher), 'dispatch', Array)
#21 /var/www/demo/releases/224/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php(87): Symfony\Component\EventDispatcher\EventDispatcher->DDTrace\Integrations\Symfony\V4\{closure}('kernel.terminat...', Object(Symfony\Component\HttpKernel\Event\PostResponseEvent))
#22 /var/www/demo/releases/224/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php(163): Symfony\Component\HttpKernel\HttpKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Symfony\Component\HttpFoundation\RedirectResponse))
#23 /var/www/demo/releases/224/web/app.php(26): Symfony\Component\HttpKernel\Kernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Symfony\Component\HttpFoundation\RedirectResponse))
#24 {main}

The code involves mailjet-apiv3-php

For a strange reason, the parameter which should be passed to requestAsync is changed at execution.

https://github.com/mailjet/mailjet-apiv3-php/blob/346a7726f57a81819e0853dde3297eb44bf1c476/src/Mailjet/Request.php#L85-L95

Can this be related to #149, #224 & #303?

Thanks!

alexsegura commented 5 years ago

By the way, the whole thing is wrapped in a register_shutdown_function, because it is an email sending, which is executed during the kernel.terminate event in Symfony

labbati commented 5 years ago

Hi! sorry for the late reply! We believe this issue is related to what we are doing in #303, as you already noticed based on the fact that the PR was already linked.

This may also be the same issue that causes in #365.

Work on #303 is currently in progress and we will make sure to notify you once this is merged.

alexsegura commented 5 years ago

Ok thank you.

Unfortunately, due to this bug I'm unable to fully experiment DataDog APM capabilities during the 14 days free trial ☹️

Is there a workaround to disable the tracer for this part of the code? In #365 I seen DD_INTEGRATIONS_DISABLED=curl, would this work?

labbati commented 5 years ago

Hi! You can temporarily disable guzzle: DD_INTEGRATIONS_DISABLED=guzzle. For the free trial expiration please reach out to me on our public slack workspace datadoghq.slack.com

labbati commented 5 years ago

Just to clarify! We are going to look into this as soon as possible and disabling the integration is not the solution. Just a temporary workaround if you like to evaluate or use other tracer integrations.

Daursu commented 5 years ago

Having the exact same issue, when I tried with any version >= 0.15.1

The issue doesn't seem to manifest in 0.13.4, which is what I ended up using temporarily, however this restricts me upgrading to Laravel 5.8

Another thing I've noticed is that not every guzzle call has this issue. In my case only one specific guzzle request failed with the above error, although all the calls are using the same classes / underlying guzzle implementation.

labbati commented 5 years ago

Thanks for reporting this as well. And useful to know that the change that broke things happened between 0.13.4 and 0.15.1. We have this in our radar and will work on it ASAP.

SammyK commented 5 years ago

Hey @alexsegura! Thanks for such a detailed report - it made replicating the issue on our end much easier. I just wanted to give you an update that #284 & #303 should get rolled out into a release next week which should fix this issue. We'll let you know when the release is available! :)

alexsegura commented 5 years ago

Thank you @SammyK Can you please close this Issue when it is released, so that I get notified? 🙂

SammyK commented 5 years ago

@alexsegura Absolutely - will do! :)

SammyK commented 5 years ago

Hey @alexsegura & @Daursu! We just released 0.21.0 of the PHP tracer which should fix this issue. I'll go ahead and close this issue, but please let us know if you're still having and issue after upgrading and we can reopen. :)