From 6b914aa3d2e23c9d73345bc3dc2102c9edc73424 Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 12:26:13 -0400 Subject: [PATCH 01/11] Cache composer api packages so these are considered each requst --- agent/lib_composer.c | 17 ++++++++++++++ agent/php_globals.c | 1 + agent/php_globals.h | 2 ++ agent/php_txn.c | 11 +++++++++ axiom/nr_php_packages.c | 50 +++++++++++++++++++++++++++++++++++++++++ axiom/nr_php_packages.h | 8 +++++++ 6 files changed, 89 insertions(+) diff --git a/agent/lib_composer.c b/agent/lib_composer.c index 674bff3c3..eeb363b57 100644 --- a/agent/lib_composer.c +++ b/agent/lib_composer.c @@ -162,6 +162,23 @@ static void nr_execute_handle_autoload_composer_get_packages_information( } } ZEND_HASH_FOREACH_END(); + + /* cache Composer packages if only running once per process */ + if (NR_PHP_PROCESS_GLOBALS(composer_api_per_process_detection)) { + nr_php_packages_destroy(&NR_PHP_PROCESS_GLOBALS(composer_php_packages)); + NR_PHP_PROCESS_GLOBALS(composer_php_packages) + = nr_php_packages_clone(NRPRG(txn)->php_packages); + if (NULL == NR_PHP_PROCESS_GLOBALS(composer_php_packages)) { + nrl_verbosedebug(NRL_INSTRUMENT, + "%s - unable to clone composer packages", __func__); + } else { + nrl_verbosedebug(NRL_INSTRUMENT, "%s - cloned %zu composer packages", + __func__, + nr_php_packages_count( + NR_PHP_PROCESS_GLOBALS(composer_php_packages))); + } + } + } else { char strbuf[80]; nr_format_zval_for_debug(&retval, strbuf, 0, sizeof(strbuf) - 1, 0); diff --git a/agent/php_globals.c b/agent/php_globals.c index 5bb372d33..d5cfe7ab8 100644 --- a/agent/php_globals.c +++ b/agent/php_globals.c @@ -42,6 +42,7 @@ static void nr_php_per_process_globals_dispose(void) { nr_free(nr_php_per_process_globals.env_labels); nr_free(nr_php_per_process_globals.apache_add); nr_free(nr_php_per_process_globals.docker_id); + nr_php_packages_destroy(&nr_php_per_process_globals.composer_php_packages); nr_memset(&nr_php_per_process_globals, 0, sizeof(nr_php_per_process_globals)); } diff --git a/agent/php_globals.h b/agent/php_globals.h index 4d0a2fe24..8ebecc8de 100644 --- a/agent/php_globals.h +++ b/agent/php_globals.h @@ -82,6 +82,8 @@ typedef struct _nrphpglobals_t { detection has run. Used in conjunction with composer_api_per_process_detection. */ char* docker_id; /* 64 byte hex docker ID parsed from /proc/self/mountinfo */ + nr_php_packages_t* composer_php_packages; /* Cache of PHP packages detected in + the current process. */ /* Original PHP callback pointer contents */ nrphperrfn_t orig_error_cb; diff --git a/agent/php_txn.c b/agent/php_txn.c index 4b92f40c6..77698b402 100644 --- a/agent/php_txn.c +++ b/agent/php_txn.c @@ -1151,6 +1151,17 @@ nr_status_t nr_php_txn_begin(const char* appnames, } } + /* preload cached composer packages if present */ + if (NR_PHP_PROCESS_GLOBALS(composer_packages_detected) + && (NULL != NR_PHP_PROCESS_GLOBALS(composer_php_packages))) { + nrl_verbosedebug(NRL_FRAMEWORK, + "composer packages already detected, using cached values"); + nr_php_packages_destroy(&NRPRG(txn)->php_packages); + NRPRG(txn)->php_packages + = nr_php_packages_clone(NR_PHP_PROCESS_GLOBALS(composer_php_packages)); + nrl_verbosedebug(NRL_FRAMEWORK, "composer packages cloned from cache"); + } + return NR_SUCCESS; } diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index b8a034086..8ccc33ca2 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -92,6 +92,56 @@ nr_php_packages_t* nr_php_packages_create() { } return h; } +static void clone_callback(void* value, + const char* key, + size_t key_len, + void* userdata) { + nr_php_packages_t* dest = NULL; + nr_php_package_t* orig_pkg = NULL; + nr_php_package_t* new_pkg = NULL; + + (void)key_len; // Unused parameter + (void)key; // Unused parameter + + if (NULL == value || NULL == userdata) { + return; + } + + if (NULL == ((nr_php_packages_t*)userdata)->data) { + return; + } + + /* Clone the package and add it to the destination hashmap */ + // userdata is a pointer to nr_php_packages_t + // value is a pointer to nr_php_package_t + // we will clone the package and add it to the destination hashmap + dest = (nr_php_packages_t*)userdata; + orig_pkg = (nr_php_package_t*)value; + if (NULL == orig_pkg) { + return; + } + new_pkg = nr_php_package_create_with_source(orig_pkg->package_name, + orig_pkg->package_version, + orig_pkg->source_priority); + nr_php_packages_add_package(dest, new_pkg); +} + +nr_php_packages_t* nr_php_packages_clone(nr_php_packages_t* pkgs) { + nr_php_packages_t* h = NULL; + + if (NULL == pkgs) { + return NULL; + } + + h = nr_php_packages_create(); + if (NULL == h) { + return NULL; + } + + nr_hashmap_apply(pkgs->data, clone_callback, h); + + return h; +} nr_php_package_t* nr_php_packages_add_package(nr_php_packages_t* h, nr_php_package_t* p) { diff --git a/axiom/nr_php_packages.h b/axiom/nr_php_packages.h index f4945c43e..9b1580124 100644 --- a/axiom/nr_php_packages.h +++ b/axiom/nr_php_packages.h @@ -70,6 +70,14 @@ extern nr_php_package_t* nr_php_package_create_with_source( extern nr_php_package_t* nr_php_package_create(const char* name, const char* version); +/* Purpose : Clone a collection of php packages + * + * Params : 1. A pointer to nr_php_packages_t + * + * Returns : A new nr_php_packages_t that is a copy of the original + */ +nr_php_packages_t* nr_php_packages_clone(nr_php_packages_t* pkgs); + /* * Purpose : Destroy/free php package * From cde82132d12ab665c182cd3bf70a62781753677e Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:17:11 -0400 Subject: [PATCH 02/11] Instead of copying cache just reference it as a const data source --- agent/php_txn.c | 5 +++++ axiom/nr_php_packages.c | 23 +++++++++++++++++++++++ axiom/nr_php_packages.h | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/agent/php_txn.c b/agent/php_txn.c index 77698b402..86757339c 100644 --- a/agent/php_txn.c +++ b/agent/php_txn.c @@ -1156,10 +1156,15 @@ nr_status_t nr_php_txn_begin(const char* appnames, && (NULL != NR_PHP_PROCESS_GLOBALS(composer_php_packages))) { nrl_verbosedebug(NRL_FRAMEWORK, "composer packages already detected, using cached values"); +#if 0 nr_php_packages_destroy(&NRPRG(txn)->php_packages); NRPRG(txn)->php_packages = nr_php_packages_clone(NR_PHP_PROCESS_GLOBALS(composer_php_packages)); nrl_verbosedebug(NRL_FRAMEWORK, "composer packages cloned from cache"); +#else + nr_php_packages_set_known(NRPRG(txn)->php_packages, + NR_PHP_PROCESS_GLOBALS(composer_php_packages)); +#endif } return NR_SUCCESS; diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index 8ccc33ca2..760dc1cf7 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -36,6 +36,15 @@ static inline const char* nr_php_package_source_priority_to_string(const nr_php_ } } +void nr_php_packages_set_known(nr_php_packages_t* pkgs, + const nr_hashmap_t* known) { + if (NULL == pkgs || NULL == known) { + return; + } + + pkgs->known_packages = known; +} + nr_php_package_t* nr_php_package_create_with_source( const char* name, const char* version, @@ -154,6 +163,20 @@ nr_php_package_t* nr_php_packages_add_package(nr_php_packages_t* h, return NULL; } + // if package is in known packages then ignore + + package = (nr_php_package_t*)nr_hashmap_get( + h->known_packages, p->package_name, nr_strlen(p->package_name)); + if (NULL != package) { + if (package->source_priority <= p->source_priority + && 0 != nr_strcmp(package->package_version, p->package_version)) { + nr_free(package->package_version); + package->package_version = nr_strdup(p->package_version); + } + nr_php_package_destroy(p); + return NULL; + } + // If package with the same key already exists, we will check if the value is // different. If it is different, then we will update the value of the package package = (nr_php_package_t*)nr_hashmap_get(h->data, p->package_name, diff --git a/axiom/nr_php_packages.h b/axiom/nr_php_packages.h index 9b1580124..aab4d3256 100644 --- a/axiom/nr_php_packages.h +++ b/axiom/nr_php_packages.h @@ -28,6 +28,7 @@ typedef struct _nr_php_package_t { typedef struct _nr_php_packages_t { nr_hashmap_t* data; + const nr_hashmap_t* known_packages; } nr_php_packages_t; typedef void(nr_php_packages_iter_t)(void* value, @@ -35,6 +36,9 @@ typedef void(nr_php_packages_iter_t)(void* value, size_t name_len, void* user_data); +void nr_php_packages_set_known(nr_php_packages_t* pkgs, + const nr_hashmap_t* known); + /* * Purpose : Create a new php package with desired source priority. If the name is null, then no package will * be created. If the version is null (version = NULL), then From 86142960f6f03c1c51e5411d80f1468e773d3866 Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:26:09 -0400 Subject: [PATCH 03/11] change temp to test - need to figure out how to make it const --- axiom/nr_php_packages.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/axiom/nr_php_packages.h b/axiom/nr_php_packages.h index aab4d3256..50fe79e63 100644 --- a/axiom/nr_php_packages.h +++ b/axiom/nr_php_packages.h @@ -28,7 +28,8 @@ typedef struct _nr_php_package_t { typedef struct _nr_php_packages_t { nr_hashmap_t* data; - const nr_hashmap_t* known_packages; + nr_hashmap_t* known_packages; + /* SHOULD BE CONST !!!*/ } nr_php_packages_t; typedef void(nr_php_packages_iter_t)(void* value, From b5b36d2aa961f585fe0b46db98b2faf0d2ab7577 Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:26:48 -0400 Subject: [PATCH 04/11] change temp to test - need to figure out how to make it const --- axiom/nr_php_packages.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index 760dc1cf7..3bc4ace15 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -37,7 +37,7 @@ static inline const char* nr_php_package_source_priority_to_string(const nr_php_ } void nr_php_packages_set_known(nr_php_packages_t* pkgs, - const nr_hashmap_t* known) { + nr_hashmap_t* known) { /* should be CONST */ if (NULL == pkgs || NULL == known) { return; } From d9dc497fd336d9af5ab16947adb68d6186e56d9d Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:28:28 -0400 Subject: [PATCH 05/11] change temp to test - need to figure out how to make it const --- axiom/nr_php_packages.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/axiom/nr_php_packages.h b/axiom/nr_php_packages.h index 50fe79e63..e8edb37b0 100644 --- a/axiom/nr_php_packages.h +++ b/axiom/nr_php_packages.h @@ -38,7 +38,7 @@ typedef void(nr_php_packages_iter_t)(void* value, void* user_data); void nr_php_packages_set_known(nr_php_packages_t* pkgs, - const nr_hashmap_t* known); + nr_hashmap_t* known); /* should be CONST! */ /* * Purpose : Create a new php package with desired source priority. If the name is null, then no package will From 650083a07becd5b6431f7afaaa31957b586802ca Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:35:55 -0400 Subject: [PATCH 06/11] debugging msgs --- axiom/nr_php_packages.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index 3bc4ace15..2e99c4bd2 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -42,6 +42,8 @@ void nr_php_packages_set_known(nr_php_packages_t* pkgs, return; } + nrl_verbosedebug(NRL_INSTRUMENT, "Setting known PHP packages to %p", known); + pkgs->known_packages = known; } @@ -165,6 +167,14 @@ nr_php_package_t* nr_php_packages_add_package(nr_php_packages_t* h, // if package is in known packages then ignore + if (h->known_packages) { + nrl_verbosedebug(NRL_INSTRUMENT, "add package -> known PHP packages = %p", + h->known_packages); + } else { + nrl_verbosedebug(NRL_INSTRUMENT, + "add package -> known PHP packages = NULL"); + } + package = (nr_php_package_t*)nr_hashmap_get( h->known_packages, p->package_name, nr_strlen(p->package_name)); if (NULL != package) { From 1c5e64ee7abcc18f414b095343dba56fedbafe72 Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:39:29 -0400 Subject: [PATCH 07/11] change so we pass in packages --- axiom/nr_php_packages.c | 6 +++--- axiom/nr_php_packages.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index 2e99c4bd2..d1099b7da 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -37,14 +37,14 @@ static inline const char* nr_php_package_source_priority_to_string(const nr_php_ } void nr_php_packages_set_known(nr_php_packages_t* pkgs, - nr_hashmap_t* known) { /* should be CONST */ - if (NULL == pkgs || NULL == known) { + nr_php_packages_t* known) { /* should be CONST */ + if (NULL == pkgs || NULL == known || NULL == known->data) { return; } nrl_verbosedebug(NRL_INSTRUMENT, "Setting known PHP packages to %p", known); - pkgs->known_packages = known; + pkgs->known_packages = known->data; } nr_php_package_t* nr_php_package_create_with_source( diff --git a/axiom/nr_php_packages.h b/axiom/nr_php_packages.h index e8edb37b0..48b81d7ad 100644 --- a/axiom/nr_php_packages.h +++ b/axiom/nr_php_packages.h @@ -38,7 +38,7 @@ typedef void(nr_php_packages_iter_t)(void* value, void* user_data); void nr_php_packages_set_known(nr_php_packages_t* pkgs, - nr_hashmap_t* known); /* should be CONST! */ + nr_php_packages_t* known); /* should be CONST! */ /* * Purpose : Create a new php package with desired source priority. If the name is null, then no package will From 2cf5726ec3ee950465ace6efb70eb82bef924509 Mon Sep 17 00:00:00 2001 From: Michael Fulbright Date: Tue, 5 Aug 2025 15:52:08 -0400 Subject: [PATCH 08/11] debug --- agent/php_txn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/php_txn.c b/agent/php_txn.c index 86757339c..4b4fb3bd6 100644 --- a/agent/php_txn.c +++ b/agent/php_txn.c @@ -1162,6 +1162,8 @@ nr_status_t nr_php_txn_begin(const char* appnames, = nr_php_packages_clone(NR_PHP_PROCESS_GLOBALS(composer_php_packages)); nrl_verbosedebug(NRL_FRAMEWORK, "composer packages cloned from cache"); #else + nrl_verbosedebug(NRL_INSTRUMENT, "passing known PHP packages = %p", + NR_PHP_PROCESS_GLOBALS(composer_php_packages)); nr_php_packages_set_known(NRPRG(txn)->php_packages, NR_PHP_PROCESS_GLOBALS(composer_php_packages)); #endif From b8b3510677f97605967f1f190644fe87bfcaf779 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 6 Aug 2025 10:12:19 -0400 Subject: [PATCH 09/11] initialize known_packages when creating php_packages object --- axiom/nr_php_packages.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index d1099b7da..98f225be2 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -101,6 +101,9 @@ nr_php_packages_t* nr_php_packages_create() { nr_free(h); return NULL; } + + h->known_packages = NULL; + return h; } static void clone_callback(void* value, From 4774823faba7b12e4601f6a7827667fc61952305 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 6 Aug 2025 10:15:16 -0400 Subject: [PATCH 10/11] make the code more readable --- axiom/nr_php_packages.c | 47 ++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/axiom/nr_php_packages.c b/axiom/nr_php_packages.c index 98f225be2..ea7df9fa5 100644 --- a/axiom/nr_php_packages.c +++ b/axiom/nr_php_packages.c @@ -157,6 +157,35 @@ nr_php_packages_t* nr_php_packages_clone(nr_php_packages_t* pkgs) { return h; } +static inline bool nr_php_packages_is_known_package(nr_php_packages_t* pkgs, + nr_php_package_t* p) { + nr_php_package_t* known = NULL; + + if (NULL == pkgs || NULL == pkgs->known_packages || NULL == p) { + return false; + } + + known = (nr_php_package_t*)nr_hashmap_get( + pkgs->known_packages, p->package_name, nr_strlen(p->package_name)); + + if (NULL == known) { + // do not ignore - package is not known yet + return false; + } + + if (known->source_priority > p->source_priority) { + // ignore - package is known and has a lower priority + return true; + } + + if (0 == nr_strcmp(known->package_version, p->package_version)) { + // ignore - package is known and has the same version + return true; + } + + return false; +} + nr_php_package_t* nr_php_packages_add_package(nr_php_packages_t* h, nr_php_package_t* p) { nr_php_package_t* package; @@ -169,23 +198,7 @@ nr_php_package_t* nr_php_packages_add_package(nr_php_packages_t* h, } // if package is in known packages then ignore - - if (h->known_packages) { - nrl_verbosedebug(NRL_INSTRUMENT, "add package -> known PHP packages = %p", - h->known_packages); - } else { - nrl_verbosedebug(NRL_INSTRUMENT, - "add package -> known PHP packages = NULL"); - } - - package = (nr_php_package_t*)nr_hashmap_get( - h->known_packages, p->package_name, nr_strlen(p->package_name)); - if (NULL != package) { - if (package->source_priority <= p->source_priority - && 0 != nr_strcmp(package->package_version, p->package_version)) { - nr_free(package->package_version); - package->package_version = nr_strdup(p->package_version); - } + if (nr_php_packages_is_known_package(h, p)) { nr_php_package_destroy(p); return NULL; } From e9e69095aeb6a8a1a71e016e35de424ce22f9fc9 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 6 Aug 2025 10:18:54 -0400 Subject: [PATCH 11/11] add test for adding packages to hashmap with known packages --- axiom/tests/test_php_packages.c | 70 +++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/axiom/tests/test_php_packages.c b/axiom/tests/test_php_packages.c index bd2323b2f..b7954daa9 100644 --- a/axiom/tests/test_php_packages.c +++ b/axiom/tests/test_php_packages.c @@ -53,6 +53,75 @@ static void test_php_adding_packages_to_hashmap(void) { tlib_pass_if_null("PHP packages hashmap destroyed", hm); } +static void test_adding_to_hashmap_with_known_packages(void) { + nr_php_package_t* package1; + nr_php_package_t* package2; + nr_php_package_t* package3; + nr_php_packages_t* hm = nr_php_packages_create(); + nr_php_packages_t* cached_packages; + int count; + + // Test: create multiple new packages and add to hashmap + package1 = nr_php_package_create("Package One", "10.1.0"); + package2 = nr_php_package_create("Package Two", "11.2.0"); + package3 = nr_php_package_create("Package Three", "12.3.0"); + /* Should not crash: */ + + nr_php_packages_add_package(NULL, package1); + nr_php_packages_add_package(hm, NULL); + nr_php_packages_add_package(hm, package1); + nr_php_packages_add_package(hm, package2); + nr_php_packages_add_package(hm, package3); + + count = nr_php_packages_count(hm); + + tlib_pass_if_int_equal("package count", 3, count); + + // Cache packages + cached_packages = nr_php_packages_clone(hm); + + nr_php_packages_destroy(&hm); + tlib_pass_if_null("Transaction packages destroyed", hm); + + tlib_pass_if_not_null("Packages cache created", cached_packages); + tlib_pass_if_int_equal("Packages cache count", 3, + nr_php_packages_count(cached_packages)); + + hm = nr_php_packages_create(); + tlib_pass_if_not_null("Transaction packages created", hm); + + nr_php_packages_set_known(hm, cached_packages); + tlib_pass_if_ptr_equal("Known packages set", + hm->known_packages, cached_packages->data); + + // Adding packages to hashmap that are already in the cache should not change + // the count of packages in the hashmap + package1 = nr_php_package_create("Package One", "10.1.0"); + package2 = nr_php_package_create("Package Two", "11.2.0"); + package3 = nr_php_package_create("Package Three", "12.3.0"); + nr_php_packages_add_package(hm, package1); + nr_php_packages_add_package(hm, package2); + nr_php_packages_add_package(hm, package3); + + count = nr_php_packages_count(hm); + + tlib_pass_if_int_equal("package count", 0, count); + + // Adding packages to hashmap that are not in the cache should change + // the count of packages in the hashmap + package1 = nr_php_package_create("Uncached package", "12.3.0"); + nr_php_packages_add_package(hm, package1); + count = nr_php_packages_count(hm); + + tlib_pass_if_int_equal("package count", 1, count); + + nr_php_packages_destroy(&hm); + tlib_pass_if_null("Transaction packages destroyed", hm); + + nr_php_packages_destroy(&cached_packages); + tlib_pass_if_null("Cached packages hashmap destroyed", cached_packages); +} + static void test_php_package_to_json(void) { char* json; nr_php_package_t* package1; @@ -375,6 +444,7 @@ tlib_parallel_info_t parallel_info void test_main(void* p NRUNUSED) { test_php_package_create_destroy(); test_php_adding_packages_to_hashmap(); + test_adding_to_hashmap_with_known_packages(); test_php_package_to_json(); test_php_packages_to_json_buffer(); test_php_packages_to_json();