feat(agent): Add basic aws-sdk-php intrumentation

[zsistla]

1. Detects the use of aws-sdk-php 3 library with a magic file
2. wraps a function to extract version number
3. Sends a supportability metric for the library and package
4. Sends PHP Package info for version
5. Multiverse tests were added in support of this

further updated to:

1. added unit tests
2. created a wrapped function that will create a metric for all detected service names. as soon as the client is initialized, the metric is automatically created.

Then the metric is generated as seen in the associated multiverse tests.

Please see the challenge with instrumenting for PHP 8.2+ and various options that were considered below.
Option 2 is the currently chosen method due to lower risk, complexity, and overhead.

Challenges:
Php8.2+ and composer autoload leads to the main magic file being optimized directly to opcache.

Options considered:

1. for PHP8.2, and only optimizable libraries, when encountering autoload.php files, ask the file what includes it added and check against only the optimizable library. Small overhead incurred when encountering an autoload file.

summary:
Magic file = {"AWS-SDK-PHP", NR_PSTR("aws-sdk-php/src/functions.php"), nr_aws_sdk_php_enable, true},

Requires bool added to library structure:

typedef struct _nr_library_table_t {
  const char* library_name;
  const char* file_to_check;
  size_t file_to_check_len;
  nr_library_enable_fn_t enable;
  bool optimizable;
} nr_library_table_t;

Additional complexity in `nr_execute_handle_library
#if amber && ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO
/*
 * Only check if files have been optimized out for PHP 8.2+ and if we know the file is optimizable.
 * It's basically preloading things into the opcache without setting the preload flag that
 * we check to see if we need to check the opcache.
 * In this case, we need to check the PHP list of files that are included to see if our magic
 * file is there. We further limit overhead by only checking when it is an autoload file.
 */
 if ((libraries[i].optimizable) && (nr_striendswith(STR_AND_LEN(filename), "autoload.php", 12)) )  {
      zend_string *zstring_fname;
      
      if (NULL != &EG(included_files)) {
        ZEND_HASH_MAP_FOREACH_STR_KEY(&EG(included_files), zstring_fname) {
              if (nr_striendswith(ZEND_STRING_VALUE(zstring_fname), ZEND_STRING_LEN(zstring_fname) ,
                                  STR_AND_LEN(libraries[i].file_to_check))) {
                nrl_debug(NRL_INSTRUMENT, "detected library=%s",
                          libraries[i].library_name);
                
                nr_fw_support_add_library_supportability_metric(NRPRG(txn), libraries[i].library_name);
                if (NULL != libraries[i].enable) {
                  libraries[i].enable();
                }
                
  
              }  
          
    
        } ZEND_HASH_FOREACH_END();
 }
#endif // PHP 8.2+     

`

2. use a file that gets called later and only when AwsClient.php file 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 when they use it first, we will not instrument it. This only detects when a Client is called to use a service so potentially misses out on other instrumentation and misses out when customers use the aws-sdk-php but use non-SDK way to interact with the service (possibly with redis/memcached). This way is definitely the least complex and lowest overhead and less complexity means lower risk as well.

3. Just directly add the wrappers to the hash map. With potentially 50ish clients to wrap, this will add overhead to every hash map lookup.

[newrelic-php-agent-bot]

Test Suite	Status	Result
Multiverse	✅	9/9 passing
SOAK	✅	56/56 passing

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 52.38095% with 20 lines in your changes missing coverage. Please review.

Project coverage is 78.12%. Comparing base (8fd1602) to head (21bf064).

Files	Patch %	Lines
agent/lib_aws_sdk_php.c	52.38%	20 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #932      +/-   ##
==========================================
- Coverage   78.16%   78.12%   -0.05%     
==========================================
  Files         193      194       +1     
  Lines       26803    26845      +42     
==========================================
+ Hits        20950    20972      +22     
- Misses       5853     5873      +20     

Flag	Coverage Δ
agent-for-php-7.2	78.12% <52.38%> (-0.05%)	⬇️
agent-for-php-7.3	78.14% <52.38%> (-0.05%)	⬇️
agent-for-php-7.4	77.84% <52.38%> (-0.05%)	⬇️
agent-for-php-8.0	77.87% <52.38%> (-0.05%)	⬇️
agent-for-php-8.1	77.86% <52.38%> (-0.05%)	⬇️
agent-for-php-8.2	77.45% <52.38%> (-0.05%)	⬇️
agent-for-php-8.3	77.45% <52.38%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lavarou]

I’m still thinking if both constructors Aws\\AwsClient::__construct and Aws\\Sdk::__construct need to be instrumented 🤔 Reason why I’m thinking about this is overhead. If only Aws\\Sdk::__construct was instrumented, then

newrelic-php-agent/agent/lib_aws_sdk_php.c

Lines 25 to 27 in 2c27a8c

	/* Use the Aws\Sdk::VERSION to determine the version */
	class_entry = nr_php_find_class("aws\\sdk");
	zval_version = nr_php_get_class_constant(class_entry, "VERSION");

would be cheap, because $this, available through nr_php_scope_get, would be of Aws\Sdk type and therefore no class lookup (class_entry = nr_php_find_class("aws\\sdk");) would be necessary, and VERSION could be pulled directly from $this - see how Monolog’s API version is retrieved:

newrelic-php-agent/agent/lib_monolog.c

Line 376 in 2c27a8c

api = nr_monolog_version(this_var TSRMLS_CC);

However, I’m not sure if Aws\\Sdk::__construct is called when only Aws\\AwsClient::__construct is used. Either way - I think using a dedicated wrapper for Aws\\Sdk::__construct to retrieve version without class lookup is warranted.

[zsistla]

I’m still thinking if both constructors Aws\\AwsClient::__construct and Aws\\Sdk::__construct need to be instrumented 🤔 Reason why I’m thinking about this is overhead. If only Aws\\Sdk::__construct was instrumented, then

newrelic-php-agent/agent/lib_aws_sdk_php.c

Lines 25 to 27 in 2c27a8c

	/* Use the Aws\Sdk::VERSION to determine the version */
	class_entry = nr_php_find_class("aws\\sdk");
	zval_version = nr_php_get_class_constant(class_entry, "VERSION");

would be cheap, because $this, available through nr_php_scope_get, would be of Aws\Sdk type and therefore no class lookup (class_entry = nr_php_find_class("aws\\sdk");) would be necessary, and VERSION could be pulled directly from $this - see how Monolog’s API version is retrieved:

newrelic-php-agent/agent/lib_monolog.c

Line 376 in 2c27a8c

api = nr_monolog_version(this_var TSRMLS_CC);

However, I’m not sure if Aws\\Sdk::__construct is called when only Aws\\AwsClient::__construct is used. Either way - I think using a dedicated wrapper for Aws\\Sdk::__construct to retrieve version without class lookup is warranted.

Thanks for the comment. After further research, Aws\\Sdk::__construct is not a good class to generate version info off of for PHP 8.2+ as it does not show up in basic testing. Additionally, we are currently detecting based off of AwsClient.php so even if aws/sdk::construct did show up (which it didn't) we would ignore it until detection kicked in. So Aws\\Sdk::__construct is undependable and should not be relied on. We only need to grab version info from one spot; otherwise, it's duplicate info and additional overhead.
Aws\\ClientResolver::_apply_user_agent is a good choice as it guarantees access to VERSION (although again, even though aws/sdk is used in this function and version is accessed, aws\sdk::construct still doesn't get called).

We can wrap the class to get metrics to determine if it is worth investing effort in instrumenting, but as of now, the focus is on Client which gets associated directly with services.

For additional commentary on this, please see a36d635

1. added unit tests
   2 created to wrapped functions that
   a) one creates a supportability metric based off of the class name of the wrapped function that points to it.
   b) one generates version info and creates package and version metrics
2. Updated how version info was generated.

[lavarou]

Leaving this for posterity: https://github.com/aws/aws-sdk-php/blob/3.316.3/src/AwsClient.php#L373-L387

[lavarou]

Nice job!