feat(agent): optionally pad agent generated trace_ids (#929)

When the agent generates trace id based on the information it received in HTTP
headers, trace id will have 32 characters. However when the agent generates the
trace id for internal use only, the trace id has 16 characters. This PR adds an
INI setting that when set, the agent will left pad its internally used trace id
with '0' so that the trace id has 32 characters.

---------

Co-authored-by: Michal Nowacki <[email protected]>

Author: ZNeumann

agent/php_newrelic.h

@@ -542,6 +542,8 @@ nrinibool_t
 nrinibool_t
     distributed_tracing_exclude_newrelic_header; /* newrelic.distributed_tracing_exclude_newrelic_header
                                                   */
+nrinibool_t
+    distributed_tracing_pad_trace_id; /* newrelic.distributed_tracing.pad_trace_id */
 nrinibool_t span_events_enabled; /* newrelic.span_events_enabled */
 nriniuint_t
     span_events_max_samples_stored; /* newrelic.span_events.max_samples_stored

agent/php_nrini.c

@@ -2879,6 +2879,21 @@ STD_PHP_INI_ENTRY_EX("newrelic.distributed_tracing_exclude_newrelic_header",
                      newrelic_globals,
                      0)
 
+/*
+ * This setting is not documented and affects the length of the interally used
+ * trace id. This INI setting should not be modified unless requested by
+ * customer.
+ */
+STD_PHP_INI_ENTRY_EX("newrelic.distributed_tracing.pad_trace_id",
+                     "",  // default is false represented as an empty string -
+                          // see notes about PHP INI parser
+                     NR_PHP_REQUEST,
+                     nr_boolean_mh,
+                     distributed_tracing_pad_trace_id,
+                     zend_newrelic_globals,
+                     newrelic_globals,
+                     nr_enabled_disabled_dh)
+
 /**
  * Flag to turn the span events on/off
  * When on, the agent will create span events. Span events require that

agent/php_txn.c

@@ -748,6 +748,8 @@ nr_status_t nr_php_txn_begin(const char* appnames,
   opts.allow_raw_exception_messages = NRINI(allow_raw_exception_messages);
   opts.custom_parameters_enabled = NRINI(custom_parameters_enabled);
   opts.distributed_tracing_enabled = NRINI(distributed_tracing_enabled);
+  opts.distributed_tracing_pad_trace_id
+      = NRINI(distributed_tracing_pad_trace_id);
   opts.distributed_tracing_exclude_newrelic_header
       = NRINI(distributed_tracing_exclude_newrelic_header);
   opts.span_events_enabled = NRINI(span_events_enabled);

agent/scripts/newrelic.ini.private.template

@@ -193,3 +193,14 @@
 ;          hooks instrumented.
 ;
 ;newrelic.framework.wordpress.hooks_skip_filename=""
+
+; Setting: newrelic.distributed_tracing.pad_trace_id
+; Type   : bool
+; Scope  : per-directory
+; Default: false
+; Info   : If set to true, the distributed trace's id generated by the agent,
+;          which is only meant to be used internally by New Relic, will be left
+;          padded with '0's to have the length of 32 characters. This will increase
+;          the amount of data sent to New Relic.
+;
+;newrelic.distributed_tracing.pad_trace_id = false

axiom/nr_distributed_trace.c

@@ -481,14 +481,30 @@ void nr_distributed_trace_set_app_id(nr_distributed_trace_t* dt,
 }
 
 void nr_distributed_trace_set_trace_id(nr_distributed_trace_t* dt,
-                                       const char* trace_id) {
+                                       const char* trace_id,
+                                       bool do_padding) {
   if (NULL == dt) {
     return;
   }
 
   nr_free(dt->trace_id);
   if (trace_id) {
-    dt->trace_id = nr_strdup(trace_id);
+    if (nrunlikely(do_padding)) {
+      int len = nr_strlen(trace_id);
+      if (len < NR_TRACE_ID_MAX_SIZE) {
+        int padding = NR_TRACE_ID_MAX_SIZE - len;
+        char* dest = (char*)nr_malloc(NR_TRACE_ID_MAX_SIZE + 1);
+        for (int i = 0; i < padding; i++) {
+          dest[i] = '0';
+        }
+        strcpy(dest + padding, trace_id);
+        dt->trace_id = dest;
+      } else {
+        dt->trace_id = nr_strdup(trace_id);
+      }
+    } else {
+      dt->trace_id = nr_strdup(trace_id);
+    }
   }
 }
 

axiom/nr_distributed_trace.h

@@ -12,6 +12,9 @@
 #include "util_time.h"
 #include "util_object.h"
 
+/* This is the maximum size of trace id that the backend will accept */
+#define NR_TRACE_ID_MAX_SIZE 32
+
 static const char NR_DISTRIBUTED_TRACE_ACCEPT_SUCCESS[]
     = "Supportability/DistributedTrace/AcceptPayload/Success";
 static const char NR_DISTRIBUTED_TRACE_ACCEPT_EXCEPTION[]
@@ -242,9 +245,12 @@ extern void nr_distributed_trace_set_app_id(nr_distributed_trace_t* dt,
  *
  * Params  : 1. The distributed trace.
  *           2. The trace id.
+ *           3. Bool where true indicates to left pad the trace_id
+ *              with '0's to make it NR_TRACE_ID_MAX_SIZE characters
  */
 void nr_distributed_trace_set_trace_id(nr_distributed_trace_t* dt,
-                                       const char* trace_id);
+                                       const char* trace_id,
+                                       bool do_padding);
 
 /*
  * Purpose : Set the distributed trace priority.

axiom/nr_txn.c

@@ -633,7 +633,8 @@ nrtxn_t* nr_txn_begin(nrapp_t* app,
    */
   guid = nr_guid_create(app->rnd);
   nr_distributed_trace_set_txn_id(nt->distributed_trace, guid);
-  nr_distributed_trace_set_trace_id(nt->distributed_trace, guid);
+  nr_distributed_trace_set_trace_id(nt->distributed_trace, guid,
+                                    opts->distributed_tracing_pad_trace_id);
 
   nr_distributed_trace_set_trusted_key(
       nt->distributed_trace,

axiom/nr_txn.h

@@ -91,6 +91,9 @@ typedef struct _nrtxnopt_t {
 
   int distributed_tracing_enabled; /* Whether distributed tracing functionality
                                       is enabled */
+  bool distributed_tracing_pad_trace_id; /* whether to pad internally generated
+                                            trace_id to NR_TRACE_ID_MAX_SIZE
+                                            characters */
   bool distributed_tracing_exclude_newrelic_header; /* Whether distributed
                                                        tracing outbound headers
                                                        should omit newrelic

axiom/tests/test_distributed_trace.c

@@ -1670,6 +1670,46 @@ static void test_distributed_trace_create_trace_parent_header(void) {
   nr_free(actual);
 }
 
+static void test_distributed_trace_set_trace_id(const bool pad_trace_id) {
+  nr_distributed_trace_t dt = {0};
+  // clang-format off
+  const char* t_input[] = {
+      "",                  // empty string
+      "1234567890",        // 10 characters
+      "1234567890abcdef",  // 16 characters
+      "1234567890abcdef1234567890abcdef", // 32 characters
+      "1234567890123456789012345678901234567890123456789012345678901234567890"  // 64 characters
+  };
+  const char* t_padded[] = {
+      "00000000000000000000000000000000",  // empty string lpadded to NR_TRACE_ID_MAX_SIZE with '0'
+      "00000000000000000000001234567890",  // 10 characters lpadded to NR_TRACE_ID_MAX_SIZE with '0'
+      "00000000000000001234567890abcdef",  // 16 characters lpadded to NR_TRACE_ID_MAX_SIZE with '0'
+      "1234567890abcdef1234567890abcdef",  // 32 characters - no padding done
+      "1234567890123456789012345678901234567890123456789012345678901234567890" // 64 characters - no padding
+    };
+  // clang-format on
+
+  /*
+   * Test : NULL input => no trace id generated
+   */
+  nr_distributed_trace_set_trace_id(&dt, NULL, pad_trace_id);
+  tlib_pass_if_null("NULL trace id", dt.trace_id);
+
+  /*
+   * Test : valid input => trace id generated
+   */
+  for (long unsigned int i = 0; i < sizeof(t_input) / sizeof(t_input[0]); i++) {
+    const char* expected = t_input[i];
+    if (pad_trace_id) {
+      expected = t_padded[i];
+    }
+    nr_distributed_trace_set_trace_id(&dt, t_input[i], pad_trace_id);
+    tlib_pass_if_not_null("trace id is set", dt.trace_id);
+    tlib_pass_if_str_equal("trace id has correct value", dt.trace_id, expected);
+    nr_free(dt.trace_id);
+  }
+}
+
 tlib_parallel_info_t parallel_info = {.suggested_nthreads = 2, .state_size = 0};
 
 void test_main(void* p NRUNUSED) {
@@ -1707,4 +1747,6 @@ void test_main(void* p NRUNUSED) {
 
   test_create_trace_state_header();
   test_distributed_trace_create_trace_parent_header();
+  test_distributed_trace_set_trace_id(false);
+  test_distributed_trace_set_trace_id(true);
 }

axiom/tests/test_txn.c

@@ -1705,6 +1705,7 @@ static void test_begin(void) {
   opts->max_segments = 0;
   opts->span_queue_batch_size = 1000;
   opts->span_queue_batch_timeout = 1 * NR_TIME_DIVISOR;
+  opts->distributed_tracing_pad_trace_id = false;
 
   app->rnd = nr_random_create();
   nr_random_seed(app->rnd, 345345);
@@ -5940,6 +5941,10 @@ static void test_create_w3c_traceparent_header(void) {
 
   nr_memset(&txn, 0, sizeof(nrtxn_t));
   txn.options.span_events_enabled = true;
+  // default value for padding can be used for this test, because other
+  // test (test_distributed_trace_create_trace_parent_header) already
+  // covers using trace_id of different lengths when w3c header is created.
+  txn.options.distributed_tracing_pad_trace_id = false;
   txn.status.recording = true;
   txn.rnd = nr_random_create();
   txn.status.recording = 1;
@@ -5969,7 +5974,9 @@ static void test_create_w3c_traceparent_header(void) {
   /*
    * Test : valid string random guid
    */
-  nr_distributed_trace_set_trace_id(txn.distributed_trace, "meatballs!");
+  nr_distributed_trace_set_trace_id(
+      txn.distributed_trace, "meatballs!",
+      txn.options.distributed_tracing_pad_trace_id);
   actual = nr_txn_create_w3c_traceparent_header(&txn, segment);
   expected = "00-0000000000000000000000meatballs!-";
   tlib_pass_if_not_null("random guid", nr_strstr(actual, expected));
@@ -6280,6 +6287,10 @@ static void test_txn_accept_distributed_trace_payload_w3c(void) {
   txn.app_connect_reply = nro_new_hash();
   txn.unscoped_metrics = nrm_table_create(0);
   nro_set_hash_string(txn.app_connect_reply, "trusted_account_key", "123");
+  // default value for padding can be used for this test, because other
+  // test (test_distributed_trace_create_trace_parent_header) already
+  // covers using trace_id of different lengths when w3c header is created.
+  txn.options.distributed_tracing_pad_trace_id = false;
 
 #define TEST_TXN_ACCEPT_DT_PAYLOAD_RESET    \
   txn.distributed_trace->inbound.set = 0;   \
@@ -6725,7 +6736,9 @@ static void test_txn_accept_distributed_trace_payload_w3c(void) {
   TEST_TXN_ACCEPT_DT_PAYLOAD_RESET
   nr_distributed_trace_destroy(&txn.distributed_trace);
   txn.distributed_trace = nr_distributed_trace_create();
-  nr_distributed_trace_set_trace_id(txn.distributed_trace, "35ff77");
+  nr_distributed_trace_set_trace_id(
+      txn.distributed_trace, "35ff77",
+      txn.options.distributed_tracing_pad_trace_id);
   traceparent = nr_txn_create_w3c_traceparent_header(&txn, NULL);
   nr_free(traceparent);
 
@@ -7525,6 +7538,7 @@ static void test_get_current_trace_id(void) {
   char* trace_id;
   nrtxn_t* txn;
   const char* txn_id;
+  const char* dt_trace_id;
 
   /* setup and start txn */
   nr_memset(&app, 0, sizeof(app));
@@ -7540,7 +7554,7 @@ static void test_get_current_trace_id(void) {
                     nr_txn_get_current_trace_id(NULL));
 
   /*
-   * Test : Correct trace id
+   * Test : trace id  == txn_id with default pad_trace_id setting - no padding
    */
   txn_id = nr_txn_get_guid(txn);
   trace_id = nr_txn_get_current_trace_id(txn);
@@ -7564,6 +7578,28 @@ static void test_get_current_trace_id(void) {
                     nr_txn_get_current_trace_id(txn));
 
   nr_txn_destroy(&txn);
+
+  /* setup and start txn */
+  nr_memset(&app, 0, sizeof(app));
+  nr_memset(&opts, 0, sizeof(opts));
+  app.state = NR_APP_OK;
+  opts.distributed_tracing_enabled = 1;
+  opts.distributed_tracing_pad_trace_id = true;
+  txn = nr_txn_begin(&app, &opts, NULL);
+  /*
+   * Test : trace id  != txn_id with trace_id padding enabled
+   */
+  txn_id = nr_txn_get_guid(txn);
+  trace_id = nr_txn_get_current_trace_id(txn);
+  dt_trace_id = nr_distributed_trace_get_trace_id(txn->distributed_trace);
+  tlib_fail_if_null("txn id", txn_id);
+  tlib_fail_if_str_equal("txn_id != trace_id", txn_id, trace_id);
+  tlib_pass_if_str_equal("txn_id = ltrim(trace_id)", txn_id,
+                         trace_id + (nr_strlen(trace_id) - NR_GUID_SIZE));
+  tlib_pass_if_str_equal("trace_id = dt_trace_id", trace_id, dt_trace_id);
+  nr_free(trace_id);
+
+  nr_txn_destroy(&txn);
 }
 
 static void test_get_current_span_id(void) {