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

Correct way to trace a plain function #113

Closed
NickStallman opened this issue Nov 13, 2018 · 11 comments
Closed

Correct way to trace a plain function #113

NickStallman opened this issue Nov 13, 2018 · 11 comments

Comments

@NickStallman
Copy link

I've got dd_trace working well with class methods however I'm trying to make an integration for Curl which requires attaching to "curl_exec" and it doesn't appear to be working.

It looks like this code should work (very short example):

        dd_trace('curl_exec', function ($ch) {
            return curl_exec($ch);
        });

But when I do that, no code in the closure is ever executed.

Also I'm a little unclear of how to call the original function. With classes $this is used but I'm not sure whether calling curl_exec inside the closure will make a infinite loop or if it'll execute the function.

Thanks

@labbati
Copy link
Member

labbati commented Nov 13, 2018

Hi @NickStallman, thank you for taking some time to contribute to this project.

It looks like this code should work (very short example):

What you wrote looks correct. I sort of replicated it in a script and the results are below.

<?php

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

// create curl resource
$ch = curl_init();
// set url
curl_setopt($ch, CURLOPT_URL, "example.com");
//return the transfer as a string
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
// $output contains the output string
$output = curl_exec($ch);
// close curl resource to free up system resources
curl_close($ch);
echo  "Received a string of " . strlen($output) . " chars\n";

It outputs

Called curl_exec
Received a string of 1270 chars

Would it be possible for you to share the remaining part of the sample script?
Would it be possible for you to run the example in the test environment that we provide so we can at least know if the issue can be related to the environment or the code?
E.g.

$ docker-compose run 7.2 bash
> composer install-ext
> php <your_script_here>.php

Also I'm a little unclear of how to call the original function. With classes $this is used but I'm not sure whether calling curl_exec inside the closure will make a infinite loop or if it'll execute the function.

Calling curl_exec from within the function is the correct way and will not cause an infinite loop.

Thanks!!!

@NickStallman
Copy link
Author

Very odd, I'm trying it out in place in a application on a dev server which probably isn't helping.

https://github.com/NickStallman/dd-trace-php/blob/curl/src/DDTrace/Integrations/Curl.php

An echo above the dd_trace works just fine, an echo inside it never executes and I've confirmed the dd_trace is being run before I'm running curl_exec.

A second pair of eyes would be welcome. :)

@labbati
Copy link
Member

labbati commented Nov 14, 2018

Hey! Once again your code looks good. This is how a suggest to proceed to debug the issue.

  1. I would put a simplified version of your script (no span and global tracer in order to restrict the scope) that I am providing here and that works in my dev environment in a php file, say playground.php
<?php
class Curl
{
    public static function load()
    {
        if (!extension_loaded('ddtrace')) {
            trigger_error('The ddtrace extension is required to instrument Memcached', E_USER_WARNING);
            return;
        }
        if (!function_exists('curl_exec')) {
            trigger_error('Curl is not loaded and cannot be instrumented', E_USER_WARNING);
            return;
        }
        echo "Before\n";
        dd_trace('curl_exec', function ($ch) {
            echo "Begin of wrapper\n";
            return curl_exec($ch);
        });
    }
}

Curl::load();

// create curl resource
$ch = curl_init();
// set url
curl_setopt($ch, CURLOPT_URL, "example.com");
//return the transfer as a string
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
// $output contains the output string
$output = curl_exec($ch);
// close curl resource to free up system resources
curl_close($ch);
echo  "Received a string of " . strlen($output) . " chars\n";
  1. I would execute this script from the command line in your dev server php playground.php, where the ddtrace C extension is build. The expected output is:
Before
Begin of wrapper
Received a string of 1270 chars

I expect that this is not working, though.

  1. If is not working (it should not) then we can focus our debugging on the C-extension (bringing in @pawelchcki as he is the C-extension master). At this point would be useful for us to know:
  • OS version (and 32 vs 64 bits)
  • php version
  • command that you are using to build the extension and the command output
  • @pawelchcki any other thing that comes to your mind?

Thanks again @NickStallman for the patience and the detailed info!

@pawelchcki
Copy link
Contributor

I've tried above snippet on all of our test docker images, and so far its working as expected.

@NickStallman would you be able to share the output of php -m as well?

@NickStallman
Copy link
Author

NickStallman commented Nov 14, 2018

Curious, that test script isn't working for me. Good to know I'm not going crazy.

# php test.php
Before
Received a string of 1270 chars

I've tried this with file_get_contents as well as a second check, and it fails the same way.
Tracing PHP defined functions is working perfectly, just not built ins apparently.

OS: Ubuntu 16.04.2 LTS x64
PHP: 7.1.24-1+ubuntu16.04.1+deb.sury.org+1 (From https://launchpad.net/~ondrej/+archive/ubuntu/php which I use extensively on numerous servers with no issues).
ddtrace: datadog-php-tracer_0.2.7-beta_amd64.deb

php -m
[PHP Modules]
bcmath
calendar
Core
ctype
curl
date
ddtrace
dom
exif
fileinfo
filter
ftp
gd
gettext
hash
iconv
imap
json
libxml
mcrypt
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
Phar
posix
readline
Reflection
session
shmop
SimpleXML
sockets
SPL
standard
sysvmsg
sysvsem
sysvshm
tokenizer
wddx
xml
xmlreader
xmlwriter
xsl
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache

@pawelchcki
Copy link
Contributor

@NickStallman I was able to reproduce the problem using PHP 7.1 on Ubuntu 16.04.
Thanks for reporting it! Working now on developing a fix in #126

@pawelchcki pawelchcki added this to the 0.4.1 milestone Nov 22, 2018
@pawelchcki
Copy link
Contributor

@NickStallman we've shipped the fix from #126 in our latest release 0.4.1.

@labbati
Copy link
Member

labbati commented Nov 23, 2018

@NickStallman did you have a chance to test version 0.4.1 which brings the fix for this issue? Is it working for you?

@labbati labbati removed this from the 0.4.1 milestone Nov 23, 2018
@labbati
Copy link
Member

labbati commented Nov 27, 2018

@NickStallman we are closing for now this issue as it is supposed to be fixed in 0.4.1. If you have a chance to test if this is also fixed in your env it would be great. If you still see any problem we will reopen this issue.

@NickStallman
Copy link
Author

@labbati sorry, I've been on holidays.

Yes I can confirm that this issue is fixed and I can now trace internal functions just fine.
However there seems to be a new edge case with curl specifically which I have opened a new issue for.

@labbati
Copy link
Member

labbati commented Nov 28, 2018

Thanks for confirming this! And welcome back from holidays 😄
Ack on the other issue, we will look into it asap.

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

No branches or pull requests

3 participants