From c5e0d2ab2e308560aff16ec7a452ac3e197f5a97 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Wed, 22 Jan 2025 15:49:16 -0700 Subject: [PATCH 1/5] feat(agent): Improve performance by delaying special segment creation. --- agent/lib_aws_sdk_php.c | 49 +++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c index b706df518..47a17bbf1 100644 --- a/agent/lib_aws_sdk_php.c +++ b/agent/lib_aws_sdk_php.c @@ -86,11 +86,13 @@ //clang-format on */ -void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* segment, +void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, char* command_name_string, size_t command_name_len, NR_EXECUTE_PROTO) { char* command_arg_value = NULL; + nr_segment_t* segment = NULL; + nr_segment_message_params_t message_params = { .library = SQS_LIBRARY_NAME, .destination_type = NR_MESSAGE_DESTINATION_TYPE_QUEUE, @@ -98,7 +100,7 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* segment, }; nr_segment_cloud_attrs_t cloud_attrs = {0}; - if (NULL == segment) { + if (NULL == auto_segment) { return; } @@ -122,6 +124,16 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* segment, } #undef AWS_COMMAND_IS + /* + * By this point, it's been determined that this call will be instrumented so + * only create the segment now, grab the parent segment start time, add our + * special segment attributes/metrics then close the newly created segment. + */ + segment = nr_segment_start(NRPRG(txn), NULL, NULL); + if (NULL == segment) { + return; + } + segment->start_time = auto_segment->start_time; cloud_attrs.aws_operation = command_name_string; command_arg_value = nr_lib_aws_sdk_php_get_command_arg_value( @@ -332,20 +344,6 @@ char* nr_lib_aws_sdk_php_get_command_arg_value(char* command_arg_name, * @throws \Exception */ -NR_PHP_WRAPPER(nr_aws_client_call_before) { - (void)wraprec; - nr_segment_t* segment = NULL; - /* - * Start a segment in case it needs to become an external, message, or - * datastore segment. - */ - segment = nr_segment_start(NRPRG(txn), NULL, NULL); - if (NULL != segment) { - segment->wraprec = auto_segment->wraprec; - } -} -NR_PHP_WRAPPER_END - NR_PHP_WRAPPER(nr_aws_client_call) { (void)wraprec; @@ -388,16 +386,6 @@ NR_PHP_WRAPPER(nr_aws_client_call) { #undef AWS_CLASS_IS - if (NR_SEGMENT_CUSTOM == auto_segment->type) { - /* - * We need to end the segment that we started in the 'before' wrapper if - * it wasn't handled and ended by the handling function. Handling - * function would have changed the segment type from from default - * (`NR_SEGMENT_CUSTOM`) if it ended it. - */ - nr_segment_discard(&auto_segment); - } - /* * Since we have klass and command_name, we can give the calling segment * a more meaningful name than Aws/AwsClient::__call We can decode it to @@ -406,11 +394,10 @@ NR_PHP_WRAPPER(nr_aws_client_call) { * EX: Aws\\Sqs\\SqsClient::sendMessage */ - segment = nr_txn_get_current_segment(NRPRG(txn), NULL); - if (NULL != segment) { + if (NULL != auto_segment) { real_class_and_command = nr_formatf("Custom/%s::%s", klass, command_name_string); - nr_segment_set_name(segment, real_class_and_command); + nr_segment_set_name(auto_segment, real_class_and_command); nr_free(real_class_and_command); } @@ -570,7 +557,7 @@ void nr_aws_sdk_php_enable() { /* We only support instrumentation above PHP 8.1 */ /* Called when a service command is issued from a Client */ nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Aws\\AwsClient::__call"), nr_aws_client_call_before, - nr_aws_client_call, nr_aws_client_call); + NR_PSTR("Aws\\AwsClient::__call"), NULL, nr_aws_client_call, + nr_aws_client_call); #endif } From 91f8e77ed8f472bfb471ba0ba5a8153f52b7a1a3 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Thu, 23 Jan 2025 11:31:09 -0700 Subject: [PATCH 2/5] Update agent/lib_aws_sdk_php.c Co-authored-by: Michal Nowacki --- agent/lib_aws_sdk_php.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c index 47a17bbf1..f5b1e7af3 100644 --- a/agent/lib_aws_sdk_php.c +++ b/agent/lib_aws_sdk_php.c @@ -129,7 +129,7 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, * only create the segment now, grab the parent segment start time, add our * special segment attributes/metrics then close the newly created segment. */ - segment = nr_segment_start(NRPRG(txn), NULL, NULL); + segment = nr_segment_start(NRPRG(txn), auto_segment, NULL); if (NULL == segment) { return; } From c7791f74acd92e33ffd7807b5ac11e7afb08e110 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Thu, 23 Jan 2025 13:27:32 -0700 Subject: [PATCH 3/5] Update agent/lib_aws_sdk_php.c Co-authored-by: Michal Nowacki --- agent/lib_aws_sdk_php.c | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c index f5b1e7af3..6377c1ce7 100644 --- a/agent/lib_aws_sdk_php.c +++ b/agent/lib_aws_sdk_php.c @@ -133,6 +133,7 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, if (NULL == segment) { return; } + /* re-use start time from auto_segment started in func_begin */ segment->start_time = auto_segment->start_time; cloud_attrs.aws_operation = command_name_string; From 33d40f166d8e2db8146409e92506d8080429d354 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Thu, 23 Jan 2025 13:36:07 -0700 Subject: [PATCH 4/5] style(agent): rename variable --- agent/lib_aws_sdk_php.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c index 6377c1ce7..841b68370 100644 --- a/agent/lib_aws_sdk_php.c +++ b/agent/lib_aws_sdk_php.c @@ -91,7 +91,7 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, size_t command_name_len, NR_EXECUTE_PROTO) { char* command_arg_value = NULL; - nr_segment_t* segment = NULL; + nr_segment_t* message_segment = NULL; nr_segment_message_params_t message_params = { .library = SQS_LIBRARY_NAME, @@ -129,12 +129,12 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, * only create the segment now, grab the parent segment start time, add our * special segment attributes/metrics then close the newly created segment. */ - segment = nr_segment_start(NRPRG(txn), auto_segment, NULL); - if (NULL == segment) { + message_segment = nr_segment_start(NRPRG(txn), auto_segment, NULL); + if (NULL == message_segment) { return; } /* re-use start time from auto_segment started in func_begin */ - segment->start_time = auto_segment->start_time; + message_segment->start_time = auto_segment->start_time; cloud_attrs.aws_operation = command_name_string; command_arg_value = nr_lib_aws_sdk_php_get_command_arg_value( @@ -149,10 +149,10 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, /* Add cloud attributes, if available. */ - nr_segment_traces_add_cloud_attributes(segment, &cloud_attrs); + nr_segment_traces_add_cloud_attributes(message_segment, &cloud_attrs); /* Now end the instrumented segment as a message segment. */ - nr_segment_message_end(&segment, &message_params); + nr_segment_message_end(&message_segment, &message_params); nr_free(command_arg_value); } From da4253ba401d3b0253f5507221105fcafdead127 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Fri, 24 Jan 2025 12:15:37 -0700 Subject: [PATCH 5/5] revert explicitly passing parent to nr_segment_start --- agent/lib_aws_sdk_php.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/lib_aws_sdk_php.c b/agent/lib_aws_sdk_php.c index 841b68370..1bd144cca 100644 --- a/agent/lib_aws_sdk_php.c +++ b/agent/lib_aws_sdk_php.c @@ -129,7 +129,7 @@ void nr_lib_aws_sdk_php_sqs_handle(nr_segment_t* auto_segment, * only create the segment now, grab the parent segment start time, add our * special segment attributes/metrics then close the newly created segment. */ - message_segment = nr_segment_start(NRPRG(txn), auto_segment, NULL); + message_segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (NULL == message_segment) { return; }