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 9 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
22 changes: 17 additions & 5 deletions axiom/nr_distributed_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "util_time.h"
#include "util_strings.h"
#include "util_logging.h"
#include "util_number_converter.h"
bduranleau-nr marked this conversation as resolved.
Show resolved Hide resolved

/*
* Purpose : Helper function to assign a string value to a field. This function
Expand Down Expand Up @@ -1164,6 +1165,8 @@ char* nr_distributed_trace_create_w3c_tracestate_header(
const char* app_id;
char* trace_context_header = NULL;
char* sampled = "0";
double priority;
bduranleau-nr marked this conversation as resolved.
Show resolved Hide resolved
char* priority_buf = NULL;

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

trace_context_header = nr_formatf(
"%s@nr=0-0-%s-%s-%s-%s-%s-%.6f-%" 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));
priority = nr_distributed_trace_get_priority(dt);

priority_buf = nr_priority_double_to_str(priority);

trace_context_header
= nr_formatf("%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, priority_buf, (nr_get_time() / NR_TIME_DIVISOR_MS));

nr_free(priority_buf);

nrl_verbosedebug(NRL_CAT,
"Outbound W3C TraceState Context Header generated: %s",
trace_context_header);
lavarou marked this conversation as resolved.
Show resolved Hide resolved

return trace_context_header;
}
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
17 changes: 17 additions & 0 deletions axiom/util_number_converter.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <stdio.h>
#include <stdlib.h>

#include "util_memory.h"
#include "util_number_converter.h"
#include "util_strings.h"

Expand Down Expand Up @@ -46,6 +47,22 @@ void nr_itoa(char* buf, size_t len, int x) {
nr_strlcpy(buf, p, len);
}

#define PRIORITY_MAXLEN 6
char* nr_priority_double_to_str(double value) {
char* buf = NULL;

buf = nr_formatf("%.6f", value);
mfulb marked this conversation as resolved.
Show resolved Hide resolved

for (int i = 0; i < PRIORITY_MAXLEN; i++) {
mfulb marked this conversation as resolved.
Show resolved Hide resolved
if (',' == buf[i]) {
buf[i] = '.';
break;
}
}

return buf;
}

lavarou marked this conversation as resolved.
Show resolved Hide resolved
int nr_double_to_str(char* buf, int buf_len, double input) {
int i;
int natural_width;
Expand Down
8 changes: 8 additions & 0 deletions axiom/util_number_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@
*/
extern void nr_itoa(char* buf, size_t len, int x);

/*
* Purpose Format double priority numbers to string.
*
* @param value double
* @return char* priority string buffer
*/
extern char* nr_priority_double_to_str(double value);

/*
* Purpose : Format double precision numbers.
*
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