Merge pull request #1007 from newrelic/dev

Release 11.5

Author: hahuja2

.github/workflows/code-coverage-baseline.yml

@@ -69,7 +69,7 @@ jobs:
       matrix:
         platform: [gnu, musl]
         arch: [amd64]
-        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3']
+        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
         include:
           - codecov: 0
           - platform: gnu
@@ -147,7 +147,7 @@ jobs:
       matrix:
         platform: [gnu, musl]
         arch: [amd64]
-        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3']
+        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
         include:
           - codecov: 0
           - platform: gnu

.github/workflows/codeql.yml

@@ -16,7 +16,7 @@ jobs:
     #   - https://gh.io/supported-runners-and-hardware-resources
     #   - https://gh.io/using-larger-runners
     # Consider using larger runners for possible analysis time improvements.
-    runs-on: 'ubuntu-latest'
+    runs-on: 'ubuntu-22.04'
     timeout-minutes: 360
     permissions:
       # required for all workflows

.github/workflows/make-agent.yml

@@ -32,7 +32,7 @@ jobs:
     strategy:
       matrix:
         platform: [gnu, musl]
-        php: ['8.0', '8.1', '8.2', '8.3']
+        php: ['8.0', '8.1', '8.2', '8.3', '8.4']
     steps:
       - name: Checkout Repo
         uses: actions/checkout@v4

.github/workflows/make-integration-tests.yml

@@ -34,7 +34,7 @@ jobs:
       fail-fast: true
       matrix:
         platform: [gnu, musl]
-        php: ['8.0', '8.1', '8.2', '8.3']
+        php: ['8.0', '8.1', '8.2', '8.3', '8.4']
     steps:
       - name: Checkout integration tests
         uses: actions/checkout@v4

.github/workflows/security-scan.yml

@@ -45,4 +45,5 @@ jobs:
         if: ${{ github.event_name == 'schedule' }}
         uses: github/codeql-action/upload-sarif@v3
         with:
+          checkout_path: ./php-agent
           sarif_file: trivy-results.sarif

.github/workflows/test-agent.yml

@@ -97,7 +97,7 @@ jobs:
       matrix:
         platform: [gnu, musl]
         arch: [amd64, arm64]
-        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3']
+        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
         exclude:
           - arch: arm64
             php: '7.2'
@@ -203,7 +203,7 @@ jobs:
       matrix:
         platform: [gnu, musl]
         arch: [amd64, arm64]
-        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3']
+        php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4']
         exclude:
           - arch: arm64
             php: '7.2'

VERSION

@@ -1 +1 @@
-11.4.0
+11.5.0

agent/config.m4

@@ -232,13 +232,16 @@ if test "$PHP_NEWRELIC" = "yes"; then
   lib_guzzle4.c lib_guzzle6.c lib_guzzle_common.c \
   lib_mongodb.c lib_phpunit.c lib_predis.c lib_zend_http.c \
   lib_composer.c"
-  PHP_NEW_EXTENSION(newrelic, $FRAMEWORKS $LIBRARIES $NEWRELIC_AGENT, $ext_shared,, \\$(NEWRELIC_CFLAGS))
+  PHP_NEW_EXTENSION(newrelic, $FRAMEWORKS $LIBRARIES $NEWRELIC_AGENT, $ext_shared,, $(NEWRELIC_CFLAGS))
 
   PHP_SUBST(NEWRELIC_CFLAGS)
 
   dnl Define $(PHP_CONFIG) so we can call it when building tests.
   PHP_SUBST(PHP_CONFIG)
 
+  dnl Make sure we include the source directory in the include search path.
+  PHP_ADD_INCLUDE([$abs_srcdir], [1])
+
   dnl Include the Makefile.frag, which we use to handle build time
   dnl dependencies.
   PHP_ADD_MAKEFILE_FRAGMENT

agent/fw_drupal8.c

@@ -9,6 +9,7 @@
 #include "php_user_instrument.h"
 #include "php_execute.h"
 #include "php_wrapper.h"
+#include "php_error.h"
 #include "fw_drupal_common.h"
 #include "fw_hooks.h"
 #include "fw_support.h"
@@ -20,6 +21,62 @@
 
 #define PHP_PACKAGE_NAME "drupal/core"
 
+NR_PHP_WRAPPER(nr_drupal_exception) {
+  int priority = nr_php_error_get_priority(E_ERROR);
+  zval* event = NULL;
+  zval* exception = NULL;
+
+  /* Warning avoidance */
+  (void)wraprec;
+
+  NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL8);
+
+  if (NR_SUCCESS != nr_txn_record_error_worthy(NRPRG(txn), priority)) {
+    NR_PHP_WRAPPER_CALL;
+    goto end;
+  }
+
+  /* Get the event that was given. */
+  event = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS);
+
+  /* Call the original function. */
+  NR_PHP_WRAPPER_CALL;
+
+  if (0 == nr_php_is_zval_valid_object(event)) {
+    nrl_verbosedebug(NRL_TXN,
+                     "Drupal: ExceptionSubscriber::onException() does not "
+                     "have an `event` parameter");
+    goto end;
+  }
+
+  /*
+   * Get the exception from the event.
+   */
+  exception = nr_php_call(event, "getThrowable");
+  if (!nr_php_is_zval_valid_object(exception)) {
+    // be abundantly cautious: free exception before attempting to re-assign
+    nr_php_zval_free(&exception);
+    exception = nr_php_call(event, "getException");
+  }
+
+  if (!nr_php_is_zval_valid_object(exception)) {
+    nrl_verbosedebug(NRL_TXN, "Drupal: getException() returned a non-object");
+    goto end;
+  }
+
+  if (NR_SUCCESS
+      != nr_php_error_record_exception(NRPRG(txn), exception, priority, true,
+                                       NULL,
+                                       &NRPRG(exception_filters))) {
+    nrl_verbosedebug(NRL_TXN, "Drupal: unable to record exception");
+  }
+
+end:
+  nr_php_arg_release(&event);
+  nr_php_zval_free(&exception);
+}
+NR_PHP_WRAPPER_END
+
 /*
  * Purpose : Convenience function to handle adding a callback to a method,
  *           given a class entry and a method name. This will check the
@@ -730,6 +787,26 @@ void nr_drupal8_enable(TSRMLS_D) {
                                     "er::getControllerFromDefinition"),
                             nr_drupal8_name_the_wt TSRMLS_CC);
 
+  /*
+   * ExceptionSubscribers handle Drupal errors and exceptions before
+   * the agent has the opportunity to capture them. Instrument several
+   * of these ExceptionSubscriber function `onException` methods in order
+   * to capture Exceptions and Errors in Drupal 9.x+
+   */
+  // clang-format off
+  /*
+   * Log exceptions without further handling.
+   */
+  nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\ExceptionLoggingSubscriber::onException"),
+                            nr_drupal_exception);
+
+  /*
+   * Last-chance handler for exceptions: the final exception subscriber.
+   */
+  nr_php_wrap_user_function(NR_PSTR("Drupal\\Core\\EventSubscriber\\FinalExceptionSubscriber::onException"),
+                            nr_drupal_exception);
+  // clang-format on
+
   /*
    * The drupal_modules config setting controls instrumentation of modules,
    * hooks, and views.

agent/lib_aws_sdk_php.c

@@ -22,49 +22,40 @@
  * In a normal course of events, the following line will always work
  * zend_eval_string("Aws\\Sdk::VERSION;", &retval, "Get AWS Version")
  * By the time we have detected the existence of the aws-sdk-php and with
- * default composer profject settings, it callable even from
+ * default composer project settings, it is callable even from
  * nr_aws_sdk_php_enable which will automatically load the class if it isn't
- * loaded yet and then evaluate the string. However, in the rare case that files
+ * loaded yet and then evaluate the string. In the rare case that files
  * are not loaded via autoloader and/or have non-default composer classload
- * settings, if the class is not found, PHP 8.2+ will generate a fatal
- * unrecoverable uncatchable error error whenever it cannot find a class. While
- * calling this from nr_aws_sdk_php_enable would have been great and would allow
- * the sdk version value to be set only once, to avoid the very unlikely but not
- * impossible fatal error, this will be called from the
- * "Aws\\ClientResolver::_apply_user_agent" wrapper which GUARANTEES that
- * aws/sdk exists and is already loaded.
- *
- *
- * Additionally given that aws-sdk-php is currently detected from the
- * AwsClient.php file, this method will always be called when a client is
- * created unlike Sdk::construct which doesn't show with PHP 8.2+.
- *
- * Using Aws/Sdk::__construct for version is currently nonviable as it is
- * unreliable as a version determiner.
- * Having separate functionality to extract from Aws/Sdk::__construct
- * is both not required and is redundant and causes additional overhead and
- * so only one function is needed to extract version.
- *
- * Aws\\ClientResolver::_apply_user_agent a reliable function as it is
- * always called on client initialization since it is key to populating
- * the request headers, and it loads Sdk by default.
- *
- * Concerns about future/past proofing to the checking prioritized the following
- * implementation vs using the eval method.
+ * settings, if the class is not found, PHP 8.2+ will generate an
+ * error whenever it cannot find a class which must be caught. Calling this
+ * from nr_aws_sdk_php_enable would allow the sdk version value to be set only
+ * once. To avoid the VERY unlikely but not impossible fatal error, we need to
+ * wrap the call in a try/catch block and make it a lambda so that we avoid
+ * fatal errors.
  */
 void nr_lib_aws_sdk_php_handle_version() {
-  zval* zval_version = NULL;
-  zend_class_entry* class_entry = NULL;
   char* version = NULL;
-
-  class_entry = nr_php_find_class("aws\\sdk");
-  if (NULL != class_entry) {
-    zval_version = nr_php_get_class_constant(class_entry, "VERSION");
-
-    if (nr_php_is_zval_non_empty_string(zval_version)) {
-      version = Z_STRVAL_P(zval_version);
+  zval retval;
+  int result = FAILURE;
+
+  result = zend_eval_string(
+      "(function() {"
+      "     $nr_aws_sdk_version = '';"
+      "     try {"
+      "          $nr_aws_sdk_version = Aws\\Sdk::VERSION;"
+      "     } catch (Throwable $e) {"
+      "     }"
+      "     return $nr_aws_sdk_version;"
+      "})();",
+      &retval, "Get nr_aws_sdk_version");
+
+  /* See if we got a non-empty/non-null string for version. */
+  if (SUCCESS == result) {
+    if (nr_php_is_zval_non_empty_string(&retval)) {
+      version = Z_STRVAL(retval);
     }
   }
+
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     /* Add php package to transaction */
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
@@ -73,7 +64,7 @@ void nr_lib_aws_sdk_php_handle_version() {
   nr_txn_suggest_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
                                                version);
 
-  nr_php_zval_free(&zval_version);
+  zval_dtor(&retval);
 }
 
 void nr_lib_aws_sdk_php_add_supportability_service_metric(
@@ -96,13 +87,6 @@ void nr_lib_aws_sdk_php_add_supportability_service_metric(
   nrm_force_add(NRPRG(txn) ? NRTXN(unscoped_metrics) : 0, buf, 0);
 }
 
-NR_PHP_WRAPPER(nr_create_aws_sdk_version_metrics) {
-  (void)wraprec;
-  NR_PHP_WRAPPER_CALL;
-  nr_lib_aws_sdk_php_handle_version();
-}
-NR_PHP_WRAPPER_END
-
 /*
  * AwsClient::parseClass
  * This is called from the base AwsClient class for every client associated
@@ -144,7 +128,7 @@ NR_PHP_WRAPPER_END
  * optimizable library. Small overhead incurred when encountering an autoload
  * file, but detects aws-sdk-php immediately before any sdk code executes
  * (changes needed for this are detailed in the original PR)
- * 2. use a file that gets called later and only when AwsClient.php file file is
+ * 2. use a file that gets called later and only when AwsClient.php file is
  * called. It's called later and we'll miss some instrumentation, but if we're
  * only ever going to be interested in Client calls anyway, maybe that's ok?
  * Doesn't detect Sdk.php (optimized out) so when customers only use that or
@@ -158,22 +142,24 @@ NR_PHP_WRAPPER_END
  * to wrap, this will add overhead to every hash map lookup. Currently
  * implemented option is 2, use the AwsClient.php as this is our main focus.
  * This means until a call to an Aws/AwsClient function,
- * all calls including aws\sdk calls are ignored.  Version detection will be
- * tied to Aws/ClientResolver::_apply_user_agent which is ALWAYS called when
- * dealing with aws clients.  It will not be computed from
- * Aws/Sdk::__constructor which would at best be duplicate info and worst would
- * never be ignored until a client is called.
+ * all calls including aws\sdk calls are ignored.
+ *
+ * Version detection will be called directly from Aws\Sdk.php
  */
 void nr_aws_sdk_php_enable() {
-  /* This will be used to extract the version. */
-  nr_php_wrap_user_function(NR_PSTR("Aws\\ClientResolver::_apply_user_agent"),
-                            nr_create_aws_sdk_version_metrics);
-  /* Called when initializing all other Clients */
-  nr_php_wrap_user_function(NR_PSTR("Aws\\AwsClient::parseClass"),
-                            nr_create_aws_service_metric);
-
+  /*
+   * Set the UNKNOWN package first, so it doesn't overwrite what we find with
+   * nr_lib_aws_sdk_php_handle_version.
+   */
   if (NRINI(vulnerability_management_package_detection_enabled)) {
     nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
                            PHP_PACKAGE_VERSION_UNKNOWN);
   }
+
+  /* Extract the version for aws-sdk 3+ */
+  nr_lib_aws_sdk_php_handle_version();
+
+  /* Called when initializing all Clients */
+  nr_php_wrap_user_function(NR_PSTR("Aws\\AwsClient::parseClass"),
+                            nr_create_aws_service_metric);
 }