From 275f8410450d91f7d4bf8c4f33b8c73bc861007c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 23 Aug 2024 11:07:57 -0600 Subject: [PATCH 01/52] create named datastore metrics for memcached --- agent/php_internal_instrument.c | 67 +++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 821379edb..4ab3ea0f4 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -13,6 +13,7 @@ #include "php_explain_mysqli.h" #include "php_file_get_contents.h" #include "php_globals.h" +#include "php_hash.h" #include "php_httprequest_send.h" #include "php_internal_instrument.h" #include "php_mysql.h" @@ -1531,6 +1532,66 @@ NR_INNER_WRAPPER(memcache_function) { INTERNAL_FUNCTION_PARAM_PASSTHRU); } +static nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( + const char* host, + zend_long port) { + nr_datastore_instance_t* instance = NULL; + if (port == 0) { // local socket + instance = nr_datastore_instance_create("localhost", host, NULL); + } else { + char* port_str = nr_formatf("%ld", (long)port); + instance = nr_datastore_instance_create(host, port_str, NULL); + nr_free(port_str); + } + return instance; +} + +NR_INNER_WRAPPER(memcached_connect_function) { + char* host = NULL; + nr_string_len_t host_len = 0; + zend_long port = 0; + zend_long weight = 0; + nr_datastore_instance_t* instance = NULL; + + if (SUCCESS + == zend_parse_parameters_ex( + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "s|ll", &host, + &host_len, &port, &weight)) { + if (NULL != host) { + instance = nr_php_memcached_create_datastore_instance(host, port); + } + } + nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, + nr_wrapper->extra, instance, + INTERNAL_FUNCTION_PARAM_PASSTHRU); +} + +NR_INNER_WRAPPER(memcached_multi_connect_function) { + zval* servers = NULL; + zval* server = NULL; + zend_ulong num_key = 0; + nr_php_string_hash_key_t* string_key = NULL; + nr_datastore_instance_t* instance = NULL; + + if (SUCCESS + == zend_parse_parameters_ex( + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "a", &servers)) { + ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(servers), num_key, string_key, server) { + (void)num_key; + (void)string_key; + zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); + zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); + if (NULL != host) { + instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); + nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, + nr_wrapper->extra, instance, + INTERNAL_FUNCTION_PARAM_PASSTHRU); + } + } + ZEND_HASH_FOREACH_END(); + } +} + /* * Handle * bool redis::connect ( string $host[, int $port = 6379 ... ] ) @@ -3098,6 +3159,8 @@ NR_OUTER_WRAPPER(memcached_set) NR_OUTER_WRAPPER(memcached_setbykey) NR_OUTER_WRAPPER(memcached_setmulti) NR_OUTER_WRAPPER(memcached_setmultibykey) +NR_OUTER_WRAPPER(memcached_addserver) +NR_OUTER_WRAPPER(memcached_addservers) NR_OUTER_WRAPPER(redis_append) NR_OUTER_WRAPPER(redis_bitcount) @@ -3511,6 +3574,10 @@ void nr_php_generate_internal_wrap_records(void) { memcache_function, 0, "set") NR_INTERNAL_WRAPREC("memcached::setmultibykey", memcached_setmultibykey, memcache_function, 0, "set") + NR_INTERNAL_WRAPREC("memcached::addServer", memcached_addserver, + memcached_connect_function, 0, "connect"); + NR_INTERNAL_WRAPREC("memcached::addServers", memcached_addservers, + memcached_multi_connect_function, 0, "connect"); NR_INTERNAL_WRAPREC("redis::connect", redis_connect, redis_connect, 0, "connect") From a462c202b903226098629c5a0dba5365385127e8 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Mon, 26 Aug 2024 12:42:42 -0600 Subject: [PATCH 02/52] restrict metrics --- agent/php_internal_instrument.c | 24 ++++++------ axiom/nr_segment_datastore.c | 67 +++++++++++++++++++-------------- axiom/nr_segment_datastore.h | 3 ++ 3 files changed, 55 insertions(+), 39 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 4ab3ea0f4..9866fa217 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -769,6 +769,7 @@ static void nr_php_instrument_datastore_operation_call( nr_datastore_t datastore, const char* operation, nr_datastore_instance_t* instance, + bool instance_only, INTERNAL_FUNCTION_PARAMETERS) { int zcaught = 0; nr_segment_t* segment = NULL; @@ -778,6 +779,7 @@ static void nr_php_instrument_datastore_operation_call( }, .operation = nr_strdup(operation), .instance = instance, + .instance_only = instance_only, .callbacks = { .backtrace = nr_php_backtrace_callback, }, @@ -833,7 +835,7 @@ NR_INNER_WRAPPER(mysqli_commit) { } nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MYSQL, - "commit", instance, + "commit", instance, false, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1528,7 +1530,7 @@ NR_INNER_WRAPPER(mysqli_stmt_prepare) { */ NR_INNER_WRAPPER(memcache_function) { nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - nr_wrapper->extra, NULL, + nr_wrapper->extra, NULL, false, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1562,7 +1564,7 @@ NR_INNER_WRAPPER(memcached_connect_function) { } } nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - nr_wrapper->extra, instance, + NULL, instance, true, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1584,7 +1586,7 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { if (NULL != host) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - nr_wrapper->extra, instance, + NULL, instance, true, INTERNAL_FUNCTION_PARAM_PASSTHRU); } } @@ -1617,7 +1619,7 @@ NR_INNER_WRAPPER(redis_connect) { } nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_REDIS, - nr_wrapper->extra, instance, + nr_wrapper->extra, instance, false, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1676,7 +1678,7 @@ NR_INNER_WRAPPER(redis_function) { instance = nr_php_redis_retrieve_datastore_instance(this_obj TSRMLS_CC); nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_REDIS, - nr_wrapper->extra, instance, + nr_wrapper->extra, instance, false, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -2669,7 +2671,7 @@ NR_INNER_WRAPPER(ob_flush_common) { */ NR_INNER_WRAPPER(mongodb_execute) { nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MONGODB, - "execute", NULL, + "execute", NULL, false, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -3574,10 +3576,10 @@ void nr_php_generate_internal_wrap_records(void) { memcache_function, 0, "set") NR_INTERNAL_WRAPREC("memcached::setmultibykey", memcached_setmultibykey, memcache_function, 0, "set") - NR_INTERNAL_WRAPREC("memcached::addServer", memcached_addserver, - memcached_connect_function, 0, "connect"); - NR_INTERNAL_WRAPREC("memcached::addServers", memcached_addservers, - memcached_multi_connect_function, 0, "connect"); + NR_INTERNAL_WRAPREC("memcached::addserver", memcached_addserver, + memcached_connect_function, 0, 0); + NR_INTERNAL_WRAPREC("memcached::addservers", memcached_addservers, + memcached_multi_connect_function, 0, 0); NR_INTERNAL_WRAPREC("redis::connect", redis_connect, redis_connect, 0, "connect") diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index 6ccc3391e..fb9d71e8c 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -12,6 +12,36 @@ #include "util_sql.h" #include "util_logging.h" +static bool create_instance_metric(nr_segment_t* segment, + const char* product, + nr_segment_datastore_t* datastore, + const nr_datastore_instance_t* instance) { + /* + * If a datastore instance was provided, we need to add the relevant data to + * the segment and the relavant metrics. + */ + nrtxn_t* txn = segment->txn; + char* instance_metric = NULL; + if (NULL == instance || 0 == txn->options.instance_reporting_enabled) { + return false; + } + + if (txn->options.database_name_reporting_enabled) { + nr_datastore_instance_set_database_name(&datastore->instance, + instance->database_name); + } + + instance_metric = nr_formatf("Datastore/instance/%s/%s/%s", product, + instance->host, instance->port_path_or_id); + nr_segment_add_metric(segment, instance_metric, false); + nr_datastore_instance_set_host(&datastore->instance, instance->host); + nr_datastore_instance_set_port_path_or_id(&datastore->instance, + instance->port_path_or_id); + + nr_free(instance_metric); + return true; +} + static char* create_metrics(nr_segment_t* segment, nrtime_t duration, const char* product, @@ -24,7 +54,6 @@ static char* create_metrics(nr_segment_t* segment, char* rollup_metric = NULL; char* scoped_metric = NULL; char* statement_metric = NULL; - char* instance_metric = NULL; nrm_force_add(txn->unscoped_metrics, "Datastore/all", duration); @@ -50,28 +79,7 @@ static char* create_metrics(nr_segment_t* segment, nr_free(operation_metric); nr_free(statement_metric); - /* - * If a datastore instance was provided, we need to add the relevant data to - * the segment and the relavant metrics. - */ - if (NULL == instance || 0 == txn->options.instance_reporting_enabled) { - return scoped_metric; - } - - if (txn->options.database_name_reporting_enabled) { - nr_datastore_instance_set_database_name(&datastore->instance, - instance->database_name); - } - - instance_metric = nr_formatf("Datastore/instance/%s/%s/%s", product, - instance->host, instance->port_path_or_id); - nr_segment_add_metric(segment, instance_metric, false); - nr_datastore_instance_set_host(&datastore->instance, instance->host); - nr_datastore_instance_set_port_path_or_id(&datastore->instance, - instance->port_path_or_id); - - nr_free(instance_metric); - + create_instance_metric(segment, product, datastore, instance); return scoped_metric; } @@ -208,11 +216,14 @@ bool nr_segment_datastore_end(nr_segment_t** segment_ptr, * The allWeb and allOther rollup metrics are created at the end of the * transaction since the background status may change. */ - scoped_metric - = create_metrics(segment, duration, datastore_string, collection, - operation, &datastore, params->instance); - - nr_segment_set_name(segment, scoped_metric); + if (!params->instance_only) { + scoped_metric + = create_metrics(segment, duration, datastore_string, collection, + operation, &datastore, params->instance); + nr_segment_set_name(segment, scoped_metric); + } else { + create_instance_metric(segment, datastore_string, &datastore, params->instance); + } /* * Add the explain plan, if we have one. diff --git a/axiom/nr_segment_datastore.h b/axiom/nr_segment_datastore.h index a89a2acdb..e1d0d6aac 100644 --- a/axiom/nr_segment_datastore.h +++ b/axiom/nr_segment_datastore.h @@ -24,6 +24,9 @@ typedef struct _nr_segment_datastore_params_t { extracted from the SQL for SQL segments. */ nr_datastore_instance_t* instance; /* Any instance information that was collected. */ + bool instance_only; /* true if only the instance metric is wanted, + collection and operation fields will not be + used or extracted from the SQL */ /* * Datastore type fields. From b5cef15ee292a77493ba191084ee0a12ecd56894 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 28 Aug 2024 12:23:17 -0600 Subject: [PATCH 03/52] fixup --- agent/php_internal_instrument.c | 42 ++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 9866fa217..3e8bef7b3 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -799,6 +799,34 @@ static void nr_php_instrument_datastore_operation_call( } } +/* + * Like nr_php_instrument_datastore_operation_call() but does not + * call the underlying internal function + */ +static void nr_php_instrument_datastore_operation( + nr_datastore_t datastore, + const char* operation, + nr_datastore_instance_t* instance, + bool instance_only, + INTERNAL_FUNCTION_PARAMETERS) { + nr_segment_t* segment = NULL; + nr_segment_datastore_params_t params = { + .datastore = { + .type = datastore, + }, + .operation = nr_strdup(operation), + .instance = instance, + .instance_only = instance_only, + .callbacks = { + .backtrace = nr_php_backtrace_callback, + }, + }; + + segment = nr_segment_start(NRPRG(txn), NULL, NULL); + nr_segment_datastore_end(&segment, ¶ms); + nr_free(params.operation); +} + /* * Handle * bool mysqli_commit ( object $link [, int $flags=0, string $name] ) @@ -1574,6 +1602,7 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { zend_ulong num_key = 0; nr_php_string_hash_key_t* string_key = NULL; nr_datastore_instance_t* instance = NULL; + int zcaught = 0; if (SUCCESS == zend_parse_parameters_ex( @@ -1585,13 +1614,20 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); if (NULL != host) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); - nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - NULL, instance, true, - INTERNAL_FUNCTION_PARAM_PASSTHRU); + nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, + NULL, instance, true, + INTERNAL_FUNCTION_PARAM_PASSTHRU); } } ZEND_HASH_FOREACH_END(); } + zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, + INTERNAL_FUNCTION_PARAM_PASSTHRU); + + if (zcaught) { + zend_bailout(); + /* NOTREACHED */ + } } /* From 65307787177fcfb6eff75ef77b34301e9e68bfdf Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 28 Aug 2024 12:23:33 -0600 Subject: [PATCH 04/52] integration tests --- .../memcached/test_add_servers.php | 34 +++++++++++++++++++ tests/integration/memcached/test_basic.php | 1 + .../memcached/test_basic_logging_off.php | 1 + tests/integration/memcached/test_by_key.php | 1 + tests/integration/memcached/test_cas.php7.php | 1 + .../memcached/test_cas_by_key.php7.php | 1 + tests/integration/memcached/test_concat.php | 1 + .../memcached/test_concat_by_key.php | 1 + .../test_concat_by_key_logging_off.php | 1 + tests/integration/memcached/test_multi.php | 1 + .../memcached/test_multi_by_key.php | 1 + 11 files changed, 44 insertions(+) create mode 100644 tests/integration/memcached/test_add_servers.php diff --git a/tests/integration/memcached/test_add_servers.php b/tests/integration/memcached/test_add_servers.php new file mode 100644 index 000000000..3151bddb9 --- /dev/null +++ b/tests/integration/memcached/test_add_servers.php @@ -0,0 +1,34 @@ + +*/ + +/*INI +*/ + +/*EXPECT_METRICS_EXIST +Datastore/instance/Memcached/host1/1 +Datastore/instance/Memcached/host2/2 +Datastore/instance/Memcached/host3/11211 +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); +require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); +require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); + +$memcached = new Memcached(); +$memcached->addServers(array( + array("host1", 1), + array("host2", 2), + array("host3", 11211))); +$memcached->quit(); diff --git a/tests/integration/memcached/test_basic.php b/tests/integration/memcached/test_basic.php index 6ccde57b7..f1f06a1e1 100644 --- a/tests/integration/memcached/test_basic.php +++ b/tests/integration/memcached/test_basic.php @@ -46,6 +46,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_basic_logging_off.php b/tests/integration/memcached/test_basic_logging_off.php index 521fc1d44..6f9e37a28 100644 --- a/tests/integration/memcached/test_basic_logging_off.php +++ b/tests/integration/memcached/test_basic_logging_off.php @@ -49,6 +49,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_by_key.php b/tests/integration/memcached/test_by_key.php index 006d6cd7d..8477fc9c0 100644 --- a/tests/integration/memcached/test_by_key.php +++ b/tests/integration/memcached/test_by_key.php @@ -48,6 +48,7 @@ [{"name":"Datastore/allOther"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [11, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas.php7.php b/tests/integration/memcached/test_cas.php7.php index b69010e59..db4afc545 100644 --- a/tests/integration/memcached/test_cas.php7.php +++ b/tests/integration/memcached/test_cas.php7.php @@ -42,6 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas_by_key.php7.php b/tests/integration/memcached/test_cas_by_key.php7.php index f43a6dc29..8fe0aece2 100644 --- a/tests/integration/memcached/test_cas_by_key.php7.php +++ b/tests/integration/memcached/test_cas_by_key.php7.php @@ -42,6 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat.php b/tests/integration/memcached/test_concat.php index 092bd5683..c82997e3d 100644 --- a/tests/integration/memcached/test_concat.php +++ b/tests/integration/memcached/test_concat.php @@ -35,6 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key.php b/tests/integration/memcached/test_concat_by_key.php index 7339eed12..46c68f032 100644 --- a/tests/integration/memcached/test_concat_by_key.php +++ b/tests/integration/memcached/test_concat_by_key.php @@ -35,6 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key_logging_off.php b/tests/integration/memcached/test_concat_by_key_logging_off.php index d4e3f2d60..8ecc8d3f7 100644 --- a/tests/integration/memcached/test_concat_by_key_logging_off.php +++ b/tests/integration/memcached/test_concat_by_key_logging_off.php @@ -38,6 +38,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi.php b/tests/integration/memcached/test_multi.php index 82328762a..4a450b24c 100644 --- a/tests/integration/memcached/test_multi.php +++ b/tests/integration/memcached/test_multi.php @@ -42,6 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi_by_key.php b/tests/integration/memcached/test_multi_by_key.php index d5867c36d..aaef9e82a 100644 --- a/tests/integration/memcached/test_multi_by_key.php +++ b/tests/integration/memcached/test_multi_by_key.php @@ -42,6 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], From 4c46e86c4bc253a92ba751740d6c7bd37b717303 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 28 Aug 2024 14:38:21 -0600 Subject: [PATCH 05/52] fix expecteds --- tests/integration/memcached/test_basic.php | 2 +- tests/integration/memcached/test_basic_logging_off.php | 2 +- tests/integration/memcached/test_by_key.php | 2 +- tests/integration/memcached/test_cas.php7.php | 2 +- tests/integration/memcached/test_cas_by_key.php7.php | 2 +- tests/integration/memcached/test_concat.php | 2 +- tests/integration/memcached/test_concat_by_key.php | 2 +- tests/integration/memcached/test_concat_by_key_logging_off.php | 2 +- tests/integration/memcached/test_multi.php | 2 +- tests/integration/memcached/test_multi_by_key.php | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integration/memcached/test_basic.php b/tests/integration/memcached/test_basic.php index f1f06a1e1..c8f601356 100644 --- a/tests/integration/memcached/test_basic.php +++ b/tests/integration/memcached/test_basic.php @@ -46,7 +46,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_basic_logging_off.php b/tests/integration/memcached/test_basic_logging_off.php index 6f9e37a28..ba2d7ac37 100644 --- a/tests/integration/memcached/test_basic_logging_off.php +++ b/tests/integration/memcached/test_basic_logging_off.php @@ -49,7 +49,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_by_key.php b/tests/integration/memcached/test_by_key.php index 8477fc9c0..ebb8c90b7 100644 --- a/tests/integration/memcached/test_by_key.php +++ b/tests/integration/memcached/test_by_key.php @@ -48,7 +48,7 @@ [{"name":"Datastore/allOther"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [11, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas.php7.php b/tests/integration/memcached/test_cas.php7.php index db4afc545..8ed54241f 100644 --- a/tests/integration/memcached/test_cas.php7.php +++ b/tests/integration/memcached/test_cas.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas_by_key.php7.php b/tests/integration/memcached/test_cas_by_key.php7.php index 8fe0aece2..31b39ce51 100644 --- a/tests/integration/memcached/test_cas_by_key.php7.php +++ b/tests/integration/memcached/test_cas_by_key.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat.php b/tests/integration/memcached/test_concat.php index c82997e3d..eb2791d22 100644 --- a/tests/integration/memcached/test_concat.php +++ b/tests/integration/memcached/test_concat.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key.php b/tests/integration/memcached/test_concat_by_key.php index 46c68f032..ce5776310 100644 --- a/tests/integration/memcached/test_concat_by_key.php +++ b/tests/integration/memcached/test_concat_by_key.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key_logging_off.php b/tests/integration/memcached/test_concat_by_key_logging_off.php index 8ecc8d3f7..520d898fb 100644 --- a/tests/integration/memcached/test_concat_by_key_logging_off.php +++ b/tests/integration/memcached/test_concat_by_key_logging_off.php @@ -38,7 +38,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi.php b/tests/integration/memcached/test_multi.php index 4a450b24c..b4e60f0d7 100644 --- a/tests/integration/memcached/test_multi.php +++ b/tests/integration/memcached/test_multi.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi_by_key.php b/tests/integration/memcached/test_multi_by_key.php index aaef9e82a..b993906c8 100644 --- a/tests/integration/memcached/test_multi_by_key.php +++ b/tests/integration/memcached/test_multi_by_key.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], From ca82d090dcba20977cb586658d7fad5ec2b36c8a Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 4 Sep 2024 13:02:56 -0600 Subject: [PATCH 06/52] refactor --- agent/config.m4 | 2 +- agent/php_internal_instrument.c | 15 +-------------- agent/php_memcached.c | 18 ++++++++++++++++++ agent/php_memcached.h | 16 ++++++++++++++++ 4 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 agent/php_memcached.c create mode 100644 agent/php_memcached.h diff --git a/agent/config.m4 b/agent/config.m4 index ee0859e30..5806caac5 100644 --- a/agent/config.m4 +++ b/agent/config.m4 @@ -215,7 +215,7 @@ if test "$PHP_NEWRELIC" = "yes"; then php_error.c php_execute.c php_explain.c php_explain_mysqli.c \ php_explain_pdo_mysql.c php_extension.c php_file_get_contents.c \ php_globals.c php_hash.c php_header.c php_httprequest_send.c \ - php_internal_instrument.c php_minit.c php_mshutdown.c php_mysql.c \ + php_internal_instrument.c php_memcached.c php_minit.c php_mshutdown.c php_mysql.c \ php_mysqli.c php_newrelic.c php_nrini.c php_observer.c php_output.c php_pdo.c \ php_pdo_mysql.c php_pdo_pgsql.c php_pgsql.c php_psr7.c php_redis.c \ php_rinit.c php_rshutdown.c php_samplers.c php_stack.c \ diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 3e8bef7b3..af09c8e38 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -16,6 +16,7 @@ #include "php_hash.h" #include "php_httprequest_send.h" #include "php_internal_instrument.h" +#include "php_memcached.h" #include "php_mysql.h" #include "php_mysqli.h" #include "php_pdo.h" @@ -1562,20 +1563,6 @@ NR_INNER_WRAPPER(memcache_function) { INTERNAL_FUNCTION_PARAM_PASSTHRU); } -static nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( - const char* host, - zend_long port) { - nr_datastore_instance_t* instance = NULL; - if (port == 0) { // local socket - instance = nr_datastore_instance_create("localhost", host, NULL); - } else { - char* port_str = nr_formatf("%ld", (long)port); - instance = nr_datastore_instance_create(host, port_str, NULL); - nr_free(port_str); - } - return instance; -} - NR_INNER_WRAPPER(memcached_connect_function) { char* host = NULL; nr_string_len_t host_len = 0; diff --git a/agent/php_memcached.c b/agent/php_memcached.c new file mode 100644 index 000000000..34f58d9c2 --- /dev/null +++ b/agent/php_memcached.c @@ -0,0 +1,18 @@ +#include "php_memcached.h" +#include "nr_datastore_instance.h" +#include "php_agent.h" + +nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( + const char* host, + zend_long port) { + nr_datastore_instance_t* instance = NULL; + if (port == 0) { // local socket + instance = nr_datastore_instance_create("localhost", host, NULL); + } else { + char* port_str = nr_formatf("%ld", (long)port); + instance = nr_datastore_instance_create(host, port_str, NULL); + nr_free(port_str); + } + return instance; +} + diff --git a/agent/php_memcached.h b/agent/php_memcached.h new file mode 100644 index 000000000..6d947e204 --- /dev/null +++ b/agent/php_memcached.h @@ -0,0 +1,16 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef PHP_MEMCACHED_HDR +#define PHP_MEMCACHED_HDR + +#include "nr_datastore_instance.h" +#include "php_includes.h" + +extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( + const char* host, + zend_long port); + +#endif From b777c76229f0f55442ea078038456fea9ce20860 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 17 Sep 2024 14:06:04 -0600 Subject: [PATCH 07/52] PR feedback --- agent/php_internal_instrument.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index af09c8e38..d60645231 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -802,14 +802,15 @@ static void nr_php_instrument_datastore_operation_call( /* * Like nr_php_instrument_datastore_operation_call() but does not - * call the underlying internal function + * call the underlying internal function. This allows multiple + * datastore metrics to be creating during a single underlying + * PHP call. */ static void nr_php_instrument_datastore_operation( nr_datastore_t datastore, const char* operation, nr_datastore_instance_t* instance, - bool instance_only, - INTERNAL_FUNCTION_PARAMETERS) { + bool instance_only) { nr_segment_t* segment = NULL; nr_segment_datastore_params_t params = { .datastore = { @@ -1602,8 +1603,7 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { if (NULL != host) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, - NULL, instance, true, - INTERNAL_FUNCTION_PARAM_PASSTHRU); + NULL, instance, true); } } ZEND_HASH_FOREACH_END(); From 31186d87da398b5c0d7a3acb190f294f11694fdb Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 20 Sep 2024 11:01:18 -0600 Subject: [PATCH 08/52] add memcached instance socket test --- tests/integration/memcached/test_socket.php | 45 +++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 tests/integration/memcached/test_socket.php diff --git a/tests/integration/memcached/test_socket.php b/tests/integration/memcached/test_socket.php new file mode 100644 index 000000000..85a07a229 --- /dev/null +++ b/tests/integration/memcached/test_socket.php @@ -0,0 +1,45 @@ + +*/ + +/*INI +*/ + +/*EXPECT_METRICS +[ + "?? agent run id", + "?? start time", + "?? stop time", + [ + [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/allOther"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/__HOST__/my_socket"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"OtherTransaction/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Supportability/Logging/Forwarding/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Supportability/Logging/Metrics/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] + ] +] +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); +require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); +require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); + +$memcached = new Memcached(); +$memcached->addServer("my_socket", 0); +$memcached->quit(); From 9fb7bd31769704433b13c93a76fb3a6b84129f7c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 24 Sep 2024 15:39:09 -0400 Subject: [PATCH 09/52] doc string --- agent/php_memcached.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agent/php_memcached.h b/agent/php_memcached.h index 6d947e204..785fd946b 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -9,6 +9,12 @@ #include "nr_datastore_instance.h" #include "php_includes.h" +/* + * Purpose : Create a datastore instance metadata for a Memcached connection. + * + * Params : 1. The memcached host or socket name as given to Memcached::addServer(). + * 2. The memcached port as given as given to Memcached::addServer(). + */ extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( const char* host, zend_long port); From 55af999f1fc34418eb3fe61ad158e3cd6e5da903 Mon Sep 17 00:00:00 2001 From: ZNeumann Date: Tue, 24 Sep 2024 13:42:06 -0600 Subject: [PATCH 10/52] Update agent/php_memcached.c Co-authored-by: Michal Nowacki --- agent/php_memcached.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/agent/php_memcached.c b/agent/php_memcached.c index 34f58d9c2..bf419560e 100644 --- a/agent/php_memcached.c +++ b/agent/php_memcached.c @@ -1,3 +1,8 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + #include "php_memcached.h" #include "nr_datastore_instance.h" #include "php_agent.h" From eb038da00507947c322c67e3021fec380319d796 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 25 Sep 2024 11:59:05 -0400 Subject: [PATCH 11/52] host_or_socket name --- agent/php_memcached.c | 6 +++--- agent/php_memcached.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php_memcached.c b/agent/php_memcached.c index bf419560e..3fc0b0bdb 100644 --- a/agent/php_memcached.c +++ b/agent/php_memcached.c @@ -8,14 +8,14 @@ #include "php_agent.h" nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( - const char* host, + const char* host_or_socket, zend_long port) { nr_datastore_instance_t* instance = NULL; if (port == 0) { // local socket - instance = nr_datastore_instance_create("localhost", host, NULL); + instance = nr_datastore_instance_create("localhost", host_or_socket, NULL); } else { char* port_str = nr_formatf("%ld", (long)port); - instance = nr_datastore_instance_create(host, port_str, NULL); + instance = nr_datastore_instance_create(host_or_socket, port_str, NULL); nr_free(port_str); } return instance; diff --git a/agent/php_memcached.h b/agent/php_memcached.h index 785fd946b..f09c11a3e 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -16,7 +16,7 @@ * 2. The memcached port as given as given to Memcached::addServer(). */ extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( - const char* host, + const char* host_or_socket, zend_long port); #endif From f7878e02de3476e0ee22a6b3a5ba552296235b90 Mon Sep 17 00:00:00 2001 From: ZNeumann Date: Thu, 26 Sep 2024 10:04:30 -0600 Subject: [PATCH 12/52] Update agent/php_internal_instrument.c Co-authored-by: Michal Nowacki --- agent/php_internal_instrument.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index d60645231..3d81db6bb 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1573,7 +1573,7 @@ NR_INNER_WRAPPER(memcached_connect_function) { if (SUCCESS == zend_parse_parameters_ex( - ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "s|ll", &host, + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|ll", &host, &host_len, &port, &weight)) { if (NULL != host) { instance = nr_php_memcached_create_datastore_instance(host, port); From db755da2e8ad75c5adeede71988047a937bd8c12 Mon Sep 17 00:00:00 2001 From: ZNeumann Date: Thu, 26 Sep 2024 10:16:21 -0600 Subject: [PATCH 13/52] Update agent/php_internal_instrument.c Co-authored-by: Michal Nowacki --- agent/php_internal_instrument.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 3d81db6bb..da79e8b8f 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1594,7 +1594,7 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { if (SUCCESS == zend_parse_parameters_ex( - ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "a", &servers)) { + ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "a", &servers)) { ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(servers), num_key, string_key, server) { (void)num_key; (void)string_key; From ee6ad5f4fc373af4b22d412d5fe6511e2c05b7fa Mon Sep 17 00:00:00 2001 From: ZNeumann Date: Thu, 26 Sep 2024 10:16:31 -0600 Subject: [PATCH 14/52] Update agent/php_internal_instrument.c Co-authored-by: Michal Nowacki --- agent/php_internal_instrument.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index da79e8b8f..592ed66db 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1595,9 +1595,7 @@ NR_INNER_WRAPPER(memcached_multi_connect_function) { if (SUCCESS == zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "a", &servers)) { - ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(servers), num_key, string_key, server) { - (void)num_key; - (void)string_key; + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); if (NULL != host) { From 330e976909d724c3cf8769a56c41fede1b55f7d1 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 12:19:36 -0400 Subject: [PATCH 15/52] doc string --- agent/php_memcached.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/php_memcached.h b/agent/php_memcached.h index f09c11a3e..f0eedcb2d 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -14,6 +14,8 @@ * * Params : 1. The memcached host or socket name as given to Memcached::addServer(). * 2. The memcached port as given as given to Memcached::addServer(). + * + * Returns: nr_datastore_instance_t* that the caller is responsible for freeing */ extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( const char* host_or_socket, From c60dacdf79bde88d6bf731f4d159bac95107c412 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 13:13:28 -0400 Subject: [PATCH 16/52] move datastore op call --- agent/php_internal_instrument.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 592ed66db..7fcb3c103 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1574,14 +1574,20 @@ NR_INNER_WRAPPER(memcached_connect_function) { if (SUCCESS == zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|ll", &host, - &host_len, &port, &weight)) { - if (NULL != host) { - instance = nr_php_memcached_create_datastore_instance(host, port); + &host_len, &port, &weight) && + NULL != host) { + instance = nr_php_memcached_create_datastore_instance(host, port); + nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, + NULL, instance, true, + INTERNAL_FUNCTION_PARAM_PASSTHRU); + } else { + zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, + INTERNAL_FUNCTION_PARAM_PASSTHRU); + if (zcaught) { + zend_bailout(); + /* NOTREACHED */ } } - nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - NULL, instance, true, - INTERNAL_FUNCTION_PARAM_PASSTHRU); } NR_INNER_WRAPPER(memcached_multi_connect_function) { From b38b9ca44d2340402254e36e030e743240e20e42 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 14:56:30 -0400 Subject: [PATCH 17/52] return status --- axiom/nr_segment_datastore.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index fb9d71e8c..8cf3bb4c6 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -12,7 +12,7 @@ #include "util_sql.h" #include "util_logging.h" -static bool create_instance_metric(nr_segment_t* segment, +static nr_status_t create_instance_metric(nr_segment_t* segment, const char* product, nr_segment_datastore_t* datastore, const nr_datastore_instance_t* instance) { @@ -23,7 +23,7 @@ static bool create_instance_metric(nr_segment_t* segment, nrtxn_t* txn = segment->txn; char* instance_metric = NULL; if (NULL == instance || 0 == txn->options.instance_reporting_enabled) { - return false; + return NR_FAILURE; } if (txn->options.database_name_reporting_enabled) { @@ -39,7 +39,7 @@ static bool create_instance_metric(nr_segment_t* segment, instance->port_path_or_id); nr_free(instance_metric); - return true; + return NR_SUCCESS; } static char* create_metrics(nr_segment_t* segment, From 1b8a8492bce6547cba81ab5bef011330d28f3aea Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 15:01:07 -0400 Subject: [PATCH 18/52] add empty addservers test --- tests/integration/memcached/test_add_servers.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/memcached/test_add_servers.php b/tests/integration/memcached/test_add_servers.php index 3151bddb9..6ef3c335d 100644 --- a/tests/integration/memcached/test_add_servers.php +++ b/tests/integration/memcached/test_add_servers.php @@ -31,4 +31,5 @@ array("host1", 1), array("host2", 2), array("host3", 11211))); +$memcached->addServers(array()); $memcached->quit(); From 5d165f2afb2c55f559b05d5b57c0fe151510795c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 15:01:13 -0400 Subject: [PATCH 19/52] fixups --- agent/php_internal_instrument.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 7fcb3c103..407ed1f0c 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1570,6 +1570,7 @@ NR_INNER_WRAPPER(memcached_connect_function) { zend_long port = 0; zend_long weight = 0; nr_datastore_instance_t* instance = NULL; + int zcaught = 0; if (SUCCESS == zend_parse_parameters_ex( @@ -1593,8 +1594,6 @@ NR_INNER_WRAPPER(memcached_connect_function) { NR_INNER_WRAPPER(memcached_multi_connect_function) { zval* servers = NULL; zval* server = NULL; - zend_ulong num_key = 0; - nr_php_string_hash_key_t* string_key = NULL; nr_datastore_instance_t* instance = NULL; int zcaught = 0; From 92ac8d56f3435978041473fb7ddf972e40d76285 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 15:29:14 -0400 Subject: [PATCH 20/52] unit test --- axiom/tests/test_segment_datastore.c | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/axiom/tests/test_segment_datastore.c b/axiom/tests/test_segment_datastore.c index cb49ddebc..15fda8ed0 100644 --- a/axiom/tests/test_segment_datastore.c +++ b/axiom/tests/test_segment_datastore.c @@ -197,6 +197,35 @@ static void test_create_metrics_no_table_no_operation(void) { nr_txn_destroy(&txn); } +static void test_create_metrics_instance_only(void) { + nrtxn_t* txn = new_txn(0); + nrtime_t duration = 4 * NR_TIME_DIVISOR; + nr_segment_datastore_params_t params = sample_segment_datastore_params(); + nr_datastore_instance_t instance = {.host = "hostname", + .port_path_or_id = "123", + .database_name = "my database"}; + nr_segment_t* segment = NULL; + char* tname = "create metrics"; + + params.instance = &instance; + params.instance_only = true; + segment = nr_segment_start(txn, NULL, NULL); + segment->start_time = 1 * NR_TIME_DIVISOR; + segment->stop_time = 1 * NR_TIME_DIVISOR + duration; + + test_segment_datastore_end_and_keep(&segment, ¶ms); + + /* + * Test : Create only the instance metric + */ + test_metric_vector_size(segment->metrics, 1); + test_segment_metric_created(tname, segment->metrics, + "Datastore/instance/MongoDB/hostname/123", + false); + + nr_txn_destroy(&txn); +} + static void test_instance_info_reporting_disabled(void) { nrtxn_t* txn = new_txn(0); nrtime_t duration = 4 * NR_TIME_DIVISOR; @@ -1127,6 +1156,7 @@ tlib_parallel_info_t parallel_info = {.suggested_nthreads = 2, .state_size = 0}; void test_main(void* p NRUNUSED) { test_bad_parameters(); test_create_metrics(); + test_create_metrics_instance_only(); test_create_metrics_no_table(); test_create_metrics_no_table_no_operation(); test_instance_info_reporting_disabled(); From 007c7748d30b42bd017d38fe9f945d8a2c3c24e1 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 15:56:45 -0400 Subject: [PATCH 21/52] more unit tests --- agent/tests/test_memcached.c | 95 ++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 agent/tests/test_memcached.c diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c new file mode 100644 index 000000000..de8e44c52 --- /dev/null +++ b/agent/tests/test_memcached.c @@ -0,0 +1,95 @@ +/* + * Copyright 2020 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +#include "tlib_php.h" +#include "tlib_datastore.h" + +#include "php_agent.h" +#include "php_memcached.h" +#include "util_system.h" + +tlib_parallel_info_t parallel_info + = {.suggested_nthreads = -1, .state_size = 0}; + +static char* system_host_name; + +static void test_create_datastore_instance(void) { + /* + * Test : Normal operation. + */ + assert_datastore_instance_equals_destroy( + "named socket", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "/tmp/memcached.sock", + }), + nr_php_memcached_create_datastore_instance("/tmp/memcached.sock", 0)); + + assert_datastore_instance_equals_destroy( + "empty socket", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "unknown", + }), + nr_php_memcached_create_datastore_instance("", 0)); + + assert_datastore_instance_equals_destroy( + "empty host", + &((nr_datastore_instance_t){ + .host = "unknown", + .database_name = "unknown", + .port_path_or_id = "6379", + }), + nr_php_memcached_create_datastore_instance("", 6379)); + + assert_datastore_instance_equals_destroy( + "localhost socket", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "localhost", + }), + nr_php_memcached_create_datastore_instance("localhost", 0)); + + assert_datastore_instance_equals_destroy( + "localhost and port", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "6379", + }), + nr_php_memcached_create_datastore_instance("localhost", 6379)); + + assert_datastore_instance_equals_destroy( + "host.name socket", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "host.name", + }), + nr_php_memcached_create_datastore_instance("host.name", 0)); + + assert_datastore_instance_equals_destroy( + "host and port", + &((nr_datastore_instance_t){ + .host = "host.name", + .database_name = "unknown", + .port_path_or_id = "6379", + }), + nr_php_memcached_create_datastore_instance("host.name", 6379)); +} + +void test_main(void* p NRUNUSED) { +#if defined(ZTS) && !defined(PHP7) + void*** tsrm_ls = NULL; +#endif /* ZTS && !PHP7 */ + + system_host_name = nr_system_get_hostname(); + + test_create_datastore_instance(); + + nr_free(system_host_name); +} From 1e97b44961004bab762b7968d051ddd25b7d9106 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 16:07:06 -0400 Subject: [PATCH 22/52] refactor lines --- axiom/nr_segment_datastore.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index 8cf3bb4c6..57406459c 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -34,9 +34,6 @@ static nr_status_t create_instance_metric(nr_segment_t* segment, instance_metric = nr_formatf("Datastore/instance/%s/%s/%s", product, instance->host, instance->port_path_or_id); nr_segment_add_metric(segment, instance_metric, false); - nr_datastore_instance_set_host(&datastore->instance, instance->host); - nr_datastore_instance_set_port_path_or_id(&datastore->instance, - instance->port_path_or_id); nr_free(instance_metric); return NR_SUCCESS; @@ -80,6 +77,9 @@ static char* create_metrics(nr_segment_t* segment, nr_free(statement_metric); create_instance_metric(segment, product, datastore, instance); + nr_datastore_instance_set_host(&datastore->instance, instance->host); + nr_datastore_instance_set_port_path_or_id(&datastore->instance, + instance->port_path_or_id); return scoped_metric; } From 6db55f6be78ccee2ae3ab68bbfc50861f5e8c6fb Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 16:13:12 -0400 Subject: [PATCH 23/52] NULL unit test --- agent/tests/test_memcached.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c index de8e44c52..72b6e909f 100644 --- a/agent/tests/test_memcached.c +++ b/agent/tests/test_memcached.c @@ -80,6 +80,15 @@ static void test_create_datastore_instance(void) { .port_path_or_id = "6379", }), nr_php_memcached_create_datastore_instance("host.name", 6379)); + + assert_datastore_instance_equals_destroy( + "NULL socket", + &((nr_datastore_instance_t){ + .host = system_host_name, + .database_name = "unknown", + .port_path_or_id = "unknown", + }), + nr_php_memcached_create_datastore_instance(NULL, 0)); } void test_main(void* p NRUNUSED) { From f447a56d29e409ba28f6492bce8254470d6eca6a Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 16:14:19 -0400 Subject: [PATCH 24/52] rename add_server --- agent/php_internal_instrument.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 407ed1f0c..38f07d155 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1564,7 +1564,7 @@ NR_INNER_WRAPPER(memcache_function) { INTERNAL_FUNCTION_PARAM_PASSTHRU); } -NR_INNER_WRAPPER(memcached_connect_function) { +NR_INNER_WRAPPER(memcached_add_server) { char* host = NULL; nr_string_len_t host_len = 0; zend_long port = 0; @@ -1591,7 +1591,7 @@ NR_INNER_WRAPPER(memcached_connect_function) { } } -NR_INNER_WRAPPER(memcached_multi_connect_function) { +NR_INNER_WRAPPER(memcached_add_servers) { zval* servers = NULL; zval* server = NULL; nr_datastore_instance_t* instance = NULL; @@ -3603,9 +3603,9 @@ void nr_php_generate_internal_wrap_records(void) { NR_INTERNAL_WRAPREC("memcached::setmultibykey", memcached_setmultibykey, memcache_function, 0, "set") NR_INTERNAL_WRAPREC("memcached::addserver", memcached_addserver, - memcached_connect_function, 0, 0); + memcached_add_server, 0, 0); NR_INTERNAL_WRAPREC("memcached::addservers", memcached_addservers, - memcached_multi_connect_function, 0, 0); + memcached_add_servers, 0, 0); NR_INTERNAL_WRAPREC("redis::connect", redis_connect, redis_connect, 0, "connect") From ae2a70b2f6169c9a0646ba49df5c68ebe0df8ee6 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 16:16:56 -0400 Subject: [PATCH 25/52] NULL check --- agent/php_internal_instrument.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 38f07d155..b80d9fce8 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1603,7 +1603,7 @@ NR_INNER_WRAPPER(memcached_add_servers) { ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); - if (NULL != host) { + if (NULL != host && NULL != port) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, NULL, instance, true); From cf1f77274d858199f189d170ea80b80afbbe76d4 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 26 Sep 2024 16:55:19 -0400 Subject: [PATCH 26/52] fix --- axiom/nr_segment_datastore.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index 57406459c..d625e46b8 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -77,9 +77,11 @@ static char* create_metrics(nr_segment_t* segment, nr_free(statement_metric); create_instance_metric(segment, product, datastore, instance); - nr_datastore_instance_set_host(&datastore->instance, instance->host); - nr_datastore_instance_set_port_path_or_id(&datastore->instance, - instance->port_path_or_id); + if (NULL != instance && 0 != txn->options.instance_reporting_enabled) { + nr_datastore_instance_set_host(&datastore->instance, instance->host); + nr_datastore_instance_set_port_path_or_id(&datastore->instance, + instance->port_path_or_id); + } return scoped_metric; } From 330b5c64d05e64e70c4979b7f0a8ac141b55f464 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 09:25:53 -0400 Subject: [PATCH 27/52] more tests --- agent/php_internal_instrument.c | 19 +++++++++++-------- .../memcached/test_add_servers.php | 5 +++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index b80d9fce8..7886138a0 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1600,16 +1600,19 @@ NR_INNER_WRAPPER(memcached_add_servers) { if (SUCCESS == zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "a", &servers)) { - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { - zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); - zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); - if (NULL != host && NULL != port) { - instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); - nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, - NULL, instance, true); + if (Z_TYPE_P(servers) == IS_ARRAY) { + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { + zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); + zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); + if (NULL != host && NULL != port && + Z_TYPE_P(host) == IS_STRING && Z_TYPE_P(port) == IS_LONG) { + instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); + nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, + NULL, instance, true); + } } + ZEND_HASH_FOREACH_END(); } - ZEND_HASH_FOREACH_END(); } zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, INTERNAL_FUNCTION_PARAM_PASSTHRU); diff --git a/tests/integration/memcached/test_add_servers.php b/tests/integration/memcached/test_add_servers.php index 6ef3c335d..113497e06 100644 --- a/tests/integration/memcached/test_add_servers.php +++ b/tests/integration/memcached/test_add_servers.php @@ -20,6 +20,7 @@ Datastore/instance/Memcached/host1/1 Datastore/instance/Memcached/host2/2 Datastore/instance/Memcached/host3/11211 +Datastore/instance/Memcached/host4/1 */ require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); @@ -32,4 +33,8 @@ array("host2", 2), array("host3", 11211))); $memcached->addServers(array()); +$memcached->addServers(array(array("host4", 1, "test field"))); +$memcached->addServers(array(array(1))); +$memcached->addServers(array(array("host1"))); +$memcached->addServers(array(array(1, "host1"))); $memcached->quit(); From c8b08807b6a0cd37f89228b56cdb46b82c144e8c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 09:41:53 -0400 Subject: [PATCH 28/52] simplify everything --- agent/php_internal_instrument.c | 61 +++++++++++---------------------- 1 file changed, 20 insertions(+), 41 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 7886138a0..a1fa77f91 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -800,35 +800,6 @@ static void nr_php_instrument_datastore_operation_call( } } -/* - * Like nr_php_instrument_datastore_operation_call() but does not - * call the underlying internal function. This allows multiple - * datastore metrics to be creating during a single underlying - * PHP call. - */ -static void nr_php_instrument_datastore_operation( - nr_datastore_t datastore, - const char* operation, - nr_datastore_instance_t* instance, - bool instance_only) { - nr_segment_t* segment = NULL; - nr_segment_datastore_params_t params = { - .datastore = { - .type = datastore, - }, - .operation = nr_strdup(operation), - .instance = instance, - .instance_only = instance_only, - .callbacks = { - .backtrace = nr_php_backtrace_callback, - }, - }; - - segment = nr_segment_start(NRPRG(txn), NULL, NULL); - nr_segment_datastore_end(&segment, ¶ms); - nr_free(params.operation); -} - /* * Handle * bool mysqli_commit ( object $link [, int $flags=0, string $name] ) @@ -1570,7 +1541,9 @@ NR_INNER_WRAPPER(memcached_add_server) { zend_long port = 0; zend_long weight = 0; nr_datastore_instance_t* instance = NULL; + char* instance_metric = NULL; int zcaught = 0; + nr_segment_t* segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (SUCCESS == zend_parse_parameters_ex( @@ -1578,16 +1551,17 @@ NR_INNER_WRAPPER(memcached_add_server) { &host_len, &port, &weight) && NULL != host) { instance = nr_php_memcached_create_datastore_instance(host, port); - nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - NULL, instance, true, - INTERNAL_FUNCTION_PARAM_PASSTHRU); - } else { - zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, - INTERNAL_FUNCTION_PARAM_PASSTHRU); - if (zcaught) { - zend_bailout(); - /* NOTREACHED */ - } + instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", + instance->host, instance->port_path_or_id); + nr_free(instance); + nr_segment_add_metric(segment, instance_metric, false); + } + zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, + INTERNAL_FUNCTION_PARAM_PASSTHRU); + nr_segment_end(&segment); + if (zcaught) { + zend_bailout(); + /* NOTREACHED */ } } @@ -1595,7 +1569,9 @@ NR_INNER_WRAPPER(memcached_add_servers) { zval* servers = NULL; zval* server = NULL; nr_datastore_instance_t* instance = NULL; + char* instance_metric = NULL; int zcaught = 0; + nr_segment_t* segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (SUCCESS == zend_parse_parameters_ex( @@ -1607,8 +1583,10 @@ NR_INNER_WRAPPER(memcached_add_servers) { if (NULL != host && NULL != port && Z_TYPE_P(host) == IS_STRING && Z_TYPE_P(port) == IS_LONG) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); - nr_php_instrument_datastore_operation(NR_DATASTORE_MEMCACHE, - NULL, instance, true); + instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", + instance->host, instance->port_path_or_id); + nr_segment_add_metric(segment, instance_metric, false); + nr_free(instance); } } ZEND_HASH_FOREACH_END(); @@ -1617,6 +1595,7 @@ NR_INNER_WRAPPER(memcached_add_servers) { zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, INTERNAL_FUNCTION_PARAM_PASSTHRU); + nr_segment_end(&segment); if (zcaught) { zend_bailout(); /* NOTREACHED */ From 8c07bd8b5fb907e1fb0b4d1bbb1d146ba3249dd3 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 10:03:34 -0400 Subject: [PATCH 29/52] test comment --- tests/integration/memcached/test_add_servers.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/memcached/test_add_servers.php b/tests/integration/memcached/test_add_servers.php index 113497e06..ca71f9621 100644 --- a/tests/integration/memcached/test_add_servers.php +++ b/tests/integration/memcached/test_add_servers.php @@ -37,4 +37,5 @@ $memcached->addServers(array(array(1))); $memcached->addServers(array(array("host1"))); $memcached->addServers(array(array(1, "host1"))); +//$memcahed->addServers("string"); crashes PHP $memcached->quit(); From 5ca4df615326fd73dfb5c642cb83afff6d434481 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 10:13:59 -0400 Subject: [PATCH 30/52] remove old code --- agent/php_internal_instrument.c | 12 +++-- axiom/nr_segment_datastore.c | 66 +++++++++++----------------- axiom/nr_segment_datastore.h | 3 -- axiom/tests/test_segment_datastore.c | 30 ------------- 4 files changed, 31 insertions(+), 80 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index a1fa77f91..28e3b8a63 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -770,7 +770,6 @@ static void nr_php_instrument_datastore_operation_call( nr_datastore_t datastore, const char* operation, nr_datastore_instance_t* instance, - bool instance_only, INTERNAL_FUNCTION_PARAMETERS) { int zcaught = 0; nr_segment_t* segment = NULL; @@ -780,7 +779,6 @@ static void nr_php_instrument_datastore_operation_call( }, .operation = nr_strdup(operation), .instance = instance, - .instance_only = instance_only, .callbacks = { .backtrace = nr_php_backtrace_callback, }, @@ -836,7 +834,7 @@ NR_INNER_WRAPPER(mysqli_commit) { } nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MYSQL, - "commit", instance, false, + "commit", instance, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1531,7 +1529,7 @@ NR_INNER_WRAPPER(mysqli_stmt_prepare) { */ NR_INNER_WRAPPER(memcache_function) { nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MEMCACHE, - nr_wrapper->extra, NULL, false, + nr_wrapper->extra, NULL, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1627,7 +1625,7 @@ NR_INNER_WRAPPER(redis_connect) { } nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_REDIS, - nr_wrapper->extra, instance, false, + nr_wrapper->extra, instance, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -1686,7 +1684,7 @@ NR_INNER_WRAPPER(redis_function) { instance = nr_php_redis_retrieve_datastore_instance(this_obj TSRMLS_CC); nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_REDIS, - nr_wrapper->extra, instance, false, + nr_wrapper->extra, instance, INTERNAL_FUNCTION_PARAM_PASSTHRU); } @@ -2679,7 +2677,7 @@ NR_INNER_WRAPPER(ob_flush_common) { */ NR_INNER_WRAPPER(mongodb_execute) { nr_php_instrument_datastore_operation_call(nr_wrapper, NR_DATASTORE_MONGODB, - "execute", NULL, false, + "execute", NULL, INTERNAL_FUNCTION_PARAM_PASSTHRU); } diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index d625e46b8..0baa91a69 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -12,33 +12,6 @@ #include "util_sql.h" #include "util_logging.h" -static nr_status_t create_instance_metric(nr_segment_t* segment, - const char* product, - nr_segment_datastore_t* datastore, - const nr_datastore_instance_t* instance) { - /* - * If a datastore instance was provided, we need to add the relevant data to - * the segment and the relavant metrics. - */ - nrtxn_t* txn = segment->txn; - char* instance_metric = NULL; - if (NULL == instance || 0 == txn->options.instance_reporting_enabled) { - return NR_FAILURE; - } - - if (txn->options.database_name_reporting_enabled) { - nr_datastore_instance_set_database_name(&datastore->instance, - instance->database_name); - } - - instance_metric = nr_formatf("Datastore/instance/%s/%s/%s", product, - instance->host, instance->port_path_or_id); - nr_segment_add_metric(segment, instance_metric, false); - - nr_free(instance_metric); - return NR_SUCCESS; -} - static char* create_metrics(nr_segment_t* segment, nrtime_t duration, const char* product, @@ -51,6 +24,7 @@ static char* create_metrics(nr_segment_t* segment, char* rollup_metric = NULL; char* scoped_metric = NULL; char* statement_metric = NULL; + char* instance_metric = NULL; nrm_force_add(txn->unscoped_metrics, "Datastore/all", duration); @@ -76,12 +50,28 @@ static char* create_metrics(nr_segment_t* segment, nr_free(operation_metric); nr_free(statement_metric); - create_instance_metric(segment, product, datastore, instance); - if (NULL != instance && 0 != txn->options.instance_reporting_enabled) { - nr_datastore_instance_set_host(&datastore->instance, instance->host); - nr_datastore_instance_set_port_path_or_id(&datastore->instance, - instance->port_path_or_id); + /* + * If a datastore instance was provided, we need to add the relevant data to + * the segment and the relavant metrics. + */ + if (NULL == instance || 0 == txn->options.instance_reporting_enabled) { + return scoped_metric; + } + + if (txn->options.database_name_reporting_enabled) { + nr_datastore_instance_set_database_name(&datastore->instance, + instance->database_name); } + + instance_metric = nr_formatf("Datastore/instance/%s/%s/%s", product, + instance->host, instance->port_path_or_id); + nr_segment_add_metric(segment, instance_metric, false); + nr_datastore_instance_set_host(&datastore->instance, instance->host); + nr_datastore_instance_set_port_path_or_id(&datastore->instance, + instance->port_path_or_id); + + nr_free(instance_metric); + return scoped_metric; } @@ -218,14 +208,10 @@ bool nr_segment_datastore_end(nr_segment_t** segment_ptr, * The allWeb and allOther rollup metrics are created at the end of the * transaction since the background status may change. */ - if (!params->instance_only) { - scoped_metric - = create_metrics(segment, duration, datastore_string, collection, - operation, &datastore, params->instance); - nr_segment_set_name(segment, scoped_metric); - } else { - create_instance_metric(segment, datastore_string, &datastore, params->instance); - } + scoped_metric + = create_metrics(segment, duration, datastore_string, collection, + operation, &datastore, params->instance); + nr_segment_set_name(segment, scoped_metric); /* * Add the explain plan, if we have one. diff --git a/axiom/nr_segment_datastore.h b/axiom/nr_segment_datastore.h index e1d0d6aac..a89a2acdb 100644 --- a/axiom/nr_segment_datastore.h +++ b/axiom/nr_segment_datastore.h @@ -24,9 +24,6 @@ typedef struct _nr_segment_datastore_params_t { extracted from the SQL for SQL segments. */ nr_datastore_instance_t* instance; /* Any instance information that was collected. */ - bool instance_only; /* true if only the instance metric is wanted, - collection and operation fields will not be - used or extracted from the SQL */ /* * Datastore type fields. diff --git a/axiom/tests/test_segment_datastore.c b/axiom/tests/test_segment_datastore.c index 15fda8ed0..cb49ddebc 100644 --- a/axiom/tests/test_segment_datastore.c +++ b/axiom/tests/test_segment_datastore.c @@ -197,35 +197,6 @@ static void test_create_metrics_no_table_no_operation(void) { nr_txn_destroy(&txn); } -static void test_create_metrics_instance_only(void) { - nrtxn_t* txn = new_txn(0); - nrtime_t duration = 4 * NR_TIME_DIVISOR; - nr_segment_datastore_params_t params = sample_segment_datastore_params(); - nr_datastore_instance_t instance = {.host = "hostname", - .port_path_or_id = "123", - .database_name = "my database"}; - nr_segment_t* segment = NULL; - char* tname = "create metrics"; - - params.instance = &instance; - params.instance_only = true; - segment = nr_segment_start(txn, NULL, NULL); - segment->start_time = 1 * NR_TIME_DIVISOR; - segment->stop_time = 1 * NR_TIME_DIVISOR + duration; - - test_segment_datastore_end_and_keep(&segment, ¶ms); - - /* - * Test : Create only the instance metric - */ - test_metric_vector_size(segment->metrics, 1); - test_segment_metric_created(tname, segment->metrics, - "Datastore/instance/MongoDB/hostname/123", - false); - - nr_txn_destroy(&txn); -} - static void test_instance_info_reporting_disabled(void) { nrtxn_t* txn = new_txn(0); nrtime_t duration = 4 * NR_TIME_DIVISOR; @@ -1156,7 +1127,6 @@ tlib_parallel_info_t parallel_info = {.suggested_nthreads = 2, .state_size = 0}; void test_main(void* p NRUNUSED) { test_bad_parameters(); test_create_metrics(); - test_create_metrics_instance_only(); test_create_metrics_no_table(); test_create_metrics_no_table_no_operation(); test_instance_info_reporting_disabled(); From 3869a624f9aef43bcd997d5ba65b8b6e6f753f1c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 11:11:23 -0400 Subject: [PATCH 31/52] split tests --- .../memcached/test_add_servers.php | 16 +++-- .../memcached/test_add_servers_bad.php | 63 +++++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 tests/integration/memcached/test_add_servers_bad.php diff --git a/tests/integration/memcached/test_add_servers.php b/tests/integration/memcached/test_add_servers.php index ca71f9621..41e9b5a63 100644 --- a/tests/integration/memcached/test_add_servers.php +++ b/tests/integration/memcached/test_add_servers.php @@ -6,7 +6,7 @@ /*DESCRIPTION The agent should report instance metrics when multiple servers are -added at once +added at once via Memcached::addServers() */ /*SKIPIF @@ -17,12 +17,14 @@ */ /*EXPECT_METRICS_EXIST -Datastore/instance/Memcached/host1/1 -Datastore/instance/Memcached/host2/2 -Datastore/instance/Memcached/host3/11211 -Datastore/instance/Memcached/host4/1 +Datastore/instance/Memcached/host1/1, 1 +Datastore/instance/Memcached/host2/2, 1 +Datastore/instance/Memcached/host3/11211, 1 +Datastore/instance/Memcached/host4/1, 1 */ +/*EXPECT_ERROR_EVENTS null */ + require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); @@ -34,8 +36,4 @@ array("host3", 11211))); $memcached->addServers(array()); $memcached->addServers(array(array("host4", 1, "test field"))); -$memcached->addServers(array(array(1))); -$memcached->addServers(array(array("host1"))); -$memcached->addServers(array(array(1, "host1"))); -//$memcahed->addServers("string"); crashes PHP $memcached->quit(); diff --git a/tests/integration/memcached/test_add_servers_bad.php b/tests/integration/memcached/test_add_servers_bad.php new file mode 100644 index 000000000..4aa5da95c --- /dev/null +++ b/tests/integration/memcached/test_add_servers_bad.php @@ -0,0 +1,63 @@ + +*/ + +/*INI +*/ + +/*EXPECT_REGEX +^\s*(PHP )?Warning:\s*could not add entry\s*$ +^\s*(PHP )?Warning:\s*could not add entry\s*$ +*/ + +/*EXPECT_ERROR_EVENTS +[ + "?? agent run id", + { + "reservoir_size": "??", + "events_seen": 1 + }, + [ + [ + { + "type": "TransactionError", + "timestamp": "??", + "error.class": "E_WARNING", + "error.message": "Memcached::addServers(): could not add entry #2 to the server list", + "transactionName": "OtherTransaction\/php__FILE__", + "duration": "??", + "nr.transactionGuid": "??", + "guid": "??", + "sampled": true, + "priority": "??", + "traceId": "??", + "spanId": "??" + }, + {}, + {} + ] + ] +] +*/ + +require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); +require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); +require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); + +$memcached = new Memcached(); +$memcached->addServers(array(array(1))); +$memcached->addServers(array(array("host1"))); +$memcached->addServers(array(array(1, "host1"))); +//$memcahed->addServers("string"); crashes PHP +$memcached->quit(); From 9fae58d31885f1601d9ec122c176826fc478253d Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 11:18:51 -0400 Subject: [PATCH 32/52] unscoped metrics --- agent/php_internal_instrument.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 28e3b8a63..226c3e8e7 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1541,7 +1541,6 @@ NR_INNER_WRAPPER(memcached_add_server) { nr_datastore_instance_t* instance = NULL; char* instance_metric = NULL; int zcaught = 0; - nr_segment_t* segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (SUCCESS == zend_parse_parameters_ex( @@ -1551,12 +1550,12 @@ NR_INNER_WRAPPER(memcached_add_server) { instance = nr_php_memcached_create_datastore_instance(host, port); instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); + nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); nr_free(instance); - nr_segment_add_metric(segment, instance_metric, false); + nr_free(instance_metric); } zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, INTERNAL_FUNCTION_PARAM_PASSTHRU); - nr_segment_end(&segment); if (zcaught) { zend_bailout(); /* NOTREACHED */ @@ -1569,7 +1568,6 @@ NR_INNER_WRAPPER(memcached_add_servers) { nr_datastore_instance_t* instance = NULL; char* instance_metric = NULL; int zcaught = 0; - nr_segment_t* segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (SUCCESS == zend_parse_parameters_ex( @@ -1583,8 +1581,9 @@ NR_INNER_WRAPPER(memcached_add_servers) { instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); - nr_segment_add_metric(segment, instance_metric, false); + nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); nr_free(instance); + nr_free(instance_metric); } } ZEND_HASH_FOREACH_END(); @@ -1593,7 +1592,6 @@ NR_INNER_WRAPPER(memcached_add_servers) { zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, INTERNAL_FUNCTION_PARAM_PASSTHRU); - nr_segment_end(&segment); if (zcaught) { zend_bailout(); /* NOTREACHED */ From a78bcf4c8dcda8f6f1fb86b48cd3f6da5a592c6f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 11:22:52 -0400 Subject: [PATCH 33/52] modify socket test --- tests/integration/memcached/test_socket.php | 22 ++++----------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/integration/memcached/test_socket.php b/tests/integration/memcached/test_socket.php index 85a07a229..126f3020b 100644 --- a/tests/integration/memcached/test_socket.php +++ b/tests/integration/memcached/test_socket.php @@ -16,26 +16,12 @@ /*INI */ -/*EXPECT_METRICS -[ - "?? agent run id", - "?? start time", - "?? stop time", - [ - [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/allOther"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/__HOST__/my_socket"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"OtherTransaction/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"Supportability/Logging/Forwarding/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"Supportability/Logging/Metrics/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] - ] -] +/*EXPECT_METRICS_EXIST +Datastore/instance/Memcached/__HOST__/my_socket, 1 */ +/*EXPECT_ERROR_EVENTS null */ + require_once(realpath (dirname ( __FILE__ )) . '/../../include/helpers.php'); require_once(realpath (dirname ( __FILE__ )) . '/../../include/tap.php'); require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); From 2125e6f222962a7bc9a5936f8415ad176d5514c7 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 11:25:02 -0400 Subject: [PATCH 34/52] remove accident --- axiom/nr_segment_datastore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/axiom/nr_segment_datastore.c b/axiom/nr_segment_datastore.c index 0baa91a69..6ccc3391e 100644 --- a/axiom/nr_segment_datastore.c +++ b/axiom/nr_segment_datastore.c @@ -211,6 +211,7 @@ bool nr_segment_datastore_end(nr_segment_t** segment_ptr, scoped_metric = create_metrics(segment, duration, datastore_string, collection, operation, &datastore, params->instance); + nr_segment_set_name(segment, scoped_metric); /* From 368a2baaf46aab9612481f75d018bd5e55a3ccf1 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 11:43:04 -0400 Subject: [PATCH 35/52] remove ZTS block --- agent/tests/test_memcached.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c index 72b6e909f..635503cee 100644 --- a/agent/tests/test_memcached.c +++ b/agent/tests/test_memcached.c @@ -92,10 +92,6 @@ static void test_create_datastore_instance(void) { } void test_main(void* p NRUNUSED) { -#if defined(ZTS) && !defined(PHP7) - void*** tsrm_ls = NULL; -#endif /* ZTS && !PHP7 */ - system_host_name = nr_system_get_hostname(); test_create_datastore_instance(); From 06cf1658254fe1ef08b77ca6feaf0509a58c4a7c Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 27 Sep 2024 12:32:53 -0400 Subject: [PATCH 36/52] fix regex --- tests/integration/memcached/test_add_servers_bad.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integration/memcached/test_add_servers_bad.php b/tests/integration/memcached/test_add_servers_bad.php index 4aa5da95c..79760b09a 100644 --- a/tests/integration/memcached/test_add_servers_bad.php +++ b/tests/integration/memcached/test_add_servers_bad.php @@ -17,8 +17,11 @@ */ /*EXPECT_REGEX -^\s*(PHP )?Warning:\s*could not add entry\s*$ -^\s*(PHP )?Warning:\s*could not add entry\s*$ + +.*(PHP )?Warning:.*could not add entry.* + +.*(PHP )?Warning:.*could not add entry.* + */ /*EXPECT_ERROR_EVENTS From 18fff7abc952385e4eea2925f631906fc7463e3b Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 1 Oct 2024 11:08:29 -0400 Subject: [PATCH 37/52] free better --- agent/php_internal_instrument.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 226c3e8e7..687cd036f 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1551,7 +1551,7 @@ NR_INNER_WRAPPER(memcached_add_server) { instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_free(instance); + nr_php_datastore_instance_destroy(instance); nr_free(instance_metric); } zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, @@ -1582,7 +1582,7 @@ NR_INNER_WRAPPER(memcached_add_servers) { instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_free(instance); + nr_php_datastore_instance_destroy(instance); nr_free(instance_metric); } } From c40cbd42f74ca295924f55fbd6a9deab996b2085 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 1 Oct 2024 11:20:52 -0400 Subject: [PATCH 38/52] fix function name --- agent/php_internal_instrument.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index 687cd036f..e3dd48723 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1551,7 +1551,7 @@ NR_INNER_WRAPPER(memcached_add_server) { instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_php_datastore_instance_destroy(instance); + nr_datastore_instance_destroy(&instance); nr_free(instance_metric); } zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, @@ -1582,7 +1582,7 @@ NR_INNER_WRAPPER(memcached_add_servers) { instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", instance->host, instance->port_path_or_id); nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_php_datastore_instance_destroy(instance); + nr_datastore_instance_destroy(&instance); nr_free(instance_metric); } } From 68843dc3a68563639bc8663ab2346429651b9988 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:23:33 -0400 Subject: [PATCH 39/52] update makefile --- agent/Makefile.frag | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/Makefile.frag b/agent/Makefile.frag index 648fc84b3..fbff46d69 100644 --- a/agent/Makefile.frag +++ b/agent/Makefile.frag @@ -93,6 +93,7 @@ TEST_BINARIES = \ tests/test_internal_instrument \ tests/test_hash \ tests/test_lib_aws_sdk_php \ + tests/test_memcached \ tests/test_mongodb \ tests/test_monolog \ tests/test_mysql \ From 63beced5d7654c7ee11853ee477a66b8ea81cfc9 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:24:30 -0400 Subject: [PATCH 40/52] PR comments --- agent/php_internal_instrument.c | 22 ++++------------------ agent/php_memcached.c | 11 +++++++++++ agent/php_memcached.h | 13 +++++++++++++ 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index e3dd48723..cf053acd1 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1538,8 +1538,6 @@ NR_INNER_WRAPPER(memcached_add_server) { nr_string_len_t host_len = 0; zend_long port = 0; zend_long weight = 0; - nr_datastore_instance_t* instance = NULL; - char* instance_metric = NULL; int zcaught = 0; if (SUCCESS @@ -1547,12 +1545,7 @@ NR_INNER_WRAPPER(memcached_add_server) { ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "s|ll", &host, &host_len, &port, &weight) && NULL != host) { - instance = nr_php_memcached_create_datastore_instance(host, port); - instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", - instance->host, instance->port_path_or_id); - nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_datastore_instance_destroy(&instance); - nr_free(instance_metric); + nr_php_memcached_create_instance_metric(host, port); } zcaught = nr_zend_call_old_handler(nr_wrapper->oldhandler, INTERNAL_FUNCTION_PARAM_PASSTHRU); @@ -1565,8 +1558,6 @@ NR_INNER_WRAPPER(memcached_add_server) { NR_INNER_WRAPPER(memcached_add_servers) { zval* servers = NULL; zval* server = NULL; - nr_datastore_instance_t* instance = NULL; - char* instance_metric = NULL; int zcaught = 0; if (SUCCESS @@ -1576,14 +1567,9 @@ NR_INNER_WRAPPER(memcached_add_servers) { ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); - if (NULL != host && NULL != port && - Z_TYPE_P(host) == IS_STRING && Z_TYPE_P(port) == IS_LONG) { - instance = nr_php_memcached_create_datastore_instance(Z_STRVAL_P(host), Z_LVAL_P(port)); - instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", - instance->host, instance->port_path_or_id); - nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); - nr_datastore_instance_destroy(&instance); - nr_free(instance_metric); + if (nr_php_is_zval_valid_string(host) && + nr_php_is_zval_valid_integer(port)) { + nr_php_memcached_create_instance_metric(Z_STRVAL_P(host), Z_LVAL_P(port)); } } ZEND_HASH_FOREACH_END(); diff --git a/agent/php_memcached.c b/agent/php_memcached.c index 3fc0b0bdb..ee13e1e73 100644 --- a/agent/php_memcached.c +++ b/agent/php_memcached.c @@ -21,3 +21,14 @@ nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( return instance; } +void nr_php_memcached_create_instance_metric( + const char* host_or_socket, + zend_long port) { + nr_datastore_instance_t* instance + = nr_php_memcached_create_datastore_instance(host_or_socket, port); + char* instance_metric = nr_formatf("Datastore/instance/Memcached/%s/%s", + instance->host, instance->port_path_or_id); + nrm_force_add(NRPRG(txn)->unscoped_metrics, instance_metric, 0); + nr_datastore_instance_destroy(&instance); + nr_free(instance_metric); +} diff --git a/agent/php_memcached.h b/agent/php_memcached.h index f0eedcb2d..0018735e9 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -21,4 +21,17 @@ extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( const char* host_or_socket, zend_long port); +/* + * Purpose : Create a memcached instance metric + * + * Params : 1. The memcached host or socket name as given to Memcached::addServer(). + * Must be non-null. + * 2. The memcached port as given as given to Memcached::addServer(). + * Must be non-null. + */ +extern void nr_php_memcached_create_instance_metric( + const char* host_or_socket, + zend_long port); + + #endif From 5597d7fccf8657254c4c9d486612ef821b2754be Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:25:12 -0400 Subject: [PATCH 41/52] concise unit tests --- agent/tests/test_memcached.c | 38 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c index 635503cee..47b82241f 100644 --- a/agent/tests/test_memcached.c +++ b/agent/tests/test_memcached.c @@ -38,57 +38,39 @@ static void test_create_datastore_instance(void) { assert_datastore_instance_equals_destroy( "empty host", - &((nr_datastore_instance_t){ - .host = "unknown", - .database_name = "unknown", - .port_path_or_id = "6379", - }), - nr_php_memcached_create_datastore_instance("", 6379)); - - assert_datastore_instance_equals_destroy( - "localhost socket", - &((nr_datastore_instance_t){ - .host = system_host_name, - .database_name = "unknown", - .port_path_or_id = "localhost", - }), - nr_php_memcached_create_datastore_instance("localhost", 0)); - - assert_datastore_instance_equals_destroy( - "localhost and port", &((nr_datastore_instance_t){ .host = system_host_name, .database_name = "unknown", - .port_path_or_id = "6379", + .port_path_or_id = "unknown", }), - nr_php_memcached_create_datastore_instance("localhost", 6379)); + nr_php_memcached_create_datastore_instance(NULL, 0)); assert_datastore_instance_equals_destroy( "host.name socket", &((nr_datastore_instance_t){ - .host = system_host_name, + .host = "host.name", .database_name = "unknown", - .port_path_or_id = "host.name", + .port_path_or_id = "11211", }), - nr_php_memcached_create_datastore_instance("host.name", 0)); + nr_php_memcached_create_datastore_instance("host.name", 11211)); assert_datastore_instance_equals_destroy( "host and port", &((nr_datastore_instance_t){ - .host = "host.name", + .host = "unknown", .database_name = "unknown", .port_path_or_id = "6379", }), - nr_php_memcached_create_datastore_instance("host.name", 6379)); + nr_php_memcached_create_datastore_instance("", 6379)); assert_datastore_instance_equals_destroy( "NULL socket", &((nr_datastore_instance_t){ - .host = system_host_name, + .host = "unknown", .database_name = "unknown", - .port_path_or_id = "unknown", + .port_path_or_id = "11211", }), - nr_php_memcached_create_datastore_instance(NULL, 0)); + nr_php_memcached_create_datastore_instance(NULL, 11211)); } void test_main(void* p NRUNUSED) { From 7cbf7f4617e6396cdf98974d38ba37d274b9a057 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:25:33 -0400 Subject: [PATCH 42/52] more integration tests --- tests/integration/memcached/test_add_servers_bad.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/memcached/test_add_servers_bad.php b/tests/integration/memcached/test_add_servers_bad.php index 79760b09a..c4887584f 100644 --- a/tests/integration/memcached/test_add_servers_bad.php +++ b/tests/integration/memcached/test_add_servers_bad.php @@ -59,6 +59,8 @@ require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); $memcached = new Memcached(); +$memcached->addServer(5, 5); +//$memcached->addServer("host", string); crashes PHP $memcached->addServers(array(array(1))); $memcached->addServers(array(array("host1"))); $memcached->addServers(array(array(1, "host1"))); From 65424c61252d2e56d9a6d6a6f2dbf979594529b1 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:26:24 -0400 Subject: [PATCH 43/52] environment variables in expecteds --- tests/integration/memcached/test_basic.php | 2 +- tests/integration/memcached/test_basic_logging_off.php | 2 +- tests/integration/memcached/test_by_key.php | 2 +- tests/integration/memcached/test_cas.php7.php | 2 +- tests/integration/memcached/test_cas_by_key.php7.php | 2 +- tests/integration/memcached/test_concat.php | 2 +- tests/integration/memcached/test_concat_by_key.php | 2 +- tests/integration/memcached/test_concat_by_key_logging_off.php | 2 +- tests/integration/memcached/test_multi.php | 2 +- tests/integration/memcached/test_multi_by_key.php | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integration/memcached/test_basic.php b/tests/integration/memcached/test_basic.php index c8f601356..ae962d4e5 100644 --- a/tests/integration/memcached/test_basic.php +++ b/tests/integration/memcached/test_basic.php @@ -46,7 +46,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_basic_logging_off.php b/tests/integration/memcached/test_basic_logging_off.php index ba2d7ac37..eb5f01a77 100644 --- a/tests/integration/memcached/test_basic_logging_off.php +++ b/tests/integration/memcached/test_basic_logging_off.php @@ -49,7 +49,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_by_key.php b/tests/integration/memcached/test_by_key.php index ebb8c90b7..19467b772 100644 --- a/tests/integration/memcached/test_by_key.php +++ b/tests/integration/memcached/test_by_key.php @@ -48,7 +48,7 @@ [{"name":"Datastore/allOther"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [11, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas.php7.php b/tests/integration/memcached/test_cas.php7.php index 8ed54241f..91b7f1316 100644 --- a/tests/integration/memcached/test_cas.php7.php +++ b/tests/integration/memcached/test_cas.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas_by_key.php7.php b/tests/integration/memcached/test_cas_by_key.php7.php index 31b39ce51..e7de5dd4c 100644 --- a/tests/integration/memcached/test_cas_by_key.php7.php +++ b/tests/integration/memcached/test_cas_by_key.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat.php b/tests/integration/memcached/test_concat.php index eb2791d22..b8d2183c0 100644 --- a/tests/integration/memcached/test_concat.php +++ b/tests/integration/memcached/test_concat.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key.php b/tests/integration/memcached/test_concat_by_key.php index ce5776310..7e8d838fa 100644 --- a/tests/integration/memcached/test_concat_by_key.php +++ b/tests/integration/memcached/test_concat_by_key.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key_logging_off.php b/tests/integration/memcached/test_concat_by_key_logging_off.php index 520d898fb..3e601cd03 100644 --- a/tests/integration/memcached/test_concat_by_key_logging_off.php +++ b/tests/integration/memcached/test_concat_by_key_logging_off.php @@ -38,7 +38,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi.php b/tests/integration/memcached/test_multi.php index b4e60f0d7..1ad1c2214 100644 --- a/tests/integration/memcached/test_multi.php +++ b/tests/integration/memcached/test_multi.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi_by_key.php b/tests/integration/memcached/test_multi_by_key.php index b993906c8..eba20d14e 100644 --- a/tests/integration/memcached/test_multi_by_key.php +++ b/tests/integration/memcached/test_multi_by_key.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/memcached/11211"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], From 8ec02684f0bb8726ae111d78d37e0dd492d662c5 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:26:38 -0400 Subject: [PATCH 44/52] socket path --- tests/integration/memcached/test_socket.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/memcached/test_socket.php b/tests/integration/memcached/test_socket.php index 126f3020b..5e1cd9b34 100644 --- a/tests/integration/memcached/test_socket.php +++ b/tests/integration/memcached/test_socket.php @@ -17,7 +17,7 @@ */ /*EXPECT_METRICS_EXIST -Datastore/instance/Memcached/__HOST__/my_socket, 1 +Datastore/instance/Memcached/__HOST__/my/socket, 1 */ /*EXPECT_ERROR_EVENTS null */ @@ -27,5 +27,5 @@ require_once(realpath (dirname ( __FILE__ )) . '/memcache.inc'); $memcached = new Memcached(); -$memcached->addServer("my_socket", 0); +$memcached->addServer("my/socket", 0); $memcached->quit(); From a40b011b3ea12d4450dc4916f1a3ac1116cc892f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 10:36:14 -0400 Subject: [PATCH 45/52] remove extern --- agent/php_memcached.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_memcached.h b/agent/php_memcached.h index 0018735e9..a2c9d2344 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -17,7 +17,7 @@ * * Returns: nr_datastore_instance_t* that the caller is responsible for freeing */ -extern nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( +nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( const char* host_or_socket, zend_long port); From 0453459f80417ff035c1cab09961c6b8e2be88d3 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 11:10:12 -0400 Subject: [PATCH 46/52] revert port --- tests/integration/memcached/test_basic.php | 2 +- tests/integration/memcached/test_basic_logging_off.php | 2 +- tests/integration/memcached/test_by_key.php | 2 +- tests/integration/memcached/test_cas.php7.php | 2 +- tests/integration/memcached/test_cas_by_key.php7.php | 2 +- tests/integration/memcached/test_concat.php | 2 +- tests/integration/memcached/test_concat_by_key.php | 2 +- tests/integration/memcached/test_concat_by_key_logging_off.php | 2 +- tests/integration/memcached/test_multi.php | 2 +- tests/integration/memcached/test_multi_by_key.php | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integration/memcached/test_basic.php b/tests/integration/memcached/test_basic.php index ae962d4e5..1e24bfd7e 100644 --- a/tests/integration/memcached/test_basic.php +++ b/tests/integration/memcached/test_basic.php @@ -46,7 +46,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_basic_logging_off.php b/tests/integration/memcached/test_basic_logging_off.php index eb5f01a77..05e87a4c1 100644 --- a/tests/integration/memcached/test_basic_logging_off.php +++ b/tests/integration/memcached/test_basic_logging_off.php @@ -49,7 +49,7 @@ [{"name":"Datastore/allOther"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [15, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [15, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_by_key.php b/tests/integration/memcached/test_by_key.php index 19467b772..ceb2758d8 100644 --- a/tests/integration/memcached/test_by_key.php +++ b/tests/integration/memcached/test_by_key.php @@ -48,7 +48,7 @@ [{"name":"Datastore/allOther"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [11, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [11, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/add", "scope":"OtherTransaction/php__FILE__"}, [2, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas.php7.php b/tests/integration/memcached/test_cas.php7.php index 91b7f1316..650bb22ec 100644 --- a/tests/integration/memcached/test_cas.php7.php +++ b/tests/integration/memcached/test_cas.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_cas_by_key.php7.php b/tests/integration/memcached/test_cas_by_key.php7.php index e7de5dd4c..10b073016 100644 --- a/tests/integration/memcached/test_cas_by_key.php7.php +++ b/tests/integration/memcached/test_cas_by_key.php7.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat.php b/tests/integration/memcached/test_concat.php index b8d2183c0..0a526adcd 100644 --- a/tests/integration/memcached/test_concat.php +++ b/tests/integration/memcached/test_concat.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key.php b/tests/integration/memcached/test_concat_by_key.php index 7e8d838fa..2cb041974 100644 --- a/tests/integration/memcached/test_concat_by_key.php +++ b/tests/integration/memcached/test_concat_by_key.php @@ -35,7 +35,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_concat_by_key_logging_off.php b/tests/integration/memcached/test_concat_by_key_logging_off.php index 3e601cd03..4d644a74d 100644 --- a/tests/integration/memcached/test_concat_by_key_logging_off.php +++ b/tests/integration/memcached/test_concat_by_key_logging_off.php @@ -38,7 +38,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/delete", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi.php b/tests/integration/memcached/test_multi.php index 1ad1c2214..78dbd2fa3 100644 --- a/tests/integration/memcached/test_multi.php +++ b/tests/integration/memcached/test_multi.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], diff --git a/tests/integration/memcached/test_multi_by_key.php b/tests/integration/memcached/test_multi_by_key.php index eba20d14e..5d4c2e9d3 100644 --- a/tests/integration/memcached/test_multi_by_key.php +++ b/tests/integration/memcached/test_multi_by_key.php @@ -42,7 +42,7 @@ [{"name":"Datastore/allOther"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/all"}, [5, "??", "??", "??", "??", "??"]], [{"name":"Datastore/Memcached/allOther"}, [5, "??", "??", "??", "??", "??"]], - [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/ENV[MEMCACHE_PORT]"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Datastore/instance/Memcached/ENV[MEMCACHE_HOST]/11211"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get"}, [4, "??", "??", "??", "??", "??"]], [{"name":"Datastore/operation/Memcached/get", "scope":"OtherTransaction/php__FILE__"}, [4, "??", "??", "??", "??", "??"]], From 62b888920b6a143f6c3d6dad923ad1fa61951af8 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 11:14:09 -0400 Subject: [PATCH 47/52] remove comment --- agent/tests/test_memcached.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c index 47b82241f..e108049ae 100644 --- a/agent/tests/test_memcached.c +++ b/agent/tests/test_memcached.c @@ -15,9 +15,6 @@ tlib_parallel_info_t parallel_info static char* system_host_name; static void test_create_datastore_instance(void) { - /* - * Test : Normal operation. - */ assert_datastore_instance_equals_destroy( "named socket", &((nr_datastore_instance_t){ From 1b227d32f7e7cd82f86796fde7cc02c7c1779e5f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 11:15:42 -0400 Subject: [PATCH 48/52] reword --- agent/php_memcached.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_memcached.h b/agent/php_memcached.h index a2c9d2344..78923693f 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -10,7 +10,7 @@ #include "php_includes.h" /* - * Purpose : Create a datastore instance metadata for a Memcached connection. + * Purpose : Create a datastore instance metadata for a Memcached server. * * Params : 1. The memcached host or socket name as given to Memcached::addServer(). * 2. The memcached port as given as given to Memcached::addServer(). From 90f1151fcd61963ffb9bf9ddd57d1caf027d6fed Mon Sep 17 00:00:00 2001 From: ZNeumann Date: Thu, 3 Oct 2024 09:17:56 -0600 Subject: [PATCH 49/52] Update agent/php_internal_instrument.c Co-authored-by: Michal Nowacki --- agent/php_internal_instrument.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/php_internal_instrument.c b/agent/php_internal_instrument.c index cf053acd1..ce6c089ec 100644 --- a/agent/php_internal_instrument.c +++ b/agent/php_internal_instrument.c @@ -1563,7 +1563,7 @@ NR_INNER_WRAPPER(memcached_add_servers) { if (SUCCESS == zend_parse_parameters_ex( ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "a", &servers)) { - if (Z_TYPE_P(servers) == IS_ARRAY) { + if (NULL != servers && Z_TYPE_P(servers) == IS_ARRAY) { ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(servers), server) { zval* host = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 0); zval* port = nr_php_zend_hash_index_find(Z_ARRVAL_P(server), 1); From 3364eb0b8bbf0e984add9030588f82ff3475ed05 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 14:21:25 -0400 Subject: [PATCH 50/52] fix comment --- tests/integration/memcached/test_socket.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/memcached/test_socket.php b/tests/integration/memcached/test_socket.php index 5e1cd9b34..19a1787e9 100644 --- a/tests/integration/memcached/test_socket.php +++ b/tests/integration/memcached/test_socket.php @@ -5,8 +5,7 @@ */ /*DESCRIPTION -The agent should report instance metrics when multiple servers are -added at once +The agent should report instance metrics for sockets when port is 0 */ /*SKIPIF From c8bdfb78b275c8e6ff9faff01a709d1c5853bc00 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 14:54:20 -0400 Subject: [PATCH 51/52] remove incorrect comment --- agent/php_memcached.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/agent/php_memcached.h b/agent/php_memcached.h index 78923693f..f449030ea 100644 --- a/agent/php_memcached.h +++ b/agent/php_memcached.h @@ -25,9 +25,7 @@ nr_datastore_instance_t* nr_php_memcached_create_datastore_instance( * Purpose : Create a memcached instance metric * * Params : 1. The memcached host or socket name as given to Memcached::addServer(). - * Must be non-null. * 2. The memcached port as given as given to Memcached::addServer(). - * Must be non-null. */ extern void nr_php_memcached_create_instance_metric( const char* host_or_socket, From 6e720c9b372aa04176e4e3aaa031681157f21680 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 3 Oct 2024 14:54:36 -0400 Subject: [PATCH 52/52] more tests --- agent/tests/test_memcached.c | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/agent/tests/test_memcached.c b/agent/tests/test_memcached.c index e108049ae..1f05c2443 100644 --- a/agent/tests/test_memcached.c +++ b/agent/tests/test_memcached.c @@ -70,10 +70,58 @@ static void test_create_datastore_instance(void) { nr_php_memcached_create_datastore_instance(NULL, 11211)); } +static void test_create_instance_metric(void) { + nrtxn_t* txn; + nrmetric_t* metric; + char* metric_str; + tlib_php_engine_create(""); + tlib_php_request_start(); + txn = NRPRG(txn); + + nr_php_memcached_create_instance_metric("host", 11211); + metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/host/11211"); + tlib_pass_if_not_null("metric found", metric); + + nr_php_memcached_create_instance_metric("", 11211); + metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/unknown/11211"); + tlib_pass_if_not_null("metric found", metric); + + nr_php_memcached_create_instance_metric(NULL, 7); + metric = nrm_find(txn->unscoped_metrics, "Datastore/instance/Memcached/unknown/7"); + tlib_pass_if_not_null("metric found", metric); + + nr_php_memcached_create_instance_metric("path/to/sock", 0); + metric_str = nr_formatf("Datastore/instance/Memcached/%s/path/to/sock", system_host_name); + metric = nrm_find(txn->unscoped_metrics, metric_str); + nr_free(metric_str); + tlib_pass_if_not_null("metric found", metric); + + nr_php_memcached_create_instance_metric("", 0); + metric_str = nr_formatf("Datastore/instance/Memcached/%s/unknown", system_host_name); + metric = nrm_find(txn->unscoped_metrics, metric_str); + nr_free(metric_str); + tlib_pass_if_not_null("metric found", metric); + + // restart the transaction because the next metric is the same as a previous metric + tlib_php_request_end(); + tlib_php_request_start(); + txn = NRPRG(txn); + + nr_php_memcached_create_instance_metric(NULL, 0); + metric_str = nr_formatf("Datastore/instance/Memcached/%s/unknown", system_host_name); + metric = nrm_find(txn->unscoped_metrics, metric_str); + nr_free(metric_str); + tlib_pass_if_not_null("metric found", metric); + + tlib_php_request_end(); + tlib_php_engine_destroy(); +} + void test_main(void* p NRUNUSED) { system_host_name = nr_system_get_hostname(); test_create_datastore_instance(); + test_create_instance_metric(); nr_free(system_host_name); }