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

Internal function tracing with callbacks failing due to private function #149

Closed
NickStallman opened this issue Nov 28, 2018 · 12 comments
Closed
Assignees
Labels
🐛 bug Something isn't working c-extension Apply this label to issues and prs related to the C-extension

Comments

@NickStallman
Copy link

This is slightly related to issue #113

It looks like there is a problem with tracing a function that uses callbacks, if that callback is private.
Specifically I am using a PHP S3 class "tpyo/amazon-s3-php-class" which is using the CURLOPT_HEADERFUNCTION feature.
curl_exec will execute the HEADERFUNCTION callback during data transmission.

As the callback is a private function of the S3 class, adding dd_trace to curl_exec throws these errors:

Message: Invalid callback S3Request::__responseHeaderCallback, cannot access private method S3Request::__responseHeaderCallback()
Message: curl_exec(): Could not call the CURLOPT_HEADERFUNCTION

So dd_trace is changing the scope and function visibility when curl_exec is run.

Curiously it doesn't appear to be all callbacks. I tried creating the below test script but it seems to function correctly.

class a
{
        function __construct()
        {
                call_user_func(array($this, 'test'));
        }

        private function test()
        {
                echo "Hello World\n";
        }
}

dd_trace('call_user_func', function ($callback) {
        call_user_func($callback);
});

new a();
@labbati labbati added 🐛 bug Something isn't working 🍏 core Changes to the core tracing functionality labels Nov 28, 2018
@labbati
Copy link
Member

labbati commented Nov 29, 2018

@pawelchcki any thoughts on this?

@pawelchcki
Copy link
Contributor

pawelchcki commented Dec 5, 2018

Hey @NickStallman sorry for late response. Were you by any chance able to reproduce it in minimal setup?
So far I wasn't able to reproduce it myself.

Is the problem showing it the same PHP version (7.1.24) and modules set as described in #113 ?

@NickStallman
Copy link
Author

Yep exactly the same configuration with tracer datadog-php-tracer_0.4.2-beta_amd64.

Here is a short test script that reliably produces the error.
Without dd_trace it executes perfectly, with dd_trace it fails with the error below:

Called curl_exec
PHP Warning:  Invalid callback a::headers, cannot access private method a::headers() in /root/test2.php on line 21

Warning: Invalid callback a::headers, cannot access private method a::headers() in /root/test2.php on line 21
PHP Warning:  curl_exec(): Could not call the CURLOPT_HEADERFUNCTION in /root/test2.php on line 21

Warning: curl_exec(): Could not call the CURLOPT_HEADERFUNCTION in /root/test2.php on line 21
Finished curl_exec
<?php

class a
{
        function __construct()
        {
                $ch = curl_init('https://www.datadog.com/');
                curl_setopt($ch, CURLOPT_HEADERFUNCTION, array($this, 'headers'));
                $data = curl_exec($ch);
        }

        private function headers($ch, $headers)
        {
                var_dump($headers);
                return strlen($headers);
        }
}

dd_trace('curl_exec', function ($ch) {
        echo "Called curl_exec\n";
        curl_exec($ch);
        echo "Finished curl_exec\n";
});

new a();

@pawelchcki
Copy link
Contributor

Thanks @NickStallman - this is very helpful! I'll post an update here once we have a fix for this/know more!

@labbati labbati added c-extension Apply this label to issues and prs related to the C-extension and removed 🍏 core Changes to the core tracing functionality labels Jan 3, 2019
@NickStallman
Copy link
Author

Has there been any progress on this issue? I notice #224 has the same issue repeated in an independent example.

This is a blocker for us to instrument one of our primary performance critical applications.

@labbati
Copy link
Member

labbati commented Feb 4, 2019

Hi @NickStallman, so far no progress on this topic and it revealed to be harder than expected!
Let us get back to you as we try to squeeze this into our current work.
Sorry again for the issue you are facing.

@Omicron7
Copy link

I just ran into this issue as well using a library called the phpCAS.

[16-Apr-2019 15:11:37 UTC] PHP Warning:  Invalid callback CAS_Request_CurlRequest::_curlReadHeaders, cannot access private method CAS_Request_CurlRequest::_curlReadHeaders() in /opt/datadog-php/dd-trace-sources/src/DDTrace/Integrations/Curl/CurlIntegration.php on line 55
[16-Apr-2019 15:11:37 UTC] PHP Warning:  curl_exec(): Could not call the CURLOPT_HEADERFUNCTION in /opt/datadog-php/dd-trace-sources/src/DDTrace/Integrations/Curl/CurlIntegration.php on line 55

Adding curl to the DD_INTEGRATIONS_DISABLED environment variable bypasses the issue for now.

@labbati
Copy link
Member

labbati commented Apr 16, 2019

Thanks for reporting this. This is related to how we pass context to our wrapper. We are actively working on this and his sibling (calls to parent::xxx from within a wrapped function). A fix for both (starting from the parent::) is expected in the next few days.

@SammyK
Copy link
Contributor

SammyK commented Apr 26, 2019

Hey @NickStallman & @Omicron7! Thanks again for reporting this issue and sorry for the wait for a fix. PR's #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! :)

@ctrlrsf
Copy link

ctrlrsf commented Apr 29, 2019

@SammyK That's great to hear. This is also affecting an internal library we use so we've disabled curl integration for now.

Error file: /opt/datadog-php/dd-trace-sources/src/DDTrace/Integrations/Curl/CurlIntegration.php
Error line: 55
Error string: curl_exec(): Could not call the CURLOPT_HEADERFUNCTION

@SammyK
Copy link
Contributor

SammyK commented May 1, 2019

Hey @NickStallman, @Omicron7, @ctrlrsf, et al! 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. :)

@SammyK SammyK closed this as completed May 1, 2019
@ctrlrsf
Copy link

ctrlrsf commented May 6, 2019

Thanks, @SammyK. Testing now and will let you know if we have any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working c-extension Apply this label to issues and prs related to the C-extension
Projects
None yet
Development

No branches or pull requests

6 participants