From a2ad43bae55356ce5c58df7b19d743b8e6dcb974 Mon Sep 17 00:00:00 2001 From: Hitesh Ahuja Date: Tue, 3 Sep 2024 11:36:52 -0700 Subject: [PATCH] chore(axiom): create nr_user_error_t struct --- agent/php_api.c | 41 +++---- axiom/nr_errors.c | 65 ++++------- axiom/nr_errors.h | 49 +++------ axiom/nr_errors_private.h | 5 +- axiom/nr_segment.c | 22 +--- axiom/nr_segment.h | 10 +- axiom/nr_segment_private.c | 4 +- axiom/nr_txn.c | 101 +++++++++--------- axiom/nr_txn.h | 7 +- .../test_transaction_additional_params2.php | 4 +- 10 files changed, 126 insertions(+), 182 deletions(-) diff --git a/agent/php_api.c b/agent/php_api.c index e0371f513..5500e5aa2 100644 --- a/agent/php_api.c +++ b/agent/php_api.c @@ -11,6 +11,7 @@ #include "php_hash.h" #include "php_user_instrument.h" #include "fw_drupal_common.h" +#include "nr_errors.h" #include "nr_rum.h" #include "util_logging.h" #include "util_memory.h" @@ -71,16 +72,17 @@ PHP_FUNCTION(newrelic_notice_error) { const char* errclass = "NoticedError"; char* errormsgstr = NULL; nr_string_len_t errormsglen = 0; - zend_long error_number = 0; - char* error_file = NULL; - nr_string_len_t error_file_len = 0; - zend_long error_line = 0; + char* user_error_message = NULL; + nr_string_len_t user_error_message_len = 0; + zend_long user_error_number = 0; + char* user_error_file = NULL; + nr_string_len_t user_error_file_len = 0; + zend_long user_error_line = 0; zval* exc = NULL; - nr_string_len_t error_context_len = 0; - char* error_context = NULL; + nr_string_len_t user_error_context_len = 0; + char* user_error_context = NULL; int priority = 0; - zval* ignore = NULL; - bool determine = false; + bool five_params = false; NR_UNUSED_RETURN_VALUE; NR_UNUSED_RETURN_VALUE_PTR; @@ -134,11 +136,11 @@ PHP_FUNCTION(newrelic_notice_error) { case 2: if (FAILURE == zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, - ZEND_NUM_ARGS() TSRMLS_CC, "zo!", &ignore, + ZEND_NUM_ARGS() TSRMLS_CC, "s!o!", &user_error_message, &user_error_message_len, &exc)) { nrl_debug(NRL_API, "newrelic_notice_error: invalid two arguments: expected " - "exception as second argument"); + "string as first argument and exception as second argument"); RETURN_NULL(); } break; @@ -147,13 +149,13 @@ PHP_FUNCTION(newrelic_notice_error) { if (FAILURE == zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS() TSRMLS_CC, "ls!s!ls!", - &error_number, &errormsgstr, &errormsglen, - &error_file, &error_file_len, &error_line, - &error_context, &error_context_len)) { + &user_error_number, &user_error_message, &user_error_message_len, + &user_error_file, &user_error_file_len, &user_error_line, + &user_error_context, &user_error_context_len)) { nrl_debug(NRL_API, "newrelic_notice_error: invalid five arguments"); RETURN_NULL(); } - determine = true; + five_params = true; break; default: @@ -172,11 +174,14 @@ PHP_FUNCTION(newrelic_notice_error) { } } - if (determine) { + if (five_params) { + nr_user_error_t* user_error = nr_user_error_create( + user_error_message, user_error_number, user_error_file, user_error_line, + user_error_context); char* stack_json = nr_php_backtrace_to_json(NULL TSRMLS_CC); - nr_txn_record_error_with_additional_attributes( - NRPRG(txn), priority, true, errormsgstr, errclass, error_file, - error_line, error_context, error_number, stack_json); + nr_txn_record_error_with_additional_attributes(NRPRG(txn), priority, true, + user_error_message, errclass, + stack_json, user_error); nr_free(stack_json); RETURN_TRUE; } diff --git a/axiom/nr_errors.c b/axiom/nr_errors.c index 7dac85976..4ec694372 100644 --- a/axiom/nr_errors.c +++ b/axiom/nr_errors.c @@ -23,17 +23,13 @@ nr_error_t* nr_error_create(int priority, if (NULL == message || NULL == klass || NULL == stacktrace_json) { return 0; } - return nr_error_create_additional_params(priority, message, klass, NULL, 0, NULL, 0, - stacktrace_json, span_id, when); + return nr_error_create_additional_params(priority, message, klass, NULL, stacktrace_json, span_id, when); } nr_error_t* nr_error_create_additional_params(int priority, const char* message, const char* klass, - const char* error_file, - int error_line, - const char* error_context, - int error_no, + nr_user_error_t* user_error, const char* stacktrace_json, const char* span_id, nrtime_t when) { @@ -47,63 +43,47 @@ nr_error_t* nr_error_create_additional_params(int priority, error->when = when; error->klass = nr_strdup(klass); error->stacktrace_json = nr_strdup(stacktrace_json); - error->error_line = error_line; - error->error_no = error_no; + error->user_error = user_error; if (NULL != message) { error->message = nr_strdup(message); } - if (NULL != error_file) { - error->error_file = nr_strdup(error_file); - } - if (NULL != error_context) { - error->error_context = nr_strdup(error_context); - } if (NULL != span_id) { error->span_id = nr_strdup(span_id); } return error; } -const char* nr_error_get_message(const nr_error_t* error) { - if (NULL == error) { - return NULL; - } - return error->message; +nr_user_error_t* nr_user_error_create(const char* user_error_message, int user_error_number, const char* user_error_file, int user_error_line, const char* user_error_context) { + nr_user_error_t* user_error = (nr_user_error_t*)nr_malloc(sizeof(nr_user_error_t)); + user_error->user_error_message = nr_strdup(user_error_message); + user_error->user_error_number = user_error_number; + user_error->user_error_file = nr_strdup(user_error_file); + user_error->user_error_line = user_error_line; + user_error->user_error_context = nr_strdup(user_error_context); + return user_error; } -const char* nr_error_get_klass(const nr_error_t* error) { - if (NULL == error) { - return NULL; +void nr_user_error_destroy(nr_user_error_t* user_error) { + if (NULL != user_error) { + nr_free(user_error->user_error_message); + nr_free(user_error->user_error_file); + nr_free(user_error->user_error_context); + nr_free(user_error); } - return error->klass; } -const char* nr_error_get_file(const nr_error_t* error) { +const char* nr_error_get_message(const nr_error_t* error) { if (NULL == error) { return NULL; } - return error->error_file; -} - -int nr_error_get_line(const nr_error_t* error) { - if (NULL == error) { - return 0; - } - return error->error_line; + return error->message; } -const char* nr_error_get_context(const nr_error_t* error) { +const char* nr_error_get_klass(const nr_error_t* error) { if (NULL == error) { return NULL; } - return error->error_context; -} - -int nr_error_get_no(const nr_error_t* error) { - if (NULL == error) { - return 0; - } - return error->error_no; + return error->klass; } nrtime_t nr_error_get_time(const nr_error_t* error) { @@ -141,8 +121,7 @@ void nr_error_destroy(nr_error_t** error_ptr) { nr_free(error->klass); nr_free(error->span_id); nr_free(error->stacktrace_json); - nr_free(error->error_file); - nr_free(error->error_context); + nr_user_error_destroy(error->user_error); nr_realfree((void**)error_ptr); } diff --git a/axiom/nr_errors.h b/axiom/nr_errors.h index ffe215fa5..4b6275b16 100644 --- a/axiom/nr_errors.h +++ b/axiom/nr_errors.h @@ -17,6 +17,14 @@ */ typedef struct _nr_error_t nr_error_t; +typedef struct _nr_user_error_t { + char* user_error_message; /* User error message */ + char* user_error_file; /* User error file */ + char* user_error_context; /* User error context */ + int user_error_line; /* User error line */ + int user_error_number; /* User error number */ +} nr_user_error_t; + /* * Purpose : Create a new error. * @@ -38,6 +46,14 @@ extern nr_error_t* nr_error_create(int priority, const char* span_id, nrtime_t when); +extern void nr_user_error_destroy(nr_user_error_t* user_error_ptr); + +extern nr_user_error_t* nr_user_error_create(const char* user_error_message, + int user_error_number, + const char* user_error_file, + int user_error_line, + const char* user_error_context); + /* * Purpose : Create a new error for the use case where additional parameters are * passed in. The following parameters are required and can not be NULL: klass @@ -64,10 +80,7 @@ extern nr_error_t* nr_error_create_additional_params( int priority, const char* message, const char* klass, - const char* error_file, - int error_line, - const char* error_context, - int error_no, + nr_user_error_t* user_error, const char* stacktrace_json, const char* span_id, nrtime_t when); @@ -89,34 +102,6 @@ extern const char* nr_error_get_message(const nr_error_t* error); */ extern const char* nr_error_get_klass(const nr_error_t* error); -/* - * Purpose : Get the error file of an error. - * - * Returns : The error file of the error or NULL if not defined. - */ -extern const char* nr_error_get_file(const nr_error_t* error); - -/* - * Purpose : Get the error line of an error. - * - * Returns : The error line of the error or 0 if not defined. - */ -extern int nr_error_get_line(const nr_error_t* error); - -/* - * Purpose : Get the error context of an error. - * - * Returns : The error context of the error or NULL if not defined. - */ -extern const char* nr_error_get_context(const nr_error_t* error); - -/* - * Purpose : Get the error number of an error. - * - * Returns : The error number of the error or 0 if not defined. - */ -extern int nr_error_get_no(const nr_error_t* error); - /* * Purpose : Get the span_id of an error. * diff --git a/axiom/nr_errors_private.h b/axiom/nr_errors_private.h index 234c8d929..4b6029b86 100644 --- a/axiom/nr_errors_private.h +++ b/axiom/nr_errors_private.h @@ -25,13 +25,10 @@ struct _nr_error_t { int priority; /* Error priority - lowest to highest */ char* message; /* Error message */ char* klass; /* Error class */ - char* error_file; /* User provided error file */ - int error_line; /* User provided error line */ - char* error_context; /* User provided error context */ - int error_no; /* User provided error number */ char* stacktrace_json; /* Stack trace in JSON format */ char* span_id; /* ID of the current executing span at the time the error occurred */ + nr_user_error_t* user_error; }; #endif /* NR_ERRORS_PRIVATE_HDR */ diff --git a/axiom/nr_segment.c b/axiom/nr_segment.c index 6848e97bd..2b0c38bc2 100644 --- a/axiom/nr_segment.c +++ b/axiom/nr_segment.c @@ -1176,17 +1176,13 @@ void nr_segment_set_error(nr_segment_t* segment, if ((NULL == segment) || (NULL == error_message && NULL == error_class)) { return; } - nr_segment_set_error_with_additional_params(segment, error_message, error_class, NULL, 0, - NULL, 0); + nr_segment_set_error_with_additional_params(segment, error_message, error_class, NULL); } void nr_segment_set_error_with_additional_params(nr_segment_t* segment, const char* error_message, const char* error_class, - const char* error_file, - int error_line, - char* error_context, - int error_no) { + nr_user_error_t* user_error) { if (NULL == segment || NULL == error_class) { return; } @@ -1197,21 +1193,13 @@ void nr_segment_set_error_with_additional_params(nr_segment_t* segment, nr_free(segment->error->error_message); nr_free(segment->error->error_class); - nr_free(segment->error->error_file); - nr_free(segment->error->error_context); + nr_free(segment->error->user_error); - segment->error->error_class = error_class ? nr_strdup(error_class) : NULL; - segment->error->error_no = error_no; - segment->error->error_line = error_line; if (NULL != error_message) { segment->error->error_message = error_message ? nr_strdup(error_message) : NULL; } - if (NULL != error_file) { - segment->error->error_file = error_file ? nr_strdup(error_file) : NULL; - } - if (NULL != error_context) { - segment->error->error_context = error_context ? nr_strdup(error_context) : NULL; - } + segment->error->error_class = error_class ? nr_strdup(error_class) : NULL; + segment->error->user_error = user_error; } bool nr_segment_attributes_user_add(nr_segment_t* segment, diff --git a/axiom/nr_segment.h b/axiom/nr_segment.h index 3aadccb4d..33ab204e5 100644 --- a/axiom/nr_segment.h +++ b/axiom/nr_segment.h @@ -117,10 +117,7 @@ typedef struct _nr_segment_metric_t { typedef struct _nr_segment_error_t { char* error_message; /* The error message that will appear on a span event. */ char* error_class; /* The error class that will appear on a span event. */ - char* error_file; /* The error file that will appear on a span event. */ - int error_line; /* The error line that will appear on a span event. */ - char* error_context; /* The error context that will appear on a span event. */ - int error_no; /* The error number that will appear on a span event. */ + nr_user_error_t* user_error; /* The user error that will appear on a span event.*/ } nr_segment_error_t; /* @@ -668,10 +665,7 @@ extern void nr_segment_set_error_with_additional_params( nr_segment_t* segment, const char* error_message, const char* error_class, - const char* errfile, - int errline, - char* errcontext, - int error_no); + nr_user_error_t* user_error); /* * Purpose : Gets the child_ix of a segment. diff --git a/axiom/nr_segment_private.c b/axiom/nr_segment_private.c index 63091bdc4..53f988f93 100644 --- a/axiom/nr_segment_private.c +++ b/axiom/nr_segment_private.c @@ -5,6 +5,7 @@ #include "nr_axiom.h" +#include "nr_errors.h" #include "nr_segment_private.h" #include "nr_segment.h" #include "nr_txn.h" @@ -87,7 +88,6 @@ void nr_segment_error_destroy_fields(nr_segment_error_t* segment_error) { nr_free(segment_error->error_message); nr_free(segment_error->error_class); - nr_free(segment_error->error_file); - nr_free(segment_error->error_context); + nr_user_error_destroy(segment_error->user_error); nr_free(segment_error); } diff --git a/axiom/nr_txn.c b/axiom/nr_txn.c index 557d7afd4..284fe1297 100644 --- a/axiom/nr_txn.c +++ b/axiom/nr_txn.c @@ -14,6 +14,8 @@ #include "nr_agent.h" #include "nr_commands.h" #include "nr_custom_events.h" +#include "nr_errors.h" +#include "nr_errors_private.h" #include "nr_guid.h" #include "nr_header.h" #include "nr_limits.h" @@ -1558,11 +1560,8 @@ void nr_txn_record_error_with_additional_attributes( bool add_to_current_segment, const char* error_message, const char* error_class, - const char* error_file, - int error_line, - char* error_context, - int error_no, - const char* stacktrace_json) { + const char* stacktrace_json, + nr_user_error_t* user_error) { nr_segment_t* current_segment = NULL; char* span_id = NULL; nr_error_t* error = NULL; @@ -1606,23 +1605,27 @@ void nr_txn_record_error_with_additional_attributes( current_segment = nr_txn_get_current_segment(txn, NULL); if (current_segment) { - nr_segment_set_error_with_additional_params( - current_segment, error_message, error_class, error_file, - error_line, error_context, error_no); + if (NULL == user_error) { + nr_segment_set_error(current_segment, error_message, error_class); nrl_verbosedebug(NRL_TXN, "recording segment error: msg='%.48s' cls='%.48s' " - "file='%.48s' line='%d' context='%.48s' num='%d'" "span_id='%.48s'", NRSAFESTR(error_message), NRSAFESTR(error_class), - NRSAFESTR(error_file), error_line, - NRSAFESTR(error_context), error_no, NRSAFESTR(span_id)); + } else { + nr_segment_set_error_with_additional_params( + current_segment, error_message, error_class, user_error); + } } } } - error = nr_error_create_additional_params( - priority, error_message, error_class, error_file, error_line, - error_context, error_no, stacktrace_json, span_id, nr_get_time()); + if (NULL == user_error) { + error = nr_error_create(priority, error_message, error_class, stacktrace_json, + span_id, nr_get_time()); + } else { + error = nr_error_create_additional_params( + priority, error_message, error_class, user_error, stacktrace_json, span_id, nr_get_time()); + } /* * Ensure previous error is destroyed only we have a valid one to replace it @@ -1662,8 +1665,7 @@ void nr_txn_record_error(nrtxn_t* txn, return; } nr_txn_record_error_with_additional_attributes( - txn, priority, add_to_current_segment, errmsg, errclass, NULL, 0, NULL, 0, - stacktrace_json); + txn, priority, add_to_current_segment, errmsg, errclass, stacktrace_json, NULL); } char* nr_txn_create_fn_supportability_metric(const char* function_name, @@ -2475,6 +2477,34 @@ static void nr_txn_add_metric_count_as_attribute(nrobj_t* attributes, } } +static nrobj_t* nr_error_user_attributes_to_nro(nr_user_error_t* user_error, nrobj_t* user_attributes) { + if (NULL == user_error) { + return user_attributes; + } + + if (user_error->user_error_message) { + nro_set_hash_string(user_attributes, "user.error.message", + user_error->user_error_message); + } + if (user_error->user_error_file) { + nro_set_hash_string(user_attributes, "user.error.file", + user_error->user_error_file); + } + if (user_error->user_error_context) { + nro_set_hash_string(user_attributes, "user.error.context", + user_error->user_error_context); + } + if (user_error->user_error_number) { + nro_set_hash_int(user_attributes, "user.error.number", + user_error->user_error_number); + } + if (user_error->user_error_line) { + nro_set_hash_int(user_attributes, "user.error.line", + user_error->user_error_line); + } + return user_attributes; +} + /* * This implements the agent Error Events spec: * We only omit 'gcCumulative' which doesn't apply and 'port' which is too @@ -2556,41 +2586,12 @@ nr_analytics_event_t* nr_error_to_event(const nrtxn_t* txn) { NR_ATTRIBUTE_DESTINATION_ERROR); user_attributes = nr_attributes_user_to_obj(txn->attributes, NR_ATTRIBUTE_DESTINATION_ERROR); - if (nr_error_get_message(txn->error)) { - if (NULL == user_attributes) { - user_attributes = nro_new_hash(); - } - nro_set_hash_string(user_attributes, "user.error.message", - nr_error_get_message(txn->error)); - } - if (nr_error_get_file(txn->error)) { - if (NULL == user_attributes) { - user_attributes = nro_new_hash(); - } - nro_set_hash_string(user_attributes, "user.error.file", - nr_error_get_file(txn->error)); - } - if (nr_error_get_context(txn->error)) { - if (NULL == user_attributes) { - user_attributes = nro_new_hash(); - } - nro_set_hash_string(user_attributes, "user.error.context", - nr_error_get_context(txn->error)); - } - if (nr_error_get_no(txn->error)) { - if (NULL == user_attributes) { - user_attributes = nro_new_hash(); - } - nro_set_hash_int(user_attributes, "user.error.number", - nr_error_get_no(txn->error)); - } - if (nr_error_get_line(txn->error)) { - if (NULL == user_attributes) { - user_attributes = nro_new_hash(); - } - nro_set_hash_int(user_attributes, "user.error.line", - nr_error_get_line(txn->error)); + + if (NULL == user_attributes) { + user_attributes = nro_new_hash(); } + + user_attributes = nr_error_user_attributes_to_nro(txn->error->user_error, user_attributes); event = nr_analytics_event_create(params, agent_attributes, user_attributes); nro_delete(params); diff --git a/axiom/nr_txn.h b/axiom/nr_txn.h index 3c24cbdb0..cecb9eea3 100644 --- a/axiom/nr_txn.h +++ b/axiom/nr_txn.h @@ -482,11 +482,8 @@ extern void nr_txn_record_error_with_additional_attributes(nrtxn_t* txn, bool add_to_current_segment, const char* error_message, const char* error_class, - const char* error_file, - int error_line, - char* error_context, - int error_no, - const char* stacktrace_json); + const char* stacktrace_json, + nr_user_error_t* user_error); /* * Purpose : Record the given error in the transaction. diff --git a/tests/integration/api/notice_error/test_transaction_additional_params2.php b/tests/integration/api/notice_error/test_transaction_additional_params2.php index d1ec3971e..93bfa5207 100644 --- a/tests/integration/api/notice_error/test_transaction_additional_params2.php +++ b/tests/integration/api/notice_error/test_transaction_additional_params2.php @@ -31,9 +31,7 @@ "traceId": "??", "spanId": "??" }, - { - "user.error.message": "Noticed exception 'Exception' with message 'Sample Exception' in __FILE__:??" - }, + {}, {} ] ]