From 61539c181a7d3e949ddbeb9c4c23aef332f4e2b9 Mon Sep 17 00:00:00 2001 From: Mike Summers Date: Tue, 14 Jan 2020 11:22:12 -0600 Subject: [PATCH] Implement newrelic_set_segment_external --- include/libnewrelic.h | 25 +++++++++ src/external.c | 50 +++++++++++++++++ tests/Makefile | 1 + tests/test_set_external.c | 111 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+) create mode 100644 tests/test_set_external.c diff --git a/include/libnewrelic.h b/include/libnewrelic.h index 80897469..1dacd7d8 100644 --- a/include/libnewrelic.h +++ b/include/libnewrelic.h @@ -910,6 +910,31 @@ newrelic_segment_t* newrelic_start_external_segment( newrelic_txn_t* transaction, const newrelic_external_segment_params_t* params); +/** + * @brief Modify an existing segment to be an external segment within a + * transaction. + * + * Given an active transaction and segment, this function modifies an existing + * segment inside of the transaction making it external. An external segment is + * generally used to represent a HTTP or RPC request. + * + * @param [in] transaction An active transaction. + * @param [in] segment An active segment. + * @param [in] params The parameters describing the external request. All + * parameters are copied, and no references to the + * pointers provided are kept after this function + * returns. + * + * @return A pointer to the modified segment. If an + * error occurs when while modifying the segment, NULL is returned, + * and a log message will be written to the SDK log at LOG_ERROR + * level. + */ +newrelic_segment_t* newrelic_set_segment_external( + newrelic_txn_t* transaction, + newrelic_segment_t* segment, + const newrelic_external_segment_params_t* params); + /** * @brief Set the parent for the given segment. * diff --git a/src/external.c b/src/external.c index db015ef3..a3aa417d 100644 --- a/src/external.c +++ b/src/external.c @@ -61,6 +61,56 @@ newrelic_segment_t* newrelic_start_external_segment( return segment; } +newrelic_segment_t* newrelic_set_segment_external( + newrelic_txn_t* transaction, + newrelic_segment_t* segment, + const newrelic_external_segment_params_t* params) { + /* Validate our inputs. */ + if (NULL == transaction) { + nrl_error(NRL_INSTRUMENT, + "cannot set as external segment on a NULL transaction"); + return NULL; + } + + if (NULL == segment) { + nrl_error(NRL_INSTRUMENT, "cannot set a NULL segment external"); + return NULL; + } + + if (NULL == params) { + nrl_error(NRL_INSTRUMENT, "params cannot be NULL"); + return NULL; + } + + if (!newrelic_validate_segment_param(params->library, "library")) { + return NULL; + } + + if (!newrelic_validate_segment_param(params->procedure, "procedure")) { + return NULL; + } + + if (NULL == params->uri) { + nrl_error(NRL_INSTRUMENT, "uri cannot be NULL"); + return NULL; + } + + nrt_mutex_lock(&transaction->lock); + { + segment->segment->type = NR_SEGMENT_EXTERNAL; + + /* Save the supplied parameters until the external segment is ended */ + segment->type.external.uri = params->uri ? nr_strdup(params->uri) : NULL; + segment->type.external.library + = params->library ? nr_strdup(params->library) : NULL; + segment->type.external.procedure + = params->procedure ? nr_strdup(params->procedure) : NULL; + } + nrt_mutex_unlock(&transaction->lock); + + return segment; +} + void newrelic_destroy_external_segment_fields(newrelic_segment_t* segment) { nr_free(segment->type.external.uri); nr_free(segment->type.external.library); diff --git a/tests/Makefile b/tests/Makefile index e83a2cef..72c85c66 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -112,6 +112,7 @@ TESTS := \ test_start_transaction \ test_txn \ test_version \ + test_set_external \ # # The list of tests to skip and tests to run. diff --git a/tests/test_set_external.c b/tests/test_set_external.c new file mode 100644 index 00000000..66767943 --- /dev/null +++ b/tests/test_set_external.c @@ -0,0 +1,111 @@ +#include +#include + +#include +#include + +#include "libnewrelic.h" +#include "external.h" +#include "transaction.h" +#include "util_memory.h" + +#include "test.h" + +/* + * Purpose: Test that newrelic_set_segment_external() handles invalid inputs + * correctly. + */ +static void test_set_external_segment_invalid(void** state) { + newrelic_txn_t* txn = (newrelic_txn_t*)*state; + newrelic_segment_t* original_segment + = newrelic_start_segment(txn, NULL, NULL); + newrelic_external_segment_params_t params = { + .uri = NULL, + }; + + assert_null(newrelic_set_segment_external(NULL, NULL, NULL)); + assert_null(newrelic_set_segment_external(txn, NULL, NULL)); + assert_null(newrelic_set_segment_external(NULL, original_segment, NULL)); + assert_null(newrelic_set_segment_external(NULL, NULL, ¶ms)); + assert_null(newrelic_set_segment_external(txn, original_segment, NULL)); + assert_null(newrelic_set_segment_external(txn, NULL, ¶ms)); + assert_null(newrelic_set_segment_external(NULL, original_segment, ¶ms)); + + /* A NULL uri should also result in a NULL. */ + assert_null(newrelic_set_segment_external(txn, original_segment, ¶ms)); + + /* Now we'll test library and procedure values including slashes, which are + * prohibited because they do terrible things to metric names and APM in + * turn. */ + params.uri = "https://newrelic.com/"; + + params.library = "foo/bar"; + assert_null(newrelic_set_segment_external(txn, original_segment, ¶ms)); + params.library = NULL; + + params.procedure = "foo/bar"; + assert_null(newrelic_set_segment_external(txn, original_segment, ¶ms)); + params.procedure = NULL; +} + +/* + * Purpose: Test that newrelic_set_segment_external() handles valid inputs + * correctly. + */ +static void test_set_external_segment_valid(void** state) { + newrelic_txn_t* txn = (newrelic_txn_t*)*state; + newrelic_segment_t* original_segment + = newrelic_start_segment(txn, NULL, NULL); + newrelic_external_segment_params_t params = { + .uri = "https://newrelic.com/", + }; + newrelic_segment_t* segment; + + /* Test without library or procedure. */ + segment = newrelic_set_segment_external(txn, original_segment, ¶ms); + assert_non_null(segment); + assert_int_equal((int)NR_SEGMENT_EXTERNAL, (int)segment->segment->type); + assert_ptr_equal(txn->txn, segment->transaction); + + /* Ensure the uri was actually copied. */ + assert_string_equal(params.uri, segment->type.external.uri); + assert_ptr_not_equal(params.uri, segment->type.external.uri); + assert_null(segment->type.external.library); + assert_null(segment->type.external.procedure); + + newrelic_segment_destroy(&segment); + + /* Now test with library and procedure. */ + params.library = "curl"; + params.procedure = "GET"; + original_segment = newrelic_start_segment(txn, NULL, NULL); + segment = newrelic_set_segment_external(txn, original_segment, ¶ms); + assert_non_null(segment); + assert_int_equal((int)NR_SEGMENT_EXTERNAL, (int)segment->segment->type); + assert_ptr_equal(txn->txn, segment->transaction); + + assert_string_equal(params.uri, segment->type.external.uri); + assert_ptr_not_equal(params.uri, segment->type.external.uri); + + assert_string_equal(params.library, segment->type.external.library); + assert_ptr_not_equal(params.library, segment->type.external.library); + + assert_string_equal(params.procedure, segment->type.external.procedure); + assert_ptr_not_equal(params.procedure, segment->type.external.procedure); + + newrelic_segment_destroy(&segment); +} + +/* + * Purpose: Main entry point (i.e. runs the tests) + */ +int main(void) { + const struct CMUnitTest external_tests[] = { + cmocka_unit_test_setup_teardown(test_set_external_segment_valid, + txn_group_setup, txn_group_teardown), + cmocka_unit_test_setup_teardown(test_set_external_segment_invalid, + txn_group_setup, txn_group_teardown), + }; + + return cmocka_run_group_tests(external_tests, NULL, NULL); +}