Skip to content

Commit 10ca5fe

Browse files
committed
fix tests
1 parent a7c2c80 commit 10ca5fe

File tree

4 files changed

+69
-43
lines changed

4 files changed

+69
-43
lines changed

dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { sentryTest } from '../../../../utils/fixtures';
44
import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers';
55

66
sentryTest(
7-
'there should be no span created for xhr requests with no active span',
7+
'should not create span for xhr requests with no active span but should attach sentry-trace header',
88
async ({ getLocalTestPath, page }) => {
99
if (shouldSkipTracingTest()) {
1010
sentryTest.skip();
@@ -13,23 +13,32 @@ sentryTest(
1313
const url = await getLocalTestPath({ testDir: __dirname });
1414

1515
let requestCount = 0;
16+
const sentryTraceHeaders: string[] = [];
1617
page.on('request', request => {
17-
expect(envelopeUrlRegex.test(request.url())).toBe(false);
18+
const url = request.url();
19+
20+
const sentryTraceHeader = request.headers()['sentry-trace'];
21+
if (sentryTraceHeader) {
22+
sentryTraceHeaders.push(sentryTraceHeader);
23+
}
24+
expect(envelopeUrlRegex.test(url)).toBe(false);
25+
26+
// We only want to count API requests
27+
if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) {
28+
return;
29+
}
1830
requestCount++;
1931
});
2032

2133
await page.goto(url);
2234

23-
// Here are the requests that should exist:
24-
// 1. HTML page
25-
// 2. Init JS bundle
26-
// 3. Subject JS bundle
27-
// 4 [OPTIONAl] CDN JS bundle
28-
// and then 3 fetch requests
29-
if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) {
30-
expect(requestCount).toBe(7);
31-
} else {
32-
expect(requestCount).toBe(6);
33-
}
35+
expect(requestCount).toBe(3);
36+
37+
expect(sentryTraceHeaders).toHaveLength(3);
38+
expect(sentryTraceHeaders).toEqual([
39+
expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/),
40+
expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/),
41+
expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/),
42+
]);
3443
},
3544
);

packages/browser/src/tracing/request.ts

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import {
77
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
88
SentryNonRecordingSpan,
9+
getActiveSpan,
910
getClient,
1011
getCurrentScope,
1112
getDynamicSamplingContextFromClient,
@@ -307,27 +308,37 @@ export function xhrCallback(
307308
return undefined;
308309
}
309310

310-
const span = shouldCreateSpanResult
311-
? startInactiveSpan({
312-
name: `${sentryXhrData.method} ${sentryXhrData.url}`,
313-
onlyIfParent: true,
314-
attributes: {
315-
type: 'xhr',
316-
'http.method': sentryXhrData.method,
317-
url: sentryXhrData.url,
318-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
319-
},
320-
op: 'http.client',
321-
})
322-
: new SentryNonRecordingSpan();
311+
const hasParent = !!getActiveSpan();
312+
313+
const span =
314+
shouldCreateSpanResult && hasParent
315+
? startInactiveSpan({
316+
name: `${sentryXhrData.method} ${sentryXhrData.url}`,
317+
attributes: {
318+
type: 'xhr',
319+
'http.method': sentryXhrData.method,
320+
url: sentryXhrData.url,
321+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
322+
},
323+
op: 'http.client',
324+
})
325+
: new SentryNonRecordingSpan();
323326

324327
xhr.__sentry_xhr_span_id__ = span.spanContext().spanId;
325328
spans[xhr.__sentry_xhr_span_id__] = span;
326329

327330
const client = getClient();
328331

329332
if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url) && client) {
330-
addTracingHeadersToXhrRequest(xhr, client, hasTracingEnabled() ? span : undefined);
333+
addTracingHeadersToXhrRequest(
334+
xhr,
335+
client,
336+
// In the following cases, we do not want to use the span as base for the trace headers,
337+
// which means that the headers will be generated from the scope:
338+
// - If tracing is disabled (TWP)
339+
// - If the span has no parent span - which means we ran into `onlyIfParent` check
340+
hasTracingEnabled() && hasParent ? span : undefined,
341+
);
331342
}
332343

333344
return span;

packages/core/src/fetch.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
} from './tracing';
1717
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
1818
import { hasTracingEnabled } from './utils/hasTracingEnabled';
19-
import { spanToTraceHeader } from './utils/spanUtils';
19+
import { getActiveSpan, spanToTraceHeader } from './utils/spanUtils';
2020

2121
type PolymorphicRequestHeaders =
2222
| Record<string, string | undefined>
@@ -81,19 +81,21 @@ export function instrumentFetchRequest(
8181

8282
const { method, url } = handlerData.fetchData;
8383

84-
const span = shouldCreateSpanResult
85-
? startInactiveSpan({
86-
name: `${method} ${url}`,
87-
onlyIfParent: true,
88-
attributes: {
89-
url,
90-
type: 'fetch',
91-
'http.method': method,
92-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
93-
},
94-
op: 'http.client',
95-
})
96-
: new SentryNonRecordingSpan();
84+
const hasParent = !!getActiveSpan();
85+
86+
const span =
87+
shouldCreateSpanResult && hasParent
88+
? startInactiveSpan({
89+
name: `${method} ${url}`,
90+
attributes: {
91+
url,
92+
type: 'fetch',
93+
'http.method': method,
94+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
95+
},
96+
op: 'http.client',
97+
})
98+
: new SentryNonRecordingSpan();
9799

98100
handlerData.fetchData.__span = span.spanContext().spanId;
99101
spans[span.spanContext().spanId] = span;
@@ -112,7 +114,11 @@ export function instrumentFetchRequest(
112114
client,
113115
scope,
114116
options,
115-
hasTracingEnabled() ? span : undefined,
117+
// In the following cases, we do not want to use the span as base for the trace headers,
118+
// which means that the headers will be generated from the scope:
119+
// - If tracing is disabled (TWP)
120+
// - If the span has no parent span - which means we ran into `onlyIfParent` check
121+
hasTracingEnabled() && hasParent ? span : undefined,
116122
);
117123
}
118124

packages/core/src/tracing/trace.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
357357
traceId,
358358
sampled,
359359
})
360-
: new SentryNonRecordingSpan();
360+
: new SentryNonRecordingSpan({ traceId });
361361

362362
addChildSpanToSpan(parentSpan, childSpan);
363363

0 commit comments

Comments
 (0)