Merge branch 'main' into split

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

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`;
 

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" \

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, &params[2], &params[3]);
+            php_error_docref(NULL, E_WARNING,
+                             "OpenTelemetry: pre hook invalid signature,"
+                             " class=%s function=%s",
+                             (Z_TYPE_P(&params[2]) == IS_NULL)
+                                 ? "null"
+                                 : Z_STRVAL_P(&params[2]),
+                             Z_STRVAL_P(&params[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, &params[4], &params[5]);
+            php_error_docref(NULL, E_WARNING,
+                             "OpenTelemetry: post hook invalid signature, "
+                             "class=%s function=%s",
+                             (Z_TYPE_P(&params[4]) == IS_NULL)
+                                 ? "null"
+                                 : Z_STRVAL_P(&params[4]),
+                             Z_STRVAL_P(&params[5]));
             continue;
         }
 

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--
+<?php if (PHP_VERSION_ID < 80200) die('skip requires PHP >= 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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -22,8 +22,11 @@ OpenTelemetry\Instrumentation\hook(
 
 var_dump(array_slice([1,2,3], 1));
 ?>
---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)
 }

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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -33,4 +31,5 @@ TestClass::test();
 --EXPECTF--
 string(3) "pre"
 string(4) "test"
-OpenTelemetry: post hook invalid signature, class=TestClass function=test
+
+Warning: TestClass::test(): OpenTelemetry: post hook invalid signature, class=TestClass function=test in %s on line %d

ext/tests/invalid_pre_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 and a message will be written to error_log.
 --EXTENSIONS--
 opentelemetry
---XFAIL--
-Providing a pre invalid callback signature causes segfault. The behaviour is currently disabled, so instead of a segfault a message is logged to error_log.
 --FILE--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -31,6 +29,6 @@ class TestClass {
 TestClass::test();
 ?>
 --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"

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--
 <?php
 OpenTelemetry\Instrumentation\hook(
@@ -30,6 +28,6 @@ function hello(): void
 hello();
 ?>
 --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"

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--
+<?php if (PHP_VERSION_ID < 80200) die('skip requires PHP >= 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"
 }