Skip to content

Commit 8501cd6

Browse files
msonnbmsonnb
committed
code review
1 parent 7704140 commit 8501cd6

13 files changed

Lines changed: 157 additions & 78 deletions

File tree

‎dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/init.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ window.Sentry = Sentry;
44

55
Sentry.init({
66
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7-
integrations: [],
87
tracesSampleRate: 1.0,
98
debug: true,
109
});

‎dev-packages/browser-integration-tests/suites/tracing/linking-addLink-static/init.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,5 @@ window.Sentry = Sentry;
55
Sentry.init({
66
traceLifecycle: 'static',
77
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8-
integrations: [],
98
tracesSampleRate: 1,
109
});

‎dev-packages/browser-integration-tests/suites/tracing/maxSpans-static/init.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
traceLifecycle: 'static',
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
integrations: [],
9+
tracesSampleRate: 1,
10+
});
11+
12+
Sentry.startSpan({ name: 'parent' }, () => {
13+
for (let i = 0; i < 5000; i++) {
14+
Sentry.startInactiveSpan({ name: `child ${i}` }).end();
15+
}
16+
});

‎dev-packages/browser-integration-tests/suites/tracing/maxSpans-static/test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../utils/fixtures';
3+
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequestOnUrl } from '../../../utils/helpers';
4+
5+
sentryTest('limits static transactions to 1000 child spans', async ({ getLocalTestUrl, page }) => {
6+
sentryTest.skip(shouldSkipTracingTest());
7+
8+
const url = await getLocalTestUrl({ testDir: __dirname });
9+
const req = await waitForTransactionRequestOnUrl(page, url);
10+
const transaction = envelopeRequestParser(req);
11+
12+
expect(transaction.spans).toHaveLength(1000);
13+
expect(transaction.spans).toContainEqual(expect.objectContaining({ description: 'child 0' }));
14+
expect(transaction.spans).toContainEqual(expect.objectContaining({ description: 'child 999' }));
15+
expect(transaction.spans).not.toContainEqual(expect.objectContaining({ description: 'child 1000' }));
16+
});

‎dev-packages/browser-integration-tests/suites/tracing/no-parent-span-client-report-static/init.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
traceLifecycle: 'static',
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false, instrumentNavigation: false })],
9+
tracesSampleRate: 1,
10+
sendClientReports: true,
11+
});

‎dev-packages/browser-integration-tests/suites/tracing/no-parent-span-client-report-static/subject.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fetch('http://sentry-test-site.example/api/test');

‎dev-packages/browser-integration-tests/suites/tracing/no-parent-span-client-report-static/test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from '@playwright/test';
2+
import type { ClientReport } from '@sentry/core';
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
hidePage,
7+
shouldSkipTracingTest,
8+
testingCdnBundle,
9+
waitForClientReportRequest,
10+
} from '../../../utils/helpers';
11+
12+
sentryTest(
13+
'records no_parent_span client report for fetch requests without an active span',
14+
async ({ getLocalTestUrl, page }) => {
15+
sentryTest.skip(shouldSkipTracingTest() || testingCdnBundle());
16+
17+
await page.route('http://sentry-test-site.example/api/test', route => {
18+
route.fulfill({
19+
status: 200,
20+
body: 'ok',
21+
headers: { 'Content-Type': 'text/plain' },
22+
});
23+
});
24+
25+
const url = await getLocalTestUrl({ testDir: __dirname });
26+
27+
const clientReportPromise = waitForClientReportRequest(page, report => {
28+
return report.discarded_events.some(e => e.reason === 'no_parent_span');
29+
});
30+
31+
await page.goto(url);
32+
33+
await hidePage(page);
34+
35+
const clientReport = envelopeRequestParser<ClientReport>(await clientReportPromise);
36+
37+
expect(clientReport.discarded_events).toEqual([
38+
{
39+
category: 'span',
40+
quantity: 1,
41+
reason: 'no_parent_span',
42+
},
43+
]);
44+
},
45+
);

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/navigation/test.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
import { expect } from '@playwright/test';
2+
import type { Event } from '@sentry/core';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
eventAndTraceHeaderRequestParser,
6+
getMultipleSentryEnvelopeRequests,
7+
shouldSkipFeedbackTest,
8+
shouldSkipTracingTest,
9+
waitForErrorRequest,
10+
} from '../../../../utils/helpers';
11+
import { getSpanOp, waitForStreamedSpan, waitForStreamedSpanEnvelope } from '../../../../utils/spanUtils';
12+
113
sentryTest('creates a new trace and sample_rand on each navigation', async ({ getLocalTestUrl, page }) => {
214
sentryTest.skip(shouldSkipTracingTest());
315

@@ -61,17 +73,6 @@ sentryTest('creates a new trace and sample_rand on each navigation', async ({ ge
6173
expect(navigation1TraceId).not.toEqual(navigation2TraceId);
6274
expect(navigation1SampleRand).not.toEqual(navigation2SampleRand);
6375
});
64-
import { expect } from '@playwright/test';
65-
import type { Event } from '@sentry/core';
66-
import { sentryTest } from '../../../../utils/fixtures';
67-
import {
68-
eventAndTraceHeaderRequestParser,
69-
getMultipleSentryEnvelopeRequests,
70-
shouldSkipFeedbackTest,
71-
shouldSkipTracingTest,
72-
waitForErrorRequest,
73-
} from '../../../../utils/helpers';
74-
import { getSpanOp, waitForStreamedSpan, waitForStreamedSpanEnvelope } from '../../../../utils/spanUtils';
7576

7677
sentryTest('error after navigation has navigation traceId', async ({ getLocalTestUrl, page }) => {
7778
sentryTest.skip(shouldSkipTracingTest());

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-headers/test.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,11 @@ sentryTest(
2525
},
2626
});
2727

28-
const [pageloadEvent, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
29-
const [navigationEvent, navigationTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, `${url}#foo`);
28+
const [pageloadSpan, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
29+
const [navigationSpan, navigationTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, `${url}#foo`);
3030

31-
const pageloadTraceContext = pageloadEvent;
32-
const navigationTraceContext = navigationEvent;
33-
34-
expect(pageloadEvent.is_segment).toBe(true);
35-
expect(pageloadTraceContext).toMatchObject({
31+
expect(pageloadSpan.is_segment).toBe(true);
32+
expect(pageloadSpan).toMatchObject({
3633
is_segment: true,
3734
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
3835
trace_id: META_TAG_TRACE_ID,
@@ -51,26 +48,26 @@ sentryTest(
5148
sample_rand: '0.42',
5249
});
5350

54-
expect(navigationEvent.is_segment).toBe(true);
55-
expect(navigationTraceContext).toMatchObject({
51+
expect(navigationSpan.is_segment).toBe(true);
52+
expect(navigationSpan).toMatchObject({
5653
is_segment: true,
5754
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'navigation' } }),
5855
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
5956
span_id: expect.stringMatching(/^[\da-f]{16}$/),
6057
});
6158
// navigation span is head of trace, so there's no parent span:
62-
expect(navigationTraceContext).not.toHaveProperty('parent_span_id');
59+
expect(navigationSpan).not.toHaveProperty('parent_span_id');
6360

6461
expect(navigationTraceHeader).toEqual({
6562
environment: 'production',
6663
public_key: 'public',
6764
sample_rate: '1',
6865
sampled: 'true',
69-
trace_id: navigationTraceContext?.trace_id,
66+
trace_id: navigationSpan?.trace_id,
7067
sample_rand: expect.any(String),
7168
});
7269

73-
expect(pageloadTraceContext?.trace_id).not.toEqual(navigationTraceContext?.trace_id);
70+
expect(pageloadSpan?.trace_id).not.toEqual(navigationSpan?.trace_id);
7471
expect(pageloadTraceHeader?.sample_rand).not.toEqual(navigationTraceHeader?.sample_rand);
7572
},
7673
);

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/pageload-meta/test.ts‎

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,11 @@ sentryTest(
2929

3030
const url = await getLocalTestUrl({ testDir: __dirname });
3131

32-
const [pageloadEvent, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
33-
const [navigationEvent, navigationTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, `${url}#foo`);
32+
const [pageloadSpan, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
33+
const [navigationSpan, navigationTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, `${url}#foo`);
3434

35-
const pageloadTraceContext = pageloadEvent;
36-
const navigationTraceContext = navigationEvent;
37-
38-
expect(pageloadEvent.is_segment).toBe(true);
39-
expect(pageloadTraceContext).toMatchObject({
35+
expect(pageloadSpan.is_segment).toBe(true);
36+
expect(pageloadSpan).toMatchObject({
4037
is_segment: true,
4138
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
4239
trace_id: META_TAG_TRACE_ID,
@@ -55,26 +52,26 @@ sentryTest(
5552
sample_rand: '0.42',
5653
});
5754

58-
expect(navigationEvent.is_segment).toBe(true);
59-
expect(navigationTraceContext).toMatchObject({
55+
expect(navigationSpan.is_segment).toBe(true);
56+
expect(navigationSpan).toMatchObject({
6057
is_segment: true,
6158
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'navigation' } }),
6259
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
6360
span_id: expect.stringMatching(/^[\da-f]{16}$/),
6461
});
6562
// navigation span is head of trace, so there's no parent span:
66-
expect(navigationTraceContext).not.toHaveProperty('parent_span_id');
63+
expect(navigationSpan).not.toHaveProperty('parent_span_id');
6764

6865
expect(navigationTraceHeader).toEqual({
6966
environment: 'production',
7067
public_key: 'public',
7168
sample_rate: '1',
7269
sampled: 'true',
73-
trace_id: navigationTraceContext?.trace_id,
70+
trace_id: navigationSpan?.trace_id,
7471
sample_rand: expect.any(String),
7572
});
7673

77-
expect(pageloadTraceContext?.trace_id).not.toEqual(navigationTraceContext?.trace_id);
74+
expect(pageloadSpan?.trace_id).not.toEqual(navigationSpan?.trace_id);
7875
expect(pageloadTraceHeader?.sample_rand).not.toEqual(navigationTraceHeader?.sample_rand);
7976
},
8077
);
@@ -86,9 +83,9 @@ sentryTest('error after <meta> tag pageload has pageload traceId', async ({ getL
8683

8784
const url = await getLocalTestUrl({ testDir: __dirname });
8885

89-
const [pageloadEvent, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
86+
const [pageloadSpan, pageloadTraceHeader] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
9087

91-
expect(pageloadEvent).toMatchObject({
88+
expect(pageloadSpan).toMatchObject({
9289
is_segment: true,
9390
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
9491
trace_id: META_TAG_TRACE_ID,
@@ -144,13 +141,13 @@ sentryTest('error during <meta> tag pageload has pageload traceId', async ({ get
144141
const errorPromise = waitForErrorRequest(page).then(eventAndTraceHeaderRequestParser);
145142
await page.goto(url);
146143
await page.locator('#errorBtn').click();
147-
const [[pageloadEvent, pageloadTraceHeader], [errorEvent, errorTraceHeader]] = await Promise.all([
144+
const [[pageloadSpan, pageloadTraceHeader], [errorEvent, errorTraceHeader]] = await Promise.all([
148145
pageloadPromise,
149146
errorPromise,
150147
]);
151148

152-
expect(pageloadEvent.is_segment).toBe(true);
153-
expect(pageloadEvent).toMatchObject({
149+
expect(pageloadSpan.is_segment).toBe(true);
150+
expect(pageloadSpan).toMatchObject({
154151
is_segment: true,
155152
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
156153
trace_id: META_TAG_TRACE_ID,
@@ -205,17 +202,17 @@ sentryTest(
205202
});
206203
});
207204

208-
const pageloadEventPromise = waitForStreamedSpanAndTraceHeader(
205+
const pageloadSpanPromise = waitForStreamedSpanAndTraceHeader(
209206
page,
210207
span => span.is_segment && getSpanOp(span) === 'pageload',
211208
);
212209
const requestPromise = page.waitForRequest('http://sentry-test-site.example/*');
213210
await page.goto(url);
214211
await page.locator('#fetchBtn').click();
215-
const [[pageloadEvent, pageloadTraceHeader], request] = await Promise.all([pageloadEventPromise, requestPromise]);
212+
const [[pageloadSpan, pageloadTraceHeader], request] = await Promise.all([pageloadSpanPromise, requestPromise]);
216213

217-
expect(pageloadEvent.is_segment).toBe(true);
218-
expect(pageloadEvent).toMatchObject({
214+
expect(pageloadSpan.is_segment).toBe(true);
215+
expect(pageloadSpan).toMatchObject({
219216
is_segment: true,
220217
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
221218
trace_id: META_TAG_TRACE_ID,
@@ -260,17 +257,17 @@ sentryTest(
260257

261258
const url = await getLocalTestUrl({ testDir: __dirname });
262259

263-
const pageloadEventPromise = waitForStreamedSpanAndTraceHeader(
260+
const pageloadSpanPromise = waitForStreamedSpanAndTraceHeader(
264261
page,
265262
span => span.is_segment && getSpanOp(span) === 'pageload',
266263
);
267264
const requestPromise = page.waitForRequest('http://sentry-test-site.example/*');
268265
await page.goto(url);
269266
await page.locator('#xhrBtn').click();
270-
const [[pageloadEvent, pageloadTraceHeader], request] = await Promise.all([pageloadEventPromise, requestPromise]);
267+
const [[pageloadSpan, pageloadTraceHeader], request] = await Promise.all([pageloadSpanPromise, requestPromise]);
271268

272-
expect(pageloadEvent.is_segment).toBe(true);
273-
expect(pageloadEvent).toMatchObject({
269+
expect(pageloadSpan.is_segment).toBe(true);
270+
expect(pageloadSpan).toMatchObject({
274271
is_segment: true,
275272
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
276273
trace_id: META_TAG_TRACE_ID,
@@ -305,10 +302,9 @@ sentryTest('user feedback event after pageload has pageload traceId in headers',
305302

306303
const url = await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true });
307304

308-
const [pageloadEvent] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
309-
const pageloadTraceContext = pageloadEvent;
305+
const [pageloadSpan] = await waitForStreamedSpanAndTraceHeaderOnUrl(page, url);
310306

311-
expect(pageloadTraceContext).toMatchObject({
307+
expect(pageloadSpan).toMatchObject({
312308
is_segment: true,
313309
attributes: expect.objectContaining({ [SENTRY_OP]: { type: 'string', value: 'pageload' } }),
314310
trace_id: META_TAG_TRACE_ID,

0 commit comments

Comments
 (0)