feat(agent): improve package major version metric creation (#965)

Change how the package major number metrics are created. This now occurs in R_SHUTDOWN. This allows the use of package versions from ALL sources including the Composer API. Whenever instrumentation for a package detects a package, it can create a package suggestion. Initially if a version is not known it is fine to use PHP_PACKAGE_VERSION_UNKNOWN. If later the instrumentation determines a version from a class constant, etc, then the suggestion can be updated with the version. At the end of the transaction the suggestions are iterated over and the actual package data (which could include Composer API data) is referenced and a major number supportability metric is created with the best version available. --------- Co-authored-by: Michal Nowacki <[email protected]>
newrelic · Sep 26, 2024 · 1fda499 · 1fda499
1 parent 51f6d43
commit 1fda499
Show file tree

Hide file tree

Showing 61 changed files with 800 additions and 207 deletions.
diff --git a/agent/Makefile.frag b/agent/Makefile.frag
@@ -107,6 +107,7 @@ TEST_BINARIES = \
 	tests/test_php_minit \
 	tests/test_php_stack \
 	tests/test_php_stacked_segment \
+	tests/test_php_txn \
 	tests/test_php_wrapper \
 	tests/test_predis \
 	tests/test_redis \

diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c
@@ -886,10 +886,7 @@ void nr_drupal_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
-
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));
 
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c
@@ -669,7 +669,6 @@ NR_PHP_WRAPPER_END
 void nr_drupal_version() {
   zval* zval_version = NULL;
   zend_class_entry* class_entry = NULL;
-  nr_php_package_t* p = NULL;
 
   class_entry = nr_php_find_class("drupal");
   if (NULL == class_entry) {
@@ -688,10 +687,8 @@ void nr_drupal_version() {
   if (nr_php_is_zval_valid_string(zval_version)) {
     char* version = Z_STRVAL_P(zval_version);
     if (NRINI(vulnerability_management_package_detection_enabled)) {
-      p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+      nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
     }
-    nr_fw_support_add_package_supportability_metric(
-        NRPRG(txn), PHP_PACKAGE_NAME, version, p);
   }
 
   nr_php_zval_free(&zval_version);
@@ -763,4 +760,7 @@ void nr_drupal8_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/fw_laminas3.c b/agent/fw_laminas3.c
@@ -168,8 +168,7 @@ void nr_laminas3_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));  
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/fw_laravel.c b/agent/fw_laravel.c
@@ -949,7 +949,6 @@ NR_PHP_WRAPPER(nr_laravel_application_construct) {
   zval* this_var = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
   ;
   char* version = NULL;
-  nr_php_package_t* p = NULL;
 
   NR_UNUSED_SPECIALFN;
   (void)wraprec;
@@ -962,10 +961,11 @@ NR_PHP_WRAPPER(nr_laravel_application_construct) {
 
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     // Add php package to transaction
-    p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
   }
-  nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
-                                                  version, p);
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               version);
 
   if (version) {
     nrl_debug(NRL_FRAMEWORK, "Laravel version is " NRP_FMT, NRP_PHP(version));

diff --git a/agent/fw_lumen.c b/agent/fw_lumen.c
@@ -239,8 +239,6 @@ void nr_lumen_enable(TSRMLS_D) {
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
 
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/fw_slim.c b/agent/fw_slim.c
@@ -152,7 +152,6 @@ NR_PHP_WRAPPER_END
 NR_PHP_WRAPPER(nr_slim_application_construct) {
   zval* this_var = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS);
   char* version = NULL;
-  nr_php_package_t* p = NULL;
 
   NR_UNUSED_SPECIALFN;
   (void)wraprec;
@@ -161,11 +160,11 @@ NR_PHP_WRAPPER(nr_slim_application_construct) {
 
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     // Add php package to transaction
-    p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
   }
 
-  nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
-                                                  version, p);
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               version);
 
   nr_free(version);
   nr_php_scope_release(&this_var);

diff --git a/agent/fw_symfony4.c b/agent/fw_symfony4.c
@@ -283,8 +283,6 @@ void nr_symfony4_enable(TSRMLS_D) {
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
 
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));  
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c
@@ -804,18 +804,17 @@ void nr_wordpress_version() {
         "})();";
 
   zval retval;
-  int result
-      = zend_eval_string(func_string, &retval, "Get Wordpress Version");
-  nr_php_package_t* p = NULL;
+  int result = zend_eval_string(func_string, &retval, "Get Wordpress Version");
   // Add php package to transaction
   if (SUCCESS == result) {
     if (nr_php_is_zval_valid_string(&retval)) {
       char* version = Z_STRVAL(retval);
       if (NRINI(vulnerability_management_package_detection_enabled)) {
-        p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+        nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
       }
-      nr_fw_support_add_package_supportability_metric(
-          NRPRG(txn), PHP_PACKAGE_NAME, version, p);
+
+      nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                                   version);
     }
     zval_dtor(&retval);
   }

diff --git a/agent/fw_yii.c b/agent/fw_yii.c
@@ -227,9 +227,7 @@ void nr_yii2_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
-
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c
@@ -56,7 +56,6 @@ void nr_lib_aws_sdk_php_handle_version() {
   zval* zval_version = NULL;
   zend_class_entry* class_entry = NULL;
   char* version = NULL;
-  nr_php_package_t* p = NULL;
 
   class_entry = nr_php_find_class("aws\\sdk");
   if (NULL != class_entry) {
@@ -68,10 +67,12 @@ void nr_lib_aws_sdk_php_handle_version() {
   }
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     /* Add php package to transaction */
-    p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
   }
-  nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
-                                                  version, p);
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               version);
+
   nr_php_zval_free(&zval_version);
 }
 

diff --git a/agent/lib_doctrine2.c b/agent/lib_doctrine2.c
@@ -111,8 +111,7 @@ void nr_doctrine2_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));  
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/lib_guzzle4.c b/agent/lib_guzzle4.c
@@ -526,10 +526,8 @@ void nr_guzzle4_enable(TSRMLS_D) {
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
 
-  nr_fw_support_add_package_supportability_metric(
-      NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-      nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                  PHP_PACKAGE_NAME));
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
 
 void nr_guzzle4_minit(TSRMLS_D) {

diff --git a/agent/lib_guzzle6.c b/agent/lib_guzzle6.c
@@ -352,23 +352,32 @@ NR_PHP_WRAPPER_START(nr_guzzle6_client_construct) {
   zval* this_var = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS);
 
   char* version = nr_php_get_object_constant(this_var, "VERSION");
-  nr_php_package_t* p = NULL;
 
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     // Add php package to transaction
-    p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
   }
 
   /*
- * If we were unable to get the full version before, at least we can extract
- * the major version to send to the supportability metric.
- * This is relevant to guzzle7+ which no longer supplies full version.
- */
+   * If we were unable to get the full version before, at least we can extract
+   * the major version to send to the supportability metric now, as
+   * this incomplete version will not be stored in a php package record
+   * and so the supportability metric cannot be created later like for
+   * most packages.
+   *
+   * This is relevant to guzzle7+ which no longer supplies full version.
+   */
   if (NULL == version) {
     version = nr_php_get_object_constant(this_var, "MAJOR_VERSION");
   }
-  nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
-                                                  version, p);
+
+  /* if version is still NULL that is OK this next call will accept
+   * that value and when supportability metrics are made for
+   * packages if another method has determined the package version
+   * (composer api for example) then it will be filled in at that time
+   */
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               version);
   nr_free(version);
 
   (void)wraprec;

diff --git a/agent/lib_mongodb.c b/agent/lib_mongodb.c
@@ -451,8 +451,6 @@ void nr_mongodb_enable() {
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
 
-  nr_fw_support_add_package_supportability_metric(
-    NRPRG(txn), PHP_PACKAGE_NAME, NULL,
-    nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                PHP_PACKAGE_NAME));
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/lib_monolog.c b/agent/lib_monolog.c
@@ -378,10 +378,8 @@ NR_PHP_WRAPPER(nr_monolog_logger_addrecord) {
         = nr_monolog_get_timestamp(api, argc, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
     char version[MAJOR_VERSION_LENGTH];
     snprintf(version, sizeof(version), "%d", api);
-    nr_fw_support_add_package_supportability_metric(
-        NRPRG(txn), PHP_PACKAGE_NAME, version,
-        nr_php_packages_get_package(NRPRG(txn)->php_packages,
-                                    PHP_PACKAGE_NAME));
+    nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                                 version);
   }
 
   /* Record the log event */
@@ -525,4 +523,7 @@ void nr_monolog_enable(TSRMLS_D) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/lib_phpunit.c b/agent/lib_phpunit.c
@@ -14,6 +14,8 @@
 #include "util_logging.h"
 #include "util_strings.h"
 
+#define PHP_PACKAGE_NAME "phpunit/phpunit"
+
 /*
  * PHPUnit instrumentation
  * =======================
@@ -697,7 +699,10 @@ void nr_phpunit_enable(TSRMLS_D) {
       nr_phpunit_instrument_testresult_adderror TSRMLS_CC);
 
   if (NRINI(vulnerability_management_package_detection_enabled)) {
-    nr_txn_add_php_package(NRPRG(txn), "phpunit/phpunit",
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               PHP_PACKAGE_VERSION_UNKNOWN);
 }
diff --git a/agent/lib_predis.c b/agent/lib_predis.c
@@ -644,7 +644,6 @@ NR_PHP_WRAPPER(nr_predis_client_construct) {
   zval* conn = NULL;
   zval* params = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
   zval* scope = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
-  nr_php_package_t* p = NULL;
 
   (void)wraprec;
 
@@ -653,10 +652,12 @@ NR_PHP_WRAPPER(nr_predis_client_construct) {
   char* version = nr_php_get_object_constant(scope, "VERSION");
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     // Add php package to transaction
-    p = nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
+    nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
   }
-  nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
-                                                  version, p);
+
+  nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
+                                               version);
+
   nr_free(version);
 
   /*