Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(axiom): handle locale for dt w3c priority #828

Merged
merged 24 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6e4cabc
fix(axiom): handle locale for dt w3c priority
bduranleau-nr Feb 6, 2024
7140c8c
test: add locale test to w3c tracestate header
bduranleau-nr Feb 6, 2024
782149f
test: add integ test for dt w3c priority locale
bduranleau-nr Feb 6, 2024
8599a3c
chore: use single quotes instead of double
bduranleau-nr Feb 6, 2024
15bda88
Update axiom/tests/test_distributed_trace.c
bduranleau-nr Feb 6, 2024
c987363
Update axiom/tests/test_distributed_trace.c
bduranleau-nr Feb 6, 2024
b06234c
fix: use php 7 friendly string search func
bduranleau-nr Feb 6, 2024
67c3f00
chore: add verbosedebug to outbound w3c ts header
bduranleau-nr Feb 6, 2024
72a524c
feat: add all locales to the docker environment
bduranleau-nr Feb 7, 2024
705c0d6
Update axiom/nr_distributed_trace.c
bduranleau-nr Feb 7, 2024
6b6d674
fix: check for debug_cat flag before logging
bduranleau-nr Feb 7, 2024
9fca58b
Update axiom/nr_distributed_trace.c
bduranleau-nr Feb 7, 2024
36f3084
chore: revert clang style change
bduranleau-nr Feb 7, 2024
49128b7
fix: move formatting function to util_sampling
bduranleau-nr Feb 8, 2024
880b18e
fix: move priority string function to nr_distributed_trace
bduranleau-nr Feb 8, 2024
f37e684
fix: use fmt value directly rather than macro
bduranleau-nr Feb 8, 2024
29a2c65
feat: add debug_dt flag
bduranleau-nr Feb 8, 2024
535f589
test: add check to verify call to setlocale
bduranleau-nr Feb 8, 2024
154ce8f
test: streamline setlocale call
bduranleau-nr Feb 8, 2024
8668b95
fix: remove test failing due to no locale on pr runner images
bduranleau-nr Feb 8, 2024
fb2e872
Update axiom/nr_distributed_trace.c
bduranleau-nr Feb 8, 2024
8d6a53c
Update axiom/nr_distributed_trace.c
bduranleau-nr Feb 8, 2024
164e873
test: add setlocale for unit test back in
bduranleau-nr Feb 8, 2024
d1110c5
Update axiom/nr_distributed_trace.c
bduranleau-nr Feb 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions agent/php_globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ typedef struct _nrphpglobals_t {
uint8_t debug_autorum;
uint8_t show_loaded_files;
uint8_t debug_cat;
uint8_t debug_dt;
uint8_t disable_laravel_queue;
} special_flags; /* special control options */
} nrphpglobals_t;
Expand Down
1 change: 1 addition & 0 deletions agent/php_minit.c
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ PHP_MINIT_FUNCTION(newrelic) {
NR_INFO_SPECIAL_FLAGS(debug_autorum);
NR_INFO_SPECIAL_FLAGS(show_loaded_files);
NR_INFO_SPECIAL_FLAGS(debug_cat);
NR_INFO_SPECIAL_FLAGS(debug_dt);
#undef NR_INFO_SPECIAL_FLAGS

nr_guzzle4_minit(TSRMLS_C);
Expand Down
5 changes: 5 additions & 0 deletions agent/php_nrini.c
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,10 @@ static void foreach_special_control_flag(const char* str,
NR_PHP_PROCESS_GLOBALS(special_flags).debug_cat = 1;
return;
}
if (0 == nr_strcmp(str, "debug_dt")) {
NR_PHP_PROCESS_GLOBALS(special_flags).debug_dt = 1;
return;
}
if (0 == nr_strcmp(str, "disable_laravel_queue")) {
NR_PHP_PROCESS_GLOBALS(special_flags).disable_laravel_queue = 1;
return;
Expand Down Expand Up @@ -977,6 +981,7 @@ static PHP_INI_MH(nr_special_mh) {
NR_PHP_PROCESS_GLOBALS(special_flags).debug_autorum = 0;
NR_PHP_PROCESS_GLOBALS(special_flags).show_loaded_files = 0;
NR_PHP_PROCESS_GLOBALS(special_flags).debug_cat = 0;
NR_PHP_PROCESS_GLOBALS(special_flags).debug_dt = 0;
NR_PHP_PROCESS_GLOBALS(special_flags).disable_laravel_queue = 0;

if (0 != NEW_VALUE_LEN) {
Expand Down
2 changes: 2 additions & 0 deletions agent/php_txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,8 @@ nr_status_t nr_php_txn_begin(const char* appnames,
= NR_PHP_PROCESS_GLOBALS(special_flags).show_sql_parsing;
NRTXN(special_flags.debug_cat)
= NR_PHP_PROCESS_GLOBALS(special_flags).debug_cat;
NRTXN(special_flags.debug_dt)
= NR_PHP_PROCESS_GLOBALS(special_flags).debug_dt;

NRTXNGLOBAL(prepared_statements)
= nr_hashmap_create(nr_php_txn_prepared_statement_destroy);
Expand Down
1 change: 1 addition & 0 deletions agent/scripts/newrelic.ini.private.template
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
; debug_autorum
; show_loaded_files
; debug_cat
; debug_dt
; disable_laravel_queue
;
;newrelic.special = show_executes, show_execute_params, show_execute_stack, show_execute_returns, show_executes_untrimmed
Expand Down
37 changes: 34 additions & 3 deletions axiom/nr_distributed_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,27 @@ static inline void set_dt_field(char** field, const char* value) {
}
}

/*
* Purpose Format trace's priority for tracestate in W3C header.
*
* @param value double
* @return char* priority string buffer
*/
static char* nr_priority_double_to_str(nr_sampling_priority_t value) {
char* buf = NULL;

buf = nr_formatf("%.6f", value);

for (int i = 0; i < nr_strlen(buf); i++) {
if (',' == buf[i]) {
buf[i] = '.';
break;
}
}

return buf;
}

nr_distributed_trace_t* nr_distributed_trace_create(void) {
nr_distributed_trace_t* dt;
dt = (nr_distributed_trace_t*)nr_zalloc(sizeof(nr_distributed_trace_t));
Expand Down Expand Up @@ -1164,6 +1185,8 @@ char* nr_distributed_trace_create_w3c_tracestate_header(
const char* app_id;
char* trace_context_header = NULL;
char* sampled = "0";
nr_sampling_priority_t priority;
char* priority_buf = NULL;

if (nrunlikely(NULL == dt)) {
return NULL;
Expand Down Expand Up @@ -1197,11 +1220,19 @@ char* nr_distributed_trace_create_w3c_tracestate_header(
sampled = "1";
}

priority = nr_distributed_trace_get_priority(dt);

priority_buf = nr_priority_double_to_str(priority);
if (NULL == priority_buf) {
nrl_verbosedebug(NRL_CAT, "Failed to allocate priority buffer");
}

trace_context_header = nr_formatf(
"%s@nr=0-0-%s-%s-%s-%s-%s-%.6f-%" PRId64, trusted_account_key, account_id,
"%s@nr=0-0-%s-%s-%s-%s-%s-%s-%" PRId64, trusted_account_key, account_id,
app_id, NRBLANKSTR(span_id), NRBLANKSTR(txn_id), sampled,
nr_distributed_trace_get_priority(dt),
(nr_get_time() / NR_TIME_DIVISOR_MS));
NRBLANKSTR(priority_buf), (nr_get_time() / NR_TIME_DIVISOR_MS));

nr_free(priority_buf);

return trace_context_header;
}
Expand Down
6 changes: 6 additions & 0 deletions axiom/nr_txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -2782,6 +2782,12 @@ char* nr_txn_create_w3c_tracestate_header(const nrtxn_t* txn,
header = nr_distributed_trace_create_w3c_tracestate_header(
txn->distributed_trace, span_id, txn_id);

if (txn->special_flags.debug_dt) {
nrl_verbosedebug(NRL_CAT,
"Outbound W3C TraceState Context Header generated: %s",
NRSAFESTR(header));
}

nr_free(txn_id);
return header;
}
Expand Down
1 change: 1 addition & 0 deletions axiom/nr_txn.h
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ typedef struct _nrtxn_t {
uint8_t no_sql_parsing; /* don't do SQL parsing */
uint8_t show_sql_parsing; /* show various steps in SQL feature parsing */
uint8_t debug_cat; /* extra logging for CAT */
uint8_t debug_dt; /* extra logging for DT */
} special_flags;

/*
Expand Down
14 changes: 14 additions & 0 deletions axiom/tests/test_distributed_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "nr_txn.h"
#include "nr_distributed_trace_private.h"
#include "util_memory.h"
#include <locale.h>

static void test_distributed_trace_create_destroy(void) {
// create a few instances to make sure state stays separate
Expand Down Expand Up @@ -1351,6 +1352,19 @@ static void test_create_trace_state_header(void) {
nr_strstr(result, expected));
nr_free(result);

/*
* Test: locale is set to use `,` instead of `.` for decimal values
*/
setlocale(LC_NUMERIC, "pl_PL");
mfulb marked this conversation as resolved.
Show resolved Hide resolved
dt->priority = 0.123456;
expected = "777@nr=0-0-1234-9876-123456789-meatball!-0-0.123456-";
result
= nr_distributed_trace_create_w3c_tracestate_header(dt, span_id, txn_id);
tlib_pass_if_not_null("locale should not affect priority formatting",
nr_strstr(result, expected));
nr_free(result);
setlocale(LC_NUMERIC, "en_US");
lavarou marked this conversation as resolved.
Show resolved Hide resolved

nr_distributed_trace_destroy(&dt);
}

Expand Down
2 changes: 2 additions & 0 deletions files/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ RUN apt-get update && apt-get install -y \
libc6-dev \
libgtest-dev \
libtool \
locales \
locales-all \
make \
perl \
strace \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php
/*
* Copyright 2020 New Relic Corporation. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/

/*DESCRIPTION
Tests that the locale is handled correctly in w3c dt tracestate header
priority for locales that use `,` instead of `.` for decimal values
*/

/*INI
newrelic.distributed_tracing_enabled = true
newrelic.distributed_tracing_exclude_newrelic_header = false
newrelic.cross_application_tracer.enabled = false
*/

/*EXPECT
ok - insert function succeeded
ok - preexisting header is present
ok - newrelic header is present
ok - tracestate header is present
ok - traceparent header is present
ok - locale handled correctly for w3c dt priority
*/

require_once(realpath (dirname ( __FILE__ )) . '/../../../include/tap.php');

$headers = array('Accept-Language' => 'en-US,en;q=0.5');
setlocale(LC_NUMERIC, 'fr_FR');
tap_assert(newrelic_insert_distributed_trace_headers($headers), 'insert function succeeded');
tap_assert(array_key_exists('Accept-Language', $headers), 'preexisting header is present');
tap_assert(array_key_exists('newrelic', $headers), 'newrelic header is present');
tap_assert(array_key_exists('tracestate', $headers), 'tracestate header is present');
tap_assert(array_key_exists('traceparent', $headers), 'traceparent header is present');
tap_refute(strpos($headers['tracestate'], ','), 'locale handled correctly for w3c dt priority');

Loading