Skip to content

Commit ac25445

Browse files
committed
src,test: preserve Latin1 strings in fastSpan APIs
Convert FastOneByteString inputs to UTF-8 before storing span string data so non-ASCII Latin-1 values such as ñ are emitted correctly by the N|Solid fast tracing bindings. Add tracing coverage for Latin-1 custom attributes and introduce a debug-only fast-path test that uses TRACK_V8_FAST_API_CALL to verify pushSpanDataString() and pushSpanDataString3() actually execute through the V8 fast API path. PR-URL: #450 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent 8aee9af commit ac25445

5 files changed

Lines changed: 113 additions & 6 deletions

File tree

src/nsolid/nsolid_api.cc

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
#include "util.h"
1111
#include "env-inl.h"
1212
#include "uv.h"
13+
#include "node_debug.h"
1314
#include "node_internals.h"
1415
#include "node_external_reference.h"
1516
#include "memory_tracker-inl.h"
1617
#include "node_perf.h"
1718
#include "node_url.h"
19+
#include "simdutf.h"
1820
#include "v8-fast-api-calls.h"
1921

2022
#include <algorithm>
@@ -93,6 +95,16 @@ constexpr size_t datapoints_q_max_size = 100;
9395

9496
static const char* get_startuptime_name(const char* name);
9597

98+
static std::string OneByteToUtf8(const FastOneByteString& input) {
99+
std::string out;
100+
out.resize(input.length * 2);
101+
102+
const size_t actual_length = simdutf::convert_latin1_to_utf8(
103+
input.data, input.length, out.data());
104+
out.resize(actual_length);
105+
return out;
106+
}
107+
96108

97109
EnvInst::EnvInst(Environment* env)
98110
: count_fields(),
@@ -2474,10 +2486,11 @@ void BindingData::FastPushSpanDataString(v8::Local<v8::Object> receiver,
24742486
uint32_t trace_id,
24752487
uint32_t type,
24762488
const FastOneByteString& val) {
2489+
TRACK_V8_FAST_API_CALL("nsolid.pushSpanDataString");
24772490
PushSpanDataStringImpl(FromJSObject<BindingData>(receiver),
24782491
trace_id,
24792492
type,
2480-
std::string(val.data, val.length));
2493+
OneByteToUtf8(val));
24812494
}
24822495

24832496

@@ -2521,12 +2534,13 @@ void BindingData::FastPushSpanDataString3(v8::Local<v8::Object> receiver,
25212534
const FastOneByteString& val1,
25222535
const FastOneByteString& val2,
25232536
const FastOneByteString& val3) {
2537+
TRACK_V8_FAST_API_CALL("nsolid.pushSpanDataString3");
25242538
PushSpanDataStringImpl3(FromJSObject<BindingData>(receiver),
2525-
trace_id,
2526-
type,
2527-
std::string(val1.data, val1.length),
2528-
std::string(val2.data, val2.length),
2529-
std::string(val3.data, val3.length));
2539+
trace_id,
2540+
type,
2541+
OneByteToUtf8(val1),
2542+
OneByteToUtf8(val2),
2543+
OneByteToUtf8(val3));
25302544
}
25312545

25322546
void BindingData::PushSpanDataStringImpl3(BindingData* data,
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Flags: --expose-internals --no-warnings --allow-natives-syntax
2+
'use strict';
3+
4+
const common = require('../../common');
5+
const assert = require('assert');
6+
const { internalBinding } = require('internal/test/binding');
7+
const { checkTracesOnExit } = require('../../common/nsolid-traces');
8+
const { setupNSolid } = require('./utils');
9+
const fixtures = require('../../common/fixtures');
10+
const addonBindingPath = require.resolve(`./build/${common.buildType}/binding`);
11+
const addonBinding = require(addonBindingPath);
12+
13+
const nsolidBinding = internalBinding('nsolid_api');
14+
const { nsolid_consts } = nsolidBinding;
15+
const nsolid = require('nsolid');
16+
const api = require(require.resolve('@opentelemetry/api',
17+
{ paths: [fixtures.fixturesDir] }));
18+
19+
const expectedTraces = [
20+
{
21+
attributes: {
22+
'http.url': 'http://localhost/ma\u00f1ana',
23+
},
24+
end_reason: addonBinding.kSpanEndOk,
25+
name: 'Espa\u00f1a',
26+
parentId: '0000000000000000',
27+
thread_id: 0,
28+
kind: addonBinding.kClient,
29+
type: addonBinding.kSpanCustom,
30+
status: {
31+
code: api.SpanStatusCode.OK,
32+
},
33+
},
34+
];
35+
36+
checkTracesOnExit(addonBinding, expectedTraces);
37+
38+
setupNSolid({ lookup: false }, common.mustCall(() => {
39+
if (!nsolid.otel.register(api)) {
40+
throw new Error('Error registering api');
41+
}
42+
43+
const tracer = api.trace.getTracer('test');
44+
const span = tracer.startSpan('initial_name', { kind: api.SpanKind.CLIENT });
45+
46+
function pushSpanName() {
47+
nsolidBinding.pushSpanDataString(span.internalId,
48+
nsolid_consts.kSpanName,
49+
'Espa\u00f1a');
50+
}
51+
52+
function pushSpanUrl() {
53+
nsolidBinding.pushSpanDataString3(span.internalId,
54+
nsolid_consts.kSpanHttpReqUrl,
55+
'http:',
56+
'localhost',
57+
'/ma\u00f1ana');
58+
}
59+
60+
if (common.isDebug) {
61+
const { getV8FastApiCallCount } = internalBinding('debug');
62+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString'), 0);
63+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString3'), 0);
64+
65+
eval('%PrepareFunctionForOptimization(pushSpanName)');
66+
pushSpanName();
67+
eval('%PrepareFunctionForOptimization(pushSpanUrl)');
68+
pushSpanUrl();
69+
70+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString'), 0);
71+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString3'), 0);
72+
73+
eval('%OptimizeFunctionOnNextCall(pushSpanName)');
74+
pushSpanName();
75+
eval('%OptimizeFunctionOnNextCall(pushSpanUrl)');
76+
pushSpanUrl();
77+
78+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString'), 1);
79+
assert.strictEqual(getV8FastApiCallCount('nsolid.pushSpanDataString3'), 1);
80+
} else {
81+
pushSpanName();
82+
pushSpanUrl();
83+
}
84+
85+
span.setStatus({ code: api.SpanStatusCode.OK });
86+
span.end();
87+
88+
setTimeout(() => {}, 100);
89+
}));

test/addons/nsolid-tracing/test-otel-basic.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const expectedTraces = [
2323
c: 3,
2424
d: 4,
2525
e: 5,
26+
latin1: 'Espa\u00f1a',
2627
},
2728
events: [
2829
{
@@ -69,6 +70,7 @@ setupNSolid(common.mustCall(() => {
6970
assert.strictEqual(span.updateName('my name'), span);
7071
assert.strictEqual(span.setAttributes({ c: 3, d: 4 }), span);
7172
assert.strictEqual(span.setAttribute('e', 5), span);
73+
assert.strictEqual(span.setAttribute('latin1', 'Espa\u00f1a'), span);
7274
assert.strictEqual(span.addEvent('my_event 1', Date.now()), span);
7375
assert.strictEqual(
7476
span.addEvent('my_event 2', { attr1: 'val1', attr2: 'val2' }, Date.now()),

test/agents/test-grpc-tracing.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const expectedCustomAttributes = (threadId) => {
5353
'b': [ 'intValue', '2', false ],
5454
'c': [ 'intValue', '3', false ],
5555
'd': [ 'intValue', '4', false ],
56+
'latin1': [ 'stringValue', 'Espa\u00f1a', false ],
5657
'e': [ 'arrayValue', [
5758
{ stringValue: 'abAD', value: 'stringValue' },
5859
{ stringValue: 'cdCF', value: 'stringValue' },

test/common/nsolid-grpc-agent/client.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ function execCustomTrace() {
9898
const span = tracer.startSpan('initial_name', { attributes: { a: 1, b: 2 },
9999
kind: api.SpanKind.CLIENT });
100100
span.setAttributes({ c: 3, d: 4 })
101+
.setAttribute('latin1', 'Espa\u00f1a')
101102
.setAttribute('e', [ 'abAD', 'cdCF'])
102103
.addEvent('my_event 1', Date.now())
103104
.addEvent('my_event 2', { attr1: 'val1', attr2: 'val2' }, Date.now());

0 commit comments

Comments
 (0)