From 72f9969aa7d6f6d893a6cc9a565d9e6170c71faa Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 26 Jan 2024 13:51:55 -0500 Subject: [PATCH 1/5] test(tracing): Add test to validate that spans only added when active span exists --- .../request/fetch-with-no-active-span/init.js | 10 +++++++ .../fetch-with-no-active-span/subject.js | 1 + .../request/fetch-with-no-active-span/test.ts | 30 +++++++++++++++++++ .../request/xhr-with-no-active-span/init.js | 10 +++++++ .../xhr-with-no-active-span/subject.js | 11 +++++++ .../request/xhr-with-no-active-span/test.ts | 30 +++++++++++++++++++ 6 files changed, 92 insertions(+) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts 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 new file mode 100644 index 000000000000..92152554ea57 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/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', + // disable pageload transaction + integrations: [Sentry.BrowserTracing({ tracingOrigins: ['http://example.com'], startTransactionOnPageLoad: false })], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/subject.js new file mode 100644 index 000000000000..f62499b1e9c5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/subject.js @@ -0,0 +1 @@ +fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); 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 new file mode 100644 index 000000000000..d66eec935b10 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-no-active-span/test.ts @@ -0,0 +1,30 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'there should be no span created for fetch requests with no active span', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + let requestCount = 0; + page.on('request', request => { + expect(request.url()).not.toContain(url); + requestCount++; + }); + + await page.goto(url); + + // There are 6 requests in the page: + // 1. HTML page + // 2. Init JS bundle + // 3. Subject JS bundle + // and then 3 fetch requests + expect(requestCount).toBe(6); + }, +); 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 new file mode 100644 index 000000000000..92152554ea57 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/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', + // disable pageload transaction + integrations: [Sentry.BrowserTracing({ tracingOrigins: ['http://example.com'], startTransactionOnPageLoad: false })], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/subject.js new file mode 100644 index 000000000000..5790c230aa66 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/subject.js @@ -0,0 +1,11 @@ +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.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-with-no-active-span/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts new file mode 100644 index 000000000000..3e25a5a30308 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-no-active-span/test.ts @@ -0,0 +1,30 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'there should be no span created for xhr requests with no active span', + async ({ getLocalTestPath, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + + let requestCount = 0; + page.on('request', request => { + expect(request.url()).not.toContain(url); + requestCount++; + }); + + await page.goto(url); + + // There are 6 requests in the page: + // 1. HTML page + // 2. Init JS bundle + // 3. Subject JS bundle + // and then 3 fetch requests + expect(requestCount).toBe(6); + }, +); From 3988ea81f43b4be2301010a9649a5f8ff981aef2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 26 Jan 2024 14:17:51 -0500 Subject: [PATCH 2/5] log out request url to see what is going on --- .../suites/tracing/request/xhr-with-no-active-span/test.ts | 1 + 1 file changed, 1 insertion(+) 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 3e25a5a30308..0809118934cb 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 @@ -14,6 +14,7 @@ sentryTest( let requestCount = 0; page.on('request', request => { + console.log(request.url()); expect(request.url()).not.toContain(url); requestCount++; }); From a4419c291c40a9eae9a96df303f61805a5f398a3 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 26 Jan 2024 14:31:22 -0500 Subject: [PATCH 3/5] account for bundles --- .../tracing/request/fetch-with-no-active-span/test.ts | 9 +++++++-- .../tracing/request/xhr-with-no-active-span/test.ts | 10 +++++++--- 2 files changed, 14 insertions(+), 5 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 d66eec935b10..957c09e47967 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 @@ -20,11 +20,16 @@ sentryTest( await page.goto(url); - // There are 6 requests in the page: + // 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 - expect(requestCount).toBe(6); + if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) { + expect(requestCount).toBe(7); + } else { + expect(requestCount).toBe(6); + } }, ); 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 0809118934cb..805a56cbf735 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 @@ -14,18 +14,22 @@ sentryTest( let requestCount = 0; page.on('request', request => { - console.log(request.url()); expect(request.url()).not.toContain(url); requestCount++; }); await page.goto(url); - // There are 6 requests in the page: + // 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 - expect(requestCount).toBe(6); + if (process.env.PW_BUNDLE && process.env.PW_BUNDLE.startsWith('bundle_')) { + expect(requestCount).toBe(7); + } else { + expect(requestCount).toBe(6); + } }, ); From b4f927b9b90eccc8e341046e04b44531232e559d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 26 Jan 2024 14:46:48 -0500 Subject: [PATCH 4/5] fix url check --- .../suites/tracing/request/fetch-with-no-active-span/test.ts | 4 ++-- dev-packages/browser-integration-tests/utils/helpers.ts | 2 +- 2 files changed, 3 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 957c09e47967..4dc5a0ac4e0a 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,7 +1,7 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { shouldSkipTracingTest } from '../../../../utils/helpers'; +import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( 'there should be no span created for fetch requests with no active span', @@ -14,7 +14,7 @@ sentryTest( let requestCount = 0; page.on('request', request => { - expect(request.url()).not.toContain(url); + expect(envelopeUrlRegex.test(request.url())).toBe(false); requestCount++; }); diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index d00f125a90c1..6027695746e9 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -1,7 +1,7 @@ import type { Page, Request } from '@playwright/test'; import type { EnvelopeItemType, Event, EventEnvelopeHeaders } from '@sentry/types'; -const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//; +export const envelopeUrlRegex = /\.sentry\.io\/api\/\d+\/envelope\//; export const envelopeParser = (request: Request | null): unknown[] => { // https://develop.sentry.dev/sdk/envelopes/ From 67296f171a882db9c6cf7128be660131fa4bb3ce Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 26 Jan 2024 17:17:22 -0500 Subject: [PATCH 5/5] fix url check again --- .../suites/tracing/request/xhr-with-no-active-span/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 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 805a56cbf735..19c1f5891a39 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,7 +1,7 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { shouldSkipTracingTest } from '../../../../utils/helpers'; +import { envelopeUrlRegex, shouldSkipTracingTest } from '../../../../utils/helpers'; sentryTest( 'there should be no span created for xhr requests with no active span', @@ -14,7 +14,7 @@ sentryTest( let requestCount = 0; page.on('request', request => { - expect(request.url()).not.toContain(url); + expect(envelopeUrlRegex.test(request.url())).toBe(false); requestCount++; });