Skip to content

fix(agent): address the aws-sdk-php versioning change #1001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 20, 2024
Merged

fix(agent): address the aws-sdk-php versioning change #1001

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bea8d5a
fix(agent): address that aws-sdk-php versioning changed
zsistla Dec 19, 2024
4804f93
fix(agent): Use lambda function instead of 2 calls
zsistla Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 43 additions & 57 deletions agent/lib_aws_sdk_php.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
Loading