Skip to content

Commit 6500c76

Browse files
authored
feat(agent): Add basic aws-sdk-php intrumentation (#932)
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.
1 parent 8fd1602 commit 6500c76

File tree

7 files changed

+382
-1
lines changed

7 files changed

+382
-1
lines changed

agent/Makefile.frag

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ TEST_BINARIES = \
8888
tests/test_globals \
8989
tests/test_internal_instrument \
9090
tests/test_hash \
91+
tests/test_lib_aws_sdk_php \
9192
tests/test_mongodb \
9293
tests/test_monolog \
9394
tests/test_mysql \

agent/config.m4

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ if test "$PHP_NEWRELIC" = "yes"; then
228228
fw_silex.c fw_slim.c fw_support.c fw_symfony4.c fw_symfony2.c \
229229
fw_symfony.c fw_symfony_common.c fw_wordpress.c fw_yii.c \
230230
fw_zend2.c fw_zend.c"
231-
LIBRARIES="lib_monolog.c lib_doctrine2.c lib_guzzle3.c \
231+
LIBRARIES="lib_aws_sdk_php.c lib_monolog.c lib_doctrine2.c lib_guzzle3.c \
232232
lib_guzzle4.c lib_guzzle6.c lib_guzzle_common.c \
233233
lib_mongodb.c lib_phpunit.c lib_predis.c lib_zend_http.c"
234234
PHP_NEW_EXTENSION(newrelic, $FRAMEWORKS $LIBRARIES $NEWRELIC_AGENT, $ext_shared,, \\$(NEWRELIC_CFLAGS))

agent/fw_hooks.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ extern void nr_zend_enable(TSRMLS_D);
4545
extern void nr_fw_zend2_enable(TSRMLS_D);
4646

4747
/* Libraries. */
48+
extern void nr_aws_sdk_php_enable();
4849
extern void nr_doctrine2_enable(TSRMLS_D);
4950
extern void nr_guzzle3_enable(TSRMLS_D);
5051
extern void nr_guzzle4_enable(TSRMLS_D);

agent/lib_aws_sdk_php.c

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Copyright 2024 New Relic Corporation. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
/*
7+
* Functions relating to instrumenting the AWS-SDK-PHP.
8+
* https://github.com/aws/aws-sdk-php
9+
*/
10+
#include "php_agent.h"
11+
#include "php_call.h"
12+
#include "php_hash.h"
13+
#include "php_wrapper.h"
14+
#include "fw_hooks.h"
15+
#include "fw_support.h"
16+
#include "util_logging.h"
17+
#include "lib_aws_sdk_php.h"
18+
19+
#define PHP_PACKAGE_NAME "aws/aws-sdk-php"
20+
21+
/*
22+
* In a normal course of events, the following line will always work
23+
* zend_eval_string("Aws\\Sdk::VERSION;", &retval, "Get AWS Version")
24+
* By the time we have detected the existence of the aws-sdk-php and with
25+
* default composer profject settings, it callable even from
26+
* nr_aws_sdk_php_enable which will automatically load the class if it isn't
27+
* loaded yet and then evaluate the string. However, in the rare case that files
28+
* are not loaded via autoloader and/or have non-default composer classload
29+
* settings, if the class is not found, PHP 8.2+ will generate a fatal
30+
* unrecoverable uncatchable error error whenever it cannot find a class. While
31+
* calling this from nr_aws_sdk_php_enable would have been great and would allow
32+
* the sdk version value to be set only once, to avoid the very unlikely but not
33+
* impossible fatal error, this will be called from the
34+
* "Aws\\ClientResolver::_apply_user_agent" wrapper which GUARANTEES that
35+
* aws/sdk exists and is already loaded.
36+
*
37+
*
38+
* Additionally given that aws-sdk-php is currently detected from the
39+
* AwsClient.php file, this method will always be called when a client is
40+
* created unlike Sdk::construct which doesn't show with PHP 8.2+.
41+
*
42+
* Using Aws/Sdk::__construct for version is currently nonviable as it is
43+
* unreliable as a version determiner.
44+
* Having separate functionality to extract from Aws/Sdk::__construct
45+
* is both not required and is redundant and causes additional overhead and
46+
* so only one function is needed to extract version.
47+
*
48+
* Aws\\ClientResolver::_apply_user_agent a reliable function as it is
49+
* always called on client initialization since it is key to populating
50+
* the request headers, and it loads Sdk by default.
51+
*
52+
* Concerns about future/past proofing to the checking prioritized the following
53+
* implementation vs using the eval method.
54+
*/
55+
void nr_lib_aws_sdk_php_handle_version() {
56+
zval* zval_version = NULL;
57+
zend_class_entry* class_entry = NULL;
58+
char* version = NULL;
59+
60+
class_entry = nr_php_find_class("aws\\sdk");
61+
if (NULL != class_entry) {
62+
zval_version = nr_php_get_class_constant(class_entry, "VERSION");
63+
64+
if (nr_php_is_zval_non_empty_string(zval_version)) {
65+
version = Z_STRVAL_P(zval_version);
66+
}
67+
}
68+
if (NRINI(vulnerability_management_package_detection_enabled)) {
69+
/* Add php package to transaction */
70+
nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
71+
}
72+
nr_fw_support_add_package_supportability_metric(NRPRG(txn), PHP_PACKAGE_NAME,
73+
version);
74+
nr_php_zval_free(&zval_version);
75+
}
76+
77+
void nr_lib_aws_sdk_php_add_supportability_service_metric(
78+
const char* service_name) {
79+
/* total MAX metric name length per agent-specs */
80+
char buf[MAX_METRIC_NAME_LEN];
81+
char* cp = NULL;
82+
83+
if (nr_strempty(service_name)) {
84+
return;
85+
}
86+
if (NULL == NRPRG(txn)) {
87+
return;
88+
}
89+
90+
cp = buf;
91+
strcpy(cp, PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX);
92+
cp += PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX_LEN - 1;
93+
strlcpy(cp, service_name, MAX_AWS_SERVICE_NAME_LEN);
94+
nrm_force_add(NRPRG(txn) ? NRTXN(unscoped_metrics) : 0, buf, 0);
95+
}
96+
97+
NR_PHP_WRAPPER(nr_create_aws_sdk_version_metrics) {
98+
(void)wraprec;
99+
NR_PHP_WRAPPER_CALL;
100+
nr_lib_aws_sdk_php_handle_version();
101+
}
102+
NR_PHP_WRAPPER_END
103+
104+
/*
105+
* AwsClient::parseClass
106+
* This is called from the base AwsClient class for every client associated
107+
* with a service during client initialization.
108+
* parseClass already computes the service name for internal use, so we don't
109+
* need to store it, we just need to snag it from the return value as it goes
110+
* through the client initialization process.
111+
*/
112+
NR_PHP_WRAPPER(nr_create_aws_service_metric) {
113+
(void)wraprec;
114+
115+
zval** ret_val_ptr = NULL;
116+
ret_val_ptr = NR_GET_RETURN_VALUE_PTR;
117+
118+
NR_PHP_WRAPPER_CALL;
119+
120+
if (NULL != ret_val_ptr && nr_php_is_zval_valid_array(*ret_val_ptr)) {
121+
/* obtain ret_val_ptr[0] which contains the service name */
122+
zval* service_name
123+
= nr_php_zend_hash_index_find(Z_ARRVAL_P(*ret_val_ptr), 0);
124+
if (nr_php_is_zval_non_empty_string(service_name)) {
125+
nr_lib_aws_sdk_php_add_supportability_service_metric(
126+
Z_STRVAL_P(service_name));
127+
}
128+
}
129+
}
130+
NR_PHP_WRAPPER_END
131+
132+
/*
133+
* The ideal file to begin immediate detection of the aws-sdk is:
134+
* aws-sdk-php/src/functions.php
135+
* Unfortunately, Php8.2+ and composer autoload leads to the
136+
* file being optimized directly and not loaded.
137+
*
138+
* Options considered:
139+
*
140+
* 1. for PHP8.2, and only optimizable libraries, when encountering autoload.php
141+
* files, ask the file what includes it added and check against only the
142+
* optimizable library. Small overhead incurred when encountering an autoload
143+
* file, but detects aws-sdk-php immediately before any sdk code executes
144+
* (changes needed for this are detailed in the original PR)
145+
* 2. use a file that gets called later and only when AwsClient.php file file is
146+
* called. It's called later and we'll miss some instrumentation, but if we're
147+
* only ever going to be interested in Client calls anyway, maybe that's ok?
148+
* Doesn't detect Sdk.php (optimized out) so when customers only use that or
149+
* when they use it first, we will not instrument it. This only detects when a
150+
* Client is called to use a service so potentially misses out on other
151+
* instrumentation and misses out when customers use the aws-sdk-php but use
152+
* non-SDK way to interact with the service (possibly with redis/memcached).
153+
* This way is definitely the least complex and lowest overhead and less
154+
* complexity means lower risk as well.
155+
* 3. Directly add the wrappers to the hash map. With potentially 50ish clients
156+
* to wrap, this will add overhead to every hash map lookup. Currently
157+
* implemented option is 2, use the AwsClient.php as this is our main focus.
158+
* This means until a call to an Aws/AwsClient function,
159+
* all calls including aws\sdk calls are ignored. Version detection will be
160+
* tied to Aws/ClientResolver::_apply_user_agent which is ALWAYS called when
161+
* dealing with aws clients. It will not be computed from
162+
* Aws/Sdk::__constructor which would at best be duplicate info and worst would
163+
* never be ignored until a client is called.
164+
*/
165+
void nr_aws_sdk_php_enable() {
166+
/* This will be used to extract the version. */
167+
nr_php_wrap_user_function(NR_PSTR("Aws\\ClientResolver::_apply_user_agent"),
168+
nr_create_aws_sdk_version_metrics);
169+
/* Called when initializing all other Clients */
170+
nr_php_wrap_user_function(NR_PSTR("Aws\\AwsClient::parseClass"),
171+
nr_create_aws_service_metric);
172+
173+
if (NRINI(vulnerability_management_package_detection_enabled)) {
174+
nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME,
175+
PHP_PACKAGE_VERSION_UNKNOWN);
176+
}
177+
}

agent/lib_aws_sdk_php.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2024 New Relic Corporation. All rights reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* Functions relating to instrumenting AWS-SDK-PHP.
6+
*/
7+
#ifndef LIB_AWS_SDK_PHP_HDR
8+
#define LIB_AWS_SDK_PHP_HDR
9+
10+
#define PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX \
11+
"Supportability/PHP/AWS/Services/"
12+
#define MAX_METRIC_NAME_LEN 256
13+
#define PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX_LEN \
14+
sizeof(PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX)
15+
#define MAX_AWS_SERVICE_NAME_LEN \
16+
(MAX_METRIC_NAME_LEN - PHP_AWS_SDK_SERVICE_NAME_METRIC_PREFIX_LEN)
17+
18+
extern void nr_aws_sdk_php_enable();
19+
extern void nr_lib_aws_sdk_php_handle_version();
20+
extern void nr_lib_aws_sdk_php_add_supportability_service_metric(
21+
const char* service_name);
22+
23+
#endif /* LIB_AWS_SDK_PHP_HDR */

agent/php_execute.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,9 @@ typedef struct _nr_library_table_t {
484484
*/
485485
// clang-format: off
486486
static nr_library_table_t libraries[] = {
487+
/* AWS-SDK-PHP 3 */
488+
{"AWS-SDK-PHP", NR_PSTR("aws-sdk-php/src/awsclient.php"), nr_aws_sdk_php_enable},
489+
487490
/* Doctrine < 2.18 */
488491
{"Doctrine 2", NR_PSTR("doctrine/orm/query.php"), nr_doctrine2_enable},
489492
/* Doctrine 2.18 reworked the directory structure */

0 commit comments

Comments
 (0)