fix(agent): fix legacy package detection issues (#1107)

When Composer runtime API is used to get packages information, the
agent should not generate packages information using legacy methods.
This solves the problem of package data detected with the help of
Composer's runtime API not to be sent to the backend in environments
using “once-per-process” setting. The reason for is that the first 
request generates package information using Composer's runtime API
but every next request only generates package information using legacy
method. This in conjunction with the fact that daemon is only sending
the last package information received from the daemon leads to a
situation that package information returned by Composer runtime API
is lost and never makes it to the backend.

Author: lavarou

agent/lib_composer.c

@@ -69,23 +69,25 @@ static int nr_execute_handle_autoload_composer_init(const char* vendor_path) {
   return NR_SUCCESS;
 }
 
-static void nr_execute_handle_autoload_composer_get_packages_information(
+static nr_composer_api_status_t
+nr_execute_handle_autoload_composer_get_packages_information(
     const char* vendor_path) {
   zval retval;  // This is used as a return value for zend_eval_string.
                 // It will only be set if the result of the eval is SUCCESS.
   int result = FAILURE;
+  nr_composer_api_status_t api_status = NR_COMPOSER_API_STATUS_UNSET;
 
   // nrunlikely because this should alredy be ensured by the caller
   if (nrunlikely(!NRINI(vulnerability_management_package_detection_enabled))) {
     // do nothing when collecting package information for vulnerability
     // management is disabled
-    return;
+    return NR_COMPOSER_API_STATUS_INVALID_USE;
   }
 
   // nrunlikely because this should alredy be ensured by the caller
   if (nrunlikely(!NRINI(vulnerability_management_composer_api_enabled))) {
     // do nothing when use of composer to collect package info is disabled
-    return;
+    return NR_COMPOSER_API_STATUS_INVALID_USE;
   }
 
   // clang-format off
@@ -124,7 +126,7 @@ static void nr_execute_handle_autoload_composer_get_packages_information(
               "%s - unable to initialize Composer runtime API - package info "
               "unavailable",
               __func__);
-    return;
+    return NR_COMPOSER_API_STATUS_INIT_FAILURE;
   }
 
   nrl_verbosedebug(NRL_INSTRUMENT, "%s - Composer runtime API available",
@@ -135,13 +137,7 @@ static void nr_execute_handle_autoload_composer_get_packages_information(
   if (SUCCESS != result) {
     nrl_verbosedebug(NRL_INSTRUMENT, "%s - composer_getallrawdata.php failed",
                      __func__);
-    return;
-  }
-
-  if (NR_PHP_PROCESS_GLOBALS(composer_api_per_process_detection)) {
-    // set the per-process flag to true to avoid re-running composer api
-    // detection when the per-process detection is enabled.
-    NR_PHP_PROCESS_GLOBALS(composer_packages_detected) = 1;
+    return NR_COMPOSER_API_STATUS_CALL_FAILURE;
   }
 
   if (IS_ARRAY == Z_TYPE(retval)) {
@@ -162,14 +158,17 @@ static void nr_execute_handle_autoload_composer_get_packages_information(
       }
     }
     ZEND_HASH_FOREACH_END();
+    api_status = NR_COMPOSER_API_STATUS_PACKAGES_COLLECTED;
   } else {
     char strbuf[80];
     nr_format_zval_for_debug(&retval, strbuf, 0, sizeof(strbuf) - 1, 0);
     nrl_verbosedebug(NRL_INSTRUMENT,
                      "%s - installed packages is: " NRP_FMT ", not an array",
                      __func__, NRP_ARGSTR(strbuf));
+    api_status = NR_COMPOSER_API_STATUS_INVALID_RESULT;
   }
   zval_dtor(&retval);
+  return api_status;
 }
 
 static char* nr_execute_handle_autoload_composer_get_vendor_path(
@@ -274,7 +273,9 @@ void nr_composer_handle_autoload(const char* filename) {
   NRPRG(txn)->composer_info.composer_detected = true;
   nr_fw_support_add_library_supportability_metric(NRPRG(txn), "Composer");
 
-  nr_execute_handle_autoload_composer_get_packages_information(vendor_path);
+  NRTXN(composer_info.api_status)
+      = nr_execute_handle_autoload_composer_get_packages_information(
+          vendor_path);
 leave:
   nr_free(vendor_path);
 }

agent/php_execute.c

@@ -875,12 +875,13 @@ static void nr_execute_handle_autoload(const char* filename,
     return;
   }
 
-  if (NR_PHP_PROCESS_GLOBALS(composer_api_per_process_detection)
-      && NR_PHP_PROCESS_GLOBALS(composer_packages_detected)) {
-    // do nothing if per-process detection is enabled and the flag to track
-    // detection is true
+  // clang-format off
+  if (NR_PHP_PROCESS_GLOBALS(composer_api_per_process_detection) &&
+      NR_COMPOSER_API_STATUS_UNSET != NR_PHP_PROCESS_GLOBALS(composer_api_status)) {
+    // do nothing if per-process detection is enabled and composer api status is set
     return;
   }
+  // clang-format on
 
   if (!nr_striendswith(STR_AND_LEN(filename), AUTOLOAD_MAGIC_FILE,
                        AUTOLOAD_MAGIC_FILE_LEN)) {

agent/php_globals.h

@@ -78,9 +78,7 @@ typedef struct _nrphpglobals_t {
   int composer_api_per_process_detection;  /* Enables per-process VM package
                                               detection when Composer API is also
                                               enabled */
-  int composer_packages_detected; /* Flag to indicate that Composer package
-                                    detection has run. Used in conjunction with
-                                    composer_api_per_process_detection. */
+  nr_composer_api_status_t composer_api_status; /* Composer API's status */
   char* docker_id; /* 64 byte hex docker ID parsed from /proc/self/mountinfo */
 
   /* Original PHP callback pointer contents */

agent/php_minit.c

@@ -456,7 +456,7 @@ PHP_MINIT_FUNCTION(newrelic) {
       = nr_php_check_for_upgrade_license_key();
   NR_PHP_PROCESS_GLOBALS(high_security) = 0;
   NR_PHP_PROCESS_GLOBALS(preload_framework_library_detection) = 1;
-  NR_PHP_PROCESS_GLOBALS(composer_packages_detected) = 0;
+  NR_PHP_PROCESS_GLOBALS(composer_api_status) = NR_COMPOSER_API_STATUS_UNSET;
   nr_php_populate_apache_process_globals();
   nr_php_api_distributed_trace_register_userland_class(TSRMLS_C);
   /*

agent/php_txn.c

@@ -1151,6 +1151,8 @@ nr_status_t nr_php_txn_begin(const char* appnames,
     }
   }
 
+  NRTXN(composer_info.api_status) = NR_PHP_PROCESS_GLOBALS(composer_api_status);
+
   return NR_SUCCESS;
 }
 
@@ -1340,6 +1342,8 @@ nr_status_t nr_php_txn_end(int ignoretxn, int in_post_deactivate TSRMLS_DC) {
       if (NR_FAILURE == ret) {
         nrl_debug(NRL_TXN, "failed to send txn");
       }
+      NR_PHP_PROCESS_GLOBALS(composer_api_status)
+          = NRTXN(composer_info.api_status);
     }
   }
 

axiom/nr_txn.c

@@ -3539,6 +3539,14 @@ nr_php_package_t* nr_txn_add_php_package_from_source(
     return NULL;
   }
 
+  if (NR_PHP_PACKAGE_SOURCE_LEGACY == source
+      && NR_COMPOSER_API_STATUS_PACKAGES_COLLECTED
+             == txn->composer_info.api_status) {
+    // don't add packages from legacy source if packages have been detected
+    // using composer runtime api
+    return NULL;
+  }
+
   p = nr_php_package_create_with_source(package_name, package_version, source);
   return nr_php_packages_add_package(txn->php_packages, p);
 }

axiom/nr_txn.h

@@ -207,9 +207,19 @@ typedef enum _nr_cpu_usage_t {
   NR_CPU_USAGE_COUNT = 2
 } nr_cpu_usage_t;
 
+typedef enum {
+  NR_COMPOSER_API_STATUS_UNSET = 0,
+  NR_COMPOSER_API_STATUS_INVALID_USE = 1,
+  NR_COMPOSER_API_STATUS_INIT_FAILURE = 2,
+  NR_COMPOSER_API_STATUS_CALL_FAILURE = 3,
+  NR_COMPOSER_API_STATUS_PACKAGES_COLLECTED = 4,
+  NR_COMPOSER_API_STATUS_INVALID_RESULT = 5,
+} nr_composer_api_status_t;
+
 typedef struct _nr_composer_info_t {
   bool autoload_detected;
   bool composer_detected;
+  nr_composer_api_status_t api_status;
 } nr_composer_info_t;
 
 /*

axiom/tests/test_txn.c

@@ -8665,6 +8665,16 @@ static void test_nr_txn_add_php_package(void) {
   tlib_pass_if_ptr_equal(
       "same package name, different version, add returns same pointer", p1, p2);
   nr_txn_destroy(&txn);
+
+  txn = new_txn(0);
+  // simulate composer api was successfully used
+  txn->composer_info.api_status = NR_COMPOSER_API_STATUS_PACKAGES_COLLECTED;
+  p1 = nr_txn_add_php_package(txn, package_name1, package_version1);
+  tlib_pass_if_null(
+      "legacy package information not added to transaction after composer api "
+      "was called successfully",
+      p1);
+  nr_txn_destroy(&txn);
 }
 
 static void test_nr_txn_add_php_package_from_source(void) {
@@ -8730,6 +8740,29 @@ static void test_nr_txn_add_php_package_from_source(void) {
                          p2->package_version);
 
   nr_txn_destroy(&txn);
+
+  txn = new_txn(0);
+  // simulate composer api was successfully used
+  txn->composer_info.api_status = NR_COMPOSER_API_STATUS_PACKAGES_COLLECTED;
+  p1 = nr_txn_add_php_package_from_source(txn, package_name1, package_version1,
+                                          NR_PHP_PACKAGE_SOURCE_LEGACY);
+  tlib_pass_if_null(
+      "legacy package information not added to transaction after composer api "
+      "was called successfully",
+      p1);
+  p1 = nr_txn_add_php_package_from_source(txn, package_name1, package_version1,
+                                          NR_PHP_PACKAGE_SOURCE_SUGGESTION);
+  tlib_pass_if_not_null(
+      "suggestion package information added to transaction even after composer "
+      "api was called successfully",
+      p1);
+  p1 = nr_txn_add_php_package_from_source(txn, package_name1, package_version1,
+                                          NR_PHP_PACKAGE_SOURCE_COMPOSER);
+  tlib_pass_if_not_null(
+      "composer package information added to transaction even after composer "
+      "api was called successfully",
+      p1);
+  nr_txn_destroy(&txn);
 }
 
 static void test_nr_txn_suggest_package_supportability_metric(void) {