fix(agent): address that aws-sdk-php versioning changed

Concerns about future/past proofing to the checking prioritized the non-eval implementation vs using the eval method for checking the version.
However, aws-sdk-php internal functions frequently change without notice in the release notes.
An extra eval is used to avoid the fatal error that would occur in the VERY unlikely event of Aws/Sdk.php not being found and which addresses the previous concerns while decoupling versioning from internal functions.

Author: zsistla

agent/lib_aws_sdk_php.c

@@ -22,49 +22,43 @@
  * 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
+ * take an extra step to wrap the call in a try/catch block. This means we need
+ * to have an additionial zend_string_eval to get the result, but 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(
+      "$nr_aws_sdk_version = '';"
+      "try {"
+      "    $nr_aws_sdk_version = Aws\\Sdk::VERSION;"
+      "} catch(Throwable $_e) {"
+      "}",
+      NULL, "Set nr_aws_sdk_version");
+
+  if (SUCCESS == result) {
+    result = zend_eval_string("$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 +67,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 +90,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 +131,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 +145,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);
 }