Merge branch 'develop' into lforst-nextjs-otel

getsentry · Oct 16, 2024 · b78f8da · b78f8da
2 parents 83d90f7 + 8d5a084
commit b78f8da
Show file tree

Hide file tree

Showing 14 changed files with 312 additions and 92 deletions.
diff --git a/.size-limit.js b/.size-limit.js
@@ -79,7 +79,7 @@ module.exports = [
     path: 'packages/browser/build/npm/esm/index.js',
     import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
     gzip: true,
-    limit: '78 KB',
+    limit: '78.1 KB',
   },
   {
     name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
@@ -138,7 +138,7 @@ module.exports = [
     import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
     ignore: ['react/jsx-runtime'],
     gzip: true,
-    limit: '39 KB',
+    limit: '39.05 KB',
   },
   // Vue SDK (ESM)
   {

diff --git a/...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js b/...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js
@@ -0,0 +1,10 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [Sentry.browserTracingIntegration()],
+  tracePropagationTargets: ['http://example.com'],
+  tracesSampleRate: 1,
+});
diff --git a/...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js b/...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js
@@ -0,0 +1,53 @@
+fetchPojo.addEventListener('click', () => {
+  const fetchOptions = {
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'sentry-release=4.2.0',
+    },
+  };
+
+  // Make two fetch requests that reuse the same fetch object
+  Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
+    fetch('http://example.com/fetch-pojo', fetchOptions)
+      .then(res => res.text())
+      .then(() =>
+        Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
+      ),
+  );
+});
+
+fetchArray.addEventListener('click', () => {
+  const fetchOptions = {
+    headers: [
+      ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
+      ['baggage', 'sentry-release=4.2.0'],
+    ],
+  };
+
+  // Make two fetch requests that reuse the same fetch object
+  Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
+    fetch('http://example.com/fetch-array', fetchOptions)
+      .then(res => res.text())
+      .then(() =>
+        Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
+      ),
+  );
+});
+
+fetchHeaders.addEventListener('click', () => {
+  const fetchOptions = {
+    headers: new Headers({
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'sentry-release=4.2.0',
+    }),
+  };
+
+  // Make two fetch requests that reuse the same fetch object
+  Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
+    fetch('http://example.com/fetch-headers', fetchOptions)
+      .then(res => res.text())
+      .then(() =>
+        Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
+      ),
+  );
+});
diff --git a/...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html b/...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button id="fetchPojo">Fetch POJO</button>
+    <button id="fetchArray">Fetch array</button>
+    <button id="fetchHeaders">Fetch Headers</button>
+  </body>
+</html>
diff --git a/...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts b/...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts
@@ -0,0 +1,64 @@
+import type { Page, Request } from '@playwright/test';
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../utils/fixtures';
+import { shouldSkipTracingTest } from '../../../../utils/helpers';
+
+async function assertRequests({
+  page,
+  buttonSelector,
+  requestMatcher,
+}: { page: Page; buttonSelector: string; requestMatcher: string }) {
+  const requests = await new Promise<Request[]>(resolve => {
+    const requests: Request[] = [];
+    page
+      .route(requestMatcher, (route, request) => {
+        requests.push(request);
+        if (requests.length === 2) {
+          resolve(requests);
+        }
+
+        return route.fulfill({
+          status: 200,
+          contentType: 'application/json',
+          body: JSON.stringify({}),
+        });
+      })
+      .then(() => {
+        page.click(buttonSelector);
+      });
+  });
+
+  requests.forEach(request => {
+    const headers = request.headers();
+
+    // No merged sentry trace headers
+    expect(headers['sentry-trace']).not.toContain(',');
+
+    // No multiple baggage entries
+    expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
+  });
+}
+
+sentryTest(
+  'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+    await page.goto(url);
+
+    await sentryTest.step('fetch with POJO', () =>
+      assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
+    );
+
+    await sentryTest.step('fetch with array', () =>
+      assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
+    );
+
+    await sentryTest.step('fetch with Headers instance', () =>
+      assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
+    );
+  },
+);
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/package.json b/dev-packages/e2e-tests/test-applications/nextjs-15/package.json
@@ -9,7 +9,8 @@
     "test:dev": "TEST_ENV=development playwright test",
     "test:build": "pnpm install && npx playwright install && pnpm build",
     "test:build-canary": "pnpm install && pnpm add next@canary && pnpm add react@beta && pnpm add react-dom@beta && npx playwright install && pnpm build",
-    "test:build-latest": "pnpm install && pnpm add next@rc && pnpm add react@beta && pnpm add react-dom@beta && npx playwright install && pnpm build",
+    "//": "15.0.0-canary.194 is the canary release attached to Next.js RC 1. We need to use the canary version instead of the RC because PPR will not work without. The specific react version is also attached to RC 1.",
+    "test:build-latest": "pnpm install && pnpm add [email protected] && pnpm add [email protected] && pnpm add [email protected] && npx playwright install && pnpm build",
     "test:assert": "pnpm test:prod && pnpm test:dev"
   },
   "dependencies": {

diff --git a/...es/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js b/...es/node-integration-tests/suites/tracing/httpIntegration/server-ignoreOutgoingRequests.js
@@ -11,10 +11,11 @@ Sentry.init({
   integrations: [
     Sentry.httpIntegration({
       ignoreOutgoingRequests: (url, request) => {
-        if (url.includes('example.com')) {
+        if (url === 'http://example.com/blockUrl') {
           return true;
         }
-        if (request.method === 'POST' && request.path === '/path') {
+
+        if (request.hostname === 'example.com' && request.path === '/blockRequest') {
           return true;
         }
         return false;
@@ -32,28 +33,37 @@ const app = express();
 
 app.use(cors());
 
-app.get('/test', (_req, response) => {
-  http
-    .request('http://example.com/', res => {
-      res.on('data', () => {});
-      res.on('end', () => {
-        response.send({ response: 'done' });
-      });
-    })
-    .end();
+app.get('/testUrl', (_req, response) => {
+  makeHttpRequest('http://example.com/blockUrl').then(() => {
+    makeHttpRequest('http://example.com/pass').then(() => {
+      response.send({ response: 'done' });
+    });
+  });
 });
 
-app.post('/testPath', (_req, response) => {
-  http
-    .request('http://example.com/path', res => {
-      res.on('data', () => {});
-      res.on('end', () => {
-        response.send({ response: 'done' });
-      });
-    })
-    .end();
+app.get('/testRequest', (_req, response) => {
+  makeHttpRequest('http://example.com/blockRequest').then(() => {
+    makeHttpRequest('http://example.com/pass').then(() => {
+      response.send({ response: 'done' });
+    });
+  });
 });
 
 Sentry.setupExpressErrorHandler(app);
 
 startExpressServerAndSendPortToRunner(app);
+
+function makeHttpRequest(url) {
+  return new Promise((resolve, reject) => {
+    http
+      .get(url, res => {
+        res.on('data', () => {});
+        res.on('end', () => {
+          resolve();
+        });
+      })
+      .on('error', error => {
+        reject(error);
+      });
+  });
+}
diff --git a/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts b/dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts
@@ -128,65 +128,45 @@ describe('httpIntegration', () => {
     });
   });
 
-  describe("doesn't create child spans for outgoing requests ignored via `ignoreOutgoingRequests`", () => {
+  describe("doesn't create child spans or breadcrumbs for outgoing requests ignored via `ignoreOutgoingRequests`", () => {
     test('via the url param', done => {
       const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js')
         .expect({
-          transaction: {
-            contexts: {
-              trace: {
-                span_id: expect.any(String),
-                trace_id: expect.any(String),
-                data: {
-                  url: expect.stringMatching(/\/test$/),
-                  'http.response.status_code': 200,
-                },
-                op: 'http.server',
-                status: 'ok',
-              },
-            },
-            transaction: 'GET /test',
-            spans: [
-              expect.objectContaining({ op: 'middleware.express', description: 'query' }),
-              expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }),
-              expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }),
-              expect.objectContaining({ op: 'request_handler.express', description: '/test' }),
-            ],
+          transaction: event => {
+            expect(event.transaction).toBe('GET /testUrl');
+
+            const requestSpans = event.spans?.filter(span => span.op === 'http.client');
+            expect(requestSpans).toHaveLength(1);
+            expect(requestSpans![0]?.description).toBe('GET http://example.com/pass');
+
+            const breadcrumbs = event.breadcrumbs?.filter(b => b.category === 'http');
+            expect(breadcrumbs).toHaveLength(1);
+            expect(breadcrumbs![0]?.data?.url).toEqual('http://example.com/pass');
           },
         })
         .start(done);
 
-      runner.makeRequest('get', '/test');
+      runner.makeRequest('get', '/testUrl');
     });
 
     test('via the request param', done => {
       const runner = createRunner(__dirname, 'server-ignoreOutgoingRequests.js')
         .expect({
-          transaction: {
-            contexts: {
-              trace: {
-                span_id: expect.any(String),
-                trace_id: expect.any(String),
-                data: {
-                  url: expect.stringMatching(/\/testPath$/),
-                  'http.response.status_code': 200,
-                },
-                op: 'http.server',
-                status: 'ok',
-              },
-            },
-            transaction: 'POST /testPath',
-            spans: [
-              expect.objectContaining({ op: 'middleware.express', description: 'query' }),
-              expect.objectContaining({ op: 'middleware.express', description: 'expressInit' }),
-              expect.objectContaining({ op: 'middleware.express', description: 'corsMiddleware' }),
-              expect.objectContaining({ op: 'request_handler.express', description: '/testPath' }),
-            ],
+          transaction: event => {
+            expect(event.transaction).toBe('GET /testRequest');
+
+            const requestSpans = event.spans?.filter(span => span.op === 'http.client');
+            expect(requestSpans).toHaveLength(1);
+            expect(requestSpans![0]?.description).toBe('GET http://example.com/pass');
+
+            const breadcrumbs = event.breadcrumbs?.filter(b => b.category === 'http');
+            expect(breadcrumbs).toHaveLength(1);
+            expect(breadcrumbs![0]?.data?.url).toEqual('http://example.com/pass');
           },
         })
         .start(done);
 
-      runner.makeRequest('post', '/testPath');
+      runner.makeRequest('get', '/testRequest');
     });
   });
 });