Add support for the CLI SAPI

[SammyK]

Description

This PR adds support for tracing custom PHP CLI scripts and Laravel Artisan. This feature is disabled by default. To enable, set the env var DD_TRACE_CLI_ENABLED=true.

This PR adds support for the CLI SAPI by removing the PHP_SAPI and getenv('APP_ENV') checks.

Readiness checklist

• Changelog entry added, if necessary
• Tests added for this feature/bug

[pawelchcki]

I feel like we still should make this optional otherwise people will start getting weird traces from Artisan or composer commands. WDYT ?

^ Other than that the change looks good :)

[SammyK]

otherwise people will start getting weird traces from Artisan

I'm not sure what kind of weird traces you're referring to. The traces from the CLI look similar to the ones from other SAPI's. :)

or composer commands

Very good point! Yeah, we should certainly make sure the tracer doesn't get run during a Composer command. I'll look into this. :)

[pawelchcki]

otherwise people will start getting weird traces from Artisan

I'm not sure what kind of weird traces you're referring to. The traces from the CLI look similar to the ones from other SAPI's. :)

By weird I meant unexpected traces of things like:

php artisan optimize
php artisan config:cache
php artisan route:cache

I have a strong suspicion that for CLI sapi the tracing needs to be opt in, since I'm not sure in other traces we do handle things like cron job run scripts etc by default.

Its also probably more of a product quesiton than engineering one though :)

[SammyK]

Very good point. Certainly "disabled by default" would make sense here. Although keeping the on-boarding experience in mind, I would err on the side of keeping things enabled by default so that people don't run into problems with seeing their first traces in the UI. And then having an option to disable tracing from the CLI. Something like DD_TRACE_CLI_ENABLED which defaults to true.

Since, as you stated, this impacts product, maybe we could have @priyanshi-gupta or @palazzem weigh in here? :)

[SammyK]

I'm going to mark this as a WIP for now since there's quite a few open questions. The CLI traces are fairly basic and nondescript at the moment.

And running a Composer command generates two traces for some reason. Ultimately we'll need to disable tracing when running a Composer command.

Also - I'm curious as to what extra metadata we should add to the root span. Perhaps the script name? I'm open to ideas! :)

[labbati]

Great work on this! Thanks @SammyK!

WRT ON/OFF by default I would go with OFF by default and activate it through env DD_TRACE_CLI_ENABLED=true. Not only because of composer, but users run some php scripts almost for sure and this may lead to unexpected behaviors.

[SammyK]

I'm moving this to 0.23.0 milestone to allow us time get this right without blocking the release of PHP 7.3 support. :)

[labbati]

👍 love it! LGTM 😄

Suggestions have been implemented :)