Auto-instrumentation in Symfony 3.4 and PHP 5.6 #262
Conversation
…e-php into labbati/improve-autoinstrumentation
…/improve-autoinstrumentation
| * @param string[] $sentinelClasses | ||
| * @return bool | ||
| */ | ||
| function any_class_exists(array $sentinelClasses) |
There was a problem hiding this comment.
In order to not fill the global namespace with functions that might collide with user's apps, can we add namespace DDTrace; to the top of this file and then we can do the same in the dd_wrap_autoloader.php file. :)
There was a problem hiding this comment.
Also if we add the namespaces, we'll need to update line 10 of dd_init.php to this:
if (!\DDTrace\dd_tracing_enabled()) {| @@ -12,16 +12,25 @@ | |||
|
|
|||
There was a problem hiding this comment.
Can we add namespace DDTrace; to the top of this file per the other comment? :)
There was a problem hiding this comment.
If it is fine for you, for consistency I would add DDTrace\Bridge as int he other files.
bridge/dd_wrap_autoloader.php
Outdated
| @@ -12,16 +12,25 @@ | |||
|
|
|||
| $dd_autoload_called = false; | |||
There was a problem hiding this comment.
Since these variables get dumped into the global scope even with the namespacing, can we wrap everything from this line down with a self-calling function?
call_user_func(function () {
$dd_autoload_called = false;
// ....
});That will prevent these variables from showing up in user's apps to potentially cause unexpected behavior. :)
| - /proc/:/host/proc/:ro | ||
| - /sys/fs/cgroup/:/host/sys/fs/cgroup:ro | ||
| environment: | ||
| - DD_API_KEY=${DATADOG_API_KEY} |
There was a problem hiding this comment.
Was this intentional to duplicate the agent container ddagent_integration here and pull the API key from the env vars instead of using the value invalid_key_but_its_ok? If so, cool, but wanted to double check. :)
There was a problem hiding this comment.
This can be used together with apache_56_server container to send real traces to our backend. I can drop this no problem (or move to a different file). But it may be useful to have some real server sending traces to our backend.
There was a problem hiding this comment.
Perfect! Just wanted to double check. :)
| rm -rf build/packages | ||
| mkdir -p build/packages | ||
|
|
||
| function build_version() { |
… still has issues in 5.6 tracing
…on, for linting purposes
Description
Readiness checklist