From 3b5765c1c973c926c868b46dc7a0666f72b3114e Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 8 Aug 2024 10:19:16 +1000 Subject: [PATCH 01/29] observe methods from attribute Add the capability to observe/hook methods and functions by adding a WithSpan attribute. The attribute is provided by the extension, as OpenTelemetry\Instumentation\WithSpan An extra parameter is passed to pre hooks, containing an array of attribute parameters which can be used to set values on the span: name, span kind, attributes. There is a restriction that WithSpan hooks can only be added to functions/methods that are not already hooked by something else (because the hooks are added at runtime: when a method is executed and has no hooks, we then check for attribute and add hooks. Lastly, the pre/post hook methods are \OpenTelemetry\Instrumentation\Handler::pre and ::post, which have been added to the tests that need them. Eventually, I suspect we'd add these to our API, and they'd be coded something like auto-instrumentation modules. --- ext/opentelemetry.c | 1 + ext/otel_observer.c | 145 +++++++++++++++--- ext/otel_observer.h | 1 + ext/tests/005.phpt | 5 +- ext/tests/006.phpt | 5 +- ...ibute_named_params_passed_to_pre_hook.phpt | 41 +++++ ext/tests/attribute_on_function.phpt | 38 +++++ ext/tests/attribute_on_method.phpt | 42 +++++ .../attribute_params_passed_to_pre_hook.phpt | 53 +++++++ .../attribute_params_skipped_if_hooked.phpt | 41 +++++ 10 files changed, 351 insertions(+), 21 deletions(-) create mode 100644 ext/tests/attribute_named_params_passed_to_pre_hook.phpt create mode 100644 ext/tests/attribute_on_function.phpt create mode 100644 ext/tests/attribute_on_method.phpt create mode 100644 ext/tests/attribute_params_passed_to_pre_hook.phpt create mode 100644 ext/tests/attribute_params_skipped_if_hooked.phpt diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index 0db31ae3..210b3972 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -128,6 +128,7 @@ PHP_MINIT_FUNCTION(opentelemetry) { if (!OTEL_G(disabled)) { opentelemetry_observer_init(INIT_FUNC_ARGS_PASSTHRU); + opentelemetry_attribute_init(); } return SUCCESS; diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 16fd2012..70f34b77 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -5,10 +5,16 @@ #include "zend_execute.h" #include "zend_extensions.h" #include "zend_exceptions.h" +#include "zend_attributes.h" #include "php_opentelemetry.h" static int op_array_extension = -1; +static char *with_span_fqn = "OpenTelemetry\\Instrumentation\\WithSpan"; +// static char *with_span_fqn_lc = "opentelemetry\\instrumentation\\withspan"; + +static char *attribute_args_keys[] = {"name", "span_kind", "attributes"}; + typedef struct otel_observer { zend_llist pre_hooks; zend_llist post_hooks; @@ -192,6 +198,37 @@ static inline void func_get_lineno(zval *zv, zend_execute_data *ex) { } } +static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { + zend_attribute *attr; + attr = zend_get_attribute_str( + ex->func->common.attributes, "opentelemetry\\instrumentation\\withspan", + sizeof("opentelemetry\\instrumentation\\withspan") - 1); + if (attr == NULL || attr->argc == 0) { + ZVAL_EMPTY_ARRAY(zv); + return; + } + + HashTable *ht; + ALLOC_HASHTABLE(ht); + zend_hash_init(ht, attr->argc, NULL, ZVAL_PTR_DTOR, 0); + zend_attribute_arg arg; + zend_string *key; + + for (uint32_t i = 0; i < attr->argc; i++) { + arg = attr->args[i]; + if (arg.name != NULL) { + zend_hash_add(ht, arg.name, &arg.value); + } else { + key = zend_string_init(attribute_args_keys[i], + strlen(attribute_args_keys[i]), 0); + zend_hash_add(ht, key, &arg.value); + zend_string_release(key); + } + } + + ZVAL_ARR(zv, ht); +} + /** * Check if the object implements or extends the specified class */ @@ -432,8 +469,8 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { return; } - zval params[6]; - uint32_t param_count = 6; + zval params[7]; + uint32_t param_count = 7; func_get_this_or_called_scope(¶ms[0], execute_data); func_get_args(¶ms[1], execute_data); @@ -441,6 +478,7 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { func_get_function_name(¶ms[3], execute_data); func_get_filename(¶ms[4], execute_data); func_get_lineno(¶ms[5], execute_data); + func_get_attribute_args(¶ms[6], execute_data); for (zend_llist_element *element = hooks->head; element; element = element->next) { @@ -759,15 +797,29 @@ static void find_class_observers(HashTable *ht, HashTable *type_visited_lookup, } } -static void find_method_observers(HashTable *ht, HashTable *type_visited_lookup, - zend_class_entry *ce, zend_string *fn, - zend_llist *pre_hooks, +static void find_method_observers(HashTable *ht, zend_class_entry *ce, + zend_string *fn, zend_llist *pre_hooks, zend_llist *post_hooks) { + // Below hashtable stores information + // whether type was already visited + // This information is used to prevent + // adding hooks more than once in the case + // of extensive class hierarchy + HashTable type_visited_lookup; + zend_hash_init(&type_visited_lookup, 8, NULL, NULL, 0); HashTable *lookup = zend_hash_find_ptr_lc(ht, fn); if (lookup) { - find_class_observers(lookup, type_visited_lookup, ce, pre_hooks, + find_class_observers(lookup, &type_visited_lookup, ce, pre_hooks, post_hooks); } + zend_hash_destroy(&type_visited_lookup); +} + +static zval create_attribute_observer_handler(char *fn) { + zval callable; + ZVAL_STRING(&callable, fn); + + return callable; } static otel_observer *resolve_observer(zend_execute_data *execute_data) { @@ -775,23 +827,24 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { if (!fbc->common.function_name) { return NULL; } + bool has_withspan_attribute = false; + + if (fbc->common.attributes != NULL) { + zend_attribute *attr; + attr = zend_get_attribute_str( + fbc->common.attributes, "opentelemetry\\instrumentation\\withspan", + sizeof("opentelemetry\\instrumentation\\withspan") - 1); + has_withspan_attribute = (attr != NULL); + } otel_observer observer_instance; init_observer(&observer_instance); if (fbc->op_array.scope) { - // Below hashtable stores information - // whether type was already visited - // This information is used to prevent - // adding hooks more than once in the case - // of extensive class hierarchy - HashTable type_visited_lookup; - zend_hash_init(&type_visited_lookup, 8, NULL, NULL, 0); - find_method_observers( - OTEL_G(observer_class_lookup), &type_visited_lookup, - fbc->op_array.scope, fbc->common.function_name, - &observer_instance.pre_hooks, &observer_instance.post_hooks); - zend_hash_destroy(&type_visited_lookup); + find_method_observers(OTEL_G(observer_class_lookup), + fbc->op_array.scope, fbc->common.function_name, + &observer_instance.pre_hooks, + &observer_instance.post_hooks); } else { find_observers(OTEL_G(observer_function_lookup), fbc->common.function_name, &observer_instance.pre_hooks, @@ -800,7 +853,39 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { if (!zend_llist_count(&observer_instance.pre_hooks) && !zend_llist_count(&observer_instance.post_hooks)) { - return NULL; + if (has_withspan_attribute) { + // there are no observers registered for this function/method, but + // it has a WithSpan attribute. Add our generic pre/post handlers + // as new observers. + zval pre = create_attribute_observer_handler( + "OpenTelemetry\\Instrumentation\\Handler::pre"); + zval post = create_attribute_observer_handler( + "OpenTelemetry\\Instrumentation\\Handler::post"); + add_observer(fbc->op_array.scope ? fbc->op_array.scope->name : NULL, + fbc->common.function_name, &pre, &post); + zval_ptr_dtor(&pre); + zval_ptr_dtor(&post); + // re-find to update pre/post hooks + if (fbc->op_array.scope) { + find_method_observers( + OTEL_G(observer_class_lookup), fbc->op_array.scope, + fbc->common.function_name, &observer_instance.pre_hooks, + &observer_instance.post_hooks); + } else { + find_observers(OTEL_G(observer_function_lookup), + fbc->common.function_name, + &observer_instance.pre_hooks, + &observer_instance.post_hooks); + } + + if (!zend_llist_count(&observer_instance.pre_hooks) && + !zend_llist_count(&observer_instance.post_hooks)) { + // failed to add hooks? + return NULL; + } + } else { + return NULL; + } } otel_observer *observer = create_observer(); copy_observer(&observer_instance, observer); @@ -939,3 +1024,25 @@ void opentelemetry_observer_init(INIT_FUNC_ARGS) { zend_get_op_array_extension_handle("opentelemetry"); } } + +void opentelemetry_attribute_init() { + zend_function_entry withspan_methods[] = {PHP_FE_END}; + zend_class_entry ce, *class_entry; + + INIT_CLASS_ENTRY(ce, with_span_fqn, withspan_methods); + class_entry = zend_register_internal_class_ex(&ce, NULL); + class_entry->ce_flags |= ZEND_ACC_FINAL; + class_entry->ce_flags |= ZEND_ATTRIBUTE_TARGET_METHOD; + class_entry->ce_flags |= ZEND_ATTRIBUTE_TARGET_FUNCTION; + + zend_string *attribute_name = + zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); + zend_attribute *withSpan = + zend_add_class_attribute(class_entry, attribute_name, 1); + zend_string_release(attribute_name); + zval attr_val; + ZVAL_LONG(&attr_val, ZEND_ATTRIBUTE_TARGET_ALL); + ZVAL_COPY_VALUE(&withSpan->args[0].value, &attr_val); + + zend_mark_internal_attribute(class_entry); +} diff --git a/ext/otel_observer.h b/ext/otel_observer.h index 47b3d942..c78f684c 100644 --- a/ext/otel_observer.h +++ b/ext/otel_observer.h @@ -3,6 +3,7 @@ #define OPENTELEMETRY_OBSERVER_H void opentelemetry_observer_init(INIT_FUNC_ARGS); +void opentelemetry_attribute_init(); void observer_globals_init(void); void observer_globals_cleanup(void); diff --git a/ext/tests/005.phpt b/ext/tests/005.phpt index b29febff..ba2fb5af 100644 --- a/ext/tests/005.phpt +++ b/ext/tests/005.phpt @@ -13,7 +13,7 @@ function helloWorld() { helloWorld(); ?> --EXPECTF-- -array(6) { +array(7) { [0]=> NULL [1]=> @@ -27,6 +27,9 @@ array(6) { string(%d) "%s%etests%e005.php" [5]=> int(4) + [6]=> + array(0) { + } } string(4) "CALL" array(8) { diff --git a/ext/tests/006.phpt b/ext/tests/006.phpt index 03ac8773..79bb6fa4 100644 --- a/ext/tests/006.phpt +++ b/ext/tests/006.phpt @@ -13,7 +13,7 @@ function helloWorld(string $a) { helloWorld('a'); ?> --EXPECTF-- -array(6) { +array(7) { [0]=> NULL [1]=> @@ -29,6 +29,9 @@ array(6) { string(%d) "%s%etests%e006.php" [5]=> int(4) + [6]=> + array(0) { + } } array(8) { [0]=> diff --git a/ext/tests/attribute_named_params_passed_to_pre_hook.phpt b/ext/tests/attribute_named_params_passed_to_pre_hook.phpt new file mode 100644 index 00000000..28178e2b --- /dev/null +++ b/ext/tests/attribute_named_params_passed_to_pre_hook.phpt @@ -0,0 +1,41 @@ +--TEST-- +Check if named attribute parameters are passed to pre hook +--EXTENSIONS-- +opentelemetry +--FILE-- +foo(); +?> +--EXPECT-- +array(1) { + ["span_kind"]=> + int(3) +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/attribute_on_function.phpt b/ext/tests/attribute_on_function.phpt new file mode 100644 index 00000000..7ef8c9f5 --- /dev/null +++ b/ext/tests/attribute_on_function.phpt @@ -0,0 +1,38 @@ +--TEST-- +Check if custom attribute loaded +--EXTENSIONS-- +opentelemetry +--FILE-- +getAttributes()[0]->getName() == WithSpan::class); + +otel_attr_test(); +?> +--EXPECT-- +bool(true) +string(3) "pre" +string(4) "test" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/attribute_on_method.phpt b/ext/tests/attribute_on_method.phpt new file mode 100644 index 00000000..86796bce --- /dev/null +++ b/ext/tests/attribute_on_method.phpt @@ -0,0 +1,42 @@ +--TEST-- +Check if custom attribute can be applied to a method +--EXTENSIONS-- +opentelemetry +--FILE-- +getAttributes()[0]->getName() == WithSpan::class); + +$c = new TestClass(); +$c->sayFoo(); +?> +--EXPECT-- +bool(true) +string(3) "pre" +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/attribute_params_passed_to_pre_hook.phpt b/ext/tests/attribute_params_passed_to_pre_hook.phpt new file mode 100644 index 00000000..05c7e33d --- /dev/null +++ b/ext/tests/attribute_params_passed_to_pre_hook.phpt @@ -0,0 +1,53 @@ +--TEST-- +Check if attribute parameters are passed to pre hook +--EXTENSIONS-- +opentelemetry +--FILE-- + 'value1', 'attr2' => 3.14])] + function foo(): void + { + var_dump('foo'); + } +} + +(new Foo())->foo(); +?> +--EXPECT-- +int(7) +array(3) { + ["name"]=> + string(6) "param1" + ["span_kind"]=> + int(99) + ["attributes"]=> + array(2) { + ["attr1"]=> + string(6) "value1" + ["attr2"]=> + float(3.14) + } +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/attribute_params_skipped_if_hooked.phpt b/ext/tests/attribute_params_skipped_if_hooked.phpt new file mode 100644 index 00000000..0cf55552 --- /dev/null +++ b/ext/tests/attribute_params_skipped_if_hooked.phpt @@ -0,0 +1,41 @@ +--TEST-- +Check if hooking a method takes priority over attribute +----DESCRIPTION-- +Attribute-based hooks are only applied if no other hooks are registered on a function or method. +--EXTENSIONS-- +opentelemetry +--FILE-- +foo(); +?> +--EXPECT-- +string(3) "pre" +string(3) "foo" +string(4) "post" \ No newline at end of file From 769a341b8ea650c1deb27ba7ff37aef6e74590d5 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 8 Aug 2024 22:10:06 +1000 Subject: [PATCH 02/29] use method available in 8.0 --- ext/otel_observer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 70f34b77..58384f58 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -1044,5 +1044,5 @@ void opentelemetry_attribute_init() { ZVAL_LONG(&attr_val, ZEND_ATTRIBUTE_TARGET_ALL); ZVAL_COPY_VALUE(&withSpan->args[0].value, &attr_val); - zend_mark_internal_attribute(class_entry); + zend_internal_attribute_register(class_entry, ZEND_ATTRIBUTE_TARGET_ALL); } From 22747ac335cdbe80b3d73b7337ddd0d769fc7ea0 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Fri, 9 Aug 2024 19:04:28 +1000 Subject: [PATCH 03/29] add SpanAttribute attribute if a SpanAttribute is applied to a function or method attribute, pass it through in an extra parameter to pre hook function. --- ext/opentelemetry.c | 2 +- ext/otel_observer.c | 68 +++++++++++++++---- ext/otel_observer.h | 2 +- ext/tests/005.phpt | 5 +- ext/tests/006.phpt | 5 +- .../span_attributes/function_params.phpt | 55 +++++++++++++++ ext/tests/span_attributes/method_params.phpt | 59 ++++++++++++++++ ...ibute_named_params_passed_to_pre_hook.phpt | 0 .../attribute_on_function.phpt | 0 .../{ => with_span}/attribute_on_method.phpt | 0 .../attribute_params_passed_to_pre_hook.phpt | 3 - .../attribute_params_skipped_if_hooked.phpt | 0 12 files changed, 178 insertions(+), 21 deletions(-) create mode 100644 ext/tests/span_attributes/function_params.phpt create mode 100644 ext/tests/span_attributes/method_params.phpt rename ext/tests/{ => with_span}/attribute_named_params_passed_to_pre_hook.phpt (100%) rename ext/tests/{ => with_span}/attribute_on_function.phpt (100%) rename ext/tests/{ => with_span}/attribute_on_method.phpt (100%) rename ext/tests/{ => with_span}/attribute_params_passed_to_pre_hook.phpt (91%) rename ext/tests/{ => with_span}/attribute_params_skipped_if_hooked.phpt (100%) diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index 210b3972..e718a99b 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -128,7 +128,7 @@ PHP_MINIT_FUNCTION(opentelemetry) { if (!OTEL_G(disabled)) { opentelemetry_observer_init(INIT_FUNC_ARGS_PASSTHRU); - opentelemetry_attribute_init(); + opentelemetry_attributes_init(); } return SUCCESS; diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 58384f58..27566532 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -11,6 +11,8 @@ static int op_array_extension = -1; static char *with_span_fqn = "OpenTelemetry\\Instrumentation\\WithSpan"; +static char *span_attribute_fqn = + "OpenTelemetry\\Instrumentation\\SpanAttribute"; // static char *with_span_fqn_lc = "opentelemetry\\instrumentation\\withspan"; static char *attribute_args_keys[] = {"name", "span_kind", "attributes"}; @@ -66,10 +68,18 @@ static inline void func_get_function_name(zval *zv, zend_execute_data *ex) { ZVAL_STR_COPY(zv, ex->func->op_array.function_name); } -static void func_get_args(zval *zv, zend_execute_data *ex) { +// get function args (invo zv) and any args with the +// SpanAttributes attribute applied (into zv_attrs) +static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, + bool check_for_attributes) { zval *p, *q; uint32_t i, first_extra_arg; uint32_t arg_count = ZEND_CALL_NUM_ARGS(ex); + HashTable *ht; // attributes from SpanAttribute + if (check_for_attributes) { + ALLOC_HASHTABLE(ht); + zend_hash_init(ht, 0, NULL, ZVAL_PTR_DTOR, 0); + } // @see // https://github.com/php/php-src/blob/php-8.1.0/Zend/zend_builtin_functions.c#L235 @@ -104,6 +114,27 @@ static void func_get_args(zval *zv, zend_execute_data *ex) { ex->func->op_array.T); } while (i < arg_count) { + if (check_for_attributes && + ex->func->type != ZEND_INTERNAL_FUNCTION) { + zend_string *arg_name = ex->func->op_array.vars[i]; + zend_attribute *attribute = + zend_get_parameter_attribute_str( + ex->func->common.attributes, + "opentelemetry\\instrumentation\\spanattribute", + sizeof("opentelemetry\\instrumentation\\spanattribu" + "te") - + 1, + i); + bool has_span_attribute = attribute != NULL; + if (has_span_attribute) { + if (attribute->argc) { + zval key = attribute->args[0].value; + zend_hash_add(ht, Z_STR(key), p); + } else { + zend_hash_add(ht, arg_name, p); + } + } + } q = p; if (EXPECTED(Z_TYPE_INFO_P(q) != IS_UNDEF)) { ZVAL_DEREF(q); @@ -124,6 +155,9 @@ static void func_get_args(zval *zv, zend_execute_data *ex) { } else { ZVAL_EMPTY_ARRAY(zv); } + if (zv_attrs) { + ZVAL_ARR(zv_attrs, ht); + } } static uint32_t func_get_arg_index_by_name(zend_execute_data *execute_data, @@ -222,7 +256,7 @@ static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { key = zend_string_init(attribute_args_keys[i], strlen(attribute_args_keys[i]), 0); zend_hash_add(ht, key, &arg.value); - zend_string_release(key); + zend_string_release(key); // todo move to end? } } @@ -469,11 +503,11 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { return; } - zval params[7]; - uint32_t param_count = 7; + zval params[8]; + uint32_t param_count = 8; func_get_this_or_called_scope(¶ms[0], execute_data); - func_get_args(¶ms[1], execute_data); + func_get_args(¶ms[1], ¶ms[7], execute_data, true); func_get_declaring_scope(¶ms[2], execute_data); func_get_function_name(¶ms[3], execute_data); func_get_filename(¶ms[4], execute_data); @@ -646,7 +680,7 @@ static void observer_end(zend_execute_data *execute_data, zval *retval, uint32_t param_count = 8; func_get_this_or_called_scope(¶ms[0], execute_data); - func_get_args(¶ms[1], execute_data); + func_get_args(¶ms[1], NULL, execute_data, false); func_get_retval(¶ms[2], retval); func_get_exception(¶ms[3]); func_get_declaring_scope(¶ms[4], execute_data); @@ -1025,24 +1059,30 @@ void opentelemetry_observer_init(INIT_FUNC_ARGS) { } } -void opentelemetry_attribute_init() { - zend_function_entry withspan_methods[] = {PHP_FE_END}; +void attribute_init(char *fqn, uint32_t flags) { + zend_function_entry attr_methods[] = {PHP_FE_END}; zend_class_entry ce, *class_entry; - INIT_CLASS_ENTRY(ce, with_span_fqn, withspan_methods); + INIT_CLASS_ENTRY(ce, fqn, attr_methods); class_entry = zend_register_internal_class_ex(&ce, NULL); - class_entry->ce_flags |= ZEND_ACC_FINAL; - class_entry->ce_flags |= ZEND_ATTRIBUTE_TARGET_METHOD; - class_entry->ce_flags |= ZEND_ATTRIBUTE_TARGET_FUNCTION; + class_entry->ce_flags |= flags; zend_string *attribute_name = zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); - zend_attribute *withSpan = + zend_attribute *attr = zend_add_class_attribute(class_entry, attribute_name, 1); zend_string_release(attribute_name); zval attr_val; ZVAL_LONG(&attr_val, ZEND_ATTRIBUTE_TARGET_ALL); - ZVAL_COPY_VALUE(&withSpan->args[0].value, &attr_val); + ZVAL_COPY_VALUE(&attr->args[0].value, &attr_val); zend_internal_attribute_register(class_entry, ZEND_ATTRIBUTE_TARGET_ALL); } + +void opentelemetry_attributes_init() { + attribute_init(with_span_fqn, ZEND_ACC_FINAL | + ZEND_ATTRIBUTE_TARGET_METHOD | + ZEND_ATTRIBUTE_TARGET_FUNCTION); + attribute_init(span_attribute_fqn, + ZEND_ACC_FINAL | ZEND_ATTRIBUTE_TARGET_PARAMETER); +} diff --git a/ext/otel_observer.h b/ext/otel_observer.h index c78f684c..5592b8d0 100644 --- a/ext/otel_observer.h +++ b/ext/otel_observer.h @@ -3,7 +3,7 @@ #define OPENTELEMETRY_OBSERVER_H void opentelemetry_observer_init(INIT_FUNC_ARGS); -void opentelemetry_attribute_init(); +void opentelemetry_attributes_init(); void observer_globals_init(void); void observer_globals_cleanup(void); diff --git a/ext/tests/005.phpt b/ext/tests/005.phpt index ba2fb5af..5e943188 100644 --- a/ext/tests/005.phpt +++ b/ext/tests/005.phpt @@ -13,7 +13,7 @@ function helloWorld() { helloWorld(); ?> --EXPECTF-- -array(7) { +array(8) { [0]=> NULL [1]=> @@ -30,6 +30,9 @@ array(7) { [6]=> array(0) { } + [7]=> + array(0) { + } } string(4) "CALL" array(8) { diff --git a/ext/tests/006.phpt b/ext/tests/006.phpt index 79bb6fa4..09d86530 100644 --- a/ext/tests/006.phpt +++ b/ext/tests/006.phpt @@ -13,7 +13,7 @@ function helloWorld(string $a) { helloWorld('a'); ?> --EXPECTF-- -array(7) { +array(8) { [0]=> NULL [1]=> @@ -32,6 +32,9 @@ array(7) { [6]=> array(0) { } + [7]=> + array(0) { + } } array(8) { [0]=> diff --git a/ext/tests/span_attributes/function_params.phpt b/ext/tests/span_attributes/function_params.phpt new file mode 100644 index 00000000..69196dfa --- /dev/null +++ b/ext/tests/span_attributes/function_params.phpt @@ -0,0 +1,55 @@ +--TEST-- +Check if function params can be passed via SpanAttribute +--EXTENSIONS-- +opentelemetry +--FILE-- + +--EXPECT-- +string(3) "pre" +array(5) { + ["renamed_one"]=> + string(3) "one" + ["two"]=> + int(99) + ["renamed_three"]=> + float(3.14159) + ["four"]=> + bool(true) + ["six"]=> + string(3) "six" +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/span_attributes/method_params.phpt b/ext/tests/span_attributes/method_params.phpt new file mode 100644 index 00000000..32af945b --- /dev/null +++ b/ext/tests/span_attributes/method_params.phpt @@ -0,0 +1,59 @@ +--TEST-- +Check if method params can be passed via SpanAttribute +--EXTENSIONS-- +opentelemetry +--FILE-- +foo('one', 99, 3.14159, true, 'five', 'six'); +?> +--EXPECT-- +string(3) "pre" +array(5) { + ["renamed_one"]=> + string(3) "one" + ["two"]=> + int(99) + ["renamed_three"]=> + float(3.14159) + ["four"]=> + bool(true) + ["six"]=> + string(3) "six" +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/attribute_named_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt similarity index 100% rename from ext/tests/attribute_named_params_passed_to_pre_hook.phpt rename to ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt diff --git a/ext/tests/attribute_on_function.phpt b/ext/tests/with_span/attribute_on_function.phpt similarity index 100% rename from ext/tests/attribute_on_function.phpt rename to ext/tests/with_span/attribute_on_function.phpt diff --git a/ext/tests/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt similarity index 100% rename from ext/tests/attribute_on_method.phpt rename to ext/tests/with_span/attribute_on_method.phpt diff --git a/ext/tests/attribute_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt similarity index 91% rename from ext/tests/attribute_params_passed_to_pre_hook.phpt rename to ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt index 05c7e33d..1cb03afc 100644 --- a/ext/tests/attribute_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt @@ -12,9 +12,7 @@ class Handler { public static function pre(): void { - //var_dump(func_get_args()); $args = func_get_args(); - var_dump(count($args)); var_dump($args[6] ?? null); } public static function post(): void @@ -35,7 +33,6 @@ class Foo (new Foo())->foo(); ?> --EXPECT-- -int(7) array(3) { ["name"]=> string(6) "param1" diff --git a/ext/tests/attribute_params_skipped_if_hooked.phpt b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt similarity index 100% rename from ext/tests/attribute_params_skipped_if_hooked.phpt rename to ext/tests/with_span/attribute_params_skipped_if_hooked.phpt From dc67507de855a4cfe4be04f8a41d59f9a4af6521 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 10 Aug 2024 14:50:17 +1000 Subject: [PATCH 04/29] configurable attribute pre/post function --- ext/opentelemetry.c | 8 ++++ ext/otel_observer.c | 4 +- ext/php_opentelemetry.h | 2 + .../span_attributes/function_params.phpt | 2 +- ext/tests/span_attributes/method_params.phpt | 2 +- ...ibute_named_params_passed_to_pre_hook.phpt | 2 +- .../with_span/attribute_on_function.phpt | 4 +- ext/tests/with_span/attribute_on_method.phpt | 2 +- .../attribute_params_passed_to_pre_hook.phpt | 2 +- ext/tests/with_span/customize_handlers.phpt | 37 +++++++++++++++++++ 10 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 ext/tests/with_span/customize_handlers.phpt diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index e718a99b..d4cc0656 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -86,6 +86,14 @@ STD_PHP_INI_ENTRY_EX("opentelemetry.allow_stack_extension", "Off", PHP_INI_ALL, OnUpdateBool, allow_stack_extension, zend_opentelemetry_globals, opentelemetry_globals, zend_ini_boolean_displayer_cb) +STD_PHP_INI_ENTRY("opentelemetry.attr_pre_handler_function", + "Opentelemetry\\API\\Instrumentation\\Handler::pre", + PHP_INI_ALL, OnUpdateString, pre_handler_function_fqn, + zend_opentelemetry_globals, opentelemetry_globals) +STD_PHP_INI_ENTRY("opentelemetry.attr_post_handler_function", + "Opentelemetry\\API\\Instrumentation\\Handler::post", + PHP_INI_ALL, OnUpdateString, post_handler_function_fqn, + zend_opentelemetry_globals, opentelemetry_globals) PHP_INI_END() PHP_FUNCTION(hook) { diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 27566532..7f061bea 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -892,9 +892,9 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { // it has a WithSpan attribute. Add our generic pre/post handlers // as new observers. zval pre = create_attribute_observer_handler( - "OpenTelemetry\\Instrumentation\\Handler::pre"); + OTEL_G(pre_handler_function_fqn)); zval post = create_attribute_observer_handler( - "OpenTelemetry\\Instrumentation\\Handler::post"); + OTEL_G(post_handler_function_fqn)); add_observer(fbc->op_array.scope ? fbc->op_array.scope->name : NULL, fbc->common.function_name, &pre, &post); zval_ptr_dtor(&pre); diff --git a/ext/php_opentelemetry.h b/ext/php_opentelemetry.h index b55302c1..8a39e4d0 100644 --- a/ext/php_opentelemetry.h +++ b/ext/php_opentelemetry.h @@ -13,6 +13,8 @@ ZEND_BEGIN_MODULE_GLOBALS(opentelemetry) char *conflicts; int disabled; // module disabled? (eg due to conflicting extension loaded) int allow_stack_extension; + char *pre_handler_function_fqn; + char *post_handler_function_fqn; ZEND_END_MODULE_GLOBALS(opentelemetry) ZEND_EXTERN_MODULE_GLOBALS(opentelemetry) diff --git a/ext/tests/span_attributes/function_params.phpt b/ext/tests/span_attributes/function_params.phpt index 69196dfa..4a665b7d 100644 --- a/ext/tests/span_attributes/function_params.phpt +++ b/ext/tests/span_attributes/function_params.phpt @@ -4,7 +4,7 @@ Check if function params can be passed via SpanAttribute opentelemetry --FILE-- getAttributes()[0]->getName() == WithSpan::class); otel_attr_test(); diff --git a/ext/tests/with_span/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt index 86796bce..565012fc 100644 --- a/ext/tests/with_span/attribute_on_method.phpt +++ b/ext/tests/with_span/attribute_on_method.phpt @@ -4,7 +4,7 @@ Check if custom attribute can be applied to a method opentelemetry --FILE-- +--EXPECT-- +string(10) "custom_pre" +string(11) "custom_post" +string(18) "custom_pre handler" +string(4) "test" +string(19) "custom_post handler" \ No newline at end of file From 1f7cb580224aa8656f3232cb6b2761a014caf154 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sat, 10 Aug 2024 15:01:29 +1000 Subject: [PATCH 05/29] disable attribute-based hooking from ini --- ext/opentelemetry.c | 4 ++++ ext/otel_observer.c | 4 ++-- ext/php_opentelemetry.h | 1 + ext/tests/with_span/disable.phpt | 34 ++++++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 ext/tests/with_span/disable.phpt diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index d4cc0656..0b33be76 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -86,6 +86,10 @@ STD_PHP_INI_ENTRY_EX("opentelemetry.allow_stack_extension", "Off", PHP_INI_ALL, OnUpdateBool, allow_stack_extension, zend_opentelemetry_globals, opentelemetry_globals, zend_ini_boolean_displayer_cb) +STD_PHP_INI_ENTRY_EX("opentelemetry.attr_hooks_enabled", "On", PHP_INI_ALL, + OnUpdateBool, attr_hooks_enabled, + zend_opentelemetry_globals, opentelemetry_globals, + zend_ini_boolean_displayer_cb) STD_PHP_INI_ENTRY("opentelemetry.attr_pre_handler_function", "Opentelemetry\\API\\Instrumentation\\Handler::pre", PHP_INI_ALL, OnUpdateString, pre_handler_function_fqn, diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 7f061bea..d373d7dc 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -114,7 +114,7 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, ex->func->op_array.T); } while (i < arg_count) { - if (check_for_attributes && + if (check_for_attributes && OTEL_G(attr_hooks_enabled) && ex->func->type != ZEND_INTERNAL_FUNCTION) { zend_string *arg_name = ex->func->op_array.vars[i]; zend_attribute *attribute = @@ -887,7 +887,7 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { if (!zend_llist_count(&observer_instance.pre_hooks) && !zend_llist_count(&observer_instance.post_hooks)) { - if (has_withspan_attribute) { + if (OTEL_G(attr_hooks_enabled) && has_withspan_attribute) { // there are no observers registered for this function/method, but // it has a WithSpan attribute. Add our generic pre/post handlers // as new observers. diff --git a/ext/php_opentelemetry.h b/ext/php_opentelemetry.h index 8a39e4d0..2288dd03 100644 --- a/ext/php_opentelemetry.h +++ b/ext/php_opentelemetry.h @@ -13,6 +13,7 @@ ZEND_BEGIN_MODULE_GLOBALS(opentelemetry) char *conflicts; int disabled; // module disabled? (eg due to conflicting extension loaded) int allow_stack_extension; + int attr_hooks_enabled; // attribute hooking enabled? char *pre_handler_function_fqn; char *post_handler_function_fqn; ZEND_END_MODULE_GLOBALS(opentelemetry) diff --git a/ext/tests/with_span/disable.phpt b/ext/tests/with_span/disable.phpt new file mode 100644 index 00000000..b9b760c7 --- /dev/null +++ b/ext/tests/with_span/disable.phpt @@ -0,0 +1,34 @@ +--TEST-- +Check if attribute hooks can be disabled by config +--EXTENSIONS-- +opentelemetry +--INI-- +opentelemetry.attr_hooks_enabled = Off +--FILE-- + +--EXPECT-- +string(4) "test" From 6e1eadbae2c74ddf6bdfdbf52df302c2b372e0e8 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sun, 11 Aug 2024 07:32:39 +1000 Subject: [PATCH 06/29] generate attribute classes, docs, ide stubs --- DEVELOPMENT.md | 26 ++++++ README.md | 81 +++++++++++++++++++ ext/opentelemetry.c | 12 ++- ext/opentelemetry.ide.stub.php | 39 +++++++++ ext/opentelemetry.stub.php | 8 ++ ext/opentelemetry_arginfo.h | 33 +++++++- ext/otel_observer.c | 40 +-------- .../function_params.phpt | 0 .../method_params.phpt | 0 .../with_span/attribute_on_interface.phpt | 44 ++++++++++ 10 files changed, 245 insertions(+), 38 deletions(-) create mode 100644 ext/opentelemetry.ide.stub.php rename ext/tests/{span_attributes => span_attribute}/function_params.phpt (100%) rename ext/tests/{span_attributes => span_attribute}/method_params.phpt (100%) create mode 100644 ext/tests/with_span/attribute_on_interface.phpt diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index a4c4d680..19fcf441 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -178,6 +178,32 @@ Basic usage is in the `tests/` directory. A more advanced example: https://github.com/open-telemetry/opentelemetry-php-contrib/pull/78/files +# `opentelemetry.stub.php` and `opentelemetry_argoinfo.h` + +The build system will generate `opentelemetry_arginfo.h` from `opentelemetry.stub.php`, but it seems +to generate an invalid output. Until that starts working as expected: + +Manually fix: + +```diff +-ZEND_FUNCTION(OpenTelemetry_Instrumentation_hook); ++ZEND_FUNCTION(hook); +``` + +```diff +-ZEND_NS_FALIAS("OpenTelemetry\\Instrumentation", hook, OpenTelemetry_Instrumentation_hook, arginfo_OpenTelemetry_Instrumentation_hook) ++ZEND_NS_FE("OpenTelemetry\\Instrumentation", hook, arginfo_OpenTelemetry_Instrumentation_hook) +``` + +Also note that the `#[Attribute]` attribute is not added to the classes in the stub file, as that could +not be handled by `gen_stub.php`. Constructors are also not added due to not building cleanly, but might +be fixable (TODO). + +You can regenerate the output by `php build/gen_stub.php`, or `make` (if the stub file has been changed). + +`opentelemetry.ide.stub.php` represents what the stub wile (probably) should be, and this file can +be used in IDEs for type-hinting. + # Further reading * https://www.phpinternalsbook.com/php7/build_system/building_extensions.html \ No newline at end of file diff --git a/README.md b/README.md index 50070af5..725f8296 100644 --- a/README.md +++ b/README.md @@ -241,5 +241,86 @@ string(3) "new" string(8) "original" ``` +## Attribute-based hooking + +By applying attributes to source code, the OpenTelemetry extension can add hooks at runtime. + +By default, the following pre/post hooks will be invoked: `OpenTelemetry\API\Instrumentation\Handler::pre` and `::post`. + +## Restrictions + +Attribute-based hooks can only be applied to a function/method that does not already have +hooks applied. +Only one hook can be applied to a function/method, including via interfaces. + +Since the attributes are evaluated at runtime, the extension checks whether a hook already +exists to decide whether it should apply a new runtime hook. + +### Configuration + +This feature can be configured via `.ini` by modifying the following entries: + +- `opentelemetry.attr_hooks_enabled` - boolean, default On +- `opentelemetry.attr_pre_handler_function` - FQN of pre method/function +- `opentelemetry.attr_post_handler_function` - FQN of post method/function + +## `WithSpan` attribute + +This attribute can be applied to a function or class method. +You can also provide optional parameters to the attribute, which control: +- span name +- span kind +- attributes + +```php +use OpenTelemetry\Instrumentation\WithSpan + +class MyClass +{ + #[WithSpan] + public function trace_me(): void + { + /* ... */ + } + + #[WithSpan('custom_span_name', SpanKind::KIND_INTERNAL, ['my-attr' => 'value'])] + public function trace_me_with_customization(): void + { + /* ... */ + } +} + +#[WithSpan] +function my_function(): void +{ + /* ... */ +} +``` + +## `SpanAttribute` attribute + +This attribute should be used in conjunction with `WithSpan`. It is applied to function/method +parameters, and causes those parameters and values to be passed through to the `pre` hook function +where they can be added as trace attributes. +There is one optional parameter, which controls the attribute key. If not set, the parameter name +is used. + +```php +use OpenTelemetry\Instrumentation\WithSpan +use OpenTelemetry\Instrumentation\SpanAttribute + +class MyClass +{ + #[WithSpan] + public function add_user( + #[SpanAttribute] string $username, + string $password, + #[SpanAttribute('a_better_attribute_name')] string $foo_bar_baz, + ): void + { + /* ... */ + } +``` + ## Contributing See [DEVELOPMENT.md](DEVELOPMENT.md) and https://github.com/open-telemetry/opentelemetry-php/blob/main/CONTRIBUTING.md diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index 0b33be76..19d10913 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -10,6 +10,7 @@ #include "otel_observer.h" #include "stdlib.h" #include "string.h" +#include "zend_attributes.h" #include "zend_closures.h" static int check_conflict(HashTable *registry, const char *extension_name) { @@ -140,7 +141,16 @@ PHP_MINIT_FUNCTION(opentelemetry) { if (!OTEL_G(disabled)) { opentelemetry_observer_init(INIT_FUNC_ARGS_PASSTHRU); - opentelemetry_attributes_init(); + zend_class_entry *ce_with_span = + register_class_OpenTelemetry_Instrumentation_WithSpan(); + zend_class_entry *ce_span_attribute = + register_class_OpenTelemetry_Instrumentation_SpanAttribute(); + // todo: gen_stubs.php should do this + zend_internal_attribute_register(ce_with_span, + ZEND_ATTRIBUTE_TARGET_METHOD | + ZEND_ATTRIBUTE_TARGET_FUNCTION); + zend_internal_attribute_register(ce_span_attribute, + ZEND_ATTRIBUTE_TARGET_PARAMETER); } return SUCCESS; diff --git a/ext/opentelemetry.ide.stub.php b/ext/opentelemetry.ide.stub.php new file mode 100644 index 00000000..a80fcbb5 --- /dev/null +++ b/ext/opentelemetry.ide.stub.php @@ -0,0 +1,39 @@ +ce_flags |= ZEND_ACC_FINAL; + + return class_entry; +} + +static zend_class_entry * +register_class_OpenTelemetry_Instrumentation_SpanAttribute(void) { + zend_class_entry ce, *class_entry; + + INIT_NS_CLASS_ENTRY( + ce, "OpenTelemetry\\Instrumentation", "SpanAttribute", + class_OpenTelemetry_Instrumentation_SpanAttribute_methods); + class_entry = zend_register_internal_class_ex(&ce, NULL); + class_entry->ce_flags |= ZEND_ACC_FINAL; + + return class_entry; +} diff --git a/ext/otel_observer.c b/ext/otel_observer.c index d373d7dc..a3a11052 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -10,12 +10,8 @@ static int op_array_extension = -1; -static char *with_span_fqn = "OpenTelemetry\\Instrumentation\\WithSpan"; -static char *span_attribute_fqn = - "OpenTelemetry\\Instrumentation\\SpanAttribute"; -// static char *with_span_fqn_lc = "opentelemetry\\instrumentation\\withspan"; - -static char *attribute_args_keys[] = {"name", "span_kind", "attributes"}; +static char *with_span_attribute_args_keys[] = {"name", "span_kind", + "attributes"}; typedef struct otel_observer { zend_llist pre_hooks; @@ -253,8 +249,8 @@ static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { if (arg.name != NULL) { zend_hash_add(ht, arg.name, &arg.value); } else { - key = zend_string_init(attribute_args_keys[i], - strlen(attribute_args_keys[i]), 0); + key = zend_string_init(with_span_attribute_args_keys[i], + strlen(with_span_attribute_args_keys[i]), 0); zend_hash_add(ht, key, &arg.value); zend_string_release(key); // todo move to end? } @@ -1058,31 +1054,3 @@ void opentelemetry_observer_init(INIT_FUNC_ARGS) { zend_get_op_array_extension_handle("opentelemetry"); } } - -void attribute_init(char *fqn, uint32_t flags) { - zend_function_entry attr_methods[] = {PHP_FE_END}; - zend_class_entry ce, *class_entry; - - INIT_CLASS_ENTRY(ce, fqn, attr_methods); - class_entry = zend_register_internal_class_ex(&ce, NULL); - class_entry->ce_flags |= flags; - - zend_string *attribute_name = - zend_string_init_interned("Attribute", sizeof("Attribute") - 1, 1); - zend_attribute *attr = - zend_add_class_attribute(class_entry, attribute_name, 1); - zend_string_release(attribute_name); - zval attr_val; - ZVAL_LONG(&attr_val, ZEND_ATTRIBUTE_TARGET_ALL); - ZVAL_COPY_VALUE(&attr->args[0].value, &attr_val); - - zend_internal_attribute_register(class_entry, ZEND_ATTRIBUTE_TARGET_ALL); -} - -void opentelemetry_attributes_init() { - attribute_init(with_span_fqn, ZEND_ACC_FINAL | - ZEND_ATTRIBUTE_TARGET_METHOD | - ZEND_ATTRIBUTE_TARGET_FUNCTION); - attribute_init(span_attribute_fqn, - ZEND_ACC_FINAL | ZEND_ATTRIBUTE_TARGET_PARAMETER); -} diff --git a/ext/tests/span_attributes/function_params.phpt b/ext/tests/span_attribute/function_params.phpt similarity index 100% rename from ext/tests/span_attributes/function_params.phpt rename to ext/tests/span_attribute/function_params.phpt diff --git a/ext/tests/span_attributes/method_params.phpt b/ext/tests/span_attribute/method_params.phpt similarity index 100% rename from ext/tests/span_attributes/method_params.phpt rename to ext/tests/span_attribute/method_params.phpt diff --git a/ext/tests/with_span/attribute_on_interface.phpt b/ext/tests/with_span/attribute_on_interface.phpt new file mode 100644 index 00000000..97b7a0ff --- /dev/null +++ b/ext/tests/with_span/attribute_on_interface.phpt @@ -0,0 +1,44 @@ +--TEST-- +Check if custom attribute can be applied to an interface +--XFAIL-- +Not implemented +--EXTENSIONS-- +opentelemetry +--FILE-- +sayFoo(); +?> +--EXPECT-- +string(3) "pre" +string(3) "foo" +string(4) "post" \ No newline at end of file From efda093e6f2c3626e0348f2546d1cc7f764343c8 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sun, 11 Aug 2024 07:40:09 +1000 Subject: [PATCH 07/29] move attribute find into function --- ext/otel_observer.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index a3a11052..73a2e338 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -852,6 +852,15 @@ static zval create_attribute_observer_handler(char *fn) { return callable; } +static zend_attribute *find_withspan_attribute(zend_function *func) { + zend_attribute *attr; + attr = zend_get_attribute_str( + func->common.attributes, "opentelemetry\\instrumentation\\withspan", + sizeof("opentelemetry\\instrumentation\\withspan") - 1); + + return attr; +} + static otel_observer *resolve_observer(zend_execute_data *execute_data) { zend_function *fbc = execute_data->func; if (!fbc->common.function_name) { @@ -859,11 +868,8 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { } bool has_withspan_attribute = false; - if (fbc->common.attributes != NULL) { - zend_attribute *attr; - attr = zend_get_attribute_str( - fbc->common.attributes, "opentelemetry\\instrumentation\\withspan", - sizeof("opentelemetry\\instrumentation\\withspan") - 1); + if(OTEL_G(attr_hooks_enabled) && fbc->common.attributes != NULL) { + zend_attribute * attr = find_withspan_attribute(fbc); has_withspan_attribute = (attr != NULL); } From 6877f79291ec75aa0fa0c676ccf643dd4df774aa Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sun, 11 Aug 2024 09:18:05 +1000 Subject: [PATCH 08/29] check interfaces for WithSpan --- ext/otel_observer.c | 46 +++++++++++++-- .../attribute_on_interface.phpt | 51 +++++++++++++++++ .../with_span/attribute_on_interface.phpt | 25 +++++++-- .../attribute_on_interface_with_params.phpt | 56 +++++++++++++++++++ ext/tests/with_span/attribute_on_method.phpt | 2 +- .../attribute_params_passed_to_pre_hook.phpt | 2 +- .../attribute_params_skipped_if_hooked.phpt | 2 +- ext/tests/with_span/customize_handlers.phpt | 2 +- 8 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 ext/tests/span_attribute/attribute_on_interface.phpt create mode 100644 ext/tests/with_span/attribute_on_interface_with_params.phpt diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 73a2e338..2faee3ac 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -852,13 +852,51 @@ static zval create_attribute_observer_handler(char *fn) { return callable; } +static zend_function *find_function(zend_class_entry *ce, zend_string *name) { + zend_function *func; + ZEND_HASH_FOREACH_PTR(&ce->function_table, func) { + if (zend_string_equals(func->common.function_name, name)) { + return func; + } + } + ZEND_HASH_FOREACH_END(); + return NULL; +} + +// find WithSpan in attributes, or in interface method attributes static zend_attribute *find_withspan_attribute(zend_function *func) { zend_attribute *attr; + // const char *s = "opentelemetry\\instrumentation\\withspan"; attr = zend_get_attribute_str( func->common.attributes, "opentelemetry\\instrumentation\\withspan", sizeof("opentelemetry\\instrumentation\\withspan") - 1); - - return attr; + if (attr != NULL) { + return attr; + } + zend_class_entry *ce = func->common.scope; + // if a method, check interfaces + if (ce && ce->num_interfaces > 0) { + zend_class_entry *interface_ce; + // zend_attribute *attr; + for (uint32_t i = 0; i < ce->num_interfaces; i++) { + interface_ce = ce->interfaces[i]; + if (interface_ce != NULL) { + zend_function *iface_func = + find_function(interface_ce, func->common.function_name); + if (iface_func) { + // Method found in the interface, now check for attributes + attr = zend_get_attribute_str( + iface_func->common.attributes, + "opentelemetry\\instrumentation\\withspan", + sizeof("opentelemetry\\instrumentation\\withspan") - 1); + if (attr) { + return attr; + } + } + } + } + } + return NULL; } static otel_observer *resolve_observer(zend_execute_data *execute_data) { @@ -868,8 +906,8 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { } bool has_withspan_attribute = false; - if(OTEL_G(attr_hooks_enabled) && fbc->common.attributes != NULL) { - zend_attribute * attr = find_withspan_attribute(fbc); + if (OTEL_G(attr_hooks_enabled)) { + zend_attribute *attr = find_withspan_attribute(fbc); has_withspan_attribute = (attr != NULL); } diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt new file mode 100644 index 00000000..8efc47dc --- /dev/null +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -0,0 +1,51 @@ +--TEST-- +Check if SpanAttribute can be applied to interface +--EXTENSIONS-- +opentelemetry +--XFAIL-- +Not implemented +--FILE-- +foo('one'); +?> +--EXPECT-- +string(3) "pre" +array(1) { + ["renamed"]=> + string(3) "one" +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/with_span/attribute_on_interface.phpt b/ext/tests/with_span/attribute_on_interface.phpt index 97b7a0ff..80fb6d88 100644 --- a/ext/tests/with_span/attribute_on_interface.phpt +++ b/ext/tests/with_span/attribute_on_interface.phpt @@ -1,7 +1,5 @@ --TEST-- Check if custom attribute can be applied to an interface ---XFAIL-- -Not implemented --EXTENSIONS-- opentelemetry --FILE-- @@ -25,20 +23,35 @@ class Handler interface TestInterface { #[WithSpan] - function sayFoo(): void; + public function sayFoo(): void; } -class TestClass implements TestInterface +interface OtherInterface { - function sayFoo(): void + #[WithSpan] + public function sayBar(): void; +} + +class TestClass implements TestInterface, OtherInterface +{ + public function sayFoo(): void { var_dump('foo'); } + public function sayBar(): void + { + var_dump('bar'); + } } -(new TestClass())->sayFoo(); +$c = new TestClass(); +$c->sayFoo(); +$c->sayBar(); ?> --EXPECT-- string(3) "pre" string(3) "foo" +string(4) "post" +string(3) "pre" +string(3) "bar" string(4) "post" \ No newline at end of file diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt new file mode 100644 index 00000000..f45112cd --- /dev/null +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -0,0 +1,56 @@ +--TEST-- +Check if WithSpan can be applied to an interface with attribute args +--EXTENSIONS-- +opentelemetry +--XFAIL-- +Not implemented +--FILE-- + 'bar'])] + function sayFoo(): void; +} + +class TestClass implements TestInterface +{ + function sayFoo(): void + { + var_dump('foo'); + } +} + +(new TestClass())->sayFoo(); +?> +--EXPECT-- +string(3) "pre" +array(3) { + ["name"]=> + string(3) "one" + ["span_kind"]=> + int(99) + ["attributes"]=> + array(1) { + ["foo"]=> + string(3) "bar" + } +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/with_span/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt index 565012fc..80b20226 100644 --- a/ext/tests/with_span/attribute_on_method.phpt +++ b/ext/tests/with_span/attribute_on_method.phpt @@ -1,5 +1,5 @@ --TEST-- -Check if custom attribute can be applied to a method +Check if WithSpan can be applied to a method --EXTENSIONS-- opentelemetry --FILE-- diff --git a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt index 2213d9bb..ed23f3b6 100644 --- a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt @@ -1,5 +1,5 @@ --TEST-- -Check if attribute parameters are passed to pre hook +Check if WithSpan parameters are passed to pre hook --EXTENSIONS-- opentelemetry --FILE-- diff --git a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt index 0cf55552..ce0774c4 100644 --- a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt +++ b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt @@ -1,5 +1,5 @@ --TEST-- -Check if hooking a method takes priority over attribute +Check if hooking a method takes priority over WithSpan ----DESCRIPTION-- Attribute-based hooks are only applied if no other hooks are registered on a function or method. --EXTENSIONS-- diff --git a/ext/tests/with_span/customize_handlers.phpt b/ext/tests/with_span/customize_handlers.phpt index 3d9bb032..9d0e6834 100644 --- a/ext/tests/with_span/customize_handlers.phpt +++ b/ext/tests/with_span/customize_handlers.phpt @@ -1,5 +1,5 @@ --TEST-- -Check if custom attribute handlers can be changed via config +Check if WithSpan handlers can be changed via config --EXTENSIONS-- opentelemetry --INI-- From f0f12a6b3a5d3b5fe4d27c96477500048b297d9f Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sun, 11 Aug 2024 09:26:27 +1000 Subject: [PATCH 09/29] use a const for attribute fqns --- ext/otel_observer.c | 29 ++++++++----------- .../attribute_on_interface_with_params.phpt | 2 +- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 2faee3ac..2139c355 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -10,6 +10,9 @@ static int op_array_extension = -1; +const char *withspan_fqn_lc = "opentelemetry\\instrumentation\\withspan"; +const char *spanattribute_fqn_lc = + "opentelemetry\\instrumentation\\spanattribute"; static char *with_span_attribute_args_keys[] = {"name", "span_kind", "attributes"}; @@ -115,12 +118,8 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, zend_string *arg_name = ex->func->op_array.vars[i]; zend_attribute *attribute = zend_get_parameter_attribute_str( - ex->func->common.attributes, - "opentelemetry\\instrumentation\\spanattribute", - sizeof("opentelemetry\\instrumentation\\spanattribu" - "te") - - 1, - i); + ex->func->common.attributes, spanattribute_fqn_lc, + strlen(spanattribute_fqn_lc), i); bool has_span_attribute = attribute != NULL; if (has_span_attribute) { if (attribute->argc) { @@ -230,9 +229,8 @@ static inline void func_get_lineno(zval *zv, zend_execute_data *ex) { static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { zend_attribute *attr; - attr = zend_get_attribute_str( - ex->func->common.attributes, "opentelemetry\\instrumentation\\withspan", - sizeof("opentelemetry\\instrumentation\\withspan") - 1); + attr = zend_get_attribute_str(ex->func->common.attributes, withspan_fqn_lc, + strlen(withspan_fqn_lc)); if (attr == NULL || attr->argc == 0) { ZVAL_EMPTY_ARRAY(zv); return; @@ -866,10 +864,8 @@ static zend_function *find_function(zend_class_entry *ce, zend_string *name) { // find WithSpan in attributes, or in interface method attributes static zend_attribute *find_withspan_attribute(zend_function *func) { zend_attribute *attr; - // const char *s = "opentelemetry\\instrumentation\\withspan"; - attr = zend_get_attribute_str( - func->common.attributes, "opentelemetry\\instrumentation\\withspan", - sizeof("opentelemetry\\instrumentation\\withspan") - 1); + attr = zend_get_attribute_str(func->common.attributes, withspan_fqn_lc, + strlen(withspan_fqn_lc)); if (attr != NULL) { return attr; } @@ -885,10 +881,9 @@ static zend_attribute *find_withspan_attribute(zend_function *func) { find_function(interface_ce, func->common.function_name); if (iface_func) { // Method found in the interface, now check for attributes - attr = zend_get_attribute_str( - iface_func->common.attributes, - "opentelemetry\\instrumentation\\withspan", - sizeof("opentelemetry\\instrumentation\\withspan") - 1); + attr = zend_get_attribute_str(iface_func->common.attributes, + withspan_fqn_lc, + strlen(withspan_fqn_lc)); if (attr) { return attr; } diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt index f45112cd..060a2982 100644 --- a/ext/tests/with_span/attribute_on_interface_with_params.phpt +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -15,7 +15,7 @@ class Handler public static function pre(): void { var_dump('pre'); - var_dump(func_get_args()[7]) + var_dump(func_get_args()[7]); } public static function post(): void { From b407ce367120ce942e4a6186508b590cb288ff8f Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Sun, 11 Aug 2024 10:32:43 +1000 Subject: [PATCH 10/29] implement WithSpan on interfaces --- ext/otel_observer.c | 102 +++++++++--------- .../attribute_on_interface_with_params.phpt | 4 +- 2 files changed, 53 insertions(+), 53 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 2139c355..9ecf30fd 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -67,10 +67,54 @@ static inline void func_get_function_name(zval *zv, zend_execute_data *ex) { ZVAL_STR_COPY(zv, ex->func->op_array.function_name); } +static zend_function *find_function(zend_class_entry *ce, zend_string *name) { + zend_function *func; + ZEND_HASH_FOREACH_PTR(&ce->function_table, func) { + if (zend_string_equals(func->common.function_name, name)) { + return func; + } + } + ZEND_HASH_FOREACH_END(); + return NULL; +} + +// find WithSpan in attributes, or in interface method attributes +static zend_attribute *find_withspan_attribute(zend_function *func) { + zend_attribute *attr; + attr = zend_get_attribute_str(func->common.attributes, withspan_fqn_lc, + strlen(withspan_fqn_lc)); + if (attr != NULL) { + return attr; + } + zend_class_entry *ce = func->common.scope; + // if a method, check interfaces + if (ce && ce->num_interfaces > 0) { + zend_class_entry *interface_ce; + for (uint32_t i = 0; i < ce->num_interfaces; i++) { + interface_ce = ce->interfaces[i]; + if (interface_ce != NULL) { + zend_function *iface_func = + find_function(interface_ce, func->common.function_name); + if (iface_func) { + // Method found in the interface, now check for attributes + attr = zend_get_attribute_str(iface_func->common.attributes, + withspan_fqn_lc, + strlen(withspan_fqn_lc)); + if (attr) { + return attr; + } + } + } + } + } + return NULL; +} + // get function args (invo zv) and any args with the // SpanAttributes attribute applied (into zv_attrs) static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, - bool check_for_attributes) { + bool is_pre_hook) { + bool check_for_attributes = is_pre_hook && OTEL_G(attr_hooks_enabled); zval *p, *q; uint32_t i, first_extra_arg; uint32_t arg_count = ZEND_CALL_NUM_ARGS(ex); @@ -113,7 +157,7 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, ex->func->op_array.T); } while (i < arg_count) { - if (check_for_attributes && OTEL_G(attr_hooks_enabled) && + if (check_for_attributes && ex->func->type != ZEND_INTERNAL_FUNCTION) { zend_string *arg_name = ex->func->op_array.vars[i]; zend_attribute *attribute = @@ -228,9 +272,11 @@ static inline void func_get_lineno(zval *zv, zend_execute_data *ex) { } static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { - zend_attribute *attr; - attr = zend_get_attribute_str(ex->func->common.attributes, withspan_fqn_lc, - strlen(withspan_fqn_lc)); + if (!OTEL_G(attr_hooks_enabled)) { + ZVAL_EMPTY_ARRAY(zv); + return; + } + zend_attribute *attr = find_withspan_attribute(ex->func); if (attr == NULL || attr->argc == 0) { ZVAL_EMPTY_ARRAY(zv); return; @@ -250,7 +296,7 @@ static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { key = zend_string_init(with_span_attribute_args_keys[i], strlen(with_span_attribute_args_keys[i]), 0); zend_hash_add(ht, key, &arg.value); - zend_string_release(key); // todo move to end? + zend_string_release(key); } } @@ -850,50 +896,6 @@ static zval create_attribute_observer_handler(char *fn) { return callable; } -static zend_function *find_function(zend_class_entry *ce, zend_string *name) { - zend_function *func; - ZEND_HASH_FOREACH_PTR(&ce->function_table, func) { - if (zend_string_equals(func->common.function_name, name)) { - return func; - } - } - ZEND_HASH_FOREACH_END(); - return NULL; -} - -// find WithSpan in attributes, or in interface method attributes -static zend_attribute *find_withspan_attribute(zend_function *func) { - zend_attribute *attr; - attr = zend_get_attribute_str(func->common.attributes, withspan_fqn_lc, - strlen(withspan_fqn_lc)); - if (attr != NULL) { - return attr; - } - zend_class_entry *ce = func->common.scope; - // if a method, check interfaces - if (ce && ce->num_interfaces > 0) { - zend_class_entry *interface_ce; - // zend_attribute *attr; - for (uint32_t i = 0; i < ce->num_interfaces; i++) { - interface_ce = ce->interfaces[i]; - if (interface_ce != NULL) { - zend_function *iface_func = - find_function(interface_ce, func->common.function_name); - if (iface_func) { - // Method found in the interface, now check for attributes - attr = zend_get_attribute_str(iface_func->common.attributes, - withspan_fqn_lc, - strlen(withspan_fqn_lc)); - if (attr) { - return attr; - } - } - } - } - } - return NULL; -} - static otel_observer *resolve_observer(zend_execute_data *execute_data) { zend_function *fbc = execute_data->func; if (!fbc->common.function_name) { diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt index 060a2982..f3cc535c 100644 --- a/ext/tests/with_span/attribute_on_interface_with_params.phpt +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -2,8 +2,6 @@ Check if WithSpan can be applied to an interface with attribute args --EXTENSIONS-- opentelemetry ---XFAIL-- -Not implemented --FILE-- Date: Sun, 11 Aug 2024 11:22:44 +1000 Subject: [PATCH 11/29] implement SpanAttribute on interface --- ext/otel_observer.c | 43 +++++++++++++++++-- .../attribute_on_interface.phpt | 19 +++++--- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 9ecf30fd..35380bc8 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -78,6 +78,42 @@ static zend_function *find_function(zend_class_entry *ce, zend_string *name) { return NULL; } +// find SpanAttribute attribute on a parameter, or on a parameter of +// an interface +static zend_attribute *find_spanattribute_attribute(zend_function *func, + uint32_t i) { + zend_attribute *attr = zend_get_parameter_attribute_str( + func->common.attributes, spanattribute_fqn_lc, + strlen(spanattribute_fqn_lc), i); + + if (attr != NULL) { + return attr; + } + zend_class_entry *ce = func->common.scope; + if (ce && ce->num_interfaces > 0) { + zend_class_entry *interface_ce; + for (uint32_t i = 0; i < ce->num_interfaces; i++) { + interface_ce = ce->interfaces[i]; + if (interface_ce != NULL) { + // does the interface have the function we are looking for? + zend_function *iface_func = + find_function(interface_ce, func->common.function_name); + if (iface_func != NULL) { + // method found, check positional arg for attribute + attr = zend_get_parameter_attribute_str( + iface_func->common.attributes, spanattribute_fqn_lc, + strlen(spanattribute_fqn_lc), i); + if (attr != NULL) { + return attr; + } + } + } + } + } + + return NULL; +} + // find WithSpan in attributes, or in interface method attributes static zend_attribute *find_withspan_attribute(zend_function *func) { zend_attribute *attr; @@ -93,9 +129,10 @@ static zend_attribute *find_withspan_attribute(zend_function *func) { for (uint32_t i = 0; i < ce->num_interfaces; i++) { interface_ce = ce->interfaces[i]; if (interface_ce != NULL) { + // does the interface have the function we are looking for? zend_function *iface_func = find_function(interface_ce, func->common.function_name); - if (iface_func) { + if (iface_func != NULL) { // Method found in the interface, now check for attributes attr = zend_get_attribute_str(iface_func->common.attributes, withspan_fqn_lc, @@ -161,9 +198,7 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, ex->func->type != ZEND_INTERNAL_FUNCTION) { zend_string *arg_name = ex->func->op_array.vars[i]; zend_attribute *attribute = - zend_get_parameter_attribute_str( - ex->func->common.attributes, spanattribute_fqn_lc, - strlen(spanattribute_fqn_lc), i); + find_spanattribute_attribute(ex->func, i); bool has_span_attribute = attribute != NULL; if (has_span_attribute) { if (attribute->argc) { diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt index 8efc47dc..3586c7bd 100644 --- a/ext/tests/span_attribute/attribute_on_interface.phpt +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -2,13 +2,12 @@ Check if SpanAttribute can be applied to interface --EXTENSIONS-- opentelemetry ---XFAIL-- -Not implemented --FILE-- foo('one'); +(new TestClass())->foo('one', 'two'); ?> --EXPECT-- string(3) "pre" -array(1) { - ["renamed"]=> +array(2) { + ["renamed_one_from_interface"]=> string(3) "one" + ["renamed_two_from_class"]=> + string(3) "two" } string(3) "foo" string(4) "post" \ No newline at end of file From 77249123ff0227f6bfb84634384e7cf0bb0f8cc7 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 12 Aug 2024 18:17:26 +1000 Subject: [PATCH 12/29] combine WithSpan and SpanAttribute attributes into one array --- Makefile | 4 +- ext/opentelemetry.ide.stub.php | 2 +- ext/opentelemetry.stub.php | 2 +- ext/otel_observer.c | 61 +++++++++++-------- .../span_attribute_attribute_priority.phpt | 48 +++++++++++++++ .../attribute_on_interface_with_params.phpt | 12 ++-- .../attribute_params_passed_to_pre_hook.phpt | 16 ++--- 7 files changed, 102 insertions(+), 43 deletions(-) create mode 100644 ext/tests/span_attribute/span_attribute_attribute_priority.phpt diff --git a/Makefile b/Makefile index 55d9f0f8..e6c74336 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -PHP_VERSION ?= 8.3.0 +PHP_VERSION ?= 8.3.10 DISTRO ?= debian .DEFAULT_GOAL : help @@ -21,4 +21,6 @@ build: ## Build extension docker compose run $(DISTRO) ./build.sh test: ## Run tests docker compose run $(DISTRO) make test +remove-orphans: ## Remove orphaned containers + docker compose down --remove-orphans .PHONY: clean build test git-clean diff --git a/ext/opentelemetry.ide.stub.php b/ext/opentelemetry.ide.stub.php index a80fcbb5..8ffff7b2 100644 --- a/ext/opentelemetry.ide.stub.php +++ b/ext/opentelemetry.ide.stub.php @@ -5,7 +5,7 @@ /** * @param string|null $class The (optional) hooked function's class. Null for a global/built-in function. * @param string $function The hooked function's name. - * @param \Closure|null $pre function($class, array $params, string $class, string $function, ?string $filename, ?int $lineno): $params + * @param \Closure|null $pre function($class, array $params, string $class, string $function, ?string $filename, ?int $lineno, ?array $span_args, ?array $span_attributes): $params * You may optionally return modified parameters. * @param \Closure|null $post function($class, array $params, $returnValue, ?Throwable $exception): $returnValue * You may optionally return modified return value. diff --git a/ext/opentelemetry.stub.php b/ext/opentelemetry.stub.php index 1483814c..1cf6638b 100644 --- a/ext/opentelemetry.stub.php +++ b/ext/opentelemetry.stub.php @@ -7,7 +7,7 @@ /** * @param string|null $class The (optional) hooked function's class. Null for a global/built-in function. * @param string $function The hooked function's name. - * @param \Closure|null $pre function($class, array $params, string $class, string $function, ?string $filename, ?int $lineno): $params + * @param \Closure|null $pre function($class, array $params, string $class, string $function, ?string $filename, ?int $lineno, ?array $span_args, ?array $span_attributes): $params * You may optionally return modified parameters. * @param \Closure|null $post function($class, array $params, $returnValue, ?Throwable $exception): $returnValue * You may optionally return modified return value. diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 35380bc8..db579a0e 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -13,8 +13,7 @@ static int op_array_extension = -1; const char *withspan_fqn_lc = "opentelemetry\\instrumentation\\withspan"; const char *spanattribute_fqn_lc = "opentelemetry\\instrumentation\\spanattribute"; -static char *with_span_attribute_args_keys[] = {"name", "span_kind", - "attributes"}; +static char *with_span_attribute_args_keys[] = {"name", "span_kind"}; typedef struct otel_observer { zend_llist pre_hooks; @@ -149,17 +148,12 @@ static zend_attribute *find_withspan_attribute(zend_function *func) { // get function args (invo zv) and any args with the // SpanAttributes attribute applied (into zv_attrs) -static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, - bool is_pre_hook) { +static void func_get_args(zval *zv, HashTable *attributes, + zend_execute_data *ex, bool is_pre_hook) { bool check_for_attributes = is_pre_hook && OTEL_G(attr_hooks_enabled); zval *p, *q; uint32_t i, first_extra_arg; uint32_t arg_count = ZEND_CALL_NUM_ARGS(ex); - HashTable *ht; // attributes from SpanAttribute - if (check_for_attributes) { - ALLOC_HASHTABLE(ht); - zend_hash_init(ht, 0, NULL, ZVAL_PTR_DTOR, 0); - } // @see // https://github.com/php/php-src/blob/php-8.1.0/Zend/zend_builtin_functions.c#L235 @@ -199,13 +193,14 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, zend_string *arg_name = ex->func->op_array.vars[i]; zend_attribute *attribute = find_spanattribute_attribute(ex->func, i); - bool has_span_attribute = attribute != NULL; - if (has_span_attribute) { + if (attribute != NULL) { if (attribute->argc) { - zval key = attribute->args[0].value; - zend_hash_add(ht, Z_STR(key), p); + zend_string *key = Z_STR(attribute->args[0].value); + zend_hash_del(attributes, key); + zend_hash_add(attributes, key, p); } else { - zend_hash_add(ht, arg_name, p); + zend_hash_del(attributes, arg_name); + zend_hash_add(attributes, arg_name, p); } } } @@ -229,9 +224,6 @@ static void func_get_args(zval *zv, zval *zv_attrs, zend_execute_data *ex, } else { ZVAL_EMPTY_ARRAY(zv); } - if (zv_attrs) { - ZVAL_ARR(zv_attrs, ht); - } } static uint32_t func_get_arg_index_by_name(zend_execute_data *execute_data, @@ -306,7 +298,8 @@ static inline void func_get_lineno(zval *zv, zend_execute_data *ex) { } } -static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { +static inline void func_get_attribute_args(zval *zv, HashTable *attributes, + zend_execute_data *ex) { if (!OTEL_G(attr_hooks_enabled)) { ZVAL_EMPTY_ARRAY(zv); return; @@ -325,13 +318,24 @@ static inline void func_get_attribute_args(zval *zv, zend_execute_data *ex) { for (uint32_t i = 0; i < attr->argc; i++) { arg = attr->args[i]; - if (arg.name != NULL) { - zend_hash_add(ht, arg.name, &arg.value); + if (i == 2 || + (arg.name && zend_string_equals_literal(arg.name, "attributes"))) { + // attributes, append to a separate HashTable + if (Z_TYPE(arg.value) == IS_ARRAY) { + zend_hash_clean(attributes); // should already be empty + HashTable *array_ht = Z_ARRVAL_P(&arg.value); + zend_hash_copy(attributes, array_ht, zval_add_ref); + } } else { - key = zend_string_init(with_span_attribute_args_keys[i], - strlen(with_span_attribute_args_keys[i]), 0); - zend_hash_add(ht, key, &arg.value); - zend_string_release(key); + if (arg.name != NULL) { + zend_hash_add(ht, arg.name, &arg.value); + } else { + key = zend_string_init(with_span_attribute_args_keys[i], + strlen(with_span_attribute_args_keys[i]), + 0); + zend_hash_add(ht, key, &arg.value); + zend_string_release(key); + } } } @@ -580,14 +584,19 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { zval params[8]; uint32_t param_count = 8; + HashTable *attributes; + ALLOC_HASHTABLE(attributes); + zend_hash_init(attributes, 0, NULL, ZVAL_PTR_DTOR, 0); func_get_this_or_called_scope(¶ms[0], execute_data); - func_get_args(¶ms[1], ¶ms[7], execute_data, true); + func_get_attribute_args(¶ms[6], attributes, execute_data); + func_get_args(¶ms[1], attributes, execute_data, true); func_get_declaring_scope(¶ms[2], execute_data); func_get_function_name(¶ms[3], execute_data); func_get_filename(¶ms[4], execute_data); func_get_lineno(¶ms[5], execute_data); - func_get_attribute_args(¶ms[6], execute_data); + + ZVAL_ARR(¶ms[7], attributes); for (zend_llist_element *element = hooks->head; element; element = element->next) { diff --git a/ext/tests/span_attribute/span_attribute_attribute_priority.phpt b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt new file mode 100644 index 00000000..42483786 --- /dev/null +++ b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt @@ -0,0 +1,48 @@ +--TEST-- +Check if attributes from SpanAttribute replace attributes with same name from WithSpan +--EXTENSIONS-- +opentelemetry +--FILE-- + 'one_from_withspan', 'two' => 'two_from_withspan'])] + function foo( + #[SpanAttribute] string $one, + ): void + { + var_dump('foo'); + } +} + +$c = new TestClass(); +$c->foo('one'); +?> +--EXPECT-- +string(3) "pre" +array(2) { + ["two"]=> + string(17) "two_from_withspan" + ["one"]=> + string(3) "one" +} +string(3) "foo" +string(4) "post" \ No newline at end of file diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt index f3cc535c..55fc4fa3 100644 --- a/ext/tests/with_span/attribute_on_interface_with_params.phpt +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -14,6 +14,7 @@ class Handler { var_dump('pre'); var_dump(func_get_args()[6]); + var_dump(func_get_args()[7]); } public static function post(): void { @@ -39,16 +40,15 @@ class TestClass implements TestInterface ?> --EXPECT-- string(3) "pre" -array(3) { +array(2) { ["name"]=> string(3) "one" ["span_kind"]=> int(99) - ["attributes"]=> - array(1) { - ["foo"]=> - string(3) "bar" - } +} +array(1) { + ["foo"]=> + string(3) "bar" } string(3) "foo" string(4) "post" \ No newline at end of file diff --git a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt index ed23f3b6..1631c1c9 100644 --- a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt @@ -14,6 +14,7 @@ class Handler { $args = func_get_args(); var_dump($args[6] ?? null); + var_dump($args[7] ?? null); } public static function post(): void { @@ -33,18 +34,17 @@ class Foo (new Foo())->foo(); ?> --EXPECT-- -array(3) { +array(2) { ["name"]=> string(6) "param1" ["span_kind"]=> int(99) - ["attributes"]=> - array(2) { - ["attr1"]=> - string(6) "value1" - ["attr2"]=> - float(3.14) - } +} +array(2) { + ["attr1"]=> + string(6) "value1" + ["attr2"]=> + float(3.14) } string(3) "foo" string(4) "post" \ No newline at end of file From d5ad17d540d93467a607cb3b68f14c6e97081e00 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Mon, 12 Aug 2024 18:39:21 +1000 Subject: [PATCH 13/29] update comments, readme --- README.md | 5 ++++- ext/otel_observer.c | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 725f8296..746d0dfe 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,10 @@ Issues have been disabled for this repo in order to help maintain consistency be 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 +- adding attributes to functions and methods to enable observers at runtime In PHP 8.2+, internal/built-in PHP functions can also be observed. diff --git a/ext/otel_observer.c b/ext/otel_observer.c index db579a0e..ba5324ef 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -146,8 +146,8 @@ static zend_attribute *find_withspan_attribute(zend_function *func) { return NULL; } -// get function args (invo zv) and any args with the -// SpanAttributes attribute applied (into zv_attrs) +// get function args. any args with the +// SpanAttributes attribute are added to the attributes HashTable static void func_get_args(zval *zv, HashTable *attributes, zend_execute_data *ex, bool is_pre_hook) { bool check_for_attributes = is_pre_hook && OTEL_G(attr_hooks_enabled); @@ -970,8 +970,8 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { !zend_llist_count(&observer_instance.post_hooks)) { if (OTEL_G(attr_hooks_enabled) && has_withspan_attribute) { // there are no observers registered for this function/method, but - // it has a WithSpan attribute. Add our generic pre/post handlers - // as new observers. + // it has a WithSpan attribute. Add configured attribute-based + // pre/post handlers as new observers. zval pre = create_attribute_observer_handler( OTEL_G(pre_handler_function_fqn)); zval post = create_attribute_observer_handler( From 3c1bef3b4b5a562f74475929b291f4ae4f197c69 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 13 Aug 2024 14:02:15 +1000 Subject: [PATCH 14/29] add test for invalid callback --- ext/tests/with_span/invalid_callback.phpt | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 ext/tests/with_span/invalid_callback.phpt diff --git a/ext/tests/with_span/invalid_callback.phpt b/ext/tests/with_span/invalid_callback.phpt new file mode 100644 index 00000000..3e3e4502 --- /dev/null +++ b/ext/tests/with_span/invalid_callback.phpt @@ -0,0 +1,25 @@ +--TEST-- +Invalid callback is ignored +--EXTENSIONS-- +opentelemetry +--INI-- +opentelemetry.attr_pre_handler_function = "Invalid::pre" +opentelemetry.attr_post_handler_function = "Also\Invalid::post" +--FILE-- + +--EXPECT-- +string(12) "Invalid::pre" +string(18) "Also\Invalid::post" +string(4) "test" \ No newline at end of file From e2f4cc251d3674788b4444922364d2753d1461c3 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 21 Aug 2024 10:52:25 +1000 Subject: [PATCH 15/29] rename default handler to WithSpanHandler --- docker-compose.yaml | 5 ++--- ext/opentelemetry.c | 4 ++-- ext/tests/span_attribute/attribute_on_interface.phpt | 2 +- ext/tests/span_attribute/function_params.phpt | 2 +- ext/tests/span_attribute/method_params.phpt | 2 +- .../span_attribute/span_attribute_attribute_priority.phpt | 2 +- .../with_span/attribute_named_params_passed_to_pre_hook.phpt | 2 +- ext/tests/with_span/attribute_on_function.phpt | 2 +- ext/tests/with_span/attribute_on_interface.phpt | 2 +- ext/tests/with_span/attribute_on_interface_with_params.phpt | 2 +- ext/tests/with_span/attribute_on_method.phpt | 2 +- ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt | 2 +- ext/tests/with_span/attribute_params_skipped_if_hooked.phpt | 4 ++-- ext/tests/with_span/disable.phpt | 2 +- 14 files changed, 17 insertions(+), 18 deletions(-) diff --git a/docker-compose.yaml b/docker-compose.yaml index 617b0e2c..adce8b80 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,11 +1,10 @@ -version: '3.7' services: debian: build: context: docker dockerfile: Dockerfile.debian args: - PHP_VERSION: ${PHP_VERSION:-8.3.0} + PHP_VERSION: ${PHP_VERSION:-8.3.10} volumes: - ./ext:/usr/src/myapp environment: @@ -15,7 +14,7 @@ services: context: docker dockerfile: Dockerfile.alpine args: - PHP_VERSION: ${PHP_VERSION:-8.3.0} + PHP_VERSION: ${PHP_VERSION:-8.3.10} volumes: - ./ext:/usr/src/myapp environment: diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index 19d10913..a9e2d1d8 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -92,11 +92,11 @@ STD_PHP_INI_ENTRY_EX("opentelemetry.attr_hooks_enabled", "On", PHP_INI_ALL, zend_opentelemetry_globals, opentelemetry_globals, zend_ini_boolean_displayer_cb) STD_PHP_INI_ENTRY("opentelemetry.attr_pre_handler_function", - "Opentelemetry\\API\\Instrumentation\\Handler::pre", + "Opentelemetry\\API\\Instrumentation\\WithSpanHandler::pre", PHP_INI_ALL, OnUpdateString, pre_handler_function_fqn, zend_opentelemetry_globals, opentelemetry_globals) STD_PHP_INI_ENTRY("opentelemetry.attr_post_handler_function", - "Opentelemetry\\API\\Instrumentation\\Handler::post", + "Opentelemetry\\API\\Instrumentation\\WithSpanHandler::post", PHP_INI_ALL, OnUpdateString, post_handler_function_fqn, zend_opentelemetry_globals, opentelemetry_globals) PHP_INI_END() diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt index 3586c7bd..e8baecce 100644 --- a/ext/tests/span_attribute/attribute_on_interface.phpt +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -9,7 +9,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; use OpenTelemetry\Instrumentation\SpanAttribute; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/span_attribute/function_params.phpt b/ext/tests/span_attribute/function_params.phpt index 4a665b7d..b551bc0f 100644 --- a/ext/tests/span_attribute/function_params.phpt +++ b/ext/tests/span_attribute/function_params.phpt @@ -9,7 +9,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; use OpenTelemetry\Instrumentation\SpanAttribute; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/span_attribute/method_params.phpt b/ext/tests/span_attribute/method_params.phpt index 638c70b6..6d5fc073 100644 --- a/ext/tests/span_attribute/method_params.phpt +++ b/ext/tests/span_attribute/method_params.phpt @@ -9,7 +9,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; use OpenTelemetry\Instrumentation\SpanAttribute; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/span_attribute/span_attribute_attribute_priority.phpt b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt index 42483786..7109debe 100644 --- a/ext/tests/span_attribute/span_attribute_attribute_priority.phpt +++ b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt @@ -9,7 +9,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; use OpenTelemetry\Instrumentation\SpanAttribute; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt index 29cceb37..064b5b68 100644 --- a/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_on_function.phpt b/ext/tests/with_span/attribute_on_function.phpt index abc45783..3de49084 100644 --- a/ext/tests/with_span/attribute_on_function.phpt +++ b/ext/tests/with_span/attribute_on_function.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_on_interface.phpt b/ext/tests/with_span/attribute_on_interface.phpt index 80fb6d88..a9372878 100644 --- a/ext/tests/with_span/attribute_on_interface.phpt +++ b/ext/tests/with_span/attribute_on_interface.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt index 55fc4fa3..d6ffe88b 100644 --- a/ext/tests/with_span/attribute_on_interface_with_params.phpt +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt index 80b20226..9387b694 100644 --- a/ext/tests/with_span/attribute_on_method.phpt +++ b/ext/tests/with_span/attribute_on_method.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt index 1631c1c9..1545d308 100644 --- a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt @@ -8,7 +8,7 @@ namespace OpenTelemetry\API\Instrumentation; use OpenTelemetry\Instrumentation\WithSpan; -class Handler +class WithSpanHandler { public static function pre(): void { diff --git a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt index ce0774c4..11fd6f69 100644 --- a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt +++ b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt @@ -6,11 +6,11 @@ Attribute-based hooks are only applied if no other hooks are registered on a fun opentelemetry --FILE-- Date: Wed, 21 Aug 2024 19:31:42 +1000 Subject: [PATCH 16/29] only fetch attribute if WithSpan, warn if disabled and used --- ext/otel_observer.c | 22 +++++++++++++++------- ext/tests/with_span/disable.phpt | 3 ++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/ext/otel_observer.c b/ext/otel_observer.c index ba5324ef..2eb66c85 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -146,11 +146,16 @@ static zend_attribute *find_withspan_attribute(zend_function *func) { return NULL; } +static bool func_has_withspan_attribute(zend_execute_data *ex) { + zend_attribute *attr = find_withspan_attribute(ex->func); + + return attr != NULL; +} + // get function args. any args with the // SpanAttributes attribute are added to the attributes HashTable static void func_get_args(zval *zv, HashTable *attributes, - zend_execute_data *ex, bool is_pre_hook) { - bool check_for_attributes = is_pre_hook && OTEL_G(attr_hooks_enabled); + zend_execute_data *ex, bool check_for_attributes) { zval *p, *q; uint32_t i, first_extra_arg; uint32_t arg_count = ZEND_CALL_NUM_ARGS(ex); @@ -587,10 +592,12 @@ static void observer_begin(zend_execute_data *execute_data, zend_llist *hooks) { HashTable *attributes; ALLOC_HASHTABLE(attributes); zend_hash_init(attributes, 0, NULL, ZVAL_PTR_DTOR, 0); + bool check_for_attributes = + OTEL_G(attr_hooks_enabled) && func_has_withspan_attribute(execute_data); func_get_this_or_called_scope(¶ms[0], execute_data); func_get_attribute_args(¶ms[6], attributes, execute_data); - func_get_args(¶ms[1], attributes, execute_data, true); + func_get_args(¶ms[1], attributes, execute_data, check_for_attributes); func_get_declaring_scope(¶ms[2], execute_data); func_get_function_name(¶ms[3], execute_data); func_get_filename(¶ms[4], execute_data); @@ -945,11 +952,12 @@ static otel_observer *resolve_observer(zend_execute_data *execute_data) { if (!fbc->common.function_name) { return NULL; } - bool has_withspan_attribute = false; + bool has_withspan_attribute = func_has_withspan_attribute(execute_data); - if (OTEL_G(attr_hooks_enabled)) { - zend_attribute *attr = find_withspan_attribute(fbc); - has_withspan_attribute = (attr != NULL); + if (OTEL_G(attr_hooks_enabled) == false && has_withspan_attribute) { + php_error_docref(NULL, E_CORE_WARNING, + "OpenTelemetry: WithSpan attribute found but " + "attribute hooks disabled"); } otel_observer observer_instance; diff --git a/ext/tests/with_span/disable.phpt b/ext/tests/with_span/disable.phpt index e852f06f..43304355 100644 --- a/ext/tests/with_span/disable.phpt +++ b/ext/tests/with_span/disable.phpt @@ -30,5 +30,6 @@ function otel_attr_test(): void otel_attr_test(); ?> ---EXPECT-- +--EXPECTF-- +Warning: %s: OpenTelemetry: WithSpan attribute found but attribute hooks disabled in Unknown on line %d string(4) "test" From 50e8be92630941eb2d4f6404fbc1a5024433675c Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 21 Aug 2024 19:37:09 +1000 Subject: [PATCH 17/29] test whether non-simple types can be passed via SpanAttribute --- .../function_params_non_simple.phpt | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 ext/tests/span_attribute/function_params_non_simple.phpt diff --git a/ext/tests/span_attribute/function_params_non_simple.phpt b/ext/tests/span_attribute/function_params_non_simple.phpt new file mode 100644 index 00000000..cea2b605 --- /dev/null +++ b/ext/tests/span_attribute/function_params_non_simple.phpt @@ -0,0 +1,61 @@ +--TEST-- +Check if function non-simple types can be passed as function params +--EXTENSIONS-- +opentelemetry +--FILE-- + 'bar'], + new \stdClass(), + function(){return 'fn';}, + null, +); +?> +--EXPECT-- +string(3) "pre" +array(4) { + ["one"]=> + array(1) { + ["foo"]=> + string(3) "bar" + } + ["two"]=> + object(stdClass)#1 (0) { + } + ["three"]=> + object(Closure)#2 (0) { + } + ["four"]=> + NULL +} +string(3) "foo" +string(4) "post" \ No newline at end of file From 08058c8df7fc7e9c59faa389ab83f3458b80e42b Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 21 Aug 2024 20:48:31 +1000 Subject: [PATCH 18/29] disable by default --- ext/opentelemetry.c | 2 +- ext/tests/span_attribute/attribute_on_interface.phpt | 2 ++ ext/tests/span_attribute/function_params.phpt | 2 ++ ext/tests/span_attribute/function_params_non_simple.phpt | 2 ++ ext/tests/span_attribute/method_params.phpt | 2 ++ ext/tests/span_attribute/span_attribute_attribute_priority.phpt | 2 ++ .../with_span/attribute_named_params_passed_to_pre_hook.phpt | 2 ++ ext/tests/with_span/attribute_on_function.phpt | 2 ++ ext/tests/with_span/attribute_on_interface.phpt | 2 ++ ext/tests/with_span/attribute_on_interface_with_params.phpt | 2 ++ ext/tests/with_span/attribute_on_method.phpt | 2 ++ ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt | 2 ++ ext/tests/with_span/attribute_params_skipped_if_hooked.phpt | 2 ++ ext/tests/with_span/customize_handlers.phpt | 1 + ext/tests/with_span/invalid_callback.phpt | 1 + 15 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index a9e2d1d8..2d40aa6c 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -87,7 +87,7 @@ STD_PHP_INI_ENTRY_EX("opentelemetry.allow_stack_extension", "Off", PHP_INI_ALL, OnUpdateBool, allow_stack_extension, zend_opentelemetry_globals, opentelemetry_globals, zend_ini_boolean_displayer_cb) -STD_PHP_INI_ENTRY_EX("opentelemetry.attr_hooks_enabled", "On", PHP_INI_ALL, +STD_PHP_INI_ENTRY_EX("opentelemetry.attr_hooks_enabled", "Off", PHP_INI_ALL, OnUpdateBool, attr_hooks_enabled, zend_opentelemetry_globals, opentelemetry_globals, zend_ini_boolean_displayer_cb) diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt index e8baecce..a07132a3 100644 --- a/ext/tests/span_attribute/attribute_on_interface.phpt +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -2,6 +2,8 @@ Check if SpanAttribute can be applied to interface --EXTENSIONS-- opentelemetry +--INI-- +opentelemetry.attr_hooks_enabled = On --FILE-- Date: Wed, 21 Aug 2024 20:50:36 +1000 Subject: [PATCH 19/29] remove null type --- ext/tests/span_attribute/function_params_non_simple.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/tests/span_attribute/function_params_non_simple.phpt b/ext/tests/span_attribute/function_params_non_simple.phpt index 0d43e5c1..fae8ea23 100644 --- a/ext/tests/span_attribute/function_params_non_simple.phpt +++ b/ext/tests/span_attribute/function_params_non_simple.phpt @@ -29,7 +29,7 @@ function foo( #[SpanAttribute] array $one, #[SpanAttribute] object $two, #[SpanAttribute] callable $three, - #[SpanAttribute] null $four, + #[SpanAttribute] $four, ): void { var_dump('foo'); From 00416fe4f8049779231422a3ff46b739f444a722 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 10:23:17 +1000 Subject: [PATCH 20/29] removing attributes the attributes will be provided by our API, so update the references to match, and drop in an implementation of each for tests --- ext/opentelemetry.c | 10 ---- ext/opentelemetry.ide.stub.php | 39 -------------- ext/opentelemetry.stub.php | 10 +--- ext/opentelemetry_arginfo.h | 51 ++++--------------- ext/otel_observer.c | 4 +- .../attribute_on_interface.phpt | 6 ++- ext/tests/span_attribute/function_params.phpt | 6 ++- .../function_params_non_simple.phpt | 6 ++- ext/tests/span_attribute/method_params.phpt | 6 ++- .../span_attribute_attribute_priority.phpt | 6 ++- ...ibute_named_params_passed_to_pre_hook.phpt | 3 +- .../with_span/attribute_on_function.phpt | 3 +- .../with_span/attribute_on_interface.phpt | 3 +- .../attribute_on_interface_with_params.phpt | 3 +- ext/tests/with_span/attribute_on_method.phpt | 3 +- .../attribute_params_passed_to_pre_hook.phpt | 3 +- .../attribute_params_skipped_if_hooked.phpt | 3 +- ext/tests/with_span/customize_handlers.phpt | 3 +- ext/tests/with_span/disable.phpt | 4 +- 19 files changed, 53 insertions(+), 119 deletions(-) delete mode 100644 ext/opentelemetry.ide.stub.php diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index 2d40aa6c..dd89ae45 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -141,16 +141,6 @@ PHP_MINIT_FUNCTION(opentelemetry) { if (!OTEL_G(disabled)) { opentelemetry_observer_init(INIT_FUNC_ARGS_PASSTHRU); - zend_class_entry *ce_with_span = - register_class_OpenTelemetry_Instrumentation_WithSpan(); - zend_class_entry *ce_span_attribute = - register_class_OpenTelemetry_Instrumentation_SpanAttribute(); - // todo: gen_stubs.php should do this - zend_internal_attribute_register(ce_with_span, - ZEND_ATTRIBUTE_TARGET_METHOD | - ZEND_ATTRIBUTE_TARGET_FUNCTION); - zend_internal_attribute_register(ce_span_attribute, - ZEND_ATTRIBUTE_TARGET_PARAMETER); } return SUCCESS; diff --git a/ext/opentelemetry.ide.stub.php b/ext/opentelemetry.ide.stub.php deleted file mode 100644 index 8ffff7b2..00000000 --- a/ext/opentelemetry.ide.stub.php +++ /dev/null @@ -1,39 +0,0 @@ -ce_flags |= ZEND_ACC_FINAL; - - return class_entry; -} - -static zend_class_entry * -register_class_OpenTelemetry_Instrumentation_SpanAttribute(void) { - zend_class_entry ce, *class_entry; +ZEND_FUNCTION(hook); - INIT_NS_CLASS_ENTRY( - ce, "OpenTelemetry\\Instrumentation", "SpanAttribute", - class_OpenTelemetry_Instrumentation_SpanAttribute_methods); - class_entry = zend_register_internal_class_ex(&ce, NULL); - class_entry->ce_flags |= ZEND_ACC_FINAL; - return class_entry; -} +static const zend_function_entry ext_functions[] = { + ZEND_NS_FE("OpenTelemetry\\Instrumentation", hook, arginfo_OpenTelemetry_Instrumentation_hook) + ZEND_FE_END +}; diff --git a/ext/otel_observer.c b/ext/otel_observer.c index 2eb66c85..2b808da2 100644 --- a/ext/otel_observer.c +++ b/ext/otel_observer.c @@ -10,9 +10,9 @@ static int op_array_extension = -1; -const char *withspan_fqn_lc = "opentelemetry\\instrumentation\\withspan"; +const char *withspan_fqn_lc = "opentelemetry\\api\\instrumentation\\withspan"; const char *spanattribute_fqn_lc = - "opentelemetry\\instrumentation\\spanattribute"; + "opentelemetry\\api\\instrumentation\\spanattribute"; static char *with_span_attribute_args_keys[] = {"name", "span_kind"}; typedef struct otel_observer { diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt index a07132a3..af447eba 100644 --- a/ext/tests/span_attribute/attribute_on_interface.phpt +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -8,8 +8,10 @@ opentelemetry.attr_hooks_enabled = On Date: Thu, 29 Aug 2024 10:28:57 +1000 Subject: [PATCH 21/29] fix wonky arginfo generation now we don't need to fix the generated arginfo if stubs change --- DEVELOPMENT.md | 28 +--------------------------- ext/opentelemetry.c | 2 +- ext/opentelemetry_arginfo.h | 4 ++-- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 19fcf441..e00a318d 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -178,32 +178,6 @@ Basic usage is in the `tests/` directory. A more advanced example: https://github.com/open-telemetry/opentelemetry-php-contrib/pull/78/files -# `opentelemetry.stub.php` and `opentelemetry_argoinfo.h` - -The build system will generate `opentelemetry_arginfo.h` from `opentelemetry.stub.php`, but it seems -to generate an invalid output. Until that starts working as expected: - -Manually fix: - -```diff --ZEND_FUNCTION(OpenTelemetry_Instrumentation_hook); -+ZEND_FUNCTION(hook); -``` - -```diff --ZEND_NS_FALIAS("OpenTelemetry\\Instrumentation", hook, OpenTelemetry_Instrumentation_hook, arginfo_OpenTelemetry_Instrumentation_hook) -+ZEND_NS_FE("OpenTelemetry\\Instrumentation", hook, arginfo_OpenTelemetry_Instrumentation_hook) -``` - -Also note that the `#[Attribute]` attribute is not added to the classes in the stub file, as that could -not be handled by `gen_stub.php`. Constructors are also not added due to not building cleanly, but might -be fixable (TODO). - -You can regenerate the output by `php build/gen_stub.php`, or `make` (if the stub file has been changed). - -`opentelemetry.ide.stub.php` represents what the stub wile (probably) should be, and this file can -be used in IDEs for type-hinting. - # Further reading -* https://www.phpinternalsbook.com/php7/build_system/building_extensions.html \ No newline at end of file +* https://www.phpinternalsbook.com/php7/build_system/building_extensions.html diff --git a/ext/opentelemetry.c b/ext/opentelemetry.c index dd89ae45..4ae6aee2 100644 --- a/ext/opentelemetry.c +++ b/ext/opentelemetry.c @@ -101,7 +101,7 @@ STD_PHP_INI_ENTRY("opentelemetry.attr_post_handler_function", zend_opentelemetry_globals, opentelemetry_globals) PHP_INI_END() -PHP_FUNCTION(hook) { +PHP_FUNCTION(OpenTelemetry_Instrumentation_hook) { zend_string *class_name; zend_string *function_name; zval *pre = NULL; diff --git a/ext/opentelemetry_arginfo.h b/ext/opentelemetry_arginfo.h index 8f25d3c7..e6901290 100644 --- a/ext/opentelemetry_arginfo.h +++ b/ext/opentelemetry_arginfo.h @@ -9,10 +9,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_OpenTelemetry_Instrumentation_ho ZEND_END_ARG_INFO() -ZEND_FUNCTION(hook); +ZEND_FUNCTION(OpenTelemetry_Instrumentation_hook); static const zend_function_entry ext_functions[] = { - ZEND_NS_FE("OpenTelemetry\\Instrumentation", hook, arginfo_OpenTelemetry_Instrumentation_hook) + ZEND_NS_FALIAS("OpenTelemetry\\Instrumentation", hook, OpenTelemetry_Instrumentation_hook, arginfo_OpenTelemetry_Instrumentation_hook) ZEND_FE_END }; From 9ca3c9e181730c1da42ceef1d101a2615978d1b1 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 10:29:31 +1000 Subject: [PATCH 22/29] format --- ext/opentelemetry_arginfo.h | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/ext/opentelemetry_arginfo.h b/ext/opentelemetry_arginfo.h index e6901290..a130e22c 100644 --- a/ext/opentelemetry_arginfo.h +++ b/ext/opentelemetry_arginfo.h @@ -1,18 +1,16 @@ /* This is a generated file, edit the .stub.php file instead. * Stub hash: aa29142596154400c530f1194a7f29fbb9036929 */ -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_OpenTelemetry_Instrumentation_hook, 0, 2, _IS_BOOL, 0) - ZEND_ARG_TYPE_INFO(0, class, IS_STRING, 1) - ZEND_ARG_TYPE_INFO(0, function, IS_STRING, 0) - ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(0, pre, Closure, 1, "null") - ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(0, post, Closure, 1, "null") +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX( + arginfo_OpenTelemetry_Instrumentation_hook, 0, 2, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, class, IS_STRING, 1) + ZEND_ARG_TYPE_INFO(0, function, IS_STRING, 0) + ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(0, pre, Closure, 1, "null") + ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(0, post, Closure, 1, "null") ZEND_END_ARG_INFO() - ZEND_FUNCTION(OpenTelemetry_Instrumentation_hook); - -static const zend_function_entry ext_functions[] = { - ZEND_NS_FALIAS("OpenTelemetry\\Instrumentation", hook, OpenTelemetry_Instrumentation_hook, arginfo_OpenTelemetry_Instrumentation_hook) - ZEND_FE_END -}; +static const zend_function_entry ext_functions[] = {ZEND_NS_FALIAS( + "OpenTelemetry\\Instrumentation", hook, OpenTelemetry_Instrumentation_hook, + arginfo_OpenTelemetry_Instrumentation_hook) ZEND_FE_END}; From 2abf899a020ae551a77c709d0e0fb72d7e4fe535 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 10:33:16 +1000 Subject: [PATCH 23/29] force add mocks --- ext/tests/mocks/SpanAttribute.php | 14 ++++++++++++++ ext/tests/mocks/WithSpan.php | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 ext/tests/mocks/SpanAttribute.php create mode 100644 ext/tests/mocks/WithSpan.php diff --git a/ext/tests/mocks/SpanAttribute.php b/ext/tests/mocks/SpanAttribute.php new file mode 100644 index 00000000..48bea6e5 --- /dev/null +++ b/ext/tests/mocks/SpanAttribute.php @@ -0,0 +1,14 @@ + Date: Thu, 29 Aug 2024 10:35:00 +1000 Subject: [PATCH 24/29] don't gitignore mocks --- ext/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/.gitignore b/ext/.gitignore index 646379ad..86ef14d7 100644 --- a/ext/.gitignore +++ b/ext/.gitignore @@ -37,6 +37,7 @@ run-tests.php tests/**/*.diff tests/**/*.out tests/**/*.php +!tests/mocks/*.php tests/**/*.exp tests/**/*.log tests/**/*.sh From 11bcb5f95ec4415e7ed1cfca7bd3c40e76614dd7 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 11:11:39 +1000 Subject: [PATCH 25/29] skip attribute tests for 8.0 --- ext/tests/span_attribute/attribute_on_interface.phpt | 2 ++ ext/tests/span_attribute/function_params.phpt | 2 ++ ext/tests/span_attribute/function_params_non_simple.phpt | 2 ++ ext/tests/span_attribute/method_params.phpt | 2 ++ ext/tests/span_attribute/span_attribute_attribute_priority.phpt | 2 ++ .../with_span/attribute_named_params_passed_to_pre_hook.phpt | 2 ++ ext/tests/with_span/attribute_on_function.phpt | 2 ++ ext/tests/with_span/attribute_on_interface.phpt | 2 ++ ext/tests/with_span/attribute_on_interface_with_params.phpt | 2 ++ ext/tests/with_span/attribute_on_method.phpt | 2 ++ ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt | 2 ++ ext/tests/with_span/attribute_params_skipped_if_hooked.phpt | 2 ++ ext/tests/with_span/customize_handlers.phpt | 2 ++ ext/tests/with_span/disable.phpt | 2 ++ 14 files changed, 28 insertions(+) diff --git a/ext/tests/span_attribute/attribute_on_interface.phpt b/ext/tests/span_attribute/attribute_on_interface.phpt index af447eba..90735943 100644 --- a/ext/tests/span_attribute/attribute_on_interface.phpt +++ b/ext/tests/span_attribute/attribute_on_interface.phpt @@ -1,5 +1,7 @@ --TEST-- Check if SpanAttribute can be applied to interface +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/span_attribute/function_params.phpt b/ext/tests/span_attribute/function_params.phpt index c2ea5a8b..c12c1440 100644 --- a/ext/tests/span_attribute/function_params.phpt +++ b/ext/tests/span_attribute/function_params.phpt @@ -1,5 +1,7 @@ --TEST-- Check if function params can be passed via SpanAttribute +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/span_attribute/function_params_non_simple.phpt b/ext/tests/span_attribute/function_params_non_simple.phpt index 260ae7db..b8ae29b3 100644 --- a/ext/tests/span_attribute/function_params_non_simple.phpt +++ b/ext/tests/span_attribute/function_params_non_simple.phpt @@ -1,5 +1,7 @@ --TEST-- Check if function non-simple types can be passed as function params +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/span_attribute/method_params.phpt b/ext/tests/span_attribute/method_params.phpt index 13fbe928..e260d0c2 100644 --- a/ext/tests/span_attribute/method_params.phpt +++ b/ext/tests/span_attribute/method_params.phpt @@ -1,5 +1,7 @@ --TEST-- Check if method params can be passed via SpanAttribute +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/span_attribute/span_attribute_attribute_priority.phpt b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt index de44b7af..104d39d8 100644 --- a/ext/tests/span_attribute/span_attribute_attribute_priority.phpt +++ b/ext/tests/span_attribute/span_attribute_attribute_priority.phpt @@ -1,5 +1,7 @@ --TEST-- Check if attributes from SpanAttribute replace attributes with same name from WithSpan +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt index 2c375ce2..0d66cf88 100644 --- a/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_named_params_passed_to_pre_hook.phpt @@ -1,5 +1,7 @@ --TEST-- Check if named attribute parameters are passed to pre hook +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_on_function.phpt b/ext/tests/with_span/attribute_on_function.phpt index e02e9e22..d8feb5ff 100644 --- a/ext/tests/with_span/attribute_on_function.phpt +++ b/ext/tests/with_span/attribute_on_function.phpt @@ -1,5 +1,7 @@ --TEST-- Check if custom attribute loaded +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_on_interface.phpt b/ext/tests/with_span/attribute_on_interface.phpt index 6bc8cc7b..8f4f20e9 100644 --- a/ext/tests/with_span/attribute_on_interface.phpt +++ b/ext/tests/with_span/attribute_on_interface.phpt @@ -1,5 +1,7 @@ --TEST-- Check if custom attribute can be applied to an interface +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_on_interface_with_params.phpt b/ext/tests/with_span/attribute_on_interface_with_params.phpt index 5e81b495..b307c069 100644 --- a/ext/tests/with_span/attribute_on_interface_with_params.phpt +++ b/ext/tests/with_span/attribute_on_interface_with_params.phpt @@ -1,5 +1,7 @@ --TEST-- Check if WithSpan can be applied to an interface with attribute args +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt index 3baf04d4..39a1cb2f 100644 --- a/ext/tests/with_span/attribute_on_method.phpt +++ b/ext/tests/with_span/attribute_on_method.phpt @@ -1,5 +1,7 @@ --TEST-- Check if WithSpan can be applied to a method +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt index f6a40cae..6255a971 100644 --- a/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt +++ b/ext/tests/with_span/attribute_params_passed_to_pre_hook.phpt @@ -1,5 +1,7 @@ --TEST-- Check if WithSpan parameters are passed to pre hook +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt index 82b5cb18..837b08cf 100644 --- a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt +++ b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt @@ -1,5 +1,7 @@ --TEST-- Check if hooking a method takes priority over WithSpan +--SKIPIF-- += 8.1'); ?> ----DESCRIPTION-- Attribute-based hooks are only applied if no other hooks are registered on a function or method. --EXTENSIONS-- diff --git a/ext/tests/with_span/customize_handlers.phpt b/ext/tests/with_span/customize_handlers.phpt index 4fba63fc..a27291eb 100644 --- a/ext/tests/with_span/customize_handlers.phpt +++ b/ext/tests/with_span/customize_handlers.phpt @@ -1,5 +1,7 @@ --TEST-- Check if WithSpan handlers can be changed via config +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- diff --git a/ext/tests/with_span/disable.phpt b/ext/tests/with_span/disable.phpt index dc4989df..6f56f4fd 100644 --- a/ext/tests/with_span/disable.phpt +++ b/ext/tests/with_span/disable.phpt @@ -1,5 +1,7 @@ --TEST-- Check if attribute hooks can be disabled by config +--SKIPIF-- += 8.1'); ?> --EXTENSIONS-- opentelemetry --INI-- From 77f2bc7317c9f3f82b699875bd36bfec70fe4392 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 11:11:46 +1000 Subject: [PATCH 26/29] update readme --- README.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 746d0dfe..efa3647a 100644 --- a/README.md +++ b/README.md @@ -248,7 +248,10 @@ string(8) "original" By applying attributes to source code, the OpenTelemetry extension can add hooks at runtime. -By default, the following pre/post hooks will be invoked: `OpenTelemetry\API\Instrumentation\Handler::pre` and `::post`. +Default pre and post hook methods are provided by the OpenTelemetry API: `OpenTelemetry\API\Instrumentation\Handler::pre` +and `::post`. + +This feature is disabled by default, but can be enabled by setting `opentelemetry.attr_hooks_enabled = On` in php.ini ## Restrictions @@ -259,24 +262,25 @@ Only one hook can be applied to a function/method, including via interfaces. Since the attributes are evaluated at runtime, the extension checks whether a hook already exists to decide whether it should apply a new runtime hook. -### Configuration +## Configuration This feature can be configured via `.ini` by modifying the following entries: -- `opentelemetry.attr_hooks_enabled` - boolean, default On +- `opentelemetry.attr_hooks_enabled` - boolean, default Off - `opentelemetry.attr_pre_handler_function` - FQN of pre method/function - `opentelemetry.attr_post_handler_function` - FQN of post method/function -## `WithSpan` attribute +## `OpenTelemetry\API\Instrumentation\WithSpan` attribute + +This attribute is provided by the OpenTelemetry API can be applied to a function or class method. -This attribute can be applied to a function or class method. You can also provide optional parameters to the attribute, which control: - span name - span kind - attributes ```php -use OpenTelemetry\Instrumentation\WithSpan +use OpenTelemetry\API\Instrumentation\WithSpan class MyClass { @@ -300,7 +304,7 @@ function my_function(): void } ``` -## `SpanAttribute` attribute +## `OpenTelemetry\API\Instrumentation\SpanAttribute` attribute This attribute should be used in conjunction with `WithSpan`. It is applied to function/method parameters, and causes those parameters and values to be passed through to the `pre` hook function @@ -309,8 +313,8 @@ There is one optional parameter, which controls the attribute key. If not set, t is used. ```php -use OpenTelemetry\Instrumentation\WithSpan -use OpenTelemetry\Instrumentation\SpanAttribute +use OpenTelemetry\API\Instrumentation\WithSpan +use OpenTelemetry\API\Instrumentation\SpanAttribute class MyClass { From 610380a764a65610a06173e66f04d82c6d6bde5b Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 11:16:06 +1000 Subject: [PATCH 27/29] un-bork --- ext/tests/with_span/attribute_params_skipped_if_hooked.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt index 837b08cf..d184e87e 100644 --- a/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt +++ b/ext/tests/with_span/attribute_params_skipped_if_hooked.phpt @@ -2,7 +2,7 @@ Check if hooking a method takes priority over WithSpan --SKIPIF-- = 8.1'); ?> -----DESCRIPTION-- +--DESCRIPTION-- Attribute-based hooks are only applied if no other hooks are registered on a function or method. --EXTENSIONS-- opentelemetry From ccb719a6ecc0161a5857450295db4ad123b67c62 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 11:33:54 +1000 Subject: [PATCH 28/29] tidy attribute tests --- ext/tests/mocks/WithSpanHandler.php | 15 +++++++++++++++ .../mocks/WithSpanHandlerDumpAttributes.php | 16 ++++++++++++++++ .../span_attribute/attribute_on_interface.phpt | 14 +------------- ext/tests/span_attribute/function_params.phpt | 14 +------------- .../function_params_non_simple.phpt | 14 +------------- ext/tests/span_attribute/method_params.phpt | 14 +------------- .../span_attribute_attribute_priority.phpt | 14 +------------- ext/tests/with_span/attribute_on_function.phpt | 13 +------------ ext/tests/with_span/attribute_on_interface.phpt | 13 +------------ ext/tests/with_span/attribute_on_method.phpt | 13 +------------ 10 files changed, 39 insertions(+), 101 deletions(-) create mode 100644 ext/tests/mocks/WithSpanHandler.php create mode 100644 ext/tests/mocks/WithSpanHandlerDumpAttributes.php diff --git a/ext/tests/mocks/WithSpanHandler.php b/ext/tests/mocks/WithSpanHandler.php new file mode 100644 index 00000000..339133b0 --- /dev/null +++ b/ext/tests/mocks/WithSpanHandler.php @@ -0,0 +1,15 @@ + 'one_from_withspan', 'two' => 'two_from_withspan'])] diff --git a/ext/tests/with_span/attribute_on_function.phpt b/ext/tests/with_span/attribute_on_function.phpt index d8feb5ff..41434501 100644 --- a/ext/tests/with_span/attribute_on_function.phpt +++ b/ext/tests/with_span/attribute_on_function.phpt @@ -11,20 +11,9 @@ opentelemetry.attr_hooks_enabled = On namespace OpenTelemetry\API\Instrumentation; include dirname(__DIR__) . '/mocks/WithSpan.php'; +include dirname(__DIR__) . '/mocks/WithSpanHandler.php'; use OpenTelemetry\API\Instrumentation\WithSpan; -class WithSpanHandler -{ - public static function pre(): void - { - var_dump('pre'); - } - public static function post(): void - { - var_dump('post'); - } -} - #[WithSpan] function otel_attr_test(): void { diff --git a/ext/tests/with_span/attribute_on_interface.phpt b/ext/tests/with_span/attribute_on_interface.phpt index 8f4f20e9..9309ffd3 100644 --- a/ext/tests/with_span/attribute_on_interface.phpt +++ b/ext/tests/with_span/attribute_on_interface.phpt @@ -11,20 +11,9 @@ opentelemetry.attr_hooks_enabled = On namespace OpenTelemetry\API\Instrumentation; include dirname(__DIR__) . '/mocks/WithSpan.php'; +include dirname(__DIR__) . '/mocks/WithSpanHandler.php'; use OpenTelemetry\API\Instrumentation\WithSpan; -class WithSpanHandler -{ - public static function pre(): void - { - var_dump('pre'); - } - public static function post(): void - { - var_dump('post'); - } -} - interface TestInterface { #[WithSpan] diff --git a/ext/tests/with_span/attribute_on_method.phpt b/ext/tests/with_span/attribute_on_method.phpt index 39a1cb2f..024a08cd 100644 --- a/ext/tests/with_span/attribute_on_method.phpt +++ b/ext/tests/with_span/attribute_on_method.phpt @@ -11,20 +11,9 @@ opentelemetry.attr_hooks_enabled = On namespace OpenTelemetry\API\Instrumentation; include dirname(__DIR__) . '/mocks/WithSpan.php'; +include dirname(__DIR__) . '/mocks/WithSpanHandler.php'; use OpenTelemetry\API\Instrumentation\WithSpan; -class WithSpanHandler -{ - public static function pre(): void - { - var_dump('pre'); - } - public static function post(): void - { - var_dump('post'); - } -} - class TestClass { #[WithSpan] From 0d0a4d6e6d2ab5671cb76b39bf46f22efbf3a61f Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 29 Aug 2024 11:41:40 +1000 Subject: [PATCH 29/29] remove unimplemented function from header --- ext/otel_observer.h | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/otel_observer.h b/ext/otel_observer.h index 5592b8d0..47b3d942 100644 --- a/ext/otel_observer.h +++ b/ext/otel_observer.h @@ -3,7 +3,6 @@ #define OPENTELEMETRY_OBSERVER_H void opentelemetry_observer_init(INIT_FUNC_ARGS); -void opentelemetry_attributes_init(); void observer_globals_init(void); void observer_globals_cleanup(void);