diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 5ff4b88..a4c4d68 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -80,7 +80,7 @@ needed related to installing and configuring some dependencies. Next is to add extension to ini file as described above. -After finising all above steps, you can start debugging session. There are few debuggers that +After finishing all above steps, you can start a debugging session. There are few debuggers that can be used for debugging process, lldb, gdb or visual studio debugger on windows. To trigger auto instrumentation code you need to invoke observer api functionality. Following, very simple script can be used as reference example, created and saved as test.php @@ -146,6 +146,28 @@ tests/name_of_test.sh valgrind # will run test and display valgrind report Further reading: https://www.phpinternalsbook.com/php7/memory_management/memory_debugging.html#debugging-memory +### gdbserver + +To debug tests running in a docker container, you can use `gdbserver`: + +```shell +docker build --build-arg PHP_VERSION=8.2.11 -f docker/Dockerfile.debian . -t otel:8.2.11 +docker run --rm -it -p "2345:2345" -v $(pwd)/ext:/usr/src/myapp otel:8.2.11 bash +``` + +Then, inside the container: + +```shell +phpize +./configure +make +gdbserver :2345 php -d extension=$(pwd)/modules/opentelemetry.so /path/to/file.php +``` + +Now, gdbserver should be running and awaiting a connection. Configure your IDE to connect via +`gdb` to `127.0.0.1:2345` and start debugging, which should connect to the waiting server +and start execution. + # Packaging for PECL See https://github.com/opentelemetry-php/dev-tools#pecl-release-tool diff --git a/README.md b/README.md index 2025f37..9c8f354 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,9 @@ This is a PHP extension for OpenTelemetry, to enable auto-instrumentation. It is based on [zend_observer](https://www.datadoghq.com/blog/engineering/php-8-observability-baked-right-in/) and requires php8+ -The extension allows creating `pre` and `post` hook functions to arbitrary PHP functions and methods, which allows those methods to be wrapped with telemetry. +The extension allows creating `pre` and `post` hook functions to arbitrary PHP functions and methods, which allows those methods to be wrapped with telemetry. + +In PHP 8.2+, internal/built-in PHP functions can also be observed. ## Requirements - PHP 8+ @@ -74,9 +76,8 @@ opentelemetry.validate_hook_functions => On => On ### Invalid pre/post hooks -Invalid argument types in pre and post callbacks can cause fatal errors. Runtime checking is performed on the -hook functions to ensure they are compatible. If not, the hook will not be executed and an error will be logged -to error_log. +Invalid argument types in `pre` and `post` callbacks can cause fatal errors. Runtime checking is performed on the +hook functions to ensure they are compatible. If not, the hook will not be executed and an error will be generated. This feature can be disabled by setting the `opentelemetry.validate_hook_functions` ini value to `Off`; diff --git a/docker/Dockerfile.debian b/docker/Dockerfile.debian index 3478347..ac5e0fb 100644 --- a/docker/Dockerfile.debian +++ b/docker/Dockerfile.debian @@ -37,6 +37,7 @@ RUN apt-get update -y \ ARG PHP_VERSION ENV PHP_URL="https://www.php.net/distributions/php-${PHP_VERSION}.tar.xz" ENV CFLAGS="-ggdb3" +ENV TEST_PHP_ARGS="-q" RUN echo "$PHP_URL" \ && curl -fsSL -o php.tar.xz "$PHP_URL" \ diff --git a/ext/otel_observer.c b/ext/otel_observer.c index d0130c8..c3ca393 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -187,36 +187,6 @@ static inline bool is_valid_signature(zend_fcall_info fci, return true; } -static void log_invalid_message(char *msg, zval *scope, zval *function) { - char *s; - if (Z_TYPE_P(scope) == IS_NULL) { - s = "null"; - } else { - s = Z_STRVAL_P(scope); - } - char *f = Z_STRVAL_P(function); - - // Calculate the size of the formatted message. - int formatted_size = strlen(msg) + strlen(s) + strlen(f) + 1; - - // Allocate a buffer for the formatted message. - char *formatted = malloc(formatted_size); - if (formatted == NULL) { - php_log_err("OpenTelemetry: Failed to allocate " - "memory for formatted message."); - return; - } - - // Format the message. - snprintf(formatted, formatted_size, msg, s, f); - - // Log the message. - php_log_err(formatted); - - // Free the allocated memory. - free(formatted); -} - static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { if (!zend_llist_count(hooks)) { return; @@ -249,9 +219,13 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { fci.retval = &ret; if (!is_valid_signature(fci, fcc)) { - char *msg = "OpenTelemetry: pre hook invalid signature, class=%s " - "function=%s"; - log_invalid_message(msg, ¶ms[2], ¶ms[3]); + php_error_docref(NULL, E_WARNING, + "OpenTelemetry: pre hook invalid signature," + " class=%s function=%s", + (Z_TYPE_P(¶ms[2]) == IS_NULL) + ? "null" + : Z_STRVAL_P(¶ms[2]), + Z_STRVAL_P(¶ms[3])); continue; } @@ -276,8 +250,10 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { if (func->type == ZEND_INTERNAL_FUNCTION) { // TODO expanding args for internal functions causes // segfault - php_log_err("OpenTelemetry: expanding args of " - "internal functions not supported"); + php_error_docref( + NULL, E_NOTICE, + "OpenTelemetry: expanding args of " + "internal functions not supported"); continue; } // TODO Extend call frame? @@ -367,9 +343,13 @@ static void observer_end(zend_execute_data *execute_data, zval *retval, fci.retval = &ret; if (!is_valid_signature(fci, fcc)) { - char *msg = "OpenTelemetry: post hook invalid signature, class=%s " - "function=%s"; - log_invalid_message(msg, ¶ms[4], ¶ms[5]); + php_error_docref(NULL, E_WARNING, + "OpenTelemetry: post hook invalid signature, " + "class=%s function=%s", + (Z_TYPE_P(¶ms[4]) == IS_NULL) + ? "null" + : Z_STRVAL_P(¶ms[4]), + Z_STRVAL_P(¶ms[5])); continue; } diff --git a/ext/tests/expand_params_internal.phpt b/ext/tests/expand_params_internal.phpt index bfc0260..88faf62 100644 --- a/ext/tests/expand_params_internal.phpt +++ b/ext/tests/expand_params_internal.phpt @@ -5,10 +5,10 @@ Removing the `post` callback avoids the segfault. The segfault is actually durin from `zend_observer_fcall_end_all` (https://github.com/php/php-src/blob/php-8.2.8/Zend/zend_observer.c#L291). When it traverses back through zend_execute_data, the top-level frame appears corrupted, and any attempt to reference it is what causes the segfault. +--SKIPIF-- += 8.2'); ?> --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a post callback when expanding params of internal function causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- ---EXPECT-- -array(1) { +--EXPECTF-- +Notice: array_slice(): OpenTelemetry: expanding args of internal functions not supported in %s on line %d +array(2) { [0]=> int(2) + [1]=> + int(3) } diff --git a/ext/tests/invalid_post_callback_signature.phpt b/ext/tests/invalid_post_callback_signature.phpt index acef320..536afb6 100644 --- a/ext/tests/invalid_post_callback_signature.phpt +++ b/ext/tests/invalid_post_callback_signature.phpt @@ -5,8 +5,6 @@ The invalid callback signature should not cause a fatal, so it is checked before is invalid, the callback will not be called. --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a post invalid callback signature causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- --EXPECTF-- -OpenTelemetry: pre hook invalid signature, class=TestClass function=test +Warning: TestClass::test(): OpenTelemetry: pre hook invalid signature, class=TestClass function=test in %s on line %d string(4) "test" string(4) "post" diff --git a/ext/tests/invalid_pre_callback_signature_with_function.phpt b/ext/tests/invalid_pre_callback_signature_with_function.phpt index 1cfb335..81af076 100644 --- a/ext/tests/invalid_pre_callback_signature_with_function.phpt +++ b/ext/tests/invalid_pre_callback_signature_with_function.phpt @@ -6,8 +6,6 @@ is invalid, the callback will not be called and a message will be written to err null class. --EXTENSIONS-- opentelemetry ---XFAIL-- -Providing a invalid pre callback signature for function without class causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log. --FILE-- --EXPECTF-- -OpenTelemetry: pre hook invalid signature, class=null function=hello +Warning: hello(): OpenTelemetry: pre hook invalid signature, class=null function=hello in %s on line %d string(5) "hello" string(4) "post" diff --git a/ext/tests/return_expanded_params_internal.phpt b/ext/tests/return_expanded_params_internal.phpt index 385f847..85d64e1 100644 --- a/ext/tests/return_expanded_params_internal.phpt +++ b/ext/tests/return_expanded_params_internal.phpt @@ -2,6 +2,8 @@ Check if pre hook can expand and then return $params of internal function --DESCRIPTION-- The existence of a post callback is part of the failure preconditions. +--SKIPIF-- += 8.2'); ?> --EXTENSIONS-- opentelemetry --FILE-- @@ -18,10 +20,11 @@ opentelemetry var_dump(array_slice(['a', 'b', 'c'], 1)); ?> ---XFAIL-- -Core dump (same issue as in return_expanded_params.phpt) ---EXPECT-- -array(1) { +--EXPECTF-- +Notice: array_slice(): OpenTelemetry: expanding args of internal functions not supported in %s on line %d +array(2) { [0]=> string(1) "b" + [1]=> + string(1) "c" }