From aa7831e3ea029455b60c2fde188d8fec2b7dd5a5 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Tue, 16 Jun 2026 21:52:37 +0200 Subject: [PATCH 01/15] feat(ext): add exception to data in end callback --- sentry.c | 9 ++++ tests/test_cought_exceptions_do_not_leak.phpt | 45 +++++++++++++++++++ tests/test_exception_set_in_end_callback.phpt | 32 +++++++++++++ tests/test_metadata_key_always_exists.phpt | 27 +++++++++++ ..._metadata_key_always_exists_attribute.phpt | 27 +++++++++++ ...est_nested_exceptions_properly_scoped.phpt | 43 ++++++++++++++++++ 6 files changed, 183 insertions(+) create mode 100644 tests/test_cought_exceptions_do_not_leak.phpt create mode 100644 tests/test_exception_set_in_end_callback.phpt create mode 100644 tests/test_metadata_key_always_exists.phpt create mode 100644 tests/test_metadata_key_always_exists_attribute.phpt create mode 100644 tests/test_nested_exceptions_properly_scoped.phpt diff --git a/sentry.c b/sentry.c index e19a6d1..e9fdbb2 100644 --- a/sentry.c +++ b/sentry.c @@ -512,6 +512,15 @@ static void sentry_observer_end(zend_execute_data *execute_data, zval *return_va ZVAL_COPY(&metadata_zv, &state->metadata); add_assoc_zval(&event, "metadata", &metadata_zv); + zend_object *exception = EG(exception); + if (exception != NULL) { + zval exception_zv; + ZVAL_OBJ_COPY(&exception_zv, exception); + add_assoc_zval(&event, "exception", &exception_zv); + } else { + add_assoc_null(&event, "exception"); + } + ZVAL_COPY_VALUE(¶ms[0], &event); ZVAL_COPY_VALUE(¶ms[1], &state->user_state); diff --git a/tests/test_cought_exceptions_do_not_leak.phpt b/tests/test_cought_exceptions_do_not_leak.phpt new file mode 100644 index 0000000..55ec6f9 --- /dev/null +++ b/tests/test_cought_exceptions_do_not_leak.phpt @@ -0,0 +1,45 @@ +--TEST-- +Tests that re-throwing different exceptions properly populates the 'exception' key. +--EXTENSIONS-- +sentry +--FILE-- +getMessage() . PHP_EOL; + echo get_class($exception) . PHP_EOL; + } else { + echo 'No exception here' . PHP_EOL; + } +}); + +\Sentry\instrument(null, 'test_throw'); +\Sentry\instrument(null, 'test_rethrow'); +try { +test_rethrow(); +} catch (Throwable $t) { + +} + +?> +--EXPECTF-- +Oh no +CustomException +Exception caught +No exception here \ No newline at end of file diff --git a/tests/test_exception_set_in_end_callback.phpt b/tests/test_exception_set_in_end_callback.phpt new file mode 100644 index 0000000..32ed6db --- /dev/null +++ b/tests/test_exception_set_in_end_callback.phpt @@ -0,0 +1,32 @@ +--TEST-- +Tests that a thrown exception is captured and stored under the 'exception' key in the end callback +--EXTENSIONS-- +sentry +--FILE-- +getMessage() . PHP_EOL; + echo get_class($exception) . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_throw'); +try { +test_throw(); +} catch (Throwable $t) { + +} + +?> +--EXPECTF-- +Oh no +CustomException \ No newline at end of file diff --git a/tests/test_metadata_key_always_exists.phpt b/tests/test_metadata_key_always_exists.phpt new file mode 100644 index 0000000..2812077 --- /dev/null +++ b/tests/test_metadata_key_always_exists.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that the 'metadata' key in data in the end callback is always populated and never null (Function) +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +[] \ No newline at end of file diff --git a/tests/test_metadata_key_always_exists_attribute.phpt b/tests/test_metadata_key_always_exists_attribute.phpt new file mode 100644 index 0000000..c61c671 --- /dev/null +++ b/tests/test_metadata_key_always_exists_attribute.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that the 'metadata' key in data in the end callback is always populated and never null (Attribute) +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +[] \ No newline at end of file diff --git a/tests/test_nested_exceptions_properly_scoped.phpt b/tests/test_nested_exceptions_properly_scoped.phpt new file mode 100644 index 0000000..08d6d76 --- /dev/null +++ b/tests/test_nested_exceptions_properly_scoped.phpt @@ -0,0 +1,43 @@ +--TEST-- +Tests that catching an exception in a function will make 'exception' be null in the end callback +--EXTENSIONS-- +sentry +--FILE-- +getMessage() . PHP_EOL; + echo get_class($exception) . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_throw'); +\Sentry\instrument(null, 'test_rethrow'); +try { +test_rethrow(); +} catch (Throwable $t) { + +} + +?> +--EXPECTF-- +Oh no +CustomException +throw different exception +TestException \ No newline at end of file From 2ea926396650c01016d371f20225bd6c6cb704a3 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Tue, 16 Jun 2026 22:37:18 +0200 Subject: [PATCH 02/15] add test --- tests/test_fields_always_present.phpt | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/test_fields_always_present.phpt diff --git a/tests/test_fields_always_present.phpt b/tests/test_fields_always_present.phpt new file mode 100644 index 0000000..21b5ac3 --- /dev/null +++ b/tests/test_fields_always_present.phpt @@ -0,0 +1,40 @@ +--TEST-- +Tests that some fields are always present in the start and end callback and also assert their type. +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +Start callback +start_time: float +name: string +End callback +start_time: float +name: string +duration: float +end_time: float +metadata: array From 4fe222d0724fd9457992b378ddf89144385ea8bd Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 09:49:00 +0200 Subject: [PATCH 03/15] feat: add support for variadic named arguments --- sentry.c | 200 +++++++++++++++--- sentry.stub.php | 4 +- sentry_arginfo.h | 6 +- tests/test_variadic_metdata_attribute.phpt | 32 +++ ...riadic_metdata_attribute_list_ignored.phpt | 27 +++ ...dic_metdata_attribute_numeric_ignored.phpt | 28 +++ ...variadic_metdata_attributes_overwrite.phpt | 34 +++ tests/test_variadic_metdata_list_ignored.phpt | 27 +++ ...test_variadic_metdata_numeric_ignored.phpt | 28 +++ tests/test_variadic_metdata_overwrite.phpt | 34 +++ 10 files changed, 384 insertions(+), 36 deletions(-) create mode 100644 tests/test_variadic_metdata_attribute.phpt create mode 100644 tests/test_variadic_metdata_attribute_list_ignored.phpt create mode 100644 tests/test_variadic_metdata_attribute_numeric_ignored.phpt create mode 100644 tests/test_variadic_metdata_attributes_overwrite.phpt create mode 100644 tests/test_variadic_metdata_list_ignored.phpt create mode 100644 tests/test_variadic_metdata_numeric_ignored.phpt create mode 100644 tests/test_variadic_metdata_overwrite.phpt diff --git a/sentry.c b/sentry.c index e9fdbb2..288a8ad 100644 --- a/sentry.c +++ b/sentry.c @@ -105,6 +105,86 @@ static bool sentry_has_trace_attribute(zend_execute_data *execute_data) { return sentry_get_trace_attribute(execute_data) != NULL; } +static void sentry_clear_pending_exception(void) { + zend_object *ex = EG(exception); + + if (ex != NULL) { + EG(exception) = NULL; + OBJ_RELEASE(ex); + } +} + +static bool sentry_is_attribute_arg(zend_string *name, zval *value) { + return name != NULL + && zend_string_equals_literal(name, "attributes") + && Z_TYPE_P(value) == IS_ARRAY; +} + +/** + * Copies all elements from source to dest if they are using string keys. + * If source is a list, it will not do anything. + * Integer keys are ignored and will not be copied. + */ +static void sentry_merge_array(zval *dest, zval *source) { + if (zend_array_is_list(Z_ARRVAL_P(source))) { + return; + } + zend_string *str_key; + zval *value; + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(source), str_key, value) { + if (str_key != NULL) { + zval copied_value; + ZVAL_COPY(&copied_value, value); + + zend_hash_update( + Z_ARRVAL_P(dest), + str_key, + &copied_value + ); + } + } + ZEND_HASH_FOREACH_END(); +} + +static void sentry_add_named_metadata_arg( + zval *metadata, + zval *deferred_attributes, + zend_string *name, + zval *value +) { + if (sentry_is_attribute_arg(name, value)) { + if (!Z_ISUNDEF_P(deferred_attributes)) { + zval_ptr_dtor(deferred_attributes); + } + + ZVAL_COPY(deferred_attributes, value); + return; + } + + zval copied_value; + ZVAL_COPY(&copied_value, value); + + zend_hash_update( + Z_ARRVAL_P(metadata), + name, + &copied_value + ); +} + +static void sentry_merge_deferred_attributes( + zval *metadata, + zval *deferred_attributes +) { + if (Z_ISUNDEF_P(deferred_attributes)) { + return; + } + + sentry_merge_array(metadata, deferred_attributes); + zval_ptr_dtor(deferred_attributes); + ZVAL_UNDEF(deferred_attributes); +} + static void sentry_get_attribute_metadata( zend_execute_data *execute_data, zval *metadata @@ -112,42 +192,68 @@ static void sentry_get_attribute_metadata( array_init(metadata); zend_attribute *attribute = sentry_get_trace_attribute(execute_data); - if (attribute != NULL && attribute->argc > 0) { + if (attribute == NULL || attribute->argc == 0) { + return; + } + + // this gets populated if the "attribute" named argument exists. + // We will store it here and merge it last so that it always overwrites existing keys. + zval attribute_list; + ZVAL_UNDEF(&attribute_list); + + for (uint32_t i = 0; i < attribute->argc; i++) { zval attribute_arg; ZVAL_UNDEF(&attribute_arg); - if (zend_get_attribute_value( - &attribute_arg, - attribute, - 0, - execute_data->func->common.scope - ) == SUCCESS && Z_TYPE(attribute_arg) == IS_ARRAY) { - zend_hash_copy( - Z_ARRVAL_P(metadata), - Z_ARRVAL(attribute_arg), - zval_add_ref - ); + const zend_result result = zend_get_attribute_value(&attribute_arg, attribute, i, execute_data->func->common.scope); + // result can be unsuccessful if e.g. constants are references that do not exist. + if (result != SUCCESS) { + // clear exception if the attribute has an invalid value so that we don't crash + // user code. + sentry_clear_pending_exception(); + + if (!Z_ISUNDEF(attribute_arg)) { + zval_ptr_dtor(&attribute_arg); + } + + continue; } - zend_object *ex = EG(exception); - if (ex != NULL) { - EG(exception) = NULL; - OBJ_RELEASE(ex); + zend_string *arg_name = attribute->args[i].name; + + if (arg_name != NULL) { + sentry_add_named_metadata_arg( + metadata, + &attribute_list, + arg_name, + &attribute_arg + ); + } else if (Z_TYPE(attribute_arg) == IS_ARRAY) { + sentry_merge_array(metadata, &attribute_arg); } + sentry_clear_pending_exception(); + if (!Z_ISUNDEF(attribute_arg)) { zval_ptr_dtor(&attribute_arg); } + + } + + if (!Z_ISUNDEF(attribute_list)) { + sentry_merge_array(metadata, &attribute_list); + zval_ptr_dtor(&attribute_list); } } ZEND_METHOD(Sentry_Trace, __construct) { - zval *metadata = NULL; + zval *args = NULL; + uint32_t argc = 0; + HashTable *named_args = NULL; - ZEND_PARSE_PARAMETERS_START(0, 1) - Z_PARAM_OPTIONAL - Z_PARAM_ARRAY(metadata) + ZEND_PARSE_PARAMETERS_START(0, -1) + Z_PARAM_VARIADIC_WITH_NAMED(args, argc, named_args) ZEND_PARSE_PARAMETERS_END(); } @@ -279,14 +385,17 @@ static zend_string *sentry_build_key(zend_string *class_name, zend_string *funct ZEND_FUNCTION(Sentry_instrument) { zend_string *class_name = NULL; zend_string *function_name; - zval metadata; - zval *extra_metadata = NULL; + // zval metadata; + // zval *extra_metadata = NULL; - ZEND_PARSE_PARAMETERS_START(2,3) + zval *metadata_args = NULL; + uint32_t metadata_argc = 0; + HashTable *named_metadata = NULL; + + ZEND_PARSE_PARAMETERS_START(2,-1) Z_PARAM_STR_OR_NULL(class_name) Z_PARAM_STR(function_name) - Z_PARAM_OPTIONAL - Z_PARAM_ARRAY(extra_metadata) + Z_PARAM_VARIADIC_WITH_NAMED(metadata_args, metadata_argc, named_metadata) ZEND_PARSE_PARAMETERS_END(); // If a subclass doesn't override a method from the parent, the scope will @@ -307,15 +416,44 @@ ZEND_FUNCTION(Sentry_instrument) { } } + zval metadata; array_init(&metadata); - if (extra_metadata != NULL) { - zend_hash_copy( - Z_ARRVAL(metadata), - Z_ARRVAL_P(extra_metadata), - zval_add_ref - ); + zval attribute_list; + ZVAL_UNDEF(&attribute_list); + + for (uint32_t i = 0; i < metadata_argc; i++) { + if (Z_TYPE(metadata_args[i]) == IS_ARRAY) { + sentry_merge_array(&metadata, &metadata_args[i]); + } + } + + if (named_metadata != NULL) { + zend_string *name; + zval *value; + + ZEND_HASH_FOREACH_STR_KEY_VAL(named_metadata, name, value) { + if (name != NULL) { + sentry_add_named_metadata_arg( + &metadata, + &attribute_list, + name, + value + ); + } + } + ZEND_HASH_FOREACH_END(); } + // + // if (extra_metadata != NULL) { + // zend_hash_copy( + // Z_ARRVAL(metadata), + // Z_ARRVAL_P(extra_metadata), + // zval_add_ref + // ); + // } + + sentry_merge_deferred_attributes(&metadata, &attribute_list); zend_string *key = sentry_build_key(class_name, function_name); diff --git a/sentry.stub.php b/sentry.stub.php index c75592b..0f9f91a 100644 --- a/sentry.stub.php +++ b/sentry.stub.php @@ -10,7 +10,7 @@ function instrument( ?string $class_name, string $function_name, - array $extra_metadata = [] + mixed ...$metadata ): bool {} /** @@ -25,6 +25,6 @@ function setStartCallback(callable $callback): bool {} #[\Attribute(\Attribute::TARGET_FUNCTION | \Attribute::TARGET_METHOD)] final class Trace { - public function __construct(array $metadata = []) {} + public function __construct(mixed ...$metadata) {} } } diff --git a/sentry_arginfo.h b/sentry_arginfo.h index 0d510b1..b79bac2 100644 --- a/sentry_arginfo.h +++ b/sentry_arginfo.h @@ -1,10 +1,10 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: a8f0df9a0ea7aa5af60848effeda1f1ac2952c61 */ + * Stub hash: 8f9ab3a122f4878e771f49c4114c3703cefba0cd */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_Sentry_instrument, 0, 2, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, class_name, IS_STRING, 1) ZEND_ARG_TYPE_INFO(0, function_name, IS_STRING, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, extra_metadata, IS_ARRAY, 0, "[]") + ZEND_ARG_VARIADIC_TYPE_INFO(0, metadata, IS_MIXED, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_Sentry_setEndCallback, 0, 1, _IS_BOOL, 0) @@ -14,7 +14,7 @@ ZEND_END_ARG_INFO() #define arginfo_Sentry_setStartCallback arginfo_Sentry_setEndCallback ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Sentry_Trace___construct, 0, 0, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, metadata, IS_ARRAY, 0, "[]") + ZEND_ARG_VARIADIC_TYPE_INFO(0, metadata, IS_MIXED, 0) ZEND_END_ARG_INFO() ZEND_FUNCTION(Sentry_instrument); diff --git a/tests/test_variadic_metdata_attribute.phpt b/tests/test_variadic_metdata_attribute.phpt new file mode 100644 index 0000000..c6b6222 --- /dev/null +++ b/tests/test_variadic_metdata_attribute.phpt @@ -0,0 +1,32 @@ +--TEST-- +Tests that the Trace annotation can use named arguments and they get passed into metadata +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +Name: test_instrumented +Duration: %f +OP: foo +Custom: oh no \ No newline at end of file diff --git a/tests/test_variadic_metdata_attribute_list_ignored.phpt b/tests/test_variadic_metdata_attribute_list_ignored.phpt new file mode 100644 index 0000000..bcfabae --- /dev/null +++ b/tests/test_variadic_metdata_attribute_list_ignored.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that numeric array keys in attributes are ignored +--EXTENSIONS-- +sentry +--FILE-- + $value) { + echo $key . ": " . $value . PHP_EOL; + } +}); + +test_instrumented(); + +?> +--EXPECTF-- \ No newline at end of file diff --git a/tests/test_variadic_metdata_attribute_numeric_ignored.phpt b/tests/test_variadic_metdata_attribute_numeric_ignored.phpt new file mode 100644 index 0000000..872412c --- /dev/null +++ b/tests/test_variadic_metdata_attribute_numeric_ignored.phpt @@ -0,0 +1,28 @@ +--TEST-- +Tests that numeric array keys in attributes are ignored +--EXTENSIONS-- +sentry +--FILE-- + 'test', 1 => 'abc', 'test' => 'example'])] +function test_instrumented() { + $result = 0; + for($i = 0; $i < 1000; $i++) { + $result += $i; + } + + return $result; +} + +\Sentry\setEndCallback(static function (array $data) { + foreach ($data['metadata'] as $key => $value) { + echo $key . ": " . $value . PHP_EOL; + } +}); + +test_instrumented(); + +?> +--EXPECTF-- +test: example \ No newline at end of file diff --git a/tests/test_variadic_metdata_attributes_overwrite.phpt b/tests/test_variadic_metdata_attributes_overwrite.phpt new file mode 100644 index 0000000..465bb15 --- /dev/null +++ b/tests/test_variadic_metdata_attributes_overwrite.phpt @@ -0,0 +1,34 @@ +--TEST-- +Tests that the Trace attribute using name arguments has special handling for the attributes named argument +--EXTENSIONS-- +sentry +--FILE-- + "overwrite", "test" => "example"])] +function test_instrumented() { + $result = 0; + for($i = 0; $i < 1000; $i++) { + $result += $i; + } + + return $result; +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Name: " . $data['name'] . PHP_EOL; + echo "Duration: " . $data['duration'] . PHP_EOL; + echo "OP: " . $data['metadata']['op'] . PHP_EOL; + echo "Test: " . $data['metadata']['test'] . PHP_EOL; + echo "Custom: " . $data['metadata']['custom'] . PHP_EOL; +}); + +test_instrumented(); + +?> +--EXPECTF-- +Name: test_instrumented +Duration: %f +OP: foo +Test: example +Custom: overwrite \ No newline at end of file diff --git a/tests/test_variadic_metdata_list_ignored.phpt b/tests/test_variadic_metdata_list_ignored.phpt new file mode 100644 index 0000000..2bbdfc3 --- /dev/null +++ b/tests/test_variadic_metdata_list_ignored.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that numeric array keys in attributes are ignored +--EXTENSIONS-- +sentry +--FILE-- + $value) { + echo $key . ": " . $value . PHP_EOL; + } +}); + +\Sentry\instrument(null, "test_instrumented", attributes: ['test', 'example', 'bar']); +test_instrumented(); + +?> +--EXPECTF-- \ No newline at end of file diff --git a/tests/test_variadic_metdata_numeric_ignored.phpt b/tests/test_variadic_metdata_numeric_ignored.phpt new file mode 100644 index 0000000..cf5b400 --- /dev/null +++ b/tests/test_variadic_metdata_numeric_ignored.phpt @@ -0,0 +1,28 @@ +--TEST-- +Tests that numeric array keys in attributes are ignored +--EXTENSIONS-- +sentry +--FILE-- + $value) { + echo $key . ": " . $value . PHP_EOL; + } +}); + +\Sentry\instrument(null, "test_instrumented", attributes: [0 => 'test', 1 => 'abc', 'test' => 'example']); +test_instrumented(); + +?> +--EXPECTF-- +test: example \ No newline at end of file diff --git a/tests/test_variadic_metdata_overwrite.phpt b/tests/test_variadic_metdata_overwrite.phpt new file mode 100644 index 0000000..dc4849d --- /dev/null +++ b/tests/test_variadic_metdata_overwrite.phpt @@ -0,0 +1,34 @@ +--TEST-- +Tests that the attributes named parameter has special handling and overwrites previously declared params. +--EXTENSIONS-- +sentry +--FILE-- + "abc", "test" => "example"]); +test_instrumented(); + +?> +--EXPECTF-- +Name: test_instrumented +Duration: %f +OP: foo +Test: example +Custom: abc \ No newline at end of file From d70cb2715b082554cfbaee532b2e752d2d689d91 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 11:15:04 +0200 Subject: [PATCH 04/15] polyfill zend_array_is_list --- sentry.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/sentry.c b/sentry.c index 288a8ad..bca192b 100644 --- a/sentry.c +++ b/sentry.c @@ -52,6 +52,24 @@ ZEND_TSRMLS_CACHE_DEFINE(); */ static zend_string *sentry_trace_attribute_lcname; +static bool sentry_array_is_list(const zend_array *array) { +#if PHP_VERSION_ID >= 80100 + return zend_array_is_list(array); +#else + zend_ulong expected_key = 0; + zend_ulong num_key; + zend_string *str_key; + + ZEND_HASH_FOREACH_KEY(array, num_key, str_key) { + if (str_key != NULL || num_key != expected_key++) { + return false; + } + } ZEND_HASH_FOREACH_END(); + + return true; +#endif +} + // ==== CALL STATE BEGIN ==== @@ -126,7 +144,7 @@ static bool sentry_is_attribute_arg(zend_string *name, zval *value) { * Integer keys are ignored and will not be copied. */ static void sentry_merge_array(zval *dest, zval *source) { - if (zend_array_is_list(Z_ARRVAL_P(source))) { + if (sentry_array_is_list(Z_ARRVAL_P(source))) { return; } zend_string *str_key; @@ -444,14 +462,6 @@ ZEND_FUNCTION(Sentry_instrument) { } ZEND_HASH_FOREACH_END(); } - // - // if (extra_metadata != NULL) { - // zend_hash_copy( - // Z_ARRVAL(metadata), - // Z_ARRVAL_P(extra_metadata), - // zval_add_ref - // ); - // } sentry_merge_deferred_attributes(&metadata, &attribute_list); From 13112e58203e6b34ccbf570030a28601fb3a7bbb Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 11:50:48 +0200 Subject: [PATCH 05/15] fix clearing exception --- sentry.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sentry.c b/sentry.c index bca192b..72b48ab 100644 --- a/sentry.c +++ b/sentry.c @@ -70,7 +70,6 @@ static bool sentry_array_is_list(const zend_array *array) { #endif } - // ==== CALL STATE BEGIN ==== /** @@ -250,8 +249,6 @@ static void sentry_get_attribute_metadata( sentry_merge_array(metadata, &attribute_arg); } - sentry_clear_pending_exception(); - if (!Z_ISUNDEF(attribute_arg)) { zval_ptr_dtor(&attribute_arg); } @@ -479,9 +476,6 @@ ZEND_FUNCTION(Sentry_instrument) { RETURN_BOOL(inserted != NULL); } - - - ZEND_FUNCTION(Sentry_setEndCallback) { zval *callback; From 6918ad0d1f56d62a8ea65efd5561ff810c905b94 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 11:57:08 +0200 Subject: [PATCH 06/15] add tests --- .../test_invalid_attribute_doesnt_crash.phpt | 28 +++++++++++++++++ tests/test_invalid_attributes_mixed.phpt | 30 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 tests/test_invalid_attribute_doesnt_crash.phpt create mode 100644 tests/test_invalid_attributes_mixed.phpt diff --git a/tests/test_invalid_attribute_doesnt_crash.phpt b/tests/test_invalid_attribute_doesnt_crash.phpt new file mode 100644 index 0000000..351712d --- /dev/null +++ b/tests/test_invalid_attribute_doesnt_crash.phpt @@ -0,0 +1,28 @@ +--TEST-- +Tests that referencing a non existent constant doesnt cause any errors +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +Name: test_instrumented +Duration: %f \ No newline at end of file diff --git a/tests/test_invalid_attributes_mixed.phpt b/tests/test_invalid_attributes_mixed.phpt new file mode 100644 index 0000000..1a6f790 --- /dev/null +++ b/tests/test_invalid_attributes_mixed.phpt @@ -0,0 +1,30 @@ +--TEST-- +Tests that using invalid and valid attributes only extract the valid ones +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +Name: test_instrumented +Duration: %f +Description: Test \ No newline at end of file From 0970385da939b0cdcda285f49928ffb1ab8f235e Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 11:58:32 +0200 Subject: [PATCH 07/15] update test --- tests/test_invalid_attributes_mixed.phpt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_invalid_attributes_mixed.phpt b/tests/test_invalid_attributes_mixed.phpt index 1a6f790..75c82b8 100644 --- a/tests/test_invalid_attributes_mixed.phpt +++ b/tests/test_invalid_attributes_mixed.phpt @@ -19,6 +19,9 @@ function test_instrumented() { echo "Name: " . $data['name'] . PHP_EOL; echo "Duration: " . $data['duration'] . PHP_EOL; echo "Description: " . $data['metadata']['description'] . PHP_EOL; + if (!isset($data['metadata']['name'])) { + echo "Works as expected" . PHP_EOL; + } }); test_instrumented(); @@ -27,4 +30,5 @@ test_instrumented(); --EXPECTF-- Name: test_instrumented Duration: %f -Description: Test \ No newline at end of file +Description: Test +Works as expected \ No newline at end of file From fa6633f65b37a68efbcbd320e00436439ce9a94b Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 13:06:52 +0200 Subject: [PATCH 08/15] fix --- sentry.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sentry.c b/sentry.c index 72b48ab..2e6661f 100644 --- a/sentry.c +++ b/sentry.c @@ -123,11 +123,8 @@ static bool sentry_has_trace_attribute(zend_execute_data *execute_data) { } static void sentry_clear_pending_exception(void) { - zend_object *ex = EG(exception); - - if (ex != NULL) { - EG(exception) = NULL; - OBJ_RELEASE(ex); + if (EG(exception) != NULL || EG(prev_exception) != NULL) { + zend_clear_exception(); } } From 305d17fc666d7b2952aa477713ec1d56319ad02a Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Fri, 19 Jun 2026 13:14:46 +0200 Subject: [PATCH 09/15] fix --- sentry.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/sentry.c b/sentry.c index 2e6661f..77f23a6 100644 --- a/sentry.c +++ b/sentry.c @@ -222,14 +222,7 @@ static void sentry_get_attribute_metadata( const zend_result result = zend_get_attribute_value(&attribute_arg, attribute, i, execute_data->func->common.scope); // result can be unsuccessful if e.g. constants are references that do not exist. if (result != SUCCESS) { - // clear exception if the attribute has an invalid value so that we don't crash - // user code. sentry_clear_pending_exception(); - - if (!Z_ISUNDEF(attribute_arg)) { - zval_ptr_dtor(&attribute_arg); - } - continue; } From c5749ad30e56d39f95da67cdb2e68eca75d66d8f Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 24 Jun 2026 23:32:05 +0200 Subject: [PATCH 10/15] feat(attributes): add support for spanAttributes, which allows to capture values --- config.m4 | 4 +- config.w32 | 2 +- sentry.c | 185 ++++++-------- sentry_internal.c | 216 ++++++++++++++++ sentry_internal.h | 30 +++ span_attributes.c | 232 ++++++++++++++++++ span_attributes.h | 54 ++++ tests/test_span_attribute_by_reference.phpt | 25 ++ .../test_span_attribute_invoke_function.phpt | 29 +++ ...ttribute_invoke_function_doesnt_exist.phpt | 33 +++ ...attribute_invoke_function_path_throws.phpt | 41 ++++ ...span_attribute_invoke_function_throws.phpt | 37 +++ tests/test_span_attribute_magic_getter.phpt | 29 +++ ..._attribute_magic_getter_reentry_guard.phpt | 34 +++ ...st_span_attribute_magic_getter_throws.phpt | 33 +++ .../test_span_attribute_private_property.phpt | 27 ++ ..._span_attribute_property_doesnt_exist.phpt | 31 +++ ...est_span_attribute_protected_property.phpt | 27 ++ .../test_span_attribute_public_property.phpt | 27 ++ tests/test_span_attribute_uninitialized.phpt | 29 +++ ...span_attributes_different_invocations.phpt | 39 +++ ...est_span_attributes_different_strings.phpt | 27 ++ tests/test_span_attributes_false.phpt | 25 ++ tests/test_span_attributes_float.phpt | 23 ++ tests/test_span_attributes_int.phpt | 23 ++ tests/test_span_attributes_multiple.phpt | 32 +++ tests/test_span_attributes_null.phpt | 25 ++ tests/test_span_attributes_string.phpt | 23 ++ tests/test_span_attributes_true.phpt | 25 ++ 29 files changed, 1256 insertions(+), 111 deletions(-) create mode 100644 sentry_internal.c create mode 100644 sentry_internal.h create mode 100644 span_attributes.c create mode 100644 span_attributes.h create mode 100644 tests/test_span_attribute_by_reference.phpt create mode 100644 tests/test_span_attribute_invoke_function.phpt create mode 100644 tests/test_span_attribute_invoke_function_doesnt_exist.phpt create mode 100644 tests/test_span_attribute_invoke_function_path_throws.phpt create mode 100644 tests/test_span_attribute_invoke_function_throws.phpt create mode 100644 tests/test_span_attribute_magic_getter.phpt create mode 100644 tests/test_span_attribute_magic_getter_reentry_guard.phpt create mode 100644 tests/test_span_attribute_magic_getter_throws.phpt create mode 100644 tests/test_span_attribute_private_property.phpt create mode 100644 tests/test_span_attribute_property_doesnt_exist.phpt create mode 100644 tests/test_span_attribute_protected_property.phpt create mode 100644 tests/test_span_attribute_public_property.phpt create mode 100644 tests/test_span_attribute_uninitialized.phpt create mode 100644 tests/test_span_attributes_different_invocations.phpt create mode 100644 tests/test_span_attributes_different_strings.phpt create mode 100644 tests/test_span_attributes_false.phpt create mode 100644 tests/test_span_attributes_float.phpt create mode 100644 tests/test_span_attributes_int.phpt create mode 100644 tests/test_span_attributes_multiple.phpt create mode 100644 tests/test_span_attributes_null.phpt create mode 100644 tests/test_span_attributes_string.phpt create mode 100644 tests/test_span_attributes_true.phpt diff --git a/config.m4 b/config.m4 index d9e2d86..707b07a 100644 --- a/config.m4 +++ b/config.m4 @@ -2,5 +2,5 @@ PHP_ARG_ENABLE(sentry, whether to enable Sentry support, [ --enable-sentry Enable Sentry support]) if test "$PHP_SENTRY" = "yes"; then AC_DEFINE(HAVE_SENTRY, 1, [Whether you have Sentry]) - PHP_NEW_EXTENSION(sentry, sentry.c, $ext_shared) -fi \ No newline at end of file + PHP_NEW_EXTENSION(sentry, sentry.c span_attributes.c sentry_internal.c, $ext_shared) +fi diff --git a/config.w32 b/config.w32 index a6197ba..2ea6f4c 100644 --- a/config.w32 +++ b/config.w32 @@ -1,5 +1,5 @@ ARG_ENABLE("sentry", "Enable Sentry support", "no"); if (PHP_SENTRY == "yes") { AC_DEFINE("HAVE_SENTRY", 1, "Whether you have Sentry"); - EXTENSION("Sentry", "sentry.c", true); + EXTENSION("Sentry", "sentry.c span_attributes.c sentry_internal.c", true); } diff --git a/sentry.c b/sentry.c index 77f23a6..e55febd 100644 --- a/sentry.c +++ b/sentry.c @@ -18,6 +18,8 @@ #else #include #endif +#include "span_attributes.h" +#include "sentry_internal.h" ZEND_BEGIN_MODULE_GLOBALS(sentry) // Functions that should be observed. Values are the metadata arrays per instrumented call. @@ -266,94 +268,28 @@ ZEND_METHOD(Sentry_Trace, __construct) { // ===== EXCEPTION ISOLATION START === -// Stores exception and opline information before invoking the start or end callback. -// We do that so that the callback runs without any interference from exceptions that might -// be set from code before. -typedef struct { - zend_object *exception; - zend_object *prev_exception; - const zend_op *opline_before_exception; - bool has_opline; - const zend_op *opline; -} sentry_exception_state; - -static void sentry_exception_isolation_start(sentry_exception_state *state) { - state->exception = EG(exception); - state->prev_exception = EG(prev_exception); - state->opline_before_exception = EG(opline_before_exception); - - EG(exception) = NULL; - EG(prev_exception) = NULL; - EG(opline_before_exception) = NULL; - - const zend_execute_data *execute_data = EG(current_execute_data); - state->has_opline = execute_data != NULL; - state->opline = execute_data ? execute_data->opline : NULL; -} - -static zend_object *sentry_exception_isolation_end(sentry_exception_state *state) { - zend_object *suppressed = EG(exception); - - // exit() unwinds via a fake exception that must not be intercepted: leave it - // pending so the engine keeps tearing down the stack, and abandon the saved - // exception — nothing will ever catch it, so we release our references here. - if (UNEXPECTED(suppressed && zend_is_unwind_exit(suppressed))) { - if (state->exception != NULL) { - OBJ_RELEASE(state->exception); - } - if (state->prev_exception != NULL) { - OBJ_RELEASE(state->prev_exception); - } - return NULL; - } - - // Detach the exception the callback itself may have thrown: it lives on in - // `suppressed` and the caller owns and releases it. It must not stay in - // EG(exception), where it would mask the original exception we are about to - // restore. - // We have to set it to NULL otherwise zend_clear_exception will invoke - // the exception handler. - EG(exception) = NULL; - zend_clear_exception(); - - EG(exception) = state->exception; - EG(prev_exception) = state->prev_exception; - EG(opline_before_exception) = state->opline_before_exception; +bool sentry_enter_internal_call() { + bool previous_in_callback = SENTRY_G(in_callback); + SENTRY_G(in_callback) = true; - zend_execute_data *execute_data = EG(current_execute_data); - if (execute_data != NULL && state->has_opline) { - execute_data->opline = state->opline; - } + return previous_in_callback; +} - return suppressed; +void sentry_leave_internal_call(bool previous) { + SENTRY_G(in_callback) = previous; } -static void sentry_call_user_function_isolated( +static void sentry_call_user_function_guarded( zval *callback, zval *retval, uint32_t param_count, zval *params ) { - SENTRY_G(in_callback) = true; - - sentry_exception_state state; - sentry_exception_isolation_start(&state); + bool previous = sentry_enter_internal_call(); - call_user_function( - EG(function_table), - NULL, - callback, - retval, - param_count, - params - ); + sentry_call_user_function_isolated(callback, retval, param_count, params); - zend_object *suppressed = sentry_exception_isolation_end(&state); - if (suppressed != NULL) { - OBJ_RELEASE(suppressed); - } - - SENTRY_G(in_callback) = false; + sentry_leave_internal_call(previous); } // ===== EXCEPTION ISOLATION END ==== @@ -390,8 +326,6 @@ static zend_string *sentry_build_key(zend_string *class_name, zend_string *funct ZEND_FUNCTION(Sentry_instrument) { zend_string *class_name = NULL; zend_string *function_name; - // zval metadata; - // zval *extra_metadata = NULL; zval *metadata_args = NULL; uint32_t metadata_argc = 0; @@ -403,33 +337,44 @@ ZEND_FUNCTION(Sentry_instrument) { Z_PARAM_VARIADIC_WITH_NAMED(metadata_args, metadata_argc, named_metadata) ZEND_PARSE_PARAMETERS_END(); - // If a subclass doesn't override a method from the parent, the scope will - // remain of the parent. For example, if A defined method food and B extends A - // without overriding, doing (new B())->foo() will show up as A::foo in the - // extension. This means that declaring an instrumentation on B::foo will never - // trigger. The code below changes the classname so that it will correctly work - // for subclasses. + zend_function *instrumented_func = NULL; + zend_string *lc_func = zend_string_tolower(function_name); if (class_name != NULL) { + // If a subclass doesn't override a method from the parent, the scope will + // remain of the parent. For example, if A defined method food and B extends A + // without overriding, doing (new B())->foo() will show up as A::foo in the + // extension. This means that declaring an instrumentation on B::foo will never + // trigger. The code below changes the classname so that it will correctly work + // for subclasses. zend_class_entry *ce = zend_lookup_class(class_name); if (ce != NULL) { - zend_string *lc_func = zend_string_tolower(function_name); zend_function *func = zend_hash_find_ptr(&ce->function_table, lc_func); if (func != NULL && func->common.scope != NULL) { class_name = func->common.scope->name; + instrumented_func = func; } - zend_string_release(lc_func); } + } else { + instrumented_func = zend_hash_find_ptr(CG(function_table), lc_func); } + zend_string_release(lc_func); - zval metadata; - array_init(&metadata); + sentry_instrumentation *instrumentation = emalloc(sizeof(sentry_instrumentation)); + array_init(&instrumentation->metadata); + zend_hash_init( + &instrumentation->span_attributes, + 4, + NULL, + sentry_span_attribute_rule_dtor, + 0 + ); zval attribute_list; ZVAL_UNDEF(&attribute_list); for (uint32_t i = 0; i < metadata_argc; i++) { if (Z_TYPE(metadata_args[i]) == IS_ARRAY) { - sentry_merge_array(&metadata, &metadata_args[i]); + sentry_merge_array(&instrumentation->metadata, &metadata_args[i]); } } @@ -438,9 +383,21 @@ ZEND_FUNCTION(Sentry_instrument) { zval *value; ZEND_HASH_FOREACH_STR_KEY_VAL(named_metadata, name, value) { - if (name != NULL) { + if (name == NULL) { + continue; + } + + if (sentry_is_span_attributes_arg(name, value)) { + if (instrumented_func != NULL) { + sentry_add_span_attribute_rules( + &instrumentation->span_attributes, + instrumented_func, + value + ); + } + } else { sentry_add_named_metadata_arg( - &metadata, + &instrumentation->metadata, &attribute_list, name, value @@ -450,15 +407,17 @@ ZEND_FUNCTION(Sentry_instrument) { ZEND_HASH_FOREACH_END(); } - sentry_merge_deferred_attributes(&metadata, &attribute_list); + sentry_merge_deferred_attributes(&instrumentation->metadata, &attribute_list); zend_string *key = sentry_build_key(class_name, function_name); - const zval* inserted = zend_hash_add(&SENTRY_G(instrumented_functions), key, &metadata); + zval instrumentation_zv; + ZVAL_PTR(&instrumentation_zv, instrumentation); + const zval* inserted = zend_hash_add(&SENTRY_G(instrumented_functions), key, &instrumentation_zv); // If the element wasn't inserted we have to manually destroy the local value to prevent memory leaks if (inserted == NULL) { - zval_ptr_dtor(&metadata); + sentry_instrumentation_dtor(&instrumentation_zv); } zend_string_release(key); @@ -547,13 +506,19 @@ static void sentry_observer_begin(zend_execute_data *execute_data) { execute_data->func->common.scope == NULL ? NULL : execute_data->func->common.scope->name, execute_data->func->common.function_name ); - zval *metadata = zend_hash_find(&SENTRY_G(instrumented_functions), key); + + zval *instrumentation_zv = zend_hash_find(&SENTRY_G(instrumented_functions), key); zend_string_release(key); + zval *metadata = NULL; + sentry_instrumentation *instrumentation = NULL; zval attribute_metadata; bool using_attribute_metadata = false; - if (metadata == NULL) { + if (instrumentation_zv != NULL) { + instrumentation = Z_PTR_P(instrumentation_zv); + metadata = &instrumentation->metadata; + } else { sentry_get_attribute_metadata(execute_data, &attribute_metadata); metadata = &attribute_metadata; using_attribute_metadata = true; @@ -572,6 +537,14 @@ static void sentry_observer_begin(zend_execute_data *execute_data) { ZVAL_COPY(&state->metadata, metadata); + if (instrumentation != NULL) { + sentry_apply_span_attribute_rules( + instrumentation, + execute_data, + &state->metadata + ); + } + if (!Z_ISUNDEF(SENTRY_G(start_callback))) { zval data; array_init(&data); @@ -580,13 +553,13 @@ static void sentry_observer_begin(zend_execute_data *execute_data) { add_assoc_double(&data, "start_time", state->start_time); zval metadata_zv; - ZVAL_COPY(&metadata_zv, metadata); + ZVAL_COPY(&metadata_zv, &state->metadata); add_assoc_zval(&data, "metadata", &metadata_zv); zval params[1]; ZVAL_COPY_VALUE(¶ms[0], &data); - sentry_call_user_function_isolated( + sentry_call_user_function_guarded( &SENTRY_G(start_callback), &retval, 1, @@ -657,16 +630,14 @@ static void sentry_observer_end(zend_execute_data *execute_data, zval *return_va ZVAL_COPY_VALUE(¶ms[1], &state->user_state); - sentry_call_user_function_isolated( + sentry_call_user_function_guarded( &SENTRY_G(end_callback), &retval, 2, params ); - if (!Z_ISUNDEF(retval)) { - zval_ptr_dtor(&retval); - } + sentry_zval_ptr_dtor_undef(&retval); zval_ptr_dtor(&event); } @@ -740,7 +711,7 @@ static PHP_GINIT_FUNCTION(sentry) { PHP_RINIT_FUNCTION(sentry) { SENTRY_G(in_callback) = false; - zend_hash_init(&SENTRY_G(instrumented_functions), 8, NULL, ZVAL_PTR_DTOR, 0); + zend_hash_init(&SENTRY_G(instrumented_functions), 8, NULL, sentry_instrumentation_dtor, 0); zend_hash_init(&SENTRY_G(active_calls), 8, NULL, sentry_call_state_dtor, 0); ZVAL_UNDEF(&SENTRY_G(start_callback)); @@ -754,13 +725,11 @@ PHP_RSHUTDOWN_FUNCTION(sentry) { zend_hash_destroy(&SENTRY_G(active_calls)); if (!Z_ISUNDEF(SENTRY_G(start_callback))) { - zval_ptr_dtor(&SENTRY_G(start_callback)); - ZVAL_UNDEF(&SENTRY_G(start_callback)); + sentry_zval_ptr_dtor_undef(&SENTRY_G(start_callback)); } if (!Z_ISUNDEF(SENTRY_G(end_callback))) { - zval_ptr_dtor(&SENTRY_G(end_callback)); - ZVAL_UNDEF(&SENTRY_G(end_callback)); + sentry_zval_ptr_dtor_undef(&SENTRY_G(end_callback)); } return SUCCESS; diff --git a/sentry_internal.c b/sentry_internal.c new file mode 100644 index 0000000..30365c5 --- /dev/null +++ b/sentry_internal.c @@ -0,0 +1,216 @@ +#include "php.h" +#include "Zend/zend_exceptions.h" +#include "Zend/zend_interfaces.h" +#include "sentry_internal.h" + +void sentry_zval_ptr_dtor_undef(zval *zv) { + if (!Z_ISUNDEF_P(zv)) { + zval_ptr_dtor(zv); + ZVAL_UNDEF(zv); + } +} + +typedef struct { + zval *object; + zend_string *name; +} sentry_operation_context; + +static bool sentry_call_method_operation(void *context, zval *retval) { + sentry_operation_context *ctx = context; + + zend_result result = zend_call_method_if_exists( + Z_OBJ_P(ctx->object), + ctx->name, + retval, + 0, + NULL + ); + + return result == SUCCESS; +} + +static bool sentry_read_property_operation(void *context, zval *retval) { + sentry_operation_context *ctx = context; + + zval property_value; + ZVAL_UNDEF(&property_value); + + zval *value = zend_read_property_ex( + Z_OBJCE_P(ctx->object), + Z_OBJ_P(ctx->object), + ctx->name, + true, + &property_value + ); + + if (value == NULL || Z_ISUNDEF_P(value)) { + sentry_zval_ptr_dtor_undef(&property_value); + return false; + } + + ZVAL_COPY(retval, value); + + sentry_zval_ptr_dtor_undef(&property_value); + + return true; +} + +// Stores exception and opline information before invoking the start or end callback. +// We do that so that the callback runs without any interference from exceptions that might +// be set from code before. +typedef struct { + zend_object *exception; + zend_object *prev_exception; + const zend_op *opline_before_exception; + bool has_opline; + const zend_op *opline; +} sentry_exception_state; + +static void sentry_exception_isolation_start(sentry_exception_state *state) { + state->exception = EG(exception); + state->prev_exception = EG(prev_exception); + state->opline_before_exception = EG(opline_before_exception); + + EG(exception) = NULL; + EG(prev_exception) = NULL; + EG(opline_before_exception) = NULL; + + const zend_execute_data *execute_data = EG(current_execute_data); + state->has_opline = execute_data != NULL; + state->opline = execute_data ? execute_data->opline : NULL; +} + +static zend_object *sentry_exception_isolation_end(sentry_exception_state *state) { + zend_object *suppressed = EG(exception); + + // exit() unwinds via a fake exception that must not be intercepted: leave it + // pending so the engine keeps tearing down the stack, and abandon the saved + // exception — nothing will ever catch it, so we release our references here. + if (UNEXPECTED(suppressed && zend_is_unwind_exit(suppressed))) { + if (state->exception != NULL) { + OBJ_RELEASE(state->exception); + } + if (state->prev_exception != NULL) { + OBJ_RELEASE(state->prev_exception); + } + return NULL; + } + + // Detach the exception the callback itself may have thrown: it lives on in + // `suppressed` and the caller owns and releases it. It must not stay in + // EG(exception), where it would mask the original exception we are about to + // restore. + // We have to set it to NULL otherwise zend_clear_exception will invoke + // the exception handler. + EG(exception) = NULL; + zend_clear_exception(); + + EG(exception) = state->exception; + EG(prev_exception) = state->prev_exception; + EG(opline_before_exception) = state->opline_before_exception; + + zend_execute_data *execute_data = EG(current_execute_data); + if (execute_data != NULL && state->has_opline) { + execute_data->opline = state->opline; + } + + return suppressed; +} + + +typedef bool (*sentry_guarded_operation)(void *context, zval *retval); + +static bool sentry_run_internal_call_guarded( + sentry_guarded_operation operation, + void *context, + zval *retval +) { + ZVAL_UNDEF(retval); + + sentry_exception_state state; + sentry_exception_isolation_start(&state); + + bool previous = sentry_enter_internal_call(); + bool ok = operation(context, retval); + sentry_leave_internal_call(previous); + + zend_object *suppressed = sentry_exception_isolation_end(&state); + if (suppressed != NULL) { + OBJ_RELEASE(suppressed); + + sentry_zval_ptr_dtor_undef(retval); + return false; + } + + if (!ok || Z_ISUNDEF_P(retval)) { + sentry_zval_ptr_dtor_undef(retval); + return false; + } + return true; +} + +void sentry_call_user_function_isolated( + zval *callback, + zval *retval, + uint32_t param_count, + zval *params +) { + sentry_exception_state state; + sentry_exception_isolation_start(&state); + + call_user_function( + EG(function_table), + NULL, + callback, + retval, + param_count, + params + ); + + zend_object *suppressed = sentry_exception_isolation_end(&state); + if (suppressed != NULL) { + OBJ_RELEASE(suppressed); + } +} + +bool sentry_call_method_guarded( + zval *object, + zend_string *method_name, + zval *retval +) { + if (Z_TYPE_P(object) != IS_OBJECT) { + return false; + } + + sentry_operation_context ctx = { + object, + method_name, + }; + + return sentry_run_internal_call_guarded( + sentry_call_method_operation, + &ctx, + retval + ); +} + +bool sentry_read_property_guarded( + zval *object, + zend_string *property_name, + zval *retval +) { + if (Z_TYPE_P(object) != IS_OBJECT) { + return false; + } + + sentry_operation_context ctx = { + object, + property_name, + }; + + return sentry_run_internal_call_guarded( + sentry_read_property_operation, + &ctx, + retval + ); +} \ No newline at end of file diff --git a/sentry_internal.h b/sentry_internal.h new file mode 100644 index 0000000..aaba8e1 --- /dev/null +++ b/sentry_internal.h @@ -0,0 +1,30 @@ +#ifndef SENTRY_PHP_TRACER_SENTRY_INTERNAL_H +#define SENTRY_PHP_TRACER_SENTRY_INTERNAL_H + +#include "php.h" + +void sentry_zval_ptr_dtor_undef(zval *zv); + +bool sentry_call_method_guarded( + zval *object, + zend_string *method_name, + zval *retval +); + +void sentry_call_user_function_isolated( + zval *callback, + zval *retval, + uint32_t param_count, + zval *params +); + +bool sentry_enter_internal_call(void); +void sentry_leave_internal_call(bool previous_in_callback); + +bool sentry_read_property_guarded( + zval *object, + zend_string *property_name, + zval *retval + ); + +#endif //SENTRY_PHP_TRACER_SENTRY_INTERNAL_H diff --git a/span_attributes.c b/span_attributes.c new file mode 100644 index 0000000..47686d8 --- /dev/null +++ b/span_attributes.c @@ -0,0 +1,232 @@ +#include "php.h" +#include "Zend/zend_interfaces.h" +#include "span_attributes.h" +#include "sentry_internal.h" + +static bool sentry_path_is_method_call(zend_string *path) { + size_t length = ZSTR_LEN(path); + + if (length <= 2) { + return false; + } + + return ZSTR_VAL(path)[length - 2] == '(' + && ZSTR_VAL(path)[length - 1] == ')'; +} + +static zend_string *sentry_path_method_name(zend_string *path) { + return zend_string_init( + ZSTR_VAL(path), + ZSTR_LEN(path) - 2, + 0 + ); +} + +void sentry_span_attribute_rule_dtor(zval *zv) { + sentry_span_attribute_rule *rule = Z_PTR_P(zv); + + zend_string_release(rule->name); + + if (rule->path != NULL) { + zend_string_release(rule->path); + } + + efree(rule); +} + +void sentry_instrumentation_dtor(zval *zv) { + sentry_instrumentation *instrumentation = Z_PTR_P(zv); + + zval_ptr_dtor(&instrumentation->metadata); + zend_hash_destroy(&instrumentation->span_attributes); + + efree(instrumentation); +} + +bool sentry_is_span_attributes_arg(zend_string *name, zval *value) { + return name != NULL + && Z_TYPE_P(value) == IS_ARRAY + && zend_string_equals_literal(name, "spanAttributes"); +} + +bool sentry_resolve_param_index( + zend_function *func, + zend_string *param_name, + uint32_t *param_index +) { + if (func->common.arg_info == NULL) { + return false; + } + + for (uint32_t i = 0; i < func->common.num_args; i++) { + zend_arg_info *arg_info = &func->common.arg_info[i]; + + if (arg_info->name != NULL && zend_string_equals(arg_info->name, param_name)) { + *param_index = i; + return true; + } + } + + return false; +} + +void sentry_add_span_attribute_rules( + HashTable *rules, + zend_function *func, + zval *span_attributes +) { + zend_string *attribute_name; + zval *definition; + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(span_attributes), attribute_name, definition) { + if (attribute_name == NULL || Z_TYPE_P(definition) != IS_ARRAY) { + continue; + } + + zval *param_name = zend_hash_index_find(Z_ARRVAL_P(definition), 0); + zval *path = zend_hash_index_find(Z_ARRVAL_P(definition), 1); + + if (param_name == NULL || Z_TYPE_P(param_name) != IS_STRING) { + continue; + } + + if (path != NULL && Z_TYPE_P(path) != IS_STRING) { + continue; + } + + uint32_t param_index = 0; + if (!sentry_resolve_param_index(func, Z_STR_P(param_name), ¶m_index)) { + continue; + } + + sentry_span_attribute_rule *rule = emalloc(sizeof(sentry_span_attribute_rule)); + rule->name = zend_string_copy(attribute_name); + rule->param_index = param_index; + + if (path == NULL) { + rule->access_kind = SENTRY_SPAN_ATTRIBUTE_ACCESS_DIRECT; + rule->path = NULL; + } else if (sentry_path_is_method_call(Z_STR_P(path))) { + rule->access_kind = SENTRY_SPAN_ATTRIBUTE_ACCESS_METHOD; + rule->path = sentry_path_method_name(Z_STR_P(path)); + } else { + rule->access_kind = SENTRY_SPAN_ATTRIBUTE_ACCESS_PROPERTY; + rule->path = zend_string_copy(Z_STR_P(path)); + } + + zval rule_zv; + ZVAL_PTR(&rule_zv, rule); + if (zend_hash_next_index_insert(rules, &rule_zv) == NULL) { + sentry_span_attribute_rule_dtor(&rule_zv); + } + } ZEND_HASH_FOREACH_END(); +} + +static bool sentry_copy_span_attribute_value(zval *source, zval *dest) { + ZVAL_DEREF(source); + + switch (Z_TYPE_P(source)) { + case IS_NULL: + case IS_FALSE: + case IS_TRUE: + case IS_LONG: + case IS_DOUBLE: + ZVAL_COPY_VALUE(dest, source); + return true; + case IS_STRING: + ZVAL_COPY(dest, source); + return true; + default: + return false; + } +} + + +static bool sentry_evaluate_method_path( + zval *root, + zend_string *method_name, + zval *result +) { + if (Z_TYPE_P(root) != IS_OBJECT) { + return false; + } + + zval retval; + ZVAL_UNDEF(&retval); + + bool called = sentry_call_method_guarded(root, method_name, &retval); + + if (!called) { + return false; + } + + if (!sentry_copy_span_attribute_value(&retval, result)) { + zval_ptr_dtor(&retval); + return false; + } + + zval_ptr_dtor(&retval); + return true; +} + +static bool sentry_evaluate_property_path( + zval *root, + zend_string *path, + zval *result +) { + zval property_value; + ZVAL_UNDEF(&property_value); + + if (!sentry_read_property_guarded(root, path, &property_value)) { + return false; + } + + bool copied = sentry_copy_span_attribute_value(&property_value, result); + zval_ptr_dtor(&property_value); + + return copied; +} + +void sentry_apply_span_attribute_rules( + sentry_instrumentation *instrumentation, + zend_execute_data *execute_data, + zval *metadata +) { + zval *rule_zv; + + ZEND_HASH_FOREACH_VAL(&instrumentation->span_attributes, rule_zv) { + sentry_span_attribute_rule *rule = Z_PTR_P(rule_zv); + + if (rule->param_index >= ZEND_CALL_NUM_ARGS(execute_data)) { + continue; + } + + zval *argument = ZEND_CALL_ARG(execute_data, rule->param_index + 1); + + zval copied_value; + bool copied; + + switch (rule->access_kind) { + case SENTRY_SPAN_ATTRIBUTE_ACCESS_DIRECT: + copied = sentry_copy_span_attribute_value(argument, &copied_value); + break; + case SENTRY_SPAN_ATTRIBUTE_ACCESS_METHOD: + copied = sentry_evaluate_method_path(argument, rule->path, &copied_value); + break; + + case SENTRY_SPAN_ATTRIBUTE_ACCESS_PROPERTY: + copied = sentry_evaluate_property_path(argument, rule->path, &copied_value); + break; + + default: + copied = false; + } + + if (!copied) { + continue; + } + + zend_hash_update(Z_ARRVAL_P(metadata), rule->name, &copied_value); + } ZEND_HASH_FOREACH_END(); +} + diff --git a/span_attributes.h b/span_attributes.h new file mode 100644 index 0000000..455b611 --- /dev/null +++ b/span_attributes.h @@ -0,0 +1,54 @@ +#ifndef SENTRY_SPAN_ATTRIBUTES_H +#define SENTRY_SPAN_ATTRIBUTES_H + +#include "php.h" + +typedef struct { + zval metadata; + // contains sentry_span_attribute_rule for one instrumented function + HashTable span_attributes; +} sentry_instrumentation; + +typedef enum { + SENTRY_SPAN_ATTRIBUTE_ACCESS_DIRECT, + SENTRY_SPAN_ATTRIBUTE_ACCESS_PROPERTY, + SENTRY_SPAN_ATTRIBUTE_ACCESS_METHOD +} sentry_span_attribute_access_kind; + +typedef struct { + // name of the parameter + zend_string *name; + // index of the parameter + uint32_t param_index; + // the path to invoke on the parameter to get the final data. null means it will use + // the value as-is + zend_string *path; + // the type of access. Can be either direct (if scalar), or property or method + sentry_span_attribute_access_kind access_kind; +} sentry_span_attribute_rule; + +void sentry_span_attribute_rule_dtor(zval *zv); + +void sentry_instrumentation_dtor(zval *zv); + +bool sentry_is_span_attributes_arg(zend_string *name, zval *value); + +void sentry_add_span_attribute_rules( + HashTable *rules, + zend_function *func, + zval *span_attributes +); + +bool sentry_resolve_param_index( + zend_function *func, + zend_string *param_name, + uint32_t *param_index +); + +void sentry_apply_span_attribute_rules( + sentry_instrumentation *instrumentation, + zend_execute_data *execute_data, + zval *metadata +); + +#endif diff --git a/tests/test_span_attribute_by_reference.phpt b/tests/test_span_attribute_by_reference.phpt new file mode 100644 index 0000000..edc1d0b --- /dev/null +++ b/tests/test_span_attribute_by_reference.phpt @@ -0,0 +1,25 @@ +--TEST-- +Tests that spanAttributes can capture params by reference. +--EXTENSIONS-- +sentry +--FILE-- + ['param'] +]); + +$x = "foo"; +test_instrumented($x); + +?> +--EXPECTF-- +Description: foo \ No newline at end of file diff --git a/tests/test_span_attribute_invoke_function.phpt b/tests/test_span_attribute_invoke_function.phpt new file mode 100644 index 0000000..501c55c --- /dev/null +++ b/tests/test_span_attribute_invoke_function.phpt @@ -0,0 +1,29 @@ +--TEST-- +Tests that spanAttributes can specify paths to invoke methods without params. +--EXTENSIONS-- +sentry +--FILE-- +getFoo(); +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'getFoo()'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: foo \ No newline at end of file diff --git a/tests/test_span_attribute_invoke_function_doesnt_exist.phpt b/tests/test_span_attribute_invoke_function_doesnt_exist.phpt new file mode 100644 index 0000000..431a611 --- /dev/null +++ b/tests/test_span_attribute_invoke_function_doesnt_exist.phpt @@ -0,0 +1,33 @@ +--TEST-- +Tests that spanAttributes can specify paths to invoke methods that doesn't exist. +--EXTENSIONS-- +sentry +--FILE-- +getFoo(); +} + +\Sentry\setEndCallback(static function (array $data) { + if (isset($data['metadata']['description'])) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; + } else { + echo "doesn't exist"; + } +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'getBar()'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +doesn't exist \ No newline at end of file diff --git a/tests/test_span_attribute_invoke_function_path_throws.phpt b/tests/test_span_attribute_invoke_function_path_throws.phpt new file mode 100644 index 0000000..a969a67 --- /dev/null +++ b/tests/test_span_attribute_invoke_function_path_throws.phpt @@ -0,0 +1,41 @@ +--TEST-- +Tests that spanAttributes can specify paths to invoke methods does not throw itself but the spanAttributes invocation throws. +--EXTENSIONS-- +sentry +--FILE-- + ['param', 'getFoo()'] +]); +try { +test_instrumented((new A())); +echo "after call" . PHP_EOL; +} catch (\Throwable $t) { + +} + +?> +--EXPECTF-- +test_instrumented invoked +doesn't exist +after call \ No newline at end of file diff --git a/tests/test_span_attribute_invoke_function_throws.phpt b/tests/test_span_attribute_invoke_function_throws.phpt new file mode 100644 index 0000000..8b66cb6 --- /dev/null +++ b/tests/test_span_attribute_invoke_function_throws.phpt @@ -0,0 +1,37 @@ +--TEST-- +Tests that spanAttributes can specify paths to invoke methods that throws an exception. +--EXTENSIONS-- +sentry +--FILE-- +getFoo(); +} + +\Sentry\setEndCallback(static function (array $data) { + if (isset($data['metadata']['description'])) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; + } else { + echo "doesn't exist"; + } +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'getFoo()'] +]); +try { +test_instrumented((new A())); +} catch (\Throwable $t) { + +} + +?> +--EXPECTF-- +doesn't exist \ No newline at end of file diff --git a/tests/test_span_attribute_magic_getter.phpt b/tests/test_span_attribute_magic_getter.phpt new file mode 100644 index 0000000..dc9a950 --- /dev/null +++ b/tests/test_span_attribute_magic_getter.phpt @@ -0,0 +1,29 @@ +--TEST-- +Tests that spanAttributes can invoke magic methods and get the correct return value. +--EXTENSIONS-- +sentry +--FILE-- +foo; +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'foo'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: fooMagic \ No newline at end of file diff --git a/tests/test_span_attribute_magic_getter_reentry_guard.phpt b/tests/test_span_attribute_magic_getter_reentry_guard.phpt new file mode 100644 index 0000000..6ba1ced --- /dev/null +++ b/tests/test_span_attribute_magic_getter_reentry_guard.phpt @@ -0,0 +1,34 @@ +--TEST-- +Tests that spanAttributes can invoke magic methods which do not trigger callbacks. +--EXTENSIONS-- +sentry +--FILE-- +instrumented(); + } + + public function instrumented() { + return "instrumented"; + } +} + +function test_instrumented(A $param) { + return 10; +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . ($data['metadata']['description'] ?? "Empty"). PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'foo'] +]); +\Sentry\instrument("A", "instrumented"); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: instrumented \ No newline at end of file diff --git a/tests/test_span_attribute_magic_getter_throws.phpt b/tests/test_span_attribute_magic_getter_throws.phpt new file mode 100644 index 0000000..2bf574b --- /dev/null +++ b/tests/test_span_attribute_magic_getter_throws.phpt @@ -0,0 +1,33 @@ +--TEST-- +Tests that spanAttributes can invoke magic methods that throw an exception which does not crash the user application. +--EXTENSIONS-- +sentry +--FILE-- + ['param', 'foo'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description does not exist \ No newline at end of file diff --git a/tests/test_span_attribute_private_property.phpt b/tests/test_span_attribute_private_property.phpt new file mode 100644 index 0000000..ea3d3bd --- /dev/null +++ b/tests/test_span_attribute_private_property.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that spanAttributes can specify paths to access a private class property. +--EXTENSIONS-- +sentry +--FILE-- + ['param', 'example'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: bar \ No newline at end of file diff --git a/tests/test_span_attribute_property_doesnt_exist.phpt b/tests/test_span_attribute_property_doesnt_exist.phpt new file mode 100644 index 0000000..93321ed --- /dev/null +++ b/tests/test_span_attribute_property_doesnt_exist.phpt @@ -0,0 +1,31 @@ +--TEST-- +Tests that spanAttributes can specify paths to access a class property that doesn't exist. +--EXTENSIONS-- +sentry +--FILE-- +example; +} + +\Sentry\setEndCallback(static function (array $data) { + if (isset($data['metadata']['description'])) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; + } else { + echo "doesn't exist"; + } +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'foo'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +doesn't exist \ No newline at end of file diff --git a/tests/test_span_attribute_protected_property.phpt b/tests/test_span_attribute_protected_property.phpt new file mode 100644 index 0000000..7810e45 --- /dev/null +++ b/tests/test_span_attribute_protected_property.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that spanAttributes can specify paths to access a protected class property. +--EXTENSIONS-- +sentry +--FILE-- + ['param', 'example'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: bar \ No newline at end of file diff --git a/tests/test_span_attribute_public_property.phpt b/tests/test_span_attribute_public_property.phpt new file mode 100644 index 0000000..7664021 --- /dev/null +++ b/tests/test_span_attribute_public_property.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that spanAttributes can specify paths to access a public class property. +--EXTENSIONS-- +sentry +--FILE-- +example; +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'example'] +]); +test_instrumented((new A())); + +?> +--EXPECTF-- +Description: bar \ No newline at end of file diff --git a/tests/test_span_attribute_uninitialized.phpt b/tests/test_span_attribute_uninitialized.phpt new file mode 100644 index 0000000..50bb19c --- /dev/null +++ b/tests/test_span_attribute_uninitialized.phpt @@ -0,0 +1,29 @@ +--TEST-- +Tests that spanAttributes will produce null for uninitialized properties. +--EXTENSIONS-- +sentry +--FILE-- + ['param', 'x'] +]); + +test_instrumented(new A()); + +?> +--EXPECTF-- +Description: null \ No newline at end of file diff --git a/tests/test_span_attributes_different_invocations.phpt b/tests/test_span_attributes_different_invocations.phpt new file mode 100644 index 0000000..66cb375 --- /dev/null +++ b/tests/test_span_attributes_different_invocations.phpt @@ -0,0 +1,39 @@ +--TEST-- +Tests that invoking it with different paths to different objects will get the correct result +--EXTENSIONS-- +sentry +--FILE-- +foo = $foo; + } + + public function getFoo() { + return $this->foo; + } +} + +function test_instrumented(A $test) { + return $test; +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['test', 'getFoo()'] +]); +test_instrumented(new A("foo")); +test_instrumented(new A("bar")); +test_instrumented(new A("baz")); + +?> +--EXPECTF-- +Description: foo +Description: bar +Description: baz \ No newline at end of file diff --git a/tests/test_span_attributes_different_strings.phpt b/tests/test_span_attributes_different_strings.phpt new file mode 100644 index 0000000..5cfc001 --- /dev/null +++ b/tests/test_span_attributes_different_strings.phpt @@ -0,0 +1,27 @@ +--TEST-- +Tests that invoking multiple times with different strings will produce the expected outcome +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented("foo"); +test_instrumented("bar"); +test_instrumented("baz"); + +?> +--EXPECTF-- +Description: foo +Description: bar +Description: baz \ No newline at end of file diff --git a/tests/test_span_attributes_false.phpt b/tests/test_span_attributes_false.phpt new file mode 100644 index 0000000..e8707a2 --- /dev/null +++ b/tests/test_span_attributes_false.phpt @@ -0,0 +1,25 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's false. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented(false); + +?> +--EXPECTF-- +Description: false \ No newline at end of file diff --git a/tests/test_span_attributes_float.phpt b/tests/test_span_attributes_float.phpt new file mode 100644 index 0000000..78bbdbb --- /dev/null +++ b/tests/test_span_attributes_float.phpt @@ -0,0 +1,23 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's a float. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented(123.456); + +?> +--EXPECTF-- +Description: 123.456 \ No newline at end of file diff --git a/tests/test_span_attributes_int.phpt b/tests/test_span_attributes_int.phpt new file mode 100644 index 0000000..b1eddb4 --- /dev/null +++ b/tests/test_span_attributes_int.phpt @@ -0,0 +1,23 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's an integer. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented(123); + +?> +--EXPECTF-- +Description: 123 \ No newline at end of file diff --git a/tests/test_span_attributes_multiple.phpt b/tests/test_span_attributes_multiple.phpt new file mode 100644 index 0000000..638f5ec --- /dev/null +++ b/tests/test_span_attributes_multiple.phpt @@ -0,0 +1,32 @@ +--TEST-- +Tests that multiple span attributes can be extracted from parameters +--EXTENSIONS-- +sentry +--FILE-- +getFoo(); +} + +\Sentry\setEndCallback(static function (array $data) { + echo "Description: " . $data['metadata']['description'] . PHP_EOL; + echo "Origin: " . $data['metadata']['origin'] . PHP_EOL; +}); + +\Sentry\instrument(null, 'test_instrumented', spanAttributes: [ + 'description' => ['param', 'getFoo()'], + 'origin' => ['test'] +]); +test_instrumented((new A()), 'sentry'); + +?> +--EXPECTF-- +Description: foo +Origin: sentry \ No newline at end of file diff --git a/tests/test_span_attributes_null.phpt b/tests/test_span_attributes_null.phpt new file mode 100644 index 0000000..eb276af --- /dev/null +++ b/tests/test_span_attributes_null.phpt @@ -0,0 +1,25 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's null. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented(null); + +?> +--EXPECTF-- +Description: null \ No newline at end of file diff --git a/tests/test_span_attributes_string.phpt b/tests/test_span_attributes_string.phpt new file mode 100644 index 0000000..5a3e21a --- /dev/null +++ b/tests/test_span_attributes_string.phpt @@ -0,0 +1,23 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's a string. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented("foo"); + +?> +--EXPECTF-- +Description: foo \ No newline at end of file diff --git a/tests/test_span_attributes_true.phpt b/tests/test_span_attributes_true.phpt new file mode 100644 index 0000000..2b3ffcf --- /dev/null +++ b/tests/test_span_attributes_true.phpt @@ -0,0 +1,25 @@ +--TEST-- +Tests that passing spanAttribute to \Sentry\Instrument will capture the value if it's true. +--EXTENSIONS-- +sentry +--FILE-- + ['test'] +]); +test_instrumented(true); + +?> +--EXPECTF-- +Description: true \ No newline at end of file From a88a0a673939d2de57392857e69cfc726243cb36 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 24 Jun 2026 23:45:40 +0200 Subject: [PATCH 11/15] add compat function for zend_call_method_if_exists --- sentry_internal.c | 52 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/sentry_internal.c b/sentry_internal.c index 30365c5..e470652 100644 --- a/sentry_internal.c +++ b/sentry_internal.c @@ -10,6 +10,48 @@ void sentry_zval_ptr_dtor_undef(zval *zv) { } } +static zend_result sentry_call_method_if_exists( + zval *object, + zend_string *method_name, + zval *retval +) { +#if PHP_VERSION_ID >= 80200 + return zend_call_method_if_exists( + Z_OBJ_P(object), + method_name, + retval, + 0, + NULL + ); +#else + zend_string *lc_method_name = zend_string_tolower(method_name); + zend_function *fn = zend_hash_find_ptr( + &Z_OBJCE_P(object)->function_table, + lc_method_name + ); + zend_string_release(lc_method_name); + + if (fn == NULL) { + ZVAL_UNDEF(retval); + return FAILURE; + } + + zend_call_method( + Z_OBJ_P(object), + Z_OBJCE_P(object), + NULL, + ZSTR_VAL(method_name), + ZSTR_LEN(method_name), + retval, + 0, + NULL, + NULL + ); + + return !Z_ISUNDEF_P(retval) ? SUCCESS : FAILURE; +#endif +} + typedef struct { zval *object; zend_string *name; @@ -18,12 +60,10 @@ typedef struct { static bool sentry_call_method_operation(void *context, zval *retval) { sentry_operation_context *ctx = context; - zend_result result = zend_call_method_if_exists( - Z_OBJ_P(ctx->object), + zend_result result = sentry_call_method_if_exists( + ctx->object, ctx->name, - retval, - 0, - NULL + retval ); return result == SUCCESS; @@ -213,4 +253,4 @@ bool sentry_read_property_guarded( &ctx, retval ); -} \ No newline at end of file +} From 825c27190f9b5a8914757e4da99d9e8d7510fdfc Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Thu, 25 Jun 2026 00:00:20 +0200 Subject: [PATCH 12/15] add simplified syntax --- span_attributes.c | 21 ++++++++++++++--- ...pan_attributes_isolated_between_calls.phpt | 22 ++++++++++++++++++ ...ttributes_simple_param_capture_syntax.phpt | 23 +++++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/test_span_attributes_isolated_between_calls.phpt create mode 100644 tests/test_span_attributes_simple_param_capture_syntax.phpt diff --git a/span_attributes.c b/span_attributes.c index 47686d8..da7aee0 100644 --- a/span_attributes.c +++ b/span_attributes.c @@ -79,12 +79,21 @@ void sentry_add_span_attribute_rules( zval *definition; ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(span_attributes), attribute_name, definition) { - if (attribute_name == NULL || Z_TYPE_P(definition) != IS_ARRAY) { + if (attribute_name == NULL) { continue; } - zval *param_name = zend_hash_index_find(Z_ARRVAL_P(definition), 0); - zval *path = zend_hash_index_find(Z_ARRVAL_P(definition), 1); + zval *param_name = NULL; + zval *path = NULL; + + if (Z_TYPE_P(definition) == IS_STRING) { + param_name = definition; + } else if (Z_TYPE_P(definition) == IS_ARRAY) { + param_name = zend_hash_index_find(Z_ARRVAL_P(definition), 0); + path = zend_hash_index_find(Z_ARRVAL_P(definition), 1); + } else { + continue; + } if (param_name == NULL || Z_TYPE_P(param_name) != IS_STRING) { continue; @@ -192,6 +201,12 @@ void sentry_apply_span_attribute_rules( zend_execute_data *execute_data, zval *metadata ) { + if (zend_hash_num_elements(&instrumentation->span_attributes) == 0) { + return; + } + + SEPARATE_ARRAY(metadata); + zval *rule_zv; ZEND_HASH_FOREACH_VAL(&instrumentation->span_attributes, rule_zv) { diff --git a/tests/test_span_attributes_isolated_between_calls.phpt b/tests/test_span_attributes_isolated_between_calls.phpt new file mode 100644 index 0000000..57ea6ff --- /dev/null +++ b/tests/test_span_attributes_isolated_between_calls.phpt @@ -0,0 +1,22 @@ +--TEST-- +Tests that invoking a function with default null will not cache the value. +--EXTENSIONS-- +sentry +--FILE-- + ['x']]); + +f('test'); +f(); + +?> +--EXPECTF-- +Description: test +Description: null \ No newline at end of file diff --git a/tests/test_span_attributes_simple_param_capture_syntax.phpt b/tests/test_span_attributes_simple_param_capture_syntax.phpt new file mode 100644 index 0000000..372a6b6 --- /dev/null +++ b/tests/test_span_attributes_simple_param_capture_syntax.phpt @@ -0,0 +1,23 @@ +--TEST-- +Tests that parameters can be captured by using the simplified syntax. +--EXTENSIONS-- +sentry +--FILE-- + 'test' +]); +test_instrumented(123); + +?> +--EXPECTF-- +Description: 123 \ No newline at end of file From 3a7a4d8b3f845271d53ea137ce161c3fd9f8cf87 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Thu, 25 Jun 2026 00:08:13 +0200 Subject: [PATCH 13/15] add handling for builtin --- span_attributes.c | 17 +++++++++++++++++ tests/test_builtin_functions.phpt | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/test_builtin_functions.phpt diff --git a/span_attributes.c b/span_attributes.c index da7aee0..26d7174 100644 --- a/span_attributes.c +++ b/span_attributes.c @@ -58,6 +58,23 @@ bool sentry_resolve_param_index( return false; } + // internal functions have a different argument structures than userland functions + // and need special handling + if (func->type == ZEND_INTERNAL_FUNCTION) { + zend_internal_arg_info *arg_info = func->internal_function.arg_info; + + for (uint32_t i = 0; i < func->common.num_args; i++) { + const char *name = arg_info[i].name; + + if (name != NULL && zend_string_equals_cstr(param_name, name, strlen(name))) { + *param_index = i; + return true; + } + } + + return false; + } + for (uint32_t i = 0; i < func->common.num_args; i++) { zend_arg_info *arg_info = &func->common.arg_info[i]; diff --git a/tests/test_builtin_functions.phpt b/tests/test_builtin_functions.phpt new file mode 100644 index 0000000..833a285 --- /dev/null +++ b/tests/test_builtin_functions.phpt @@ -0,0 +1,17 @@ +--TEST-- +Tests a regular function instrumented by instrument +--EXTENSIONS-- +sentry +--FILE-- + +--EXPECTF-- +Name: trim \ No newline at end of file From 000660995045e14e4fad18f92b75e8b7efec0d92 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Thu, 25 Jun 2026 00:15:55 +0200 Subject: [PATCH 14/15] do not allow builtin --- sentry.c | 6 +++++- span_attributes.c | 17 ----------------- tests/test_builtin_functions.phpt | 6 +++--- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/sentry.c b/sentry.c index e55febd..892b0c6 100644 --- a/sentry.c +++ b/sentry.c @@ -359,6 +359,10 @@ ZEND_FUNCTION(Sentry_instrument) { } zend_string_release(lc_func); + if (instrumented_func != NULL && instrumented_func->type == ZEND_INTERNAL_FUNCTION) { + RETURN_FALSE; + } + sentry_instrumentation *instrumentation = emalloc(sizeof(sentry_instrumentation)); array_init(&instrumentation->metadata); zend_hash_init( @@ -758,4 +762,4 @@ zend_module_entry sentry_module_entry = { #ifdef COMPILE_DL_SENTRY ZEND_GET_MODULE(sentry); -#endif \ No newline at end of file +#endif diff --git a/span_attributes.c b/span_attributes.c index 26d7174..da7aee0 100644 --- a/span_attributes.c +++ b/span_attributes.c @@ -58,23 +58,6 @@ bool sentry_resolve_param_index( return false; } - // internal functions have a different argument structures than userland functions - // and need special handling - if (func->type == ZEND_INTERNAL_FUNCTION) { - zend_internal_arg_info *arg_info = func->internal_function.arg_info; - - for (uint32_t i = 0; i < func->common.num_args; i++) { - const char *name = arg_info[i].name; - - if (name != NULL && zend_string_equals_cstr(param_name, name, strlen(name))) { - *param_index = i; - return true; - } - } - - return false; - } - for (uint32_t i = 0; i < func->common.num_args; i++) { zend_arg_info *arg_info = &func->common.arg_info[i]; diff --git a/tests/test_builtin_functions.phpt b/tests/test_builtin_functions.phpt index 833a285..d952fe2 100644 --- a/tests/test_builtin_functions.phpt +++ b/tests/test_builtin_functions.phpt @@ -1,5 +1,5 @@ --TEST-- -Tests a regular function instrumented by instrument +Tests that built-in functions are ignored by instrument. --EXTENSIONS-- sentry --FILE-- @@ -9,9 +9,9 @@ sentry echo "Name: " . $data['name'] . PHP_EOL; }); -\Sentry\instrument(null, 'trim'); +echo \Sentry\instrument(null, 'trim') ? "true" : "false"; trim(" abcde "); ?> --EXPECTF-- -Name: trim \ No newline at end of file +false \ No newline at end of file From 5f02f9b241e4e7d9d9184390ea6c9a5894689ed0 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Thu, 25 Jun 2026 21:34:29 +0200 Subject: [PATCH 15/15] enable capture attributes functions loaded after the instrumented call --- sentry.c | 20 +++++++++++++++++ span_attributes.c | 1 + span_attributes.h | 2 ++ tests/fixtures/functions.php | 5 +++++ ...tions_loaded_later_capture_attributes.phpt | 22 +++++++++++++++++++ 5 files changed, 50 insertions(+) create mode 100644 tests/fixtures/functions.php create mode 100644 tests/test_functions_loaded_later_capture_attributes.phpt diff --git a/sentry.c b/sentry.c index 892b0c6..d56c9da 100644 --- a/sentry.c +++ b/sentry.c @@ -372,6 +372,8 @@ ZEND_FUNCTION(Sentry_instrument) { sentry_span_attribute_rule_dtor, 0 ); + ZVAL_UNDEF(&instrumentation->span_attributes_definition); + instrumentation->span_attributes_resolved = false; zval attribute_list; ZVAL_UNDEF(&attribute_list); @@ -392,12 +394,15 @@ ZEND_FUNCTION(Sentry_instrument) { } if (sentry_is_span_attributes_arg(name, value)) { + ZVAL_COPY(&instrumentation->span_attributes_definition, value); + if (instrumented_func != NULL) { sentry_add_span_attribute_rules( &instrumentation->span_attributes, instrumented_func, value ); + instrumentation->span_attributes_resolved = true; } } else { sentry_add_named_metadata_arg( @@ -541,6 +546,21 @@ static void sentry_observer_begin(zend_execute_data *execute_data) { ZVAL_COPY(&state->metadata, metadata); + if ( + instrumentation != NULL + && !instrumentation->span_attributes_resolved + && !Z_ISUNDEF(instrumentation->span_attributes_definition) + ) { + sentry_add_span_attribute_rules( + &instrumentation->span_attributes, + execute_data->func, + &instrumentation->span_attributes_definition + ); + + instrumentation->span_attributes_resolved = true; + sentry_zval_ptr_dtor_undef(&instrumentation->span_attributes_definition); + } + if (instrumentation != NULL) { sentry_apply_span_attribute_rules( instrumentation, diff --git a/span_attributes.c b/span_attributes.c index da7aee0..9b5c404 100644 --- a/span_attributes.c +++ b/span_attributes.c @@ -37,6 +37,7 @@ void sentry_span_attribute_rule_dtor(zval *zv) { void sentry_instrumentation_dtor(zval *zv) { sentry_instrumentation *instrumentation = Z_PTR_P(zv); + sentry_zval_ptr_dtor_undef(&instrumentation->span_attributes_definition); zval_ptr_dtor(&instrumentation->metadata); zend_hash_destroy(&instrumentation->span_attributes); diff --git a/span_attributes.h b/span_attributes.h index 455b611..91094f5 100644 --- a/span_attributes.h +++ b/span_attributes.h @@ -7,6 +7,8 @@ typedef struct { zval metadata; // contains sentry_span_attribute_rule for one instrumented function HashTable span_attributes; + zval span_attributes_definition; + bool span_attributes_resolved; } sentry_instrumentation; typedef enum { diff --git a/tests/fixtures/functions.php b/tests/fixtures/functions.php new file mode 100644 index 0000000..8030c5d --- /dev/null +++ b/tests/fixtures/functions.php @@ -0,0 +1,5 @@ + 'test'])); + +require __DIR__ . '/fixtures/functions.php'; + +instrumented_function("instrumented function"); + +?> +--EXPECTF-- +bool(true) +Description: instrumented function \ No newline at end of file