Skip to content

Commit 8c16b70

Browse files
authored
fix(agent): Slim txn naming updates (#917)
1) the scheme for route run to correlate with proper OAPI naming was incorrectly listed and therefore txn naming precedence was incorrectly handled 2) this PR kept same naming scheme in route:run putting it before wrapper_call and saying ok to overwrite (instead of after and not ok to overwrite) 3) number 2 needed to incorporate fallback naming scheme 4) added fallback naming scheme (since some errors and middleware intervention give the wrong txn name) using dispatcher:dispatch. When there is an exception thrown in a route that is handled by the slim routing and error middleware, it is superseded by: Uncaught exception 'Slim\\Exception\\HttpNotFoundException' with message 'Not found.' and transformed into an http 404 error and is shown in the http.statuscode field as 404. If the middleware isn't loaded, it is recorded as an uncaught exception and status code is 200. Wrap function Slim\\Routing\\Dispatcher::dispatch It will name set the txn_name in case we hit an error and Slim\\Routing\\Route::run isn't called. It is set to be overwritable do that if we encounter Slim\\Routing\\Route::run, it can overwrite with the most accurate info (this is only relevant in the case of redirects). This allows us to give the txn_name even in error cases when Slim\\Routing\\Route::run isn't called. Corresponds with multiverse updates which passed against this branch.
1 parent cf3b2c3 commit 8c16b70

File tree

1 file changed

+68
-9
lines changed

1 file changed

+68
-9
lines changed

agent/fw_slim.c

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,28 +81,65 @@ NR_PHP_WRAPPER(nr_slim2_route_dispatch) {
8181
NR_PHP_WRAPPER_END
8282

8383
/*
84-
* Wrap the \Slim\Route::run method, which is the happy path for Slim routing.
84+
* Wrap the Slim 3\Slim\Route::run method
85+
* and
86+
* Slim 4 Slim\\Routing\\Route::run
87+
* which are the happy paths for Slim 3/4 routing.
8588
* i.e. The router has succesfully matched the URL and dispatched the request
8689
* to a route.
8790
*
88-
* In this case, `nr_txn_set_path` is called after `NR_PHP_WRAPPER_CALL` with
89-
* `NR_OK_TO_OVERWRITE` and as this corresponds to calling the wrapped function
90-
* in func_end no change is needed to ensure OAPI compatibility as it will use
91-
* the default func_end after callback. This entails that the first wrapped
92-
* function call of this type gets to name the txn.
91+
* In this case, `nr_txn_set_path` is called before `NR_PHP_WRAPPER_CALL` with
92+
* `NR_OK_TO_OVERWRITE` and as this corresponds to calling the last wrapped
93+
* function call of this type gets to name the txn; therefore needs a before
94+
* call for OAPI.
9395
*/
9496
NR_PHP_WRAPPER(nr_slim3_4_route_run) {
9597
zval* this_var = NULL;
9698
char* txn_name = NULL;
9799

98100
(void)wraprec;
101+
99102
NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_SLIM);
100103

101104
this_var = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
102105
txn_name = nr_slim_path_from_route(this_var TSRMLS_CC);
103106
nr_php_scope_release(&this_var);
104107

108+
if (txn_name) {
109+
nr_txn_set_path("Slim", NRPRG(txn), txn_name, NR_PATH_TYPE_ACTION,
110+
NR_OK_TO_OVERWRITE);
111+
nr_free(txn_name);
112+
}
113+
105114
NR_PHP_WRAPPER_CALL;
115+
}
116+
NR_PHP_WRAPPER_END
117+
118+
/*
119+
* public function dispatch(string $method, string $uri): RoutingResults
120+
* This is fallback naming mechanism for Slim 4 routing when the Slim 4
121+
* Slim\\Routing\\Route::run does not run due middlware intervening on
122+
* certain errors.
123+
* In this case, `nr_txn_set_path` is called before `NR_PHP_WRAPPER_CALL` with
124+
* `NR_NOT_OK_TO_OVERWRITE` and as this corresponds to calling the first wrapped
125+
* function in func_begin.
126+
*/
127+
NR_PHP_WRAPPER(nr_slim4_route_dispatch) {
128+
char* txn_name = NULL;
129+
zval* route_name = NULL;
130+
131+
(void)wraprec;
132+
133+
NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_SLIM);
134+
135+
/* Get the route name. The first arg is the method, 2nd arg is routename. */
136+
route_name = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS);
137+
138+
if (nr_php_is_zval_valid_string(route_name)) {
139+
txn_name = nr_strndup(Z_STRVAL_P(route_name), Z_STRLEN_P(route_name));
140+
}
141+
142+
nr_php_arg_release(&route_name);
106143

107144
if (txn_name) {
108145
nr_txn_set_path("Slim", NRPRG(txn), txn_name, NR_PATH_TYPE_ACTION,
@@ -120,7 +157,7 @@ NR_PHP_WRAPPER(nr_slim_application_construct) {
120157
(void)wraprec;
121158

122159
version = nr_php_get_object_constant(this_var, "VERSION");
123-
160+
124161
if (NRINI(vulnerability_management_package_detection_enabled)) {
125162
// Add php package to transaction
126163
nr_txn_add_php_package(NRPRG(txn), PHP_PACKAGE_NAME, version);
@@ -139,12 +176,34 @@ void nr_slim_enable(TSRMLS_D) {
139176

140177
nr_php_wrap_user_function(NR_PSTR("Slim\\Route::dispatch"),
141178
nr_slim2_route_dispatch TSRMLS_CC);
179+
180+
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
181+
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
182+
142183
/* Slim 3 */
143-
nr_php_wrap_user_function(NR_PSTR("Slim\\Route::run"),
144-
nr_slim3_4_route_run TSRMLS_CC);
184+
nr_php_wrap_user_function_before_after_clean(
185+
NR_PSTR("Slim\\Route::run"), nr_slim3_4_route_run, NULL, NULL);
186+
145187
/* Slim 4 */
188+
nr_php_wrap_user_function_before_after_clean(
189+
NR_PSTR("Slim\\Routing\\Route::run"), nr_slim3_4_route_run, NULL, NULL);
190+
191+
/* Slim 4 */
192+
nr_php_wrap_user_function_before_after_clean(
193+
NR_PSTR("Slim\\Routing\\Dispatcher::dispatch"), nr_slim4_route_dispatch,
194+
NULL, NULL);
195+
#else
196+
/* Slim 4*/
146197
nr_php_wrap_user_function(NR_PSTR("Slim\\Routing\\Route::run"),
147198
nr_slim3_4_route_run TSRMLS_CC);
199+
/* Slim 4 */
200+
nr_php_wrap_user_function(NR_PSTR("Slim\\Routing\\Dispatcher::dispatch"),
201+
nr_slim4_route_dispatch TSRMLS_CC);
202+
203+
/* Slim 3 */
204+
nr_php_wrap_user_function(NR_PSTR("Slim\\Route::run"),
205+
nr_slim3_4_route_run TSRMLS_CC);
206+
#endif
148207

149208
/* Slim 2 does not have the same path as Slim 3/4 which is why
150209
we need to separate these*/

0 commit comments

Comments
 (0)