Skip to content

Commit 240fd8a

Browse files
mydeamydea
authored
feat(opentelemetry): Ensure DSC & attributes are correctly set (#10806)
This PR fixes a few things about the OTEL handling of our SDK. 1. Ensure that attributes are actually correctly put on the trace data - we used to remove e.g. `sentry.sample_rate` which we don't do in "main" SDK. Question: Should we remove the `otel.attributes` context data then? As this is kind of redundant at this point with the `data` on the trace? 2. Ensure we put the `transaction` tag correctly on the transaction, to align with "main" SDK. 3. Ensure we store the incoming DSC as traceState, so we can pick this up. This can include e.g. transaction name and stuff like this to persist.
1 parent a5c4fcb commit 240fd8a

File tree

14 files changed

+458
-101
lines changed

14 files changed

+458
-101
lines changed

‎dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts‎

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,29 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
5252

5353
expect(outboundTransaction.contexts?.trace).toEqual({
5454
data: {
55+
'sentry.source': 'route',
56+
'sentry.origin': 'auto.http.otel.http',
57+
'sentry.op': 'http.server',
58+
'sentry.sample_rate': 1,
5559
url: 'http://localhost:3030/test-outgoing-http',
5660
'otel.kind': 'SERVER',
5761
'http.response.status_code': 200,
58-
'sentry.op': 'http.server',
59-
'sentry.origin': 'auto.http.otel.http',
60-
'sentry.source': 'route',
62+
'http.url': 'http://localhost:3030/test-outgoing-http',
63+
'http.host': 'localhost:3030',
64+
'net.host.name': 'localhost',
65+
'http.method': 'GET',
66+
'http.scheme': 'http',
67+
'http.target': '/test-outgoing-http',
68+
'http.user_agent': 'axios/1.6.7',
69+
'http.flavor': '1.1',
70+
'net.transport': 'ip_tcp',
71+
'net.host.ip': expect.any(String),
72+
'net.host.port': expect.any(Number),
73+
'net.peer.ip': expect.any(String),
74+
'net.peer.port': expect.any(Number),
75+
'http.status_code': 200,
76+
'http.status_text': 'OK',
77+
'http.route': '/test-outgoing-http',
6178
},
6279
op: 'http.server',
6380
span_id: expect.any(String),
@@ -68,12 +85,28 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
6885

6986
expect(inboundTransaction.contexts?.trace).toEqual({
7087
data: {
88+
'sentry.source': 'route',
89+
'sentry.origin': 'auto.http.otel.http',
90+
'sentry.op': 'http.server',
91+
'sentry.sample_rate': 1,
7192
url: 'http://localhost:3030/test-inbound-headers',
7293
'otel.kind': 'SERVER',
7394
'http.response.status_code': 200,
74-
'sentry.op': 'http.server',
75-
'sentry.origin': 'auto.http.otel.http',
76-
'sentry.source': 'route',
95+
'http.url': 'http://localhost:3030/test-inbound-headers',
96+
'http.host': 'localhost:3030',
97+
'net.host.name': 'localhost',
98+
'http.method': 'GET',
99+
'http.scheme': 'http',
100+
'http.target': '/test-inbound-headers',
101+
'http.flavor': '1.1',
102+
'net.transport': 'ip_tcp',
103+
'net.host.ip': expect.any(String),
104+
'net.host.port': expect.any(Number),
105+
'net.peer.ip': expect.any(String),
106+
'net.peer.port': expect.any(Number),
107+
'http.status_code': 200,
108+
'http.status_text': 'OK',
109+
'http.route': '/test-inbound-headers',
77110
},
78111
op: 'http.server',
79112
parent_span_id: outgoingHttpSpanId,
@@ -133,12 +166,29 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
133166

134167
expect(outboundTransaction.contexts?.trace).toEqual({
135168
data: {
169+
'sentry.source': 'route',
170+
'sentry.origin': 'auto.http.otel.http',
171+
'sentry.op': 'http.server',
172+
'sentry.sample_rate': 1,
136173
url: 'http://localhost:3030/test-outgoing-fetch',
137174
'otel.kind': 'SERVER',
138175
'http.response.status_code': 200,
139-
'sentry.op': 'http.server',
140-
'sentry.origin': 'auto.http.otel.http',
141-
'sentry.source': 'route',
176+
'http.url': 'http://localhost:3030/test-outgoing-fetch',
177+
'http.host': 'localhost:3030',
178+
'net.host.name': 'localhost',
179+
'http.method': 'GET',
180+
'http.scheme': 'http',
181+
'http.target': '/test-outgoing-fetch',
182+
'http.user_agent': 'axios/1.6.7',
183+
'http.flavor': '1.1',
184+
'net.transport': 'ip_tcp',
185+
'net.host.ip': expect.any(String),
186+
'net.host.port': expect.any(Number),
187+
'net.peer.ip': expect.any(String),
188+
'net.peer.port': expect.any(Number),
189+
'http.status_code': 200,
190+
'http.status_text': 'OK',
191+
'http.route': '/test-outgoing-fetch',
142192
},
143193
op: 'http.server',
144194
span_id: expect.any(String),
@@ -149,12 +199,28 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
149199

150200
expect(inboundTransaction.contexts?.trace).toEqual({
151201
data: expect.objectContaining({
202+
'sentry.source': 'route',
203+
'sentry.origin': 'auto.http.otel.http',
204+
'sentry.op': 'http.server',
205+
'sentry.sample_rate': 1,
152206
url: 'http://localhost:3030/test-inbound-headers',
153207
'otel.kind': 'SERVER',
154208
'http.response.status_code': 200,
155-
'sentry.op': 'http.server',
156-
'sentry.origin': 'auto.http.otel.http',
157-
'sentry.source': 'route',
209+
'http.url': 'http://localhost:3030/test-inbound-headers',
210+
'http.host': 'localhost:3030',
211+
'net.host.name': 'localhost',
212+
'http.method': 'GET',
213+
'http.scheme': 'http',
214+
'http.target': '/test-inbound-headers',
215+
'http.flavor': '1.1',
216+
'net.transport': 'ip_tcp',
217+
'net.host.ip': expect.any(String),
218+
'net.host.port': expect.any(Number),
219+
'net.peer.ip': expect.any(String),
220+
'net.peer.port': expect.any(Number),
221+
'http.status_code': 200,
222+
'http.status_text': 'OK',
223+
'http.route': '/test-inbound-headers',
158224
}),
159225
op: 'http.server',
160226
parent_span_id: outgoingHttpSpanId,

‎dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,29 @@ test('Sends an API route transaction', async ({ baseURL }) => {
2222

2323
expect(transactionEvent.contexts?.trace).toEqual({
2424
data: {
25+
'sentry.source': 'route',
26+
'sentry.origin': 'auto.http.otel.http',
27+
'sentry.op': 'http.server',
28+
'sentry.sample_rate': 1,
2529
url: 'http://localhost:3030/test-transaction',
2630
'otel.kind': 'SERVER',
2731
'http.response.status_code': 200,
28-
'sentry.op': 'http.server',
29-
'sentry.origin': 'auto.http.otel.http',
30-
'sentry.source': 'route',
32+
'http.url': 'http://localhost:3030/test-transaction',
33+
'http.host': 'localhost:3030',
34+
'net.host.name': 'localhost',
35+
'http.method': 'GET',
36+
'http.scheme': 'http',
37+
'http.target': '/test-transaction',
38+
'http.user_agent': 'axios/1.6.7',
39+
'http.flavor': '1.1',
40+
'net.transport': 'ip_tcp',
41+
'net.host.ip': expect.any(String),
42+
'net.host.port': expect.any(Number),
43+
'net.peer.ip': expect.any(String),
44+
'net.peer.port': expect.any(Number),
45+
'http.status_code': 200,
46+
'http.status_text': 'OK',
47+
'http.route': '/test-transaction',
3148
},
3249
op: 'http.server',
3350
span_id: expect.any(String),

‎packages/core/src/tracing/dynamicSamplingContext.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
4747
return {};
4848
}
4949

50-
// passing emit=false here to only emit later once the DSC is actually populated
5150
const dsc = getDynamicSamplingContextFromClient(spanToJSON(span).trace_id || '', client);
5251

5352
// TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext

‎packages/node-experimental/test/integration/scope.test.ts‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,22 +56,33 @@ describe('Integration | Scope', () => {
5656
await client.flush();
5757

5858
expect(beforeSend).toHaveBeenCalledTimes(1);
59+
60+
if (spanId) {
61+
expect(beforeSend).toHaveBeenCalledWith(
62+
expect.objectContaining({
63+
contexts: expect.objectContaining({
64+
trace: {
65+
span_id: spanId,
66+
trace_id: traceId,
67+
},
68+
}),
69+
}),
70+
{
71+
event_id: expect.any(String),
72+
originalException: error,
73+
syntheticException: expect.any(Error),
74+
},
75+
);
76+
}
77+
5978
expect(beforeSend).toHaveBeenCalledWith(
6079
expect.objectContaining({
61-
contexts: expect.objectContaining({
62-
trace: spanId
63-
? {
64-
span_id: spanId,
65-
trace_id: traceId,
66-
parent_span_id: undefined,
67-
}
68-
: expect.any(Object),
69-
}),
7080
tags: {
7181
tag1: 'val1',
7282
tag2: 'val2',
7383
tag3: 'val3',
7484
tag4: 'val4',
85+
...(enableTracing ? { transaction: 'outer' } : {}),
7586
},
7687
}),
7788
{
@@ -88,7 +99,12 @@ describe('Integration | Scope', () => {
8899
expect.objectContaining({
89100
contexts: expect.objectContaining({
90101
trace: {
91-
data: { 'otel.kind': 'INTERNAL', 'sentry.origin': 'manual', 'sentry.source': 'custom' },
102+
data: {
103+
'otel.kind': 'INTERNAL',
104+
'sentry.origin': 'manual',
105+
'sentry.source': 'custom',
106+
'sentry.sample_rate': 1,
107+
},
92108
span_id: spanId,
93109
status: 'ok',
94110
trace_id: traceId,
@@ -102,6 +118,7 @@ describe('Integration | Scope', () => {
102118
tag2: 'val2',
103119
tag3: 'val3',
104120
tag4: 'val4',
121+
transaction: 'outer',
105122
},
106123
timestamp: expect.any(Number),
107124
transaction: 'outer',
@@ -186,6 +203,7 @@ describe('Integration | Scope', () => {
186203
tag2: 'val2a',
187204
tag3: 'val3a',
188205
tag4: 'val4a',
206+
...(enableTracing ? { transaction: 'outer' } : {}),
189207
},
190208
}),
191209
{
@@ -211,6 +229,7 @@ describe('Integration | Scope', () => {
211229
tag2: 'val2b',
212230
tag3: 'val3b',
213231
tag4: 'val4b',
232+
...(enableTracing ? { transaction: 'outer' } : {}),
214233
},
215234
}),
216235
{

0 commit comments

Comments
 (0)