From 2c130d87a35fe1998adfc37fbb8f2b48e7fede9e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 11 Apr 2024 15:59:40 +0200 Subject: [PATCH 01/11] fix(browser): Ensure tracing without performance (TWP) works This also forward-ports the tests from https://github.com/getsentry/sentry-javascript/pull/11555, and adds some more tests. --- .../tracing/request/fetch-no-tracing/init.js | 8 ++++ .../request/fetch-no-tracing/subject.js | 5 ++ .../tracing/request/fetch-no-tracing/test.ts | 30 ++++++++++++ .../request/fetch-tracing-unsampled/init.js | 10 ++++ .../fetch-tracing-unsampled/subject.js | 5 ++ .../request/fetch-tracing-unsampled/test.ts | 43 +++++++++++++++++ .../fetch-tracing-without-performance/init.js | 10 ++++ .../subject.js | 5 ++ .../fetch-tracing-without-performance/test.ts | 46 +++++++++++++++++++ .../suites/tracing/request/fetch/subject.js | 6 ++- .../suites/tracing/request/fetch/test.ts | 31 +++++++++---- .../tracing/request/xhr-no-tracing/init.js | 8 ++++ .../tracing/request/xhr-no-tracing/subject.js | 12 +++++ .../tracing/request/xhr-no-tracing/test.ts | 30 ++++++++++++ .../request/xhr-tracing-unsampled/init.js | 10 ++++ .../request/xhr-tracing-unsampled/subject.js | 12 +++++ .../request/xhr-tracing-unsampled/test.ts | 43 +++++++++++++++++ .../xhr-tracing-without-performance/init.js | 9 ++++ .../subject.js | 12 +++++ .../xhr-tracing-without-performance/test.ts | 46 +++++++++++++++++++ .../suites/tracing/request/xhr/subject.js | 1 + .../suites/tracing/request/xhr/test.ts | 31 +++++++++---- packages/browser/src/tracing/request.ts | 38 ++++++++------- packages/core/src/fetch.ts | 12 +++-- 24 files changed, 426 insertions(+), 37 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/init.js new file mode 100644 index 000000000000..5cfae0864821 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracePropagationTargets: ['http://example.com'], +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/subject.js new file mode 100644 index 000000000000..eef6d917a2d8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/subject.js @@ -0,0 +1,5 @@ +fetch('http://example.com/0').then( + fetch('http://example.com/1', { headers: { 'X-Test-Header': 'existing-header' } }).then( + fetch('http://example.com/2'), + ), +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/test.ts new file mode 100644 index 000000000000..95c7b3052732 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-no-tracing/test.ts @@ -0,0 +1,30 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'should not attach `sentry-trace` header to fetch requests without tracing', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + for (const request of requests) { + const requestHeaders = request.headers(); + expect(requestHeaders['sentry-trace']).toBeUndefined(); + expect(requestHeaders['baggage']).toBeUndefined(); + } + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/init.js new file mode 100644 index 000000000000..203a32557f21 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], + tracesSampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/subject.js new file mode 100644 index 000000000000..eef6d917a2d8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/subject.js @@ -0,0 +1,5 @@ +fetch('http://example.com/0').then( + fetch('http://example.com/1', { headers: { 'X-Test-Header': 'existing-header' } }).then( + fetch('http://example.com/2'), + ), +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/test.ts new file mode 100644 index 000000000000..44911665af7c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-unsampled/test.ts @@ -0,0 +1,43 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest('should attach `sentry-trace` header to unsampled fetch requests', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/init.js new file mode 100644 index 000000000000..89854ab72450 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], + // no tracesSampleRate defined means TWP mode +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/subject.js new file mode 100644 index 000000000000..eef6d917a2d8 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/subject.js @@ -0,0 +1,5 @@ +fetch('http://example.com/0').then( + fetch('http://example.com/1', { headers: { 'X-Test-Header': 'existing-header' } }).then( + fetch('http://example.com/2'), + ), +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/test.ts new file mode 100644 index 000000000000..8c5c626731df --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-tracing-without-performance/test.ts @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'should attach `sentry-trace` header to tracing without performance (TWP) fetch requests', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/subject.js index f62499b1e9c5..eef6d917a2d8 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/subject.js @@ -1 +1,5 @@ -fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); +fetch('http://example.com/0').then( + fetch('http://example.com/1', { headers: { 'X-Test-Header': 'existing-header' } }).then( + fetch('http://example.com/2'), + ), +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts index 4308491f2d7f..00cf0baafc6a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts @@ -4,7 +4,7 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers'; -sentryTest('should create spans for multiple fetch requests', async ({ getLocalTestPath, page }) => { +sentryTest('should create spans for fetch requests', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -40,7 +40,7 @@ sentryTest('should create spans for multiple fetch requests', async ({ getLocalT ); }); -sentryTest('should attach `sentry-trace` header to multiple fetch requests', async ({ getLocalTestPath, page }) => { +sentryTest('should attach `sentry-trace` header to fetch requests', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -56,10 +56,25 @@ sentryTest('should attach `sentry-trace` header to multiple fetch requests', asy expect(requests).toHaveLength(3); - for (const request of requests) { - const requestHeaders = request.headers(); - expect(requestHeaders).toMatchObject({ - 'sentry-trace': expect.any(String), - }); - } + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/init.js new file mode 100644 index 000000000000..5cfae0864821 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/init.js @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracePropagationTargets: ['http://example.com'], +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/subject.js new file mode 100644 index 000000000000..cb5f05dad5c1 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/subject.js @@ -0,0 +1,12 @@ +const xhr_1 = new XMLHttpRequest(); +xhr_1.open('GET', 'http://example.com/0'); +xhr_1.send(); + +const xhr_2 = new XMLHttpRequest(); +xhr_2.open('GET', 'http://example.com/1'); +xhr_2.setRequestHeader('X-Test-Header', 'existing-header'); +xhr_2.send(); + +const xhr_3 = new XMLHttpRequest(); +xhr_3.open('GET', 'http://example.com/2'); +xhr_3.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/test.ts new file mode 100644 index 000000000000..95c7b3052732 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-no-tracing/test.ts @@ -0,0 +1,30 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'should not attach `sentry-trace` header to fetch requests without tracing', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + for (const request of requests) { + const requestHeaders = request.headers(); + expect(requestHeaders['sentry-trace']).toBeUndefined(); + expect(requestHeaders['baggage']).toBeUndefined(); + } + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/init.js new file mode 100644 index 000000000000..203a32557f21 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], + tracesSampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/subject.js new file mode 100644 index 000000000000..cb5f05dad5c1 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/subject.js @@ -0,0 +1,12 @@ +const xhr_1 = new XMLHttpRequest(); +xhr_1.open('GET', 'http://example.com/0'); +xhr_1.send(); + +const xhr_2 = new XMLHttpRequest(); +xhr_2.open('GET', 'http://example.com/1'); +xhr_2.setRequestHeader('X-Test-Header', 'existing-header'); +xhr_2.send(); + +const xhr_3 = new XMLHttpRequest(); +xhr_3.open('GET', 'http://example.com/2'); +xhr_3.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/test.ts new file mode 100644 index 000000000000..954628873383 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-unsampled/test.ts @@ -0,0 +1,43 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest('should attach `sentry-trace` header to unsampled xhr requests', async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/), + baggage: expect.any(String), + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/init.js new file mode 100644 index 000000000000..e7db5efdc388 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/init.js @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/subject.js new file mode 100644 index 000000000000..cb5f05dad5c1 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/subject.js @@ -0,0 +1,12 @@ +const xhr_1 = new XMLHttpRequest(); +xhr_1.open('GET', 'http://example.com/0'); +xhr_1.send(); + +const xhr_2 = new XMLHttpRequest(); +xhr_2.open('GET', 'http://example.com/1'); +xhr_2.setRequestHeader('X-Test-Header', 'existing-header'); +xhr_2.send(); + +const xhr_3 = new XMLHttpRequest(); +xhr_3.open('GET', 'http://example.com/2'); +xhr_3.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/test.ts new file mode 100644 index 000000000000..2e1a8ee36277 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-tracing-without-performance/test.ts @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'should attach `sentry-trace` header to tracing without performance (TWP) xhr requests', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + baggage: expect.any(String), + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/subject.js index 5790c230aa66..cb5f05dad5c1 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/subject.js @@ -4,6 +4,7 @@ xhr_1.send(); const xhr_2 = new XMLHttpRequest(); xhr_2.open('GET', 'http://example.com/1'); +xhr_2.setRequestHeader('X-Test-Header', 'existing-header'); xhr_2.send(); const xhr_3 = new XMLHttpRequest(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts index 3d329c9c5c6d..13646a34826e 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts @@ -4,7 +4,7 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; -sentryTest('should create spans for multiple XHR requests', async ({ getLocalTestPath, page }) => { +sentryTest('should create spans for XHR requests', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -28,7 +28,7 @@ sentryTest('should create spans for multiple XHR requests', async ({ getLocalTes ); }); -sentryTest('should attach `sentry-trace` header to multiple XHR requests', async ({ getLocalTestPath, page }) => { +sentryTest('should attach `sentry-trace` header to XHR requests', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -44,10 +44,25 @@ sentryTest('should attach `sentry-trace` header to multiple XHR requests', async expect(requests).toHaveLength(3); - for (const request of requests) { - const requestHeaders = request.headers(); - expect(requestHeaders).toMatchObject({ - 'sentry-trace': expect.any(String), - }); - } + const request1 = requests[0]; + const requestHeaders1 = request1.headers(); + expect(requestHeaders1).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); + + const request2 = requests[1]; + const requestHeaders2 = request2.headers(); + expect(requestHeaders2).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + 'x-test-header': 'existing-header', + }); + + const request3 = requests[2]; + const requestHeaders3 = request3.headers(); + expect(requestHeaders3).toMatchObject({ + 'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/), + baggage: expect.any(String), + }); }); diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index 374dc4feaf13..4f5a6e6560c7 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -18,7 +18,7 @@ import { spanToTraceHeader, startInactiveSpan, } from '@sentry/core'; -import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; +import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, addFetchInstrumentationHandler, @@ -285,11 +285,11 @@ export function xhrCallback( const xhr = handlerData.xhr; const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY]; - if (!hasTracingEnabled() || !xhr || xhr.__sentry_own_request__ || !sentryXhrData) { + if (!xhr || xhr.__sentry_own_request__ || !sentryXhrData) { return undefined; } - const shouldCreateSpanResult = shouldCreateSpan(sentryXhrData.url); + const shouldCreateSpanResult = hasTracingEnabled() && shouldCreateSpan(sentryXhrData.url); // check first if the request has finished and is tracked by an existing span which should now end if (handlerData.endTimestamp && shouldCreateSpanResult) { @@ -307,9 +307,6 @@ export function xhrCallback( return undefined; } - const scope = getCurrentScope(); - const isolationScope = getIsolationScope(); - const span = shouldCreateSpanResult ? startInactiveSpan({ name: `${sentryXhrData.method} ${sentryXhrData.url}`, @@ -330,21 +327,28 @@ export function xhrCallback( const client = getClient(); if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url) && client) { - const { traceId, spanId, sampled, dsc } = { - ...isolationScope.getPropagationContext(), - ...scope.getPropagationContext(), - }; + addTracingHeadersToXhrRequest(xhr, client, hasTracingEnabled() ? span : undefined); + } - const sentryTraceHeader = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, spanId, sampled); + return span; +} - const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader( - dsc || (span ? getDynamicSamplingContextFromSpan(span) : getDynamicSamplingContextFromClient(traceId, client)), - ); +function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, client: Client, span?: Span): void { + const scope = getCurrentScope(); + const isolationScope = getIsolationScope(); + const { traceId, spanId, sampled, dsc } = { + ...isolationScope.getPropagationContext(), + ...scope.getPropagationContext(), + }; - setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader); - } + const sentryTraceHeader = + span && hasTracingEnabled() ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, spanId, sampled); - return span; + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader( + dsc || (span ? getDynamicSamplingContextFromSpan(span) : getDynamicSamplingContextFromClient(traceId, client)), + ); + + setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader); } function setHeaderOnXhr( diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 8002d9280297..70e34b9a4589 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -41,11 +41,11 @@ export function instrumentFetchRequest( spans: Record, spanOrigin: SpanOrigin = 'auto.http.browser', ): Span | undefined { - if (!hasTracingEnabled() || !handlerData.fetchData) { + if (!handlerData.fetchData) { return undefined; } - const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url); + const shouldCreateSpanResult = hasTracingEnabled() && shouldCreateSpan(handlerData.fetchData.url); if (handlerData.endTimestamp && shouldCreateSpanResult) { const spanId = handlerData.fetchData.__span; @@ -107,7 +107,13 @@ export function instrumentFetchRequest( // eslint-disable-next-line @typescript-eslint/no-explicit-any const options: { [key: string]: any } = handlerData.args[1]; - options.headers = addTracingHeadersToFetchRequest(request, client, scope, options, span); + options.headers = addTracingHeadersToFetchRequest( + request, + client, + scope, + options, + hasTracingEnabled() ? span : undefined, + ); } return span; From 01f20e57b415f81713ca9bc1717a81085c4ac456 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 11 Apr 2024 16:22:55 +0200 Subject: [PATCH 02/11] fix test --- packages/vercel-edge/test/wintercg-fetch.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/vercel-edge/test/wintercg-fetch.test.ts b/packages/vercel-edge/test/wintercg-fetch.test.ts index bdcb9bedd36f..79678046d02d 100644 --- a/packages/vercel-edge/test/wintercg-fetch.test.ts +++ b/packages/vercel-edge/test/wintercg-fetch.test.ts @@ -165,7 +165,12 @@ describe('WinterCGFetch instrumentation', () => { expect(addBreadcrumbSpy).toBeCalledWith( { category: 'fetch', - data: { method: 'POST', status_code: 201, url: 'http://my-website.com/' }, + data: { + method: 'POST', + status_code: 201, + url: 'http://my-website.com/', + __span: expect.any(String), + }, type: 'http', }, { From a7c2c804f52d10d5a9bed3aa1ca554f7882a15dd Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 10:11:37 +0200 Subject: [PATCH 03/11] fix flaky test --- .../request/fetch-with-no-active-span/init.js | 9 +++-- .../request/fetch-with-no-active-span/test.ts | 35 ++++++++++++------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js index a8d8c03d1f25..24995aceb538 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js @@ -4,8 +4,13 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - // disable pageload transaction - integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false })], + // disable auto span creation + integrations: [ + Sentry.browserTracingIntegration({ + instrumentPageLoad: false, + instrumentNavigation: false, + }), + ], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index 4dc5a0ac4e0a..d0f030505c13 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -4,7 +4,7 @@ import { sentryTest } from '../../../../utils/fixtures'; import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( - 'there should be no span created for fetch requests with no active span', + 'should not create span for fetch requests with no active span but should attach sentry-trace header', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); @@ -13,23 +13,32 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); let requestCount = 0; + const sentryTraceHeaders: string[] = []; page.on('request', request => { - expect(envelopeUrlRegex.test(request.url())).toBe(false); + const url = request.url(); + + const sentryTraceHeader = request.headers()['sentry-trace']; + if (sentryTraceHeader) { + sentryTraceHeaders.push(sentryTraceHeader); + } + expect(envelopeUrlRegex.test(url)).toBe(false); + + // We only want to count API requests + if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { + return; + } requestCount++; }); await page.goto(url); - // Here are the requests that should exist: - // 1. HTML page - // 2. Init JS bundle - // 3. Subject JS bundle - // 4 [OPTIONAl] CDN JS bundle - // and then 3 fetch requests - if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) { - expect(requestCount).toBe(7); - } else { - expect(requestCount).toBe(6); - } + expect(requestCount).toBe(3); + + expect(sentryTraceHeaders).toHaveLength(3); + expect(sentryTraceHeaders).toEqual([ + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + ]); }, ); From 10ca5fe9482e63affae470e989a82eb742985ab1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 12:03:53 +0200 Subject: [PATCH 04/11] fix tests --- .../request/xhr-with-no-active-span/test.ts | 35 ++++++++++------- packages/browser/src/tracing/request.ts | 39 ++++++++++++------- packages/core/src/fetch.ts | 36 ++++++++++------- packages/core/src/tracing/trace.ts | 2 +- 4 files changed, 69 insertions(+), 43 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts index 19c1f5891a39..7476f45ae1aa 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts @@ -4,7 +4,7 @@ import { sentryTest } from '../../../../utils/fixtures'; import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( - 'there should be no span created for xhr requests with no active span', + 'should not create span for xhr requests with no active span but should attach sentry-trace header', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); @@ -13,23 +13,32 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); let requestCount = 0; + const sentryTraceHeaders: string[] = []; page.on('request', request => { - expect(envelopeUrlRegex.test(request.url())).toBe(false); + const url = request.url(); + + const sentryTraceHeader = request.headers()['sentry-trace']; + if (sentryTraceHeader) { + sentryTraceHeaders.push(sentryTraceHeader); + } + expect(envelopeUrlRegex.test(url)).toBe(false); + + // We only want to count API requests + if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { + return; + } requestCount++; }); await page.goto(url); - // Here are the requests that should exist: - // 1. HTML page - // 2. Init JS bundle - // 3. Subject JS bundle - // 4 [OPTIONAl] CDN JS bundle - // and then 3 fetch requests - if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) { - expect(requestCount).toBe(7); - } else { - expect(requestCount).toBe(6); - } + expect(requestCount).toBe(3); + + expect(sentryTraceHeaders).toHaveLength(3); + expect(sentryTraceHeaders).toEqual([ + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/), + ]); }, ); diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index 4f5a6e6560c7..2c013fe27232 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -6,6 +6,7 @@ import { import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SentryNonRecordingSpan, + getActiveSpan, getClient, getCurrentScope, getDynamicSamplingContextFromClient, @@ -307,19 +308,21 @@ export function xhrCallback( return undefined; } - const span = shouldCreateSpanResult - ? startInactiveSpan({ - name: `${sentryXhrData.method} ${sentryXhrData.url}`, - onlyIfParent: true, - attributes: { - type: 'xhr', - 'http.method': sentryXhrData.method, - url: sentryXhrData.url, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser', - }, - op: 'http.client', - }) - : new SentryNonRecordingSpan(); + const hasParent = !!getActiveSpan(); + + const span = + shouldCreateSpanResult && hasParent + ? startInactiveSpan({ + name: `${sentryXhrData.method} ${sentryXhrData.url}`, + attributes: { + type: 'xhr', + 'http.method': sentryXhrData.method, + url: sentryXhrData.url, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser', + }, + op: 'http.client', + }) + : new SentryNonRecordingSpan(); xhr.__sentry_xhr_span_id__ = span.spanContext().spanId; spans[xhr.__sentry_xhr_span_id__] = span; @@ -327,7 +330,15 @@ export function xhrCallback( const client = getClient(); if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url) && client) { - addTracingHeadersToXhrRequest(xhr, client, hasTracingEnabled() ? span : undefined); + addTracingHeadersToXhrRequest( + xhr, + client, + // In the following cases, we do not want to use the span as base for the trace headers, + // which means that the headers will be generated from the scope: + // - If tracing is disabled (TWP) + // - If the span has no parent span - which means we ran into `onlyIfParent` check + hasTracingEnabled() && hasParent ? span : undefined, + ); } return span; diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 70e34b9a4589..f9069b6b6efa 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -16,7 +16,7 @@ import { } from './tracing'; import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan'; import { hasTracingEnabled } from './utils/hasTracingEnabled'; -import { spanToTraceHeader } from './utils/spanUtils'; +import { getActiveSpan, spanToTraceHeader } from './utils/spanUtils'; type PolymorphicRequestHeaders = | Record @@ -81,19 +81,21 @@ export function instrumentFetchRequest( const { method, url } = handlerData.fetchData; - const span = shouldCreateSpanResult - ? startInactiveSpan({ - name: `${method} ${url}`, - onlyIfParent: true, - attributes: { - url, - type: 'fetch', - 'http.method': method, - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, - }, - op: 'http.client', - }) - : new SentryNonRecordingSpan(); + const hasParent = !!getActiveSpan(); + + const span = + shouldCreateSpanResult && hasParent + ? startInactiveSpan({ + name: `${method} ${url}`, + attributes: { + url, + type: 'fetch', + 'http.method': method, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin, + }, + op: 'http.client', + }) + : new SentryNonRecordingSpan(); handlerData.fetchData.__span = span.spanContext().spanId; spans[span.spanContext().spanId] = span; @@ -112,7 +114,11 @@ export function instrumentFetchRequest( client, scope, options, - hasTracingEnabled() ? span : undefined, + // In the following cases, we do not want to use the span as base for the trace headers, + // which means that the headers will be generated from the scope: + // - If tracing is disabled (TWP) + // - If the span has no parent span - which means we ran into `onlyIfParent` check + hasTracingEnabled() && hasParent ? span : undefined, ); } diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 4b6fe024fd88..791cae88a573 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -357,7 +357,7 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp traceId, sampled, }) - : new SentryNonRecordingSpan(); + : new SentryNonRecordingSpan({ traceId }); addChildSpanToSpan(parentSpan, childSpan); From 2b5ae72f9849b0b28da68ab49e8f43363fbdaf49 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 14:11:16 +0200 Subject: [PATCH 05/11] disable sessions --- .../suites/tracing/request/fetch-with-no-active-span/init.js | 1 + .../suites/tracing/request/xhr-with-no-active-span/init.js | 1 + 2 files changed, 2 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js index 24995aceb538..566461669763 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js @@ -13,4 +13,5 @@ Sentry.init({ ], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, + autoSessionTracking: false }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js index a8d8c03d1f25..cbb1f886cb9a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js @@ -8,4 +8,5 @@ Sentry.init({ integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false })], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, + autoSessionTracking: false }); From 3d4ea8390a9a0c6ca0b2f540e03d989567137f3b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 14:33:15 +0200 Subject: [PATCH 06/11] unflake test... --- .../tracing/request/fetch-with-no-active-span/test.ts | 8 +++++++- .../tracing/request/xhr-with-no-active-span/test.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index d0f030505c13..194c7248070e 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -24,7 +24,13 @@ sentryTest( expect(envelopeUrlRegex.test(url)).toBe(false); // We only want to count API requests - if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { + if ( + envelopeUrlRegex.test(url) || + url.endsWith('.html') || + url.endsWith('.js') || + url.endsWith('.css') || + url.endsWith('.map') + ) { return; } requestCount++; diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts index 7476f45ae1aa..baf39269c75d 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts @@ -21,10 +21,15 @@ sentryTest( if (sentryTraceHeader) { sentryTraceHeaders.push(sentryTraceHeader); } - expect(envelopeUrlRegex.test(url)).toBe(false); // We only want to count API requests - if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { + if ( + envelopeUrlRegex.test(url) || + url.endsWith('.html') || + url.endsWith('.js') || + url.endsWith('.css') || + url.endsWith('.map') + ) { return; } requestCount++; From eaee2e24461fc765cfbff06df8cf700ebc453b5c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 14:52:37 +0200 Subject: [PATCH 07/11] throw on request url... --- .../request/fetch-with-no-active-span/test.ts | 13 ++++++------- .../tracing/request/xhr-with-no-active-span/test.ts | 13 ++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index 194c7248070e..1cc82013514c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -23,14 +23,13 @@ sentryTest( } expect(envelopeUrlRegex.test(url)).toBe(false); + // We _should_ not have any sentry API requests + if (envelopeUrlRegex.test(url)) { + throw new Error(`${url} is an envelope URL`); + } + // We only want to count API requests - if ( - envelopeUrlRegex.test(url) || - url.endsWith('.html') || - url.endsWith('.js') || - url.endsWith('.css') || - url.endsWith('.map') - ) { + if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { return; } requestCount++; diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts index baf39269c75d..70618fe5000d 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts @@ -22,14 +22,13 @@ sentryTest( sentryTraceHeaders.push(sentryTraceHeader); } + // We _should_ not have any sentry API requests + if (envelopeUrlRegex.test(url)) { + throw new Error(`${url} is an envelope URL`); + } + // We only want to count API requests - if ( - envelopeUrlRegex.test(url) || - url.endsWith('.html') || - url.endsWith('.js') || - url.endsWith('.css') || - url.endsWith('.map') - ) { + if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { return; } requestCount++; From 6f95ef963cb3c36b21612a718e38a8483142f339 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 15:07:14 +0200 Subject: [PATCH 08/11] fix linting --- .../suites/tracing/request/fetch-with-no-active-span/init.js | 2 +- .../suites/tracing/request/xhr-with-no-active-span/init.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js index 566461669763..d4ad9bb47ce9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js @@ -13,5 +13,5 @@ Sentry.init({ ], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, - autoSessionTracking: false + autoSessionTracking: false, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js index cbb1f886cb9a..f9f82688fdae 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js @@ -8,5 +8,5 @@ Sentry.init({ integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false })], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, - autoSessionTracking: false + autoSessionTracking: false, }); From d251f6fd6d734a441a97ec057595a954680eb369 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 15:09:05 +0200 Subject: [PATCH 09/11] fix test check? --- .../suites/tracing/request/fetch-with-no-active-span/test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index 1cc82013514c..8245246a4aee 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -21,7 +21,6 @@ sentryTest( if (sentryTraceHeader) { sentryTraceHeaders.push(sentryTraceHeader); } - expect(envelopeUrlRegex.test(url)).toBe(false); // We _should_ not have any sentry API requests if (envelopeUrlRegex.test(url)) { From 56001e5a00d9acbdb550fbf16534236765073504 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 15:21:38 +0200 Subject: [PATCH 10/11] more debugging... --- .../suites/tracing/request/fetch-with-no-active-span/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index 8245246a4aee..0507e747ae59 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -24,6 +24,7 @@ sentryTest( // We _should_ not have any sentry API requests if (envelopeUrlRegex.test(url)) { + console.log(request.postData()); throw new Error(`${url} is an envelope URL`); } From ddfb8f97e9660cbd529f6585d8fde9b664411e45 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 12 Apr 2024 15:46:54 +0200 Subject: [PATCH 11/11] finally unflake the tests...? --- .../request/fetch-with-no-active-span/init.js | 6 ++++ .../request/fetch-with-no-active-span/test.ts | 34 +++++++------------ .../request/xhr-with-no-active-span/init.js | 15 ++++++-- .../request/xhr-with-no-active-span/test.ts | 33 +++++++----------- 4 files changed, 45 insertions(+), 43 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js index d4ad9bb47ce9..e94191654e74 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js @@ -2,6 +2,8 @@ import * as Sentry from '@sentry/browser'; window.Sentry = Sentry; +window._sentryTransactionsCount = 0; + Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', // disable auto span creation @@ -14,4 +16,8 @@ Sentry.init({ tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, autoSessionTracking: false, + beforeSendTransaction() { + window._sentryTransactionsCount++; + return null; + }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts index 0507e747ae59..acaf0e98d693 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -1,43 +1,35 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( 'should not create span for fetch requests with no active span but should attach sentry-trace header', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } - const url = await getLocalTestPath({ testDir: __dirname }); - - let requestCount = 0; const sentryTraceHeaders: string[] = []; - page.on('request', request => { - const url = request.url(); - const sentryTraceHeader = request.headers()['sentry-trace']; + await page.route('http://example.com/**', route => { + const sentryTraceHeader = route.request().headers()['sentry-trace']; if (sentryTraceHeader) { sentryTraceHeaders.push(sentryTraceHeader); } - // We _should_ not have any sentry API requests - if (envelopeUrlRegex.test(url)) { - console.log(request.postData()); - throw new Error(`${url} is an envelope URL`); - } - - // We only want to count API requests - if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { - return; - } - requestCount++; + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); }); - await page.goto(url); + const url = await getLocalTestUrl({ testDir: __dirname }); + + await Promise.all([page.goto(url), ...[0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))]); - expect(requestCount).toBe(3); + expect(await page.evaluate('window._sentryTransactionsCount')).toBe(0); expect(sentryTraceHeaders).toHaveLength(3); expect(sentryTraceHeaders).toEqual([ diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js index f9f82688fdae..e94191654e74 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js @@ -2,11 +2,22 @@ import * as Sentry from '@sentry/browser'; window.Sentry = Sentry; +window._sentryTransactionsCount = 0; + Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - // disable pageload transaction - integrations: [Sentry.browserTracingIntegration({ instrumentPageLoad: false })], + // disable auto span creation + integrations: [ + Sentry.browserTracingIntegration({ + instrumentPageLoad: false, + instrumentNavigation: false, + }), + ], tracePropagationTargets: ['http://example.com'], tracesSampleRate: 1, autoSessionTracking: false, + beforeSendTransaction() { + window._sentryTransactionsCount++; + return null; + }, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts index 70618fe5000d..d3c5f91362c9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts @@ -1,42 +1,35 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( 'should not create span for xhr requests with no active span but should attach sentry-trace header', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } - const url = await getLocalTestPath({ testDir: __dirname }); - - let requestCount = 0; const sentryTraceHeaders: string[] = []; - page.on('request', request => { - const url = request.url(); - const sentryTraceHeader = request.headers()['sentry-trace']; + await page.route('http://example.com/**', route => { + const sentryTraceHeader = route.request().headers()['sentry-trace']; if (sentryTraceHeader) { sentryTraceHeaders.push(sentryTraceHeader); } - // We _should_ not have any sentry API requests - if (envelopeUrlRegex.test(url)) { - throw new Error(`${url} is an envelope URL`); - } - - // We only want to count API requests - if (url.endsWith('.html') || url.endsWith('.js') || url.endsWith('.css') || url.endsWith('.map')) { - return; - } - requestCount++; + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); }); - await page.goto(url); + const url = await getLocalTestUrl({ testDir: __dirname }); + + await Promise.all([page.goto(url), ...[0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))]); - expect(requestCount).toBe(3); + expect(await page.evaluate('window._sentryTransactionsCount')).toBe(0); expect(sentryTraceHeaders).toHaveLength(3); expect(sentryTraceHeaders).toEqual([