diff --git a/.craft.yml b/.craft.yml index d387e917307d..185fa2fd0510 100644 --- a/.craft.yml +++ b/.craft.yml @@ -114,6 +114,9 @@ targets: - name: npm id: '@sentry/remix' includeNames: /^sentry-remix-\d.*\.tgz$/ + - name: npm + id: '@sentry/solidstart' + includeNames: /^sentry-solidstart-\d.*\.tgz$/ - name: npm id: '@sentry/sveltekit' includeNames: /^sentry-sveltekit-\d.*\.tgz$/ diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 437191889954..43e4e82c61b0 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -1,6 +1,6 @@ name: 🐞 Bug Report description: Tell us about something that's not working the way we (probably) intend. -labels: ['Type: Bug'] +type: 'bug' body: - type: checkboxes attributes: @@ -31,20 +31,23 @@ body: setup. options: - '@sentry/browser' - - '@sentry/astro' + - '@sentry/node' - '@sentry/angular' + - '@sentry/astro' - '@sentry/aws-serverless' - '@sentry/bun' + - '@sentry/cloudflare' - '@sentry/deno' - '@sentry/ember' - '@sentry/gatsby' - '@sentry/google-cloud-serverless' - '@sentry/nestjs' - '@sentry/nextjs' - - '@sentry/node' + - '@sentry/nuxt' - '@sentry/react' - '@sentry/remix' - '@sentry/solid' + - '@sentry/solidstart' - '@sentry/svelte' - '@sentry/sveltekit' - '@sentry/vue' diff --git a/.github/ISSUE_TEMPLATE/feature.yml b/.github/ISSUE_TEMPLATE/feature.yml index 7749deee1d44..185de4888de1 100644 --- a/.github/ISSUE_TEMPLATE/feature.yml +++ b/.github/ISSUE_TEMPLATE/feature.yml @@ -1,6 +1,6 @@ name: 💡 Feature Request description: Create a feature request for a sentry-javascript SDK. -labels: ['Type: Improvement'] +type: 'enhancement' body: - type: markdown attributes: diff --git a/.github/ISSUE_TEMPLATE/flaky.yml b/.github/ISSUE_TEMPLATE/flaky.yml index 48e6ce41c047..a679cf98d328 100644 --- a/.github/ISSUE_TEMPLATE/flaky.yml +++ b/.github/ISSUE_TEMPLATE/flaky.yml @@ -1,6 +1,7 @@ name: ❅ Flaky Test description: Report a flaky test in CI title: '[Flaky CI]: ' +type: 'task' labels: ['Type: Tests'] body: - type: dropdown diff --git a/.github/ISSUE_TEMPLATE/internal.yml b/.github/ISSUE_TEMPLATE/internal.yml index bd5b1d1f1970..308d04db7eb5 100644 --- a/.github/ISSUE_TEMPLATE/internal.yml +++ b/.github/ISSUE_TEMPLATE/internal.yml @@ -1,6 +1,10 @@ name: 💡 [Internal] Blank Issue description: Only for Sentry Employees! Create an issue without a template. +type: 'task' body: + - type: markdown + attributes: + value: Make sure to apply relevant labels and issue types before submitting. - type: textarea id: description attributes: diff --git a/.github/actions/install-playwright/action.yml b/.github/actions/install-playwright/action.yml index 29ecbcfbd2d1..7f85f5e743ba 100644 --- a/.github/actions/install-playwright/action.yml +++ b/.github/actions/install-playwright/action.yml @@ -1,5 +1,9 @@ name: "Install Playwright dependencies" description: "Installs Playwright dependencies and caches them." +inputs: + browsers: + description: 'What browsers to install.' + default: 'chromium webkit firefox' runs: using: "composite" @@ -17,12 +21,13 @@ runs: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} + # We always install all browsers, if uncached - name: Install Playwright dependencies (uncached) run: npx playwright install chromium webkit firefox --with-deps if: steps.playwright-cache.outputs.cache-hit != 'true' shell: bash - name: Install Playwright system dependencies only (cached) - run: npx playwright install-deps chromium webkit firefox + run: npx playwright install-deps ${{ inputs.browsers || 'chromium webkit firefox' }} if: steps.playwright-cache.outputs.cache-hit == 'true' shell: bash diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6eea92c884ef..8ab03a313253 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -528,7 +528,7 @@ jobs: run: yarn lerna run test --scope @sentry/profiling-node job_browser_playwright_tests: - name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests + name: Playwright ${{ matrix.bundle }}${{ matrix.project && matrix.project != 'chromium' && format(' {0}', matrix.project) || ''}}${{ matrix.shard && format(' ({0}/{1})', matrix.shard, matrix.shards) || ''}} Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04-large-js @@ -548,31 +548,30 @@ jobs: project: - chromium include: - # Only check all projects for esm & full bundle + # Only check all projects for full bundle # We also shard the tests as they take the longest - bundle: bundle_tracing_replay_feedback_min - project: '' - shard: 1 - shards: 2 + project: 'webkit' - bundle: bundle_tracing_replay_feedback_min - project: '' - shard: 2 - shards: 2 + project: 'firefox' - bundle: esm - project: '' + project: chromium shard: 1 - shards: 3 + shards: 4 - bundle: esm + project: chromium shard: 2 - shards: 3 + shards: 4 - bundle: esm - project: '' + project: chromium shard: 3 - shards: 3 + shards: 4 + - bundle: esm + project: chromium + shard: 4 + shards: 4 exclude: - # Do not run the default chromium-only tests - - bundle: bundle_tracing_replay_feedback_min - project: 'chromium' + # Do not run the un-sharded esm tests - bundle: esm project: 'chromium' @@ -592,12 +591,15 @@ jobs: - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: ${{ matrix.project }} - name: Run Playwright tests env: PW_BUNDLE: ${{ matrix.bundle }} working-directory: dev-packages/browser-integration-tests run: yarn test:ci${{ matrix.project && format(' --project={0}', matrix.project) || '' }}${{ matrix.shard && format(' --shard={0}/{1}', matrix.shard, matrix.shards) || '' }} + - name: Upload Playwright Traces uses: actions/upload-artifact@v3 if: always() @@ -606,7 +608,7 @@ jobs: path: dev-packages/browser-integration-tests/test-results job_browser_loader_tests: - name: Playwright Loader (${{ matrix.bundle }}) Tests + name: PW ${{ matrix.bundle }} Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 @@ -639,6 +641,8 @@ jobs: - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: chromium - name: Run Playwright Loader tests env: @@ -750,8 +754,12 @@ jobs: uses: ./.github/actions/restore-cache env: DEPENDENCY_CACHE_KEY: ${{ needs.job_build.outputs.dependency_cache_key }} + - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: chromium + - name: Run integration tests env: NODE_VERSION: ${{ matrix.node }} @@ -878,6 +886,7 @@ jobs: 'react-router-5', 'react-router-6', 'solid', + 'solidstart', 'svelte-5', 'sveltekit', 'sveltekit-2', @@ -952,6 +961,8 @@ jobs: - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: chromium - name: Get node version id: versions @@ -1049,6 +1060,8 @@ jobs: - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: chromium - name: Get node version id: versions @@ -1149,6 +1162,8 @@ jobs: - name: Install Playwright uses: ./.github/actions/install-playwright + with: + browsers: chromium - name: Get node version id: versions diff --git a/.github/workflows/flaky-test-detector.yml b/.github/workflows/flaky-test-detector.yml index d2238ce58e71..5d0d5af5d247 100644 --- a/.github/workflows/flaky-test-detector.yml +++ b/.github/workflows/flaky-test-detector.yml @@ -68,10 +68,10 @@ jobs: CHANGED_TEST_PATHS: ${{ steps.changed.outputs.browser_integration_files }} TEST_RUN_COUNT: 'AUTO' - - name: Artifacts upload + - name: Upload Playwright Traces uses: actions/upload-artifact@v4 if: failure() && steps.test.outcome == 'failure' with: name: playwright-test-results - path: test-results + path: dev-packages/browser-integration-tests/test-results retention-days: 5 diff --git a/.github/workflows/issue-package-label.yml b/.github/workflows/issue-package-label.yml index 2e2f41904f7c..ad5edbd3b398 100644 --- a/.github/workflows/issue-package-label.yml +++ b/.github/workflows/issue-package-label.yml @@ -29,17 +29,26 @@ jobs: # Note: Since this is handled as a regex, and JSON parse wrangles slashes /, we just use `.` instead map: | { + "@sentry.angular": { + "label": "Package: angular" + }, "@sentry.astro": { - "label": "Package: Astro" + "label": "Package: astro" }, - "@sentry.browser": { - "label": "Package: Browser" + "@sentry.aws-serverless": { + "label": "Package: aws-serverless" }, - "@sentry.angular": { - "label": "Package: Angular" + "@sentry.browser": { + "label": "Package: browser" }, "@sentry.bun": { - "label": "Package: Bun" + "label": "Package: bun" + }, + "@sentry.cloudflare": { + "label": "Package: cloudflare" + }, + "@sentry.deno": { + "label": "Package: deno" }, "@sentry.ember": { "label": "Package: ember" @@ -47,11 +56,20 @@ jobs: "@sentry.gatsby": { "label": "Package: gatbsy" }, + "@sentry.google-cloud-serverless": { + "label": "Package: google-cloud-serverless" + }, + "@sentry.nestjs": { + "label": "Package: nestjs" + }, "@sentry.nextjs": { - "label": "Package: Nextjs" + "label": "Package: nextjs" }, "@sentry.node": { - "label": "Package: Node" + "label": "Package: node" + }, + "@sentry.nuxt": { + "label": "Package: nuxt" }, "@sentry.react": { "label": "Package: react" @@ -59,15 +77,18 @@ jobs: "@sentry.remix": { "label": "Package: remix" }, - "@sentry.serverless": { - "label": "Package: Serverless" + "@sentry.solid": { + "label": "Package: solid" }, - "@sentry.sveltekit": { - "label": "Package: SvelteKit" + "@sentry.solid": { + "label": "Package: solidstart" }, "@sentry.svelte": { "label": "Package: svelte" }, + "@sentry.sveltekit": { + "label": "Package: sveltekit" + }, "@sentry.vue": { "label": "Package: vue" }, diff --git a/.size-limit.js b/.size-limit.js index 72050f7225f3..437e466a89e1 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -22,7 +22,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '72 KB', + limit: '73 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ad5a1920aae..2e8d141efd95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,28 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 8.25.0 + +### Important Changes + +- **Alpha release of Official Solid Start SDK** + +This release contains the alpha version of `@sentry/solidstart`, our SDK for [Solid Start](https://start.solidjs.com/)! +For details on how to use it, please see the [README](./packages/solidstart/README.md). Any feedback/bug reports are +greatly appreciated, please [reach out on GitHub](https://github.com/getsentry/sentry-javascript/issues/12538). + +### Other Changes + +- feat(astro): Add `bundleSizeOptimizations` vite options to integration (#13250) +- feat(astro): Always add BrowserTracing (#13244) +- feat(core): Add `getTraceMetaTags` function (#13201) +- feat(nestjs): Automatic instrumentation of nestjs exception filters (#13230) +- feat(node): Add `useOperationNameForRootSpan` to`graphqlIntegration` (#13248) +- feat(sveltekit): Add `wrapServerRouteWithSentry` wrapper (#13247) +- fix(aws-serverless): Extract sentry trace data from handler `context` over `event` (#13266) +- fix(browser): Initialize default integration if `defaultIntegrations: undefined` (#13261) +- fix(utils): Streamline IP capturing on incoming requests (#13272) + ## 8.24.0 - feat(nestjs): Filter RPC exceptions (#13227) diff --git a/dev-packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/dev-packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index ba3dd43ac3d3..aafdced81505 100644 --- a/dev-packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/dev-packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -18,49 +18,51 @@ sentryTest('mutation after threshold results in slow click', async ({ forceFlush const url = await getLocalTestUrl({ testDir: __dirname }); - await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); - await forceFlushReplay(); + const replayRequestPromise = waitForReplayRequest(page, 0); + + const segmentReqWithSlowClickBreadcrumbPromise = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); - const [req1] = await Promise.all([ - waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); + await page.goto(url); + await replayRequestPromise; - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); - }), + await forceFlushReplay(); + + await page.locator('#mutationButton').click(); - page.locator('#mutationButton').click(), - ]); + const segmentReqWithSlowClick = await segmentReqWithSlowClickBreadcrumbPromise; - const { breadcrumbs } = getCustomRecordingEvents(req1); + const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithSlowClick); const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); - expect(slowClickBreadcrumbs).toEqual([ - { - category: 'ui.slowClickDetected', - type: 'default', - data: { - endReason: 'mutation', - clickCount: 1, - node: { - attributes: { - id: 'mutationButton', - }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ********', + expect(slowClickBreadcrumbs).toContainEqual({ + category: 'ui.slowClickDetected', + type: 'default', + data: { + endReason: 'mutation', + clickCount: 1, + node: { + attributes: { + id: 'mutationButton', }, - nodeId: expect.any(Number), - timeAfterClickMs: expect.any(Number), - url: 'http://sentry-test.io/index.html', + id: expect.any(Number), + tagName: 'button', + textContent: '******* ********', }, - message: 'body > button#mutationButton', - timestamp: expect.any(Number), + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', }, - ]); + message: 'body > button#mutationButton', + timestamp: expect.any(Number), + }); expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000); - expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3500); + expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3501); }); sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => { @@ -78,49 +80,50 @@ sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => { const url = await getLocalTestUrl({ testDir: __dirname }); - await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); + const replayRequestPromise = waitForReplayRequest(page, 0); + const segmentReqWithSlowClickBreadcrumbPromise = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); - const [req1] = await Promise.all([ - waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); - }), - page.locator('#mutationButton').click({ clickCount: 4 }), - ]); + await page.goto(url); + await replayRequestPromise; - const { breadcrumbs } = getCustomRecordingEvents(req1); + await page.locator('#mutationButton').click({ clickCount: 4 }); + + const segmentReqWithSlowClick = await segmentReqWithSlowClickBreadcrumbPromise; + + const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithSlowClick); const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); const multiClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick'); - expect(slowClickBreadcrumbs).toEqual([ - { - category: 'ui.slowClickDetected', - type: 'default', - data: { - endReason: 'mutation', - clickCount: 4, - node: { - attributes: { - id: 'mutationButton', - }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ********', + expect(slowClickBreadcrumbs).toContainEqual({ + category: 'ui.slowClickDetected', + type: 'default', + data: { + endReason: expect.stringMatching(/^(mutation|timeout)$/), + clickCount: 4, + node: { + attributes: { + id: 'mutationButton', }, - nodeId: expect.any(Number), - timeAfterClickMs: expect.any(Number), - url: 'http://sentry-test.io/index.html', + id: expect.any(Number), + tagName: 'button', + textContent: '******* ********', }, - message: 'body > button#mutationButton', - timestamp: expect.any(Number), + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', }, - ]); + message: 'body > button#mutationButton', + timestamp: expect.any(Number), + }); expect(multiClickBreadcrumbs.length).toEqual(0); expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000); - expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3500); + expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3501); }); sentryTest('immediate mutation does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => { @@ -138,7 +141,15 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush const url = await getLocalTestUrl({ testDir: __dirname }); - await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); + const replayRequestPromise = waitForReplayRequest(page, 0); + const segmentReqWithClickBreadcrumbPromise = waitForReplayRequest(page, (_event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.goto(url); + await replayRequestPromise; await forceFlushReplay(); let slowClickCount = 0; @@ -150,36 +161,29 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush slowClickCount += slowClicks.length; }); - const [req1] = await Promise.all([ - waitForReplayRequest(page, (_event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); - - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); - }), - page.locator('#mutationButtonImmediately').click(), - ]); - - const { breadcrumbs } = getCustomRecordingEvents(req1); - - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mutationButtonImmediately', - }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ******** ***********', + await page.locator('#mutationButtonImmediately').click(); + + const segmentReqWithSlowClick = await segmentReqWithClickBreadcrumbPromise; + + const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithSlowClick); + + expect(breadcrumbs).toContainEqual({ + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationButtonImmediately', }, - nodeId: expect.any(Number), + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ***********', }, - message: 'body > button#mutationButtonImmediately', - timestamp: expect.any(Number), - type: 'default', + nodeId: expect.any(Number), }, - ]); + message: 'body > button#mutationButtonImmediately', + timestamp: expect.any(Number), + type: 'default', + }); // Ensure we wait for timeout, to make sure no slow click is created // Waiting for 3500 + 1s rounding room @@ -204,39 +208,41 @@ sentryTest('inline click handler does not trigger slow click', async ({ forceFlu const url = await getLocalTestUrl({ testDir: __dirname }); - await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); + const replayRequestPromise = waitForReplayRequest(page, 0); + const segmentReqWithClickBreadcrumbPromise = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.goto(url); + await replayRequestPromise; + await forceFlushReplay(); - const [req1] = await Promise.all([ - waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); - - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); - }), - page.locator('#mutationButtonInline').click(), - ]); - - const { breadcrumbs } = getCustomRecordingEvents(req1); - - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mutationButtonInline', - }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ******** ***********', + await page.locator('#mutationButtonInline').click(); + + const segmentReqWithClick = await segmentReqWithClickBreadcrumbPromise; + + const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithClick); + + expect(breadcrumbs).toContainEqual({ + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationButtonInline', }, - nodeId: expect.any(Number), + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ***********', }, - message: 'body > button#mutationButtonInline', - timestamp: expect.any(Number), - type: 'default', + nodeId: expect.any(Number), }, - ]); + message: 'body > button#mutationButtonInline', + timestamp: expect.any(Number), + type: 'default', + }); }); sentryTest('mouseDown events are considered', async ({ getLocalTestUrl, page }) => { @@ -254,36 +260,36 @@ sentryTest('mouseDown events are considered', async ({ getLocalTestUrl, page }) const url = await getLocalTestUrl({ testDir: __dirname }); - await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]); - - const [req1] = await Promise.all([ - waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); - - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); - }), - page.locator('#mouseDownButton').click(), - ]); - - const { breadcrumbs } = getCustomRecordingEvents(req1); - - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mouseDownButton', - }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ******** ** ***** ****', + const replayRequestPromise = waitForReplayRequest(page, 0); + const segmentReqWithClickBreadcrumbPromise = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.goto(url); + await replayRequestPromise; + + await page.locator('#mouseDownButton').click(); + const segmentReqWithClick = await segmentReqWithClickBreadcrumbPromise; + + const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithClick); + + expect(breadcrumbs).toContainEqual({ + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mouseDownButton', }, - nodeId: expect.any(Number), + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ** ***** ****', }, - message: 'body > button#mouseDownButton', - timestamp: expect.any(Number), - type: 'default', + nodeId: expect.any(Number), }, - ]); + message: 'body > button#mouseDownButton', + timestamp: expect.any(Number), + type: 'default', + }); }); diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index 887284585ae1..9217249faad0 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -121,3 +121,77 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { }), ); }); + +test('API route transaction includes exception filter span for global filter', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'ExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); + +test('API route transaction includes exception filter span for local filter', async ({ baseURL }) => { + const transactionEventPromise = waitForTransaction('nestjs-with-submodules', transactionEvent => { + return ( + transactionEvent?.contexts?.trace?.op === 'http.server' && + transactionEvent?.transaction === 'GET /example-module-local-filter/expected-exception' && + transactionEvent?.request?.url?.includes('/example-module-local-filter/expected-exception') + ); + }); + + const response = await fetch(`${baseURL}/example-module-local-filter/expected-exception`); + expect(response.status).toBe(400); + + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toEqual( + expect.objectContaining({ + spans: expect.arrayContaining([ + { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'middleware.nestjs', + 'sentry.origin': 'auto.middleware.nestjs', + }, + description: 'LocalExampleExceptionFilter', + parent_span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + status: 'ok', + op: 'middleware.nestjs', + origin: 'auto.middleware.nestjs', + }, + ]), + }), + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte index e7788b6433cd..0cfae1c54741 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/+page.svelte @@ -38,4 +38,7 @@
  • Component Tracking
  • +
  • + server routes +
  • diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.svelte b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.svelte new file mode 100644 index 000000000000..adc04d52c0ea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.svelte @@ -0,0 +1,7 @@ + + +

    + Message from API: {data.myMessage} +

    diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.ts new file mode 100644 index 000000000000..3f3f5942366e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/+page.ts @@ -0,0 +1,7 @@ +export const load = async ({ fetch }) => { + const res = await fetch('/wrap-server-route/api'); + const myMessage = await res.json(); + return { + myMessage: myMessage.myMessage, + }; +}; diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/api/+server.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/api/+server.ts new file mode 100644 index 000000000000..6ba210690ad5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/src/routes/wrap-server-route/api/+server.ts @@ -0,0 +1,6 @@ +import { wrapServerRouteWithSentry } from '@sentry/sveltekit'; +import { error } from '@sveltejs/kit'; + +export const GET = wrapServerRouteWithSentry(async () => { + error(500, 'error() error'); +}); diff --git a/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/errors.server.test.ts index c9dc56b9c96b..fd2e58e9c2a3 100644 --- a/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/sveltekit-2/tests/errors.server.test.ts @@ -58,4 +58,37 @@ test.describe('server-side errors', () => { expect(errorEvent.transaction).toEqual('GET /server-route-error'); }); + + test('captures error() thrown in server route with `wrapServerRouteWithSentry`', async ({ page }) => { + const errorEventPromise = waitForError('sveltekit-2', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === "'HttpError' captured as exception with keys: body, status"; + }); + + await page.goto('/wrap-server-route'); + + expect(await errorEventPromise).toMatchObject({ + exception: { + values: [ + { + value: "'HttpError' captured as exception with keys: body, status", + mechanism: { + handled: false, + data: { + function: 'serverRoute', + }, + }, + stacktrace: { frames: expect.any(Array) }, + }, + ], + }, + extra: { + __serialized__: { + body: { + message: 'error() error', + }, + status: 500, + }, + }, + }); + }); }); diff --git a/dev-packages/node-integration-tests/.eslintrc.js b/dev-packages/node-integration-tests/.eslintrc.js index df04aa267446..51b7dfbb7ed3 100644 --- a/dev-packages/node-integration-tests/.eslintrc.js +++ b/dev-packages/node-integration-tests/.eslintrc.js @@ -20,6 +20,15 @@ module.exports = { }, rules: { '@typescript-eslint/typedef': 'off', + // Explicitly allow ts-ignore with description for Node integration tests + // Reason: We run these tests on TS3.8 which doesn't support `@ts-expect-error` + '@typescript-eslint/ban-ts-comment': [ + 'error', + { + 'ts-ignore': 'allow-with-description', + 'ts-expect-error': true, + }, + ], }, }, ], diff --git a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts index aade87fe3bcf..4d1625035ebf 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts @@ -52,7 +52,17 @@ app.get('/test/error/:id', (req, res) => { Sentry.captureException(new Error(`This is an exception ${id}`)); - res.send({}); + setTimeout(() => { + // We flush to ensure we are sending exceptions in a certain order + Sentry.flush(3000).then( + () => { + res.send({}); + }, + () => { + res.send({}); + }, + ); + }, 1000); }); Sentry.setupExpressErrorHandler(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js new file mode 100644 index 000000000000..8c1817564196 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js @@ -0,0 +1,33 @@ +const { ApolloServer, gql } = require('apollo-server'); +const Sentry = require('@sentry/node'); + +module.exports = () => { + return Sentry.startSpan({ name: 'Test Server Start' }, () => { + return new ApolloServer({ + typeDefs: gql`type Query { + hello: String + world: String + } + type Mutation { + login(email: String): String + }`, + resolvers: { + Query: { + hello: () => { + return 'Hello!'; + }, + world: () => { + return 'World!'; + }, + }, + Mutation: { + login: async (_, { email }) => { + return `${email}--token`; + }, + }, + }, + introspection: false, + debug: false, + }); + }); +}; diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js index 9cecf2302315..6defe777d464 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js @@ -12,7 +12,8 @@ Sentry.init({ setInterval(() => {}, 1000); async function run() { - const { ApolloServer, gql } = require('apollo-server'); + const { gql } = require('apollo-server'); + const server = require('./apollo-server')(); await Sentry.startSpan( { @@ -20,29 +21,6 @@ async function run() { op: 'transaction', }, async span => { - const server = new ApolloServer({ - typeDefs: gql` - type Query { - hello: String - } - type Mutation { - login(email: String): String - } - `, - resolvers: { - Query: { - hello: () => { - return 'Hello world!'; - }, - }, - Mutation: { - login: async (_, { email }) => { - return `${email}--token`; - }, - }, - }, - }); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: gql`mutation Mutation($email: String){ diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js index f0c140fd4b24..b9a05c4b1c3c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js @@ -12,7 +12,7 @@ Sentry.init({ setInterval(() => {}, 1000); async function run() { - const { ApolloServer, gql } = require('apollo-server'); + const server = require('./apollo-server')(); await Sentry.startSpan( { @@ -20,21 +20,6 @@ async function run() { op: 'transaction', }, async span => { - const typeDefs = gql`type Query { hello: String }`; - - const resolvers = { - Query: { - hello: () => { - return 'Hello world!'; - }, - }, - }; - - const server = new ApolloServer({ - typeDefs, - resolvers, - }); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: '{hello}', diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts index 5bf91f7653c1..46e05acf940e 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts @@ -1,7 +1,12 @@ import { createRunner } from '../../../utils/runner'; +// Graphql Instrumentation emits some spans by default on server start +const EXPECTED_START_SERVER_TRANSACTION = { + transaction: 'Test Server Start', +}; + describe('GraphQL/Apollo Tests', () => { - test('CJS - should instrument GraphQL queries used from Apollo Server.', done => { + test('should instrument GraphQL queries used from Apollo Server.', done => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', spans: expect.arrayContaining([ @@ -18,10 +23,13 @@ describe('GraphQL/Apollo Tests', () => { ]), }; - createRunner(__dirname, 'scenario-query.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-query.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); - test('CJS - should instrument GraphQL mutations used from Apollo Server.', done => { + test('should instrument GraphQL mutations used from Apollo Server.', done => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', spans: expect.arrayContaining([ @@ -39,6 +47,9 @@ describe('GraphQL/Apollo Tests', () => { ]), }; - createRunner(__dirname, 'scenario-mutation.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-mutation.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js new file mode 100644 index 000000000000..840a5551b98a --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js @@ -0,0 +1,34 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan('test span name', async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js new file mode 100644 index 000000000000..992ff5337b46 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js @@ -0,0 +1,43 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + for (let i = 1; i < 10; i++) { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: `query GetHello${i} {hello}`, + }); + } + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js new file mode 100644 index 000000000000..d9eeca63ae10 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js @@ -0,0 +1,45 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetWorld {world}', + }); + + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js new file mode 100644 index 000000000000..8ee9154c0e51 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js @@ -0,0 +1,45 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const { gql } = require('apollo-server'); + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: gql`mutation TestMutation($email: String){ + login(email: $email) + }`, + variables: { email: 'test@email.com' }, + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js new file mode 100644 index 000000000000..14879bc0e79d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js @@ -0,0 +1,41 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js new file mode 100644 index 000000000000..4dc3357ab17f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js @@ -0,0 +1,41 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts new file mode 100644 index 000000000000..234cc4009b38 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts @@ -0,0 +1,152 @@ +import { createRunner } from '../../../../utils/runner'; + +// Graphql Instrumentation emits some spans by default on server start +const EXPECTED_START_SERVER_TRANSACTION = { + transaction: 'Test Server Start', +}; + +describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { + test('useOperationNameForRootSpan works with single query operation', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /test-graphql (query GetHello)', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetHello', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetHello {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetHello', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-query.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan works with single mutation operation', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /test-graphql (mutation TestMutation)', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.name': 'TestMutation', + 'graphql.operation.type': 'mutation', + 'graphql.source': `mutation TestMutation($email: String) { + login(email: $email) +}`, + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'mutation TestMutation', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-mutation.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan ignores an invalid root span', done => { + const EXPECTED_TRANSACTION = { + transaction: 'test span name', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetHello', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetHello {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetHello', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-invalid-root-span.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan works with single query operation without name', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /test-graphql (query)', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.type': 'query', + 'graphql.source': 'query {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-no-operation-name.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan works with multiple query operations', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /test-graphql (query GetHello, query GetWorld)', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetHello', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetHello {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetHello', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetWorld', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetWorld {world}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetWorld', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-multiple-operations.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan works with more than 5 query operations', done => { + const EXPECTED_TRANSACTION = { + transaction: + 'GET /test-graphql (query GetHello1, query GetHello2, query GetHello3, query GetHello4, query GetHello5, +4)', + }; + + createRunner(__dirname, 'scenario-multiple-operations-many.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/server.js b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server.js new file mode 100644 index 000000000000..35e204a05d6b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/server.js @@ -0,0 +1,33 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); + +// express must be required after Sentry is initialized +const express = require('express'); +const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests'); + +const app = express(); + +app.get('/test', (_req, res) => { + res.send({ + response: ` + + + ${Sentry.getTraceMetaTags()} + + + Hi :) + + + `, + }); +}); + +Sentry.setupExpressErrorHandler(app); + +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts new file mode 100644 index 000000000000..c42269dd8504 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts @@ -0,0 +1,25 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('getTraceMetaTags', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('injects sentry tracing tags', async () => { + const traceId = 'cd7ee7a6fe3ebe7ab9c3271559bc203c'; + const parentSpanId = '100ff0980e7a4ead'; + + const runner = createRunner(__dirname, 'server.js').start(); + + const response = await runner.makeRequest('get', '/test', { + 'sentry-trace': `${traceId}-${parentSpanId}-1`, + baggage: 'sentry-environment=production', + }); + + // @ts-ignore - response is defined, types just don't reflect it + const html = response?.response as unknown as string; + + expect(html).toMatch(//); + expect(html).toContain(''); + }); +}); diff --git a/packages/astro/src/client/sdk.ts b/packages/astro/src/client/sdk.ts index e38a552feb39..8a4617c652cf 100644 --- a/packages/astro/src/client/sdk.ts +++ b/packages/astro/src/client/sdk.ts @@ -4,7 +4,7 @@ import { getDefaultIntegrations as getBrowserDefaultIntegrations, init as initBrowserSdk, } from '@sentry/browser'; -import { applySdkMetadata, hasTracingEnabled } from '@sentry/core'; +import { applySdkMetadata } from '@sentry/core'; import type { Client, Integration } from '@sentry/types'; // Tree-shakable guard to remove all code related to tracing @@ -26,14 +26,12 @@ export function init(options: BrowserOptions): Client | undefined { return initBrowserSdk(opts); } -function getDefaultIntegrations(options: BrowserOptions): Integration[] | undefined { +function getDefaultIntegrations(options: BrowserOptions): Integration[] { // This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false", - // in which case everything inside will get treeshaken away + // in which case everything inside will get tree-shaken away if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) { - if (hasTracingEnabled(options)) { - return [...getBrowserDefaultIntegrations(options), browserTracingIntegration()]; - } + return [...getBrowserDefaultIntegrations(options), browserTracingIntegration()]; + } else { + return getBrowserDefaultIntegrations(options); } - - return undefined; } diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 1084643584d6..7f52ad0dc0bb 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -56,6 +56,7 @@ export { getSpanDescendants, getSpanStatusFromHttpCode, getTraceData, + getTraceMetaTags, graphqlIntegration, hapiIntegration, httpIntegration, diff --git a/packages/astro/src/integration/index.ts b/packages/astro/src/integration/index.ts index 9baee4d9dc39..0a367a1bdf5d 100644 --- a/packages/astro/src/integration/index.ts +++ b/packages/astro/src/integration/index.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import { sentryVitePlugin } from '@sentry/vite-plugin'; import type { AstroConfig, AstroIntegration } from 'astro'; +import { dropUndefinedKeys } from '@sentry/utils'; import { buildClientSnippet, buildSdkInitFileImportSnippet, buildServerSnippet } from './snippets'; import type { SentryOptions } from './types'; @@ -40,21 +41,29 @@ export const sentryAstro = (options: SentryOptions = {}): AstroIntegration => { sourcemap: true, }, plugins: [ - sentryVitePlugin({ - org: uploadOptions.org ?? env.SENTRY_ORG, - project: uploadOptions.project ?? env.SENTRY_PROJECT, - authToken: uploadOptions.authToken ?? env.SENTRY_AUTH_TOKEN, - telemetry: uploadOptions.telemetry ?? true, - sourcemaps: { - assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], - }, - _metaOptions: { - telemetry: { - metaFramework: 'astro', + sentryVitePlugin( + dropUndefinedKeys({ + org: uploadOptions.org ?? env.SENTRY_ORG, + project: uploadOptions.project ?? env.SENTRY_PROJECT, + authToken: uploadOptions.authToken ?? env.SENTRY_AUTH_TOKEN, + telemetry: uploadOptions.telemetry ?? true, + sourcemaps: { + assets: uploadOptions.assets ?? [getSourcemapsAssetsGlob(config)], }, - }, - debug: options.debug ?? false, - }), + bundleSizeOptimizations: { + ...options.bundleSizeOptimizations, + // TODO: with a future version of the vite plugin (probably 2.22.0) this re-mapping is not needed anymore + // ref: https://github.com/getsentry/sentry-javascript-bundler-plugins/pull/582 + excludePerformanceMonitoring: options.bundleSizeOptimizations?.excludeTracing, + }, + _metaOptions: { + telemetry: { + metaFramework: 'astro', + }, + }, + debug: options.debug ?? false, + }), + ), ], }, }); diff --git a/packages/astro/src/integration/snippets.ts b/packages/astro/src/integration/snippets.ts index c46267080aa8..4b784170d330 100644 --- a/packages/astro/src/integration/snippets.ts +++ b/packages/astro/src/integration/snippets.ts @@ -47,7 +47,7 @@ const buildCommonInitOptions = (options: SentryOptions): string => `dsn: ${ }`; /** - * We don't include the `BrowserTracing` integration if the tracesSampleRate is set to 0. + * We don't include the `BrowserTracing` integration if `bundleSizeOptimizations.excludeTracing` is falsy. * Likewise, we don't include the `Replay` integration if the replaysSessionSampleRate * and replaysOnErrorSampleRate are set to 0. * @@ -56,7 +56,7 @@ const buildCommonInitOptions = (options: SentryOptions): string => `dsn: ${ const buildClientIntegrations = (options: SentryOptions): string => { const integrations: string[] = []; - if (options.tracesSampleRate == null || options.tracesSampleRate) { + if (!options.bundleSizeOptimizations?.excludeTracing) { integrations.push('Sentry.browserTracingIntegration()'); } diff --git a/packages/astro/src/integration/types.ts b/packages/astro/src/integration/types.ts index f51a020bb290..026fcd01d8c4 100644 --- a/packages/astro/src/integration/types.ts +++ b/packages/astro/src/integration/types.ts @@ -25,62 +25,95 @@ type SdkInitPaths = { type SourceMapsOptions = { /** - * Options for the Sentry Vite plugin to customize the source maps upload process. + * If this flag is `true`, and an auth token is detected, the Sentry integration will + * automatically generate and upload source maps to Sentry during a production build. * - * These options are always read from the `sentryAstro` integration. - * Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files. + * @default true */ - sourceMapsUploadOptions?: { - /** - * If this flag is `true`, and an auth token is detected, the Sentry integration will - * automatically generate and upload source maps to Sentry during a production build. - * - * @default true - */ - enabled?: boolean; + enabled?: boolean; - /** - * The auth token to use when uploading source maps to Sentry. - * - * Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable. - * - * To create an auth token, follow this guide: - * @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens - */ - authToken?: string; + /** + * The auth token to use when uploading source maps to Sentry. + * + * Instead of specifying this option, you can also set the `SENTRY_AUTH_TOKEN` environment variable. + * + * To create an auth token, follow this guide: + * @see https://docs.sentry.io/product/accounts/auth-tokens/#organization-auth-tokens + */ + authToken?: string; - /** - * The organization slug of your Sentry organization. - * Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable. - */ - org?: string; + /** + * The organization slug of your Sentry organization. + * Instead of specifying this option, you can also set the `SENTRY_ORG` environment variable. + */ + org?: string; - /** - * The project slug of your Sentry project. - * Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable. - */ - project?: string; + /** + * The project slug of your Sentry project. + * Instead of specifying this option, you can also set the `SENTRY_PROJECT` environment variable. + */ + project?: string; - /** - * If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry. - * It will not collect any sensitive or user-specific data. - * - * @default true - */ - telemetry?: boolean; + /** + * If this flag is `true`, the Sentry plugin will collect some telemetry data and send it to Sentry. + * It will not collect any sensitive or user-specific data. + * + * @default true + */ + telemetry?: boolean; - /** - * A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry. - * - * If this option is not specified, sensible defaults based on your `outDir`, `rootDir` and `adapter` - * config will be used. Use this option to override these defaults, for instance if you have a - * customized build setup that diverges from Astro's defaults. - * - * The globbing patterns must follow the implementation of the `glob` package. - * @see https://www.npmjs.com/package/glob#glob-primer - */ - assets?: string | Array; - }; + /** + * A glob or an array of globs that specify the build artifacts and source maps that will be uploaded to Sentry. + * + * If this option is not specified, sensible defaults based on your `outDir`, `rootDir` and `adapter` + * config will be used. Use this option to override these defaults, for instance if you have a + * customized build setup that diverges from Astro's defaults. + * + * The globbing patterns must follow the implementation of the `glob` package. + * @see https://www.npmjs.com/package/glob#glob-primer + */ + assets?: string | Array; +}; + +type BundleSizeOptimizationOptions = { + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) any debugging code within the Sentry SDK. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * Setting this option to `true` will disable features like the SDK's `debug` option. + */ + excludeDebugStatements?: boolean; + + /** + * If set to true, the plugin will try to tree-shake performance monitoring statements out. + * Note that the success of this depends on tree shaking generally being enabled in your build. + * Attention: DO NOT enable this when you're using any performance monitoring-related SDK features (e.g. Sentry.startTransaction()). + */ + excludeTracing?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay Shadow DOM recording functionality. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * This option is safe to be used when you do not want to capture any Shadow DOM activity via Sentry Session Replay. + */ + excludeReplayShadowDom?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay `iframe` recording functionality. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * You can safely do this when you do not want to capture any `iframe` activity via Sentry Session Replay. + */ + excludeReplayIframe?: boolean; + + /** + * If set to `true`, the plugin will attempt to tree-shake (remove) code related to the Sentry SDK's Session Replay's Compression Web Worker. + * Note that the success of this depends on tree shaking being enabled in your build tooling. + * + * **Notice:** You should only do use this option if you manually host a compression worker and configure it in your Sentry Session Replay integration config via the `workerUrl` option. + */ + excludeReplayWorker?: boolean; }; type InstrumentationOptions = { @@ -138,6 +171,20 @@ type SdkEnabledOptions = { export type SentryOptions = SdkInitPaths & Pick & Pick & - SourceMapsOptions & InstrumentationOptions & - SdkEnabledOptions; + SdkEnabledOptions & { + /** + * Options for the Sentry Vite plugin to customize the source maps upload process. + * + * These options are always read from the `sentryAstro` integration. + * Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files. + */ + sourceMapsUploadOptions?: SourceMapsOptions; + /** + * Options for the Sentry Vite plugin to customize bundle size optimizations. + * + * These options are always read from the `sentryAstro` integration. + * Do not define them in the `sentry.client.config.(js|ts)` or `sentry.server.config.(js|ts)` files. + */ + bundleSizeOptimizations?: BundleSizeOptimizationOptions; + }; diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 6b668f462489..4b2f15eb3be4 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -6,6 +6,7 @@ import { getActiveSpan, getClient, getCurrentScope, + getTraceMetaTags, setHttpStatus, startSpan, withIsolationScope, @@ -14,8 +15,6 @@ import type { Client, Scope, Span, SpanAttributes } from '@sentry/types'; import { addNonEnumerableProperty, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; import type { APIContext, MiddlewareResponseHandler } from 'astro'; -import { getTraceData } from '@sentry/node'; - type MiddlewareOptions = { /** * If true, the client IP will be attached to the event by calling `setUser`. @@ -189,16 +188,13 @@ function addMetaTagToHead(htmlChunk: string, scope: Scope, client: Client, span? if (typeof htmlChunk !== 'string') { return htmlChunk; } - const { 'sentry-trace': sentryTrace, baggage } = getTraceData(span, scope, client); + const metaTags = getTraceMetaTags(span, scope, client); - if (!sentryTrace) { + if (!metaTags) { return htmlChunk; } - const sentryTraceMeta = ``; - const baggageMeta = baggage && ``; - - const content = `\n${sentryTraceMeta}`.concat(baggageMeta ? `\n${baggageMeta}` : '', '\n'); + const content = `${metaTags}`; return htmlChunk.replace('', content); } diff --git a/packages/astro/test/client/sdk.test.ts b/packages/astro/test/client/sdk.test.ts index 1ef31131cb77..41ff4bbae061 100644 --- a/packages/astro/test/client/sdk.test.ts +++ b/packages/astro/test/client/sdk.test.ts @@ -53,6 +53,7 @@ describe('Sentry client SDK', () => { ['tracesSampleRate', { tracesSampleRate: 0 }], ['tracesSampler', { tracesSampler: () => 1.0 }], ['enableTracing', { enableTracing: true }], + ['no tracing option set', {}], ])('adds browserTracingIntegration if tracing is enabled via %s', (_, tracingOptions) => { init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', @@ -66,22 +67,6 @@ describe('Sentry client SDK', () => { expect(browserTracing).toBeDefined(); }); - it.each([ - ['enableTracing', { enableTracing: false }], - ['no tracing option set', {}], - ])("doesn't add browserTracingIntegration if tracing is disabled via %s", (_, tracingOptions) => { - init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - ...tracingOptions, - }); - - const integrationsToInit = browserInit.mock.calls[0]![0]?.defaultIntegrations || []; - const browserTracing = getClient()?.getIntegrationByName('BrowserTracing'); - - expect(integrationsToInit).not.toContainEqual(expect.objectContaining({ name: 'BrowserTracing' })); - expect(browserTracing).toBeUndefined(); - }); - it("doesn't add browserTracingIntegration if `__SENTRY_TRACING__` is set to false", () => { (globalThis as any).__SENTRY_TRACING__ = false; diff --git a/packages/astro/test/integration/index.test.ts b/packages/astro/test/integration/index.test.ts index 008132264602..eb6bdf555ae3 100644 --- a/packages/astro/test/integration/index.test.ts +++ b/packages/astro/test/integration/index.test.ts @@ -57,6 +57,7 @@ describe('sentryAstro integration', () => { project: 'my-project', telemetry: false, debug: false, + bundleSizeOptimizations: {}, sourcemaps: { assets: ['out/**/*'], }, @@ -82,6 +83,7 @@ describe('sentryAstro integration', () => { project: 'my-project', telemetry: false, debug: false, + bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/**/*'], }, @@ -114,6 +116,7 @@ describe('sentryAstro integration', () => { project: 'my-project', telemetry: false, debug: false, + bundleSizeOptimizations: {}, sourcemaps: { assets: ['{.vercel,dist}/**/*'], }, @@ -151,6 +154,7 @@ describe('sentryAstro integration', () => { project: 'my-project', telemetry: true, debug: false, + bundleSizeOptimizations: {}, sourcemaps: { assets: ['dist/server/**/*, dist/client/**/*'], }, diff --git a/packages/astro/test/integration/snippets.test.ts b/packages/astro/test/integration/snippets.test.ts index 04aaa866aee9..66a3b15efd58 100644 --- a/packages/astro/test/integration/snippets.test.ts +++ b/packages/astro/test/integration/snippets.test.ts @@ -52,7 +52,25 @@ describe('buildClientSnippet', () => { `); }); - it('does not include browserTracingIntegration if tracesSampleRate is 0', () => { + it('does not include browserTracingIntegration if bundleSizeOptimizations.excludeTracing is true', () => { + const snippet = buildClientSnippet({ bundleSizeOptimizations: { excludeTracing: true } }); + expect(snippet).toMatchInlineSnapshot(` + "import * as Sentry from "@sentry/astro"; + + Sentry.init({ + dsn: import.meta.env.PUBLIC_SENTRY_DSN, + debug: false, + environment: import.meta.env.PUBLIC_VERCEL_ENV, + release: import.meta.env.PUBLIC_VERCEL_GIT_COMMIT_SHA, + tracesSampleRate: 1, + integrations: [Sentry.replayIntegration()], + replaysSessionSampleRate: 0.1, + replaysOnErrorSampleRate: 1, + });" + `); + }); + + it('still include browserTracingIntegration if tracesSampleRate is 0', () => { const snippet = buildClientSnippet({ tracesSampleRate: 0 }); expect(snippet).toMatchInlineSnapshot(` "import * as Sentry from "@sentry/astro"; @@ -63,7 +81,7 @@ describe('buildClientSnippet', () => { environment: import.meta.env.PUBLIC_VERCEL_ENV, release: import.meta.env.PUBLIC_VERCEL_GIT_COMMIT_SHA, tracesSampleRate: 0, - integrations: [Sentry.replayIntegration()], + integrations: [Sentry.browserTracingIntegration(), Sentry.replayIntegration()], replaysSessionSampleRate: 0.1, replaysOnErrorSampleRate: 1, });" diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 58405c8d1c12..bf96f6ef9046 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -34,10 +34,12 @@ describe('sentryMiddleware', () => { }); vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock); vi.spyOn(SentryNode, 'getClient').mockImplementation(() => ({}) as Client); - vi.spyOn(SentryNode, 'getTraceData').mockImplementation(() => ({ - 'sentry-trace': '123', - baggage: 'abc', - })); + vi.spyOn(SentryNode, 'getTraceMetaTags').mockImplementation( + () => ` + + + `, + ); vi.spyOn(SentryCore, 'getDynamicSamplingContextFromSpan').mockImplementation(() => ({ transaction: 'test', })); diff --git a/packages/aws-serverless/rollup.npm.config.mjs b/packages/aws-serverless/rollup.npm.config.mjs index 46e006f70b95..0ac3218144d5 100644 --- a/packages/aws-serverless/rollup.npm.config.mjs +++ b/packages/aws-serverless/rollup.npm.config.mjs @@ -9,6 +9,10 @@ export default [ entrypoints: ['src/index.ts', 'src/awslambda-auto.ts'], // packages with bundles have a different build directory structure hasBundles: true, + packageSpecificConfig: { + // Used for our custom eventContextExtractor + external: ['@opentelemetry/api'], + }, }), ), ...makeOtelLoaders('./build', 'sentry-node'), diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 95b2d553f2d4..20ef9eeaf09f 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -21,6 +21,7 @@ export { getGlobalScope, getIsolationScope, getTraceData, + getTraceMetaTags, setCurrentClient, Scope, SDK_VERSION, diff --git a/packages/aws-serverless/src/integration/awslambda.ts b/packages/aws-serverless/src/integration/awslambda.ts index c6516603b6b8..2e1479914471 100644 --- a/packages/aws-serverless/src/integration/awslambda.ts +++ b/packages/aws-serverless/src/integration/awslambda.ts @@ -1,20 +1,44 @@ import { AwsLambdaInstrumentation } from '@opentelemetry/instrumentation-aws-lambda'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration } from '@sentry/core'; -import { addOpenTelemetryInstrumentation } from '@sentry/node'; +import { generateInstrumentOnce } from '@sentry/node'; import type { IntegrationFn } from '@sentry/types'; +import { eventContextExtractor } from '../utils'; -const _awsLambdaIntegration = (() => { +interface AwsLambdaOptions { + /** + * Disables the AWS context propagation and instead uses + * Sentry's context. Defaults to `true`, in order for + * Sentry trace propagation to take precedence, but can + * be disabled if you want AWS propagation to take take + * precedence. + */ + disableAwsContextPropagation?: boolean; +} + +export const instrumentAwsLambda = generateInstrumentOnce( + 'AwsLambda', + (_options: AwsLambdaOptions = {}) => { + const options = { + disableAwsContextPropagation: true, + ..._options, + }; + + return new AwsLambdaInstrumentation({ + ...options, + eventContextExtractor, + requestHook(span) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.otel.aws-lambda'); + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'function.aws.lambda'); + }, + }); + }, +); + +const _awsLambdaIntegration = ((options: AwsLambdaOptions = {}) => { return { name: 'AwsLambda', setupOnce() { - addOpenTelemetryInstrumentation( - new AwsLambdaInstrumentation({ - requestHook(span) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.otel.aws-lambda'); - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'function.aws.lambda'); - }, - }), - ); + instrumentAwsLambda(options); }, }; }) satisfies IntegrationFn; diff --git a/packages/aws-serverless/src/sdk.ts b/packages/aws-serverless/src/sdk.ts index 8ed86d9f23d4..e052782d50eb 100644 --- a/packages/aws-serverless/src/sdk.ts +++ b/packages/aws-serverless/src/sdk.ts @@ -16,7 +16,7 @@ import { withScope, } from '@sentry/node'; import type { Integration, Options, Scope, SdkMetadata, Span } from '@sentry/types'; -import { isString, logger } from '@sentry/utils'; +import { logger } from '@sentry/utils'; import type { Context, Handler } from 'aws-lambda'; import { performance } from 'perf_hooks'; @@ -25,7 +25,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } fr import { DEBUG_BUILD } from './debug-build'; import { awsIntegration } from './integration/aws'; import { awsLambdaIntegration } from './integration/awslambda'; -import { markEventUnhandled } from './utils'; +import { getAwsTraceData, markEventUnhandled } from './utils'; const { isPromise } = types; @@ -334,15 +334,9 @@ export function wrapHandler( // Otherwise, we create two root spans (one from otel, one from our wrapper). // If Otel instrumentation didn't work or was filtered by users, we still want to trace the handler. if (options.startTrace && !isWrappedByOtel(handler)) { - const eventWithHeaders = event as { headers?: { [key: string]: string } }; + const traceData = getAwsTraceData(event as { headers?: Record }, context); - const sentryTrace = - eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace']) - ? eventWithHeaders.headers['sentry-trace'] - : undefined; - const baggage = eventWithHeaders.headers?.baggage; - - return continueTrace({ sentryTrace, baggage }, () => { + return continueTrace({ sentryTrace: traceData['sentry-trace'], baggage: traceData.baggage }, () => { return startSpanManual( { name: context.functionName, diff --git a/packages/aws-serverless/src/utils.ts b/packages/aws-serverless/src/utils.ts index 259388bb193c..f6461030c1a7 100644 --- a/packages/aws-serverless/src/utils.ts +++ b/packages/aws-serverless/src/utils.ts @@ -1,5 +1,29 @@ +import type { TextMapGetter } from '@opentelemetry/api'; +import type { Context as OtelContext } from '@opentelemetry/api'; +import { context as otelContext, propagation } from '@opentelemetry/api'; import type { Scope } from '@sentry/types'; -import { addExceptionMechanism } from '@sentry/utils'; +import { addExceptionMechanism, isString } from '@sentry/utils'; +import type { Handler } from 'aws-lambda'; +import type { APIGatewayProxyEventHeaders } from 'aws-lambda'; + +type HandlerEvent = Parameters }>>[0]; +type HandlerContext = Parameters[1]; + +type TraceData = { + 'sentry-trace'?: string; + baggage?: string; +}; + +// vendored from +// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts#L65-L72 +const headerGetter: TextMapGetter = { + keys(carrier): string[] { + return Object.keys(carrier); + }, + get(carrier, key: string) { + return carrier[key]; + }, +}; /** * Marks an event as unhandled by adding a span processor to the passed scope. @@ -12,3 +36,51 @@ export function markEventUnhandled(scope: Scope): Scope { return scope; } + +/** + * Extracts sentry trace data from the handler `context` if available and falls + * back to the `event`. + * + * When instrumenting the Lambda function with Sentry, the sentry trace data + * is placed on `context.clientContext.Custom`. Users are free to modify context + * tho and provide this data via `event` or `context`. + */ +export function getAwsTraceData(event: HandlerEvent, context?: HandlerContext): TraceData { + const headers = event.headers || {}; + + const traceData: TraceData = { + 'sentry-trace': headers['sentry-trace'], + baggage: headers.baggage, + }; + + if (context && context.clientContext && context.clientContext.Custom) { + const customContext: Record = context.clientContext.Custom; + const sentryTrace = isString(customContext['sentry-trace']) ? customContext['sentry-trace'] : undefined; + + if (sentryTrace) { + traceData['sentry-trace'] = sentryTrace; + traceData.baggage = isString(customContext.baggage) ? customContext.baggage : undefined; + } + } + + return traceData; +} + +/** + * A custom event context extractor for the aws integration. It takes sentry trace data + * from the context rather than the event, with the event being a fallback. + * + * Is only used when the handler was successfully wrapped by otel and the integration option + * `disableAwsContextPropagation` is `true`. + */ +export function eventContextExtractor(event: HandlerEvent, context?: HandlerContext): OtelContext { + // The default context extractor tries to get sampled trace headers from HTTP headers + // The otel aws integration packs these onto the context, so we try to extract them from + // there instead. + const httpHeaders = { + ...(event.headers || {}), + ...getAwsTraceData(event, context), + }; + + return propagation.extract(otelContext.active(), httpHeaders, headerGetter); +} diff --git a/packages/aws-serverless/test/utils.test.ts b/packages/aws-serverless/test/utils.test.ts new file mode 100644 index 000000000000..197c6ebdf90f --- /dev/null +++ b/packages/aws-serverless/test/utils.test.ts @@ -0,0 +1,102 @@ +import { eventContextExtractor, getAwsTraceData } from '../src/utils'; + +const mockExtractContext = jest.fn(); +jest.mock('@opentelemetry/api', () => { + const actualApi = jest.requireActual('@opentelemetry/api'); + return { + ...actualApi, + propagation: { + extract: (...args: unknown[]) => mockExtractContext(args), + }, + }; +}); + +const mockContext = { + clientContext: { + Custom: { + 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', + baggage: 'sentry-environment=production', + }, + }, +}; +const mockEvent = { + headers: { + 'sentry-trace': '12345678901234567890123456789012-1234567890123456-2', + baggage: 'sentry-environment=staging', + }, +}; + +describe('getTraceData', () => { + test('gets sentry trace data from the context', () => { + // @ts-expect-error, a partial context object is fine here + const traceData = getAwsTraceData({}, mockContext); + + expect(traceData['sentry-trace']).toEqual('12345678901234567890123456789012-1234567890123456-1'); + expect(traceData.baggage).toEqual('sentry-environment=production'); + }); + + test('gets sentry trace data from the context even if event has data', () => { + // @ts-expect-error, a partial context object is fine here + const traceData = getAwsTraceData(mockEvent, mockContext); + + expect(traceData['sentry-trace']).toEqual('12345678901234567890123456789012-1234567890123456-1'); + expect(traceData.baggage).toEqual('sentry-environment=production'); + }); + + test('gets sentry trace data from the event if no context is passed', () => { + const traceData = getAwsTraceData(mockEvent); + + expect(traceData['sentry-trace']).toEqual('12345678901234567890123456789012-1234567890123456-2'); + expect(traceData.baggage).toEqual('sentry-environment=staging'); + }); + + test('gets sentry trace data from the event if the context sentry trace is undefined', () => { + const traceData = getAwsTraceData(mockEvent, { + // @ts-expect-error, a partial context object is fine here + clientContext: { Custom: { 'sentry-trace': undefined, baggage: '' } }, + }); + + expect(traceData['sentry-trace']).toEqual('12345678901234567890123456789012-1234567890123456-2'); + expect(traceData.baggage).toEqual('sentry-environment=staging'); + }); +}); + +describe('eventContextExtractor', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + test('passes sentry trace data to the propagation extractor', () => { + // @ts-expect-error, a partial context object is fine here + eventContextExtractor(mockEvent, mockContext); + + // @ts-expect-error, a partial context object is fine here + const expectedTraceData = getAwsTraceData(mockEvent, mockContext); + + expect(mockExtractContext).toHaveBeenCalledTimes(1); + expect(mockExtractContext).toHaveBeenCalledWith(expect.arrayContaining([expectedTraceData])); + }); + + test('passes along non-sentry trace headers along', () => { + eventContextExtractor( + { + ...mockEvent, + headers: { + ...mockEvent.headers, + 'X-Custom-Header': 'Foo', + }, + }, + // @ts-expect-error, a partial context object is fine here + mockContext, + ); + + const expectedHeaders = { + 'X-Custom-Header': 'Foo', + // @ts-expect-error, a partial context object is fine here + ...getAwsTraceData(mockEvent, mockContext), + }; + + expect(mockExtractContext).toHaveBeenCalledTimes(1); + expect(mockExtractContext).toHaveBeenCalledWith(expect.arrayContaining([expectedHeaders])); + }); +}); diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 1421814ae9e5..04aa82b5f0e6 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -57,6 +57,14 @@ function applyDefaultOptions(optionsArg: BrowserOptions = {}): BrowserOptions { sendClientReports: true, }; + // TODO: Instead of dropping just `defaultIntegrations`, we should simply + // call `dropUndefinedKeys` on the entire `optionsArg`. + // However, for this to work we need to adjust the `hasTracingEnabled()` logic + // first as it differentiates between `undefined` and the key not being in the object. + if (optionsArg.defaultIntegrations == null) { + delete optionsArg.defaultIntegrations; + } + return { ...defaultOptions, ...optionsArg }; } diff --git a/packages/browser/test/sdk.test.ts b/packages/browser/test/sdk.test.ts index 80e54e3d49d2..618333532a09 100644 --- a/packages/browser/test/sdk.test.ts +++ b/packages/browser/test/sdk.test.ts @@ -6,6 +6,7 @@ import type { Mock } from 'vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as SentryCore from '@sentry/core'; import { Scope, createTransport } from '@sentry/core'; import type { Client, Integration } from '@sentry/types'; import { resolvedSyncPromise } from '@sentry/utils'; @@ -79,6 +80,18 @@ describe('init', () => { expect(DEFAULT_INTEGRATIONS[1]!.setupOnce as Mock).toHaveBeenCalledTimes(1); }); + it('installs default integrations if `defaultIntegrations: undefined`', () => { + // @ts-expect-error this is fine for testing + const initAndBindSpy = vi.spyOn(SentryCore, 'initAndBind').mockImplementationOnce(() => {}); + const options = getDefaultBrowserOptions({ dsn: PUBLIC_DSN, defaultIntegrations: undefined }); + init(options); + + expect(initAndBindSpy).toHaveBeenCalledTimes(1); + + const optionsPassed = initAndBindSpy.mock.calls[0]?.[1]; + expect(optionsPassed?.integrations?.length).toBeGreaterThan(0); + }); + test("doesn't install default integrations if told not to", () => { const DEFAULT_INTEGRATIONS: Integration[] = [ new MockIntegration('MockIntegration 0.3'), @@ -150,6 +163,7 @@ describe('init', () => { Object.defineProperty(WINDOW, 'browser', { value: undefined, writable: true }); Object.defineProperty(WINDOW, 'nw', { value: undefined, writable: true }); Object.defineProperty(WINDOW, 'window', { value: WINDOW, writable: true }); + vi.clearAllMocks(); }); it('logs a browser extension error if executed inside a Chrome extension', () => { diff --git a/packages/browser/test/utils/lazyLoadIntegration.test.ts b/packages/browser/test/utils/lazyLoadIntegration.test.ts index ec88ae49a1a9..82548a59faec 100644 --- a/packages/browser/test/utils/lazyLoadIntegration.test.ts +++ b/packages/browser/test/utils/lazyLoadIntegration.test.ts @@ -71,9 +71,11 @@ describe('lazyLoadIntegration', () => { httpClientIntegration: undefined, }; - // We do not await here, as this this does not seem to work with JSDOM :( - // We have browser integration tests to check that this actually works - void lazyLoadIntegration('httpClientIntegration'); + try { + await lazyLoadIntegration('httpClientIntegration'); + } catch { + // skip + } expect(global.document.querySelectorAll('script')).toHaveLength(1); expect(global.document.querySelector('script')?.src).toEqual( diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 287dbc26eeee..4de55fd1c5f7 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -41,6 +41,7 @@ export { getGlobalScope, getIsolationScope, getTraceData, + getTraceMetaTags, setCurrentClient, Scope, SDK_VERSION, diff --git a/packages/cloudflare/src/index.ts b/packages/cloudflare/src/index.ts index 2f77f96f4e33..0c02fb8ca810 100644 --- a/packages/cloudflare/src/index.ts +++ b/packages/cloudflare/src/index.ts @@ -56,6 +56,7 @@ export { getActiveSpan, getRootSpan, getTraceData, + getTraceMetaTags, startSpan, startInactiveSpan, startSpanManual, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5c21c8e484ed..73295f7df64c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -83,6 +83,7 @@ export { export { parseSampleRate } from './utils/parseSampleRate'; export { applySdkMetadata } from './utils/sdkMetadata'; export { getTraceData } from './utils/traceData'; +export { getTraceMetaTags } from './utils/meta'; export { DEFAULT_ENVIRONMENT } from './constants'; export { addBreadcrumb } from './breadcrumbs'; export { functionToStringIntegration } from './integrations/functiontostring'; diff --git a/packages/core/src/utils/meta.ts b/packages/core/src/utils/meta.ts new file mode 100644 index 000000000000..339dfcee2f28 --- /dev/null +++ b/packages/core/src/utils/meta.ts @@ -0,0 +1,29 @@ +import type { Client, Scope, Span } from '@sentry/types'; +import { getTraceData } from './traceData'; + +/** + * Returns a string of meta tags that represent the current trace data. + * + * You can use this to propagate a trace from your server-side rendered Html to the browser. + * This function returns up to two meta tags, `sentry-trace` and `baggage`, depending on the + * current trace data state. + * + * @example + * Usage example: + * + * ```js + * function renderHtml() { + * return ` + * + * ${getTraceMetaTags()} + * + * `; + * } + * ``` + * + */ +export function getTraceMetaTags(span?: Span, scope?: Scope, client?: Client): string { + return Object.entries(getTraceData(span, scope, client)) + .map(([key, value]) => ``) + .join('\n'); +} diff --git a/packages/core/test/lib/utils/meta.test.ts b/packages/core/test/lib/utils/meta.test.ts new file mode 100644 index 000000000000..3d78247b8951 --- /dev/null +++ b/packages/core/test/lib/utils/meta.test.ts @@ -0,0 +1,30 @@ +import { getTraceMetaTags } from '../../../src/utils/meta'; +import * as TraceDataModule from '../../../src/utils/traceData'; + +describe('getTraceMetaTags', () => { + it('renders baggage and sentry-trace values to stringified Html meta tags', () => { + jest.spyOn(TraceDataModule, 'getTraceData').mockReturnValueOnce({ + 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', + baggage: 'sentry-environment=production', + }); + + expect(getTraceMetaTags()).toBe(` +`); + }); + + it('renders just sentry-trace values to stringified Html meta tags', () => { + jest.spyOn(TraceDataModule, 'getTraceData').mockReturnValueOnce({ + 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', + }); + + expect(getTraceMetaTags()).toBe( + '', + ); + }); + + it('returns an empty string if neither sentry-trace nor baggage values are available', () => { + jest.spyOn(TraceDataModule, 'getTraceData').mockReturnValueOnce({}); + + expect(getTraceMetaTags()).toBe(''); + }); +}); diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 69b26bb1729a..1f983b476b74 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -56,6 +56,7 @@ export { getActiveSpan, getRootSpan, getTraceData, + getTraceMetaTags, startSpan, startInactiveSpan, startSpanManual, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 351f843d2c2d..73f24f9cf39e 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -21,6 +21,7 @@ export { getGlobalScope, getIsolationScope, getTraceData, + getTraceMetaTags, setCurrentClient, Scope, SDK_VERSION, diff --git a/packages/nestjs/src/setup.ts b/packages/nestjs/src/setup.ts index b068ed052a91..5e76f5cbe912 100644 --- a/packages/nestjs/src/setup.ts +++ b/packages/nestjs/src/setup.ts @@ -64,6 +64,8 @@ export { SentryTracingInterceptor }; * Global filter to handle exceptions and report them to Sentry. */ class SentryGlobalFilter extends BaseExceptionFilter { + public static readonly __SENTRY_INTERNAL__ = true; + /** * Catches exceptions and reports them to Sentry unless they are expected errors. */ @@ -84,6 +86,8 @@ export { SentryGlobalFilter }; * Service to set up Sentry performance tracing for Nest.js applications. */ class SentryService implements OnModuleInit { + public static readonly __SENTRY_INTERNAL__ = true; + /** * Initializes the Sentry service and registers span attributes. */ diff --git a/packages/nestjs/test/sdk.test.ts b/packages/nestjs/test/sdk.test.ts index c6bf7166444d..77d19fa2c797 100644 --- a/packages/nestjs/test/sdk.test.ts +++ b/packages/nestjs/test/sdk.test.ts @@ -1,7 +1,8 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + import * as SentryNode from '@sentry/node'; import { SDK_VERSION } from '@sentry/utils'; -import { vi } from 'vitest'; import { init as nestInit } from '../src/sdk'; const nodeInit = vi.spyOn(SentryNode, 'init'); diff --git a/packages/nestjs/tsconfig.test.json b/packages/nestjs/tsconfig.test.json index fc9e549d35ce..00cada2d8bcf 100644 --- a/packages/nestjs/tsconfig.test.json +++ b/packages/nestjs/tsconfig.test.json @@ -4,9 +4,6 @@ "include": ["test/**/*", "vite.config.ts"], "compilerOptions": { - // should include all types from `./tsconfig.json` plus types for all test frameworks used - "types": ["vitest/globals"] - // other package-specific, test-specific options } } diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index badd1f1a27bf..1fdc32d3d77a 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -96,6 +96,7 @@ export { getCurrentScope, getIsolationScope, getTraceData, + getTraceMetaTags, withScope, withIsolationScope, captureException, diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 097ee3ba43f8..914653ac745c 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -1,12 +1,17 @@ import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; -import { defineIntegration } from '@sentry/core'; +import { defineIntegration, getRootSpan, spanToJSON } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; import { generateInstrumentOnce } from '../../otel/instrument'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; interface GraphqlOptions { - /** Do not create spans for resolvers. */ + /** + * Do not create spans for resolvers. + * + * Defaults to true. + */ ignoreResolveSpans?: boolean; /** @@ -16,8 +21,18 @@ interface GraphqlOptions { * use the default resolver which just looks for a property with that name on the object. * If the property is not a function, it's not very interesting to trace. * This option can reduce noise and number of spans created. + * + * Defaults to true. + */ + ignoreTrivialResolveSpans?: boolean; + + /** + * If this is enabled, a http.server root span containing this span will automatically be renamed to include the operation name. + * Set this to `false` if you do not want this behavior, and want to keep the default http.server span name. + * + * Defaults to true. */ - ignoreTrivalResolveSpans?: boolean; + useOperationNameForRootSpan?: boolean; } const INTEGRATION_NAME = 'Graphql'; @@ -28,6 +43,7 @@ export const instrumentGraphql = generateInstrumentOnce( const options = { ignoreResolveSpans: true, ignoreTrivialResolveSpans: true, + useOperationNameForRootSpan: true, ..._options, }; @@ -35,6 +51,35 @@ export const instrumentGraphql = generateInstrumentOnce( ...options, responseHook(span) { addOriginToSpan(span, 'auto.graphql.otel.graphql'); + + const attributes = spanToJSON(span).data || {}; + + // If operation.name is not set, we fall back to use operation.type only + const operationType = attributes['graphql.operation.type']; + const operationName = attributes['graphql.operation.name']; + + if (options.useOperationNameForRootSpan && operationType) { + const rootSpan = getRootSpan(span); + + // We guard to only do this on http.server spans + + const rootSpanAttributes = spanToJSON(rootSpan).data || {}; + + const existingOperations = rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION] || []; + + const newOperation = operationName ? `${operationType} ${operationName}` : `${operationType}`; + + // We keep track of each operation on the root span + // This can either be a string, or an array of strings (if there are multiple operations) + if (Array.isArray(existingOperations)) { + existingOperations.push(newOperation); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, existingOperations); + } else if (existingOperations) { + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, [existingOperations, newOperation]); + } else { + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, newOperation); + } + } }, }); }, diff --git a/packages/node/src/integrations/tracing/nest/helpers.ts b/packages/node/src/integrations/tracing/nest/helpers.ts index 32eb3a0d5a39..babf80022c1f 100644 --- a/packages/node/src/integrations/tracing/nest/helpers.ts +++ b/packages/node/src/integrations/tracing/nest/helpers.ts @@ -1,6 +1,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { addNonEnumerableProperty } from '@sentry/utils'; -import type { InjectableTarget } from './types'; +import type { CatchTarget, InjectableTarget } from './types'; const sentryPatched = 'sentryPatched'; @@ -10,7 +10,7 @@ const sentryPatched = 'sentryPatched'; * We already guard duplicate patching with isWrapped. However, isWrapped checks whether a file has been patched, whereas we use this check for concrete target classes. * This check might not be necessary, but better to play it safe. */ -export function isPatched(target: InjectableTarget): boolean { +export function isPatched(target: InjectableTarget | CatchTarget): boolean { if (target.sentryPatched) { return true; } @@ -23,7 +23,7 @@ export function isPatched(target: InjectableTarget): boolean { * Returns span options for nest middleware spans. */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function getMiddlewareSpanOptions(target: InjectableTarget) { +export function getMiddlewareSpanOptions(target: InjectableTarget | CatchTarget) { return { name: target.name, attributes: { diff --git a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts index 52c3a4ad6b40..28d5a74ef63d 100644 --- a/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts +++ b/packages/node/src/integrations/tracing/nest/sentry-nest-instrumentation.ts @@ -9,7 +9,7 @@ import { getActiveSpan, startSpan, startSpanManual, withActiveSpan } from '@sent import type { Span } from '@sentry/types'; import { SDK_VERSION } from '@sentry/utils'; import { getMiddlewareSpanOptions, isPatched } from './helpers'; -import type { InjectableTarget } from './types'; +import type { CatchTarget, InjectableTarget } from './types'; const supportedVersions = ['>=8.0.0 <11']; @@ -34,7 +34,10 @@ export class SentryNestInstrumentation extends InstrumentationBase { public init(): InstrumentationNodeModuleDefinition { const moduleDef = new InstrumentationNodeModuleDefinition(SentryNestInstrumentation.COMPONENT, supportedVersions); - moduleDef.files.push(this._getInjectableFileInstrumentation(supportedVersions)); + moduleDef.files.push( + this._getInjectableFileInstrumentation(supportedVersions), + this._getCatchFileInstrumentation(supportedVersions), + ); return moduleDef; } @@ -58,10 +61,28 @@ export class SentryNestInstrumentation extends InstrumentationBase { ); } + /** + * Wraps the @Catch decorator. + */ + private _getCatchFileInstrumentation(versions: string[]): InstrumentationNodeModuleFile { + return new InstrumentationNodeModuleFile( + '@nestjs/common/decorators/core/catch.decorator.js', + versions, + (moduleExports: { Catch: CatchTarget }) => { + if (isWrapped(moduleExports.Catch)) { + this._unwrap(moduleExports, 'Catch'); + } + this._wrap(moduleExports, 'Catch', this._createWrapCatch()); + return moduleExports; + }, + (moduleExports: { Catch: CatchTarget }) => { + this._unwrap(moduleExports, 'Catch'); + }, + ); + } + /** * Creates a wrapper function for the @Injectable decorator. - * - * Wraps the use method to instrument nest class middleware. */ private _createWrapInjectable() { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -177,4 +198,33 @@ export class SentryNestInstrumentation extends InstrumentationBase { }; }; } + + /** + * Creates a wrapper function for the @Catch decorator. Used to instrument exception filters. + */ + private _createWrapCatch() { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function wrapCatch(original: any) { + return function wrappedCatch(...exceptions: unknown[]) { + return function (target: CatchTarget) { + if (typeof target.prototype.catch === 'function' && !target.__SENTRY_INTERNAL__) { + // patch only once + if (isPatched(target)) { + return original(...exceptions)(target); + } + + target.prototype.catch = new Proxy(target.prototype.catch, { + apply: (originalCatch, thisArgCatch, argsCatch) => { + return startSpan(getMiddlewareSpanOptions(target), () => { + return originalCatch.apply(thisArgCatch, argsCatch); + }); + }, + }); + } + + return original(...exceptions)(target); + }; + }; + }; + } } diff --git a/packages/node/src/integrations/tracing/nest/types.ts b/packages/node/src/integrations/tracing/nest/types.ts index 2cdd1b6aefaf..42aa0b003315 100644 --- a/packages/node/src/integrations/tracing/nest/types.ts +++ b/packages/node/src/integrations/tracing/nest/types.ts @@ -55,3 +55,15 @@ export interface InjectableTarget { intercept?: (context: unknown, next: CallHandler, ...args: any[]) => Observable; }; } + +/** + * Represents a target class in NestJS annotated with @Catch. + */ +export interface CatchTarget { + name: string; + sentryPatched?: boolean; + __SENTRY_INTERNAL__?: boolean; + prototype: { + catch?: (...args: any[]) => any; + }; +} diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index ef57ab0fff3d..98460b575c8d 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -1,3 +1,5 @@ +export { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from './semanticAttributes'; + export { getRequestSpanData } from './utils/getRequestSpanData'; export type { OpenTelemetryClient } from './types'; diff --git a/packages/opentelemetry/src/semanticAttributes.ts b/packages/opentelemetry/src/semanticAttributes.ts index 80a80f87a666..2e14c71bf5e9 100644 --- a/packages/opentelemetry/src/semanticAttributes.ts +++ b/packages/opentelemetry/src/semanticAttributes.ts @@ -1,2 +1,5 @@ /** If this attribute is true, it means that the parent is a remote span. */ export const SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE = 'sentry.parentIsRemote'; + +// These are not standardized yet, but used by the graphql instrumentation +export const SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION = 'sentry.graphql.operation'; diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts index a9d99aa91b8a..6d1c9936899b 100644 --- a/packages/opentelemetry/src/utils/parseSpanDescription.ts +++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts @@ -15,6 +15,7 @@ import type { SpanAttributes, TransactionSource } from '@sentry/types'; import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes'; import type { AbstractSpan } from '../types'; import { getSpanKind } from './getSpanKind'; import { spanHasAttributes, spanHasName } from './spanTypes'; @@ -136,8 +137,16 @@ export function descriptionForHttpMethod( return { op: opParts.join('.'), description: name, source: 'custom' }; } - // Ex. description="GET /api/users". - const description = `${httpMethod} ${urlPath}`; + const graphqlOperationsAttribute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION]; + + // Ex. GET /api/users + const baseDescription = `${httpMethod} ${urlPath}`; + + // When the http span has a graphql operation, append it to the description + // We add these in the graphqlIntegration + const description = graphqlOperationsAttribute + ? `${baseDescription} (${getGraphqlOperationNamesFromAttribute(graphqlOperationsAttribute)})` + : baseDescription; // If `httpPath` is a root path, then we can categorize the transaction source as route. const source: TransactionSource = hasRoute || urlPath === '/' ? 'route' : 'url'; @@ -162,6 +171,22 @@ export function descriptionForHttpMethod( }; } +function getGraphqlOperationNamesFromAttribute(attr: AttributeValue): string { + if (Array.isArray(attr)) { + const sorted = attr.slice().sort(); + + // Up to 5 items, we just add all of them + if (sorted.length <= 5) { + return sorted.join(', '); + } else { + // Else, we add the first 5 and the diff of other operations + return `${sorted.slice(0, 5).join(', ')}, +${sorted.length - 5}`; + } + } + + return `${attr}`; +} + /** Exported for tests only */ export function getSanitizedUrl( attributes: Attributes, diff --git a/packages/opentelemetry/src/utils/spanTypes.ts b/packages/opentelemetry/src/utils/spanTypes.ts index f92d411200a1..39c62219d2ad 100644 --- a/packages/opentelemetry/src/utils/spanTypes.ts +++ b/packages/opentelemetry/src/utils/spanTypes.ts @@ -22,7 +22,7 @@ export function spanHasAttributes( */ export function spanHasKind(span: SpanType): span is SpanType & { kind: SpanKind } { const castSpan = span as ReadableSpan; - return !!castSpan.kind; + return typeof castSpan.kind === 'number'; } /** diff --git a/packages/opentelemetry/test/utils/spanTypes.test.ts b/packages/opentelemetry/test/utils/spanTypes.test.ts index 99152204adfa..af07e5c45af5 100644 --- a/packages/opentelemetry/test/utils/spanTypes.test.ts +++ b/packages/opentelemetry/test/utils/spanTypes.test.ts @@ -24,7 +24,9 @@ describe('spanTypes', () => { it.each([ [{}, false], [{ kind: null }, false], - [{ kind: 'TEST_KIND' }, true], + [{ kind: 0 }, true], + [{ kind: 5 }, true], + [{ kind: 'TEST_KIND' }, false], ])('works with %p', (span, expected) => { const castSpan = span as unknown as Span; const actual = spanHasKind(castSpan); diff --git a/packages/remix/src/utils/web-fetch.ts b/packages/remix/src/utils/web-fetch.ts index 8450a12eb05d..6e188bd9d440 100644 --- a/packages/remix/src/utils/web-fetch.ts +++ b/packages/remix/src/utils/web-fetch.ts @@ -1,4 +1,3 @@ -/* eslint-disable complexity */ // Based on Remix's implementation of Fetch API // https://github.com/remix-run/web-std-io/blob/d2a003fe92096aaf97ab2a618b74875ccaadc280/packages/fetch/ // The MIT License (MIT) @@ -23,10 +22,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import { logger } from '@sentry/utils'; - -import { DEBUG_BUILD } from './debug-build'; -import { getClientIPAddress } from './vendor/getIpAddress'; import type { RemixRequest } from './vendor/types'; /* @@ -124,15 +119,6 @@ export const normalizeRemixRequest = (request: RemixRequest): Record = {}; - - get(key: string): string | null { - return this._headers[key] ?? null; - } - - set(key: string, value: string): void { - this._headers[key] = value; - } -} - -describe('getClientIPAddress', () => { - it.each([ - [ - '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5,2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35', - '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', - ], - [ - '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35', - '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', - ], - [ - '2a01:cb19:8350:ed00:d0dd:INVALID_IP_ADDR:8be5,141.101.69.35,2a01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', - '141.101.69.35', - ], - [ - '2b01:cb19:8350:ed00:d0dd:fa5b:nope:8be5, 2b01:cb19:NOPE:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35 ', - '141.101.69.35', - ], - ['2b01:cb19:8350:ed00:d0 dd:fa5b:de31:8be5, 141.101.69.35', '141.101.69.35'], - ])('should parse the IP from the X-Forwarded-For header %s', (headerValue, expectedIP) => { - const headers = new Headers(); - headers.set('X-Forwarded-For', headerValue); - - const ip = getClientIPAddress(headers as any); - - expect(ip).toEqual(expectedIP); - }); -}); diff --git a/packages/remix/test/utils/normalizeRemixRequest.test.ts b/packages/remix/test/utils/normalizeRemixRequest.test.ts index b627a34e4f12..64de88510014 100644 --- a/packages/remix/test/utils/normalizeRemixRequest.test.ts +++ b/packages/remix/test/utils/normalizeRemixRequest.test.ts @@ -83,7 +83,6 @@ describe('normalizeRemixRequest', () => { hostname: 'example.com', href: 'https://example.com/api/json?id=123', insecureHTTPParser: undefined, - ip: null, method: 'GET', originalUrl: 'https://example.com/api/json?id=123', path: '/api/json?id=123', diff --git a/packages/solidstart/README.md b/packages/solidstart/README.md index 61aa3b2793da..b654b9bdf744 100644 --- a/packages/solidstart/README.md +++ b/packages/solidstart/README.md @@ -161,3 +161,59 @@ render( document.getElementById('root'), ); ``` + +# Sourcemaps and Releases + +To generate and upload source maps of your Solid Start app use our Vite bundler plugin. + +1. Install the Sentry Vite plugin + +```bash +# Using npm +npm install @sentry/vite-plugin --save-dev + +# Using yarn +yarn add @sentry/vite-plugin --dev +``` + +2. Configure the vite plugin + +To upload source maps you have to configure an auth token. Auth tokens can be passed to the plugin explicitly with the +`authToken` option, with a `SENTRY_AUTH_TOKEN` environment variable, or with an `.env.sentry-build-plugin` file in the +working directory when building your project. We recommend you add the auth token to your CI/CD environment as an +environment variable. + +Learn more about configuring the plugin in our +[Sentry Vite Plugin documentation](https://www.npmjs.com/package/@sentry/vite-plugin). + +```bash +// .env.sentry-build-plugin +SENTRY_AUTH_TOKEN= +SENTRY_ORG= +SENTRY_PROJECT= +``` + +3. Finally, add the plugin to your `app.config.ts` file. + +```javascript +import { defineConfig } from '@solidjs/start/config'; +import { sentryVitePlugin } from '@sentry/vite-plugin'; + +export default defineConfig({ + // rest of your config + // ... + + vite: { + build: { + sourcemap: true, + }, + plugins: [ + sentryVitePlugin({ + org: process.env.SENTRY_ORG, + project: process.env.SENTRY_PROJECT, + authToken: process.env.SENTRY_AUTH_TOKEN, + }), + ], + }, +}); +``` diff --git a/packages/solidstart/src/middleware.ts b/packages/solidstart/src/middleware.ts index 0113cce8f988..65287d23fa0b 100644 --- a/packages/solidstart/src/middleware.ts +++ b/packages/solidstart/src/middleware.ts @@ -1,4 +1,4 @@ -import { getTraceData } from '@sentry/core'; +import { getTraceMetaTags } from '@sentry/core'; import { addNonEnumerableProperty } from '@sentry/utils'; import type { ResponseMiddleware } from '@solidjs/start/middleware'; import type { FetchEvent } from '@solidjs/start/server'; @@ -8,19 +8,13 @@ export type ResponseMiddlewareResponse = Parameters[1] & { }; function addMetaTagToHead(html: string): string { - const { 'sentry-trace': sentryTrace, baggage } = getTraceData(); + const metaTags = getTraceMetaTags(); - if (!sentryTrace) { + if (!metaTags) { return html; } - const metaTags = [``]; - - if (baggage) { - metaTags.push(``); - } - - const content = `\n${metaTags.join('\n')}\n`; + const content = `\n${metaTags}\n`; return html.replace('', content); } diff --git a/packages/solidstart/test/middleware.test.ts b/packages/solidstart/test/middleware.test.ts index 888a0fbc702d..c025dc38af97 100644 --- a/packages/solidstart/test/middleware.test.ts +++ b/packages/solidstart/test/middleware.test.ts @@ -5,10 +5,10 @@ import type { ResponseMiddlewareResponse } from '../src/middleware'; describe('middleware', () => { describe('sentryBeforeResponseMiddleware', () => { - vi.spyOn(SentryCore, 'getTraceData').mockReturnValue({ - 'sentry-trace': '123', - baggage: 'abc', - }); + vi.spyOn(SentryCore, 'getTraceMetaTags').mockReturnValue(` + , + + `); const mockFetchEvent = { request: {}, diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 56ddc23e1885..7f4b8d980ad8 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -11,21 +11,15 @@ import { withIsolationScope, } from '@sentry/core'; import { startSpan } from '@sentry/core'; -import { captureException, continueTrace } from '@sentry/node'; +import { continueTrace } from '@sentry/node'; import type { Span } from '@sentry/types'; -import { - dynamicSamplingContextToSentryBaggageHeader, - logger, - objectify, - winterCGRequestToRequestData, -} from '@sentry/utils'; +import { dynamicSamplingContextToSentryBaggageHeader, logger, winterCGRequestToRequestData } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; import { getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry'; import { DEBUG_BUILD } from '../common/debug-build'; -import { isHttpError, isRedirect } from '../common/utils'; -import { flushIfServerless, getTracePropagationData } from './utils'; +import { flushIfServerless, getTracePropagationData, sendErrorToSentry } from './utils'; export type SentryHandleOptions = { /** @@ -62,32 +56,6 @@ export type SentryHandleOptions = { fetchProxyScriptNonce?: string; }; -function sendErrorToSentry(e: unknown): unknown { - // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can - // store a seen flag on it. - const objectifiedErr = objectify(e); - - // similarly to the `load` function, we don't want to capture 4xx errors or redirects - if ( - isRedirect(objectifiedErr) || - (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) - ) { - return objectifiedErr; - } - - captureException(objectifiedErr, { - mechanism: { - type: 'sveltekit', - handled: false, - data: { - function: 'handle', - }, - }, - }); - - return objectifiedErr; -} - /** * Exported only for testing */ @@ -225,7 +193,7 @@ async function instrumentHandle( ); return resolveResult; } catch (e: unknown) { - sendErrorToSentry(e); + sendErrorToSentry(e, 'handle'); throw e; } finally { await flushIfServerless(); diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index a74e5bb89dc0..32dd6627d7a6 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -52,6 +52,7 @@ export { getSpanDescendants, getSpanStatusFromHttpCode, getTraceData, + getTraceMetaTags, graphqlIntegration, hapiIntegration, httpIntegration, @@ -128,6 +129,7 @@ export { init } from './sdk'; export { handleErrorWithSentry } from './handleError'; export { wrapLoadWithSentry, wrapServerLoadWithSentry } from './load'; export { sentryHandle } from './handle'; +export { wrapServerRouteWithSentry } from './serverRoute'; /** * Tracks the Svelte component's initialization and mounting operation as well as diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index fe61ed0913bd..82a8c548c6ec 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -1,49 +1,13 @@ -import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - captureException, - startSpan, -} from '@sentry/node'; -import { addNonEnumerableProperty, objectify } from '@sentry/utils'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/node'; +import { addNonEnumerableProperty } from '@sentry/utils'; import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { isHttpError, isRedirect } from '../common/utils'; -import { flushIfServerless } from './utils'; +import { flushIfServerless, sendErrorToSentry } from './utils'; type PatchedLoadEvent = LoadEvent & SentryWrappedFlag; type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag; -function sendErrorToSentry(e: unknown): unknown { - // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can - // store a seen flag on it. - const objectifiedErr = objectify(e); - - // The error() helper is commonly used to throw errors in load functions: https://kit.svelte.dev/docs/modules#sveltejs-kit-error - // If we detect a thrown error that is an instance of HttpError, we don't want to capture 4xx errors as they - // could be noisy. - // Also the `redirect(...)` helper is used to redirect users from one page to another. We don't want to capture thrown - // `Redirect`s as they're not errors but expected behaviour - if ( - isRedirect(objectifiedErr) || - (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) - ) { - return objectifiedErr; - } - - captureException(objectifiedErr, { - mechanism: { - type: 'sveltekit', - handled: false, - data: { - function: 'load', - }, - }, - }); - - return objectifiedErr; -} - /** * @inheritdoc */ @@ -81,7 +45,7 @@ export function wrapLoadWithSentry any>(origLoad: T) () => wrappingTarget.apply(thisArg, args), ); } catch (e) { - sendErrorToSentry(e); + sendErrorToSentry(e, 'load'); throw e; } finally { await flushIfServerless(); @@ -146,7 +110,7 @@ export function wrapServerLoadWithSentry any>(origSe () => wrappingTarget.apply(thisArg, args), ); } catch (e: unknown) { - sendErrorToSentry(e); + sendErrorToSentry(e, 'load'); throw e; } finally { await flushIfServerless(); diff --git a/packages/sveltekit/src/server/serverRoute.ts b/packages/sveltekit/src/server/serverRoute.ts new file mode 100644 index 000000000000..a5f13f9a73ca --- /dev/null +++ b/packages/sveltekit/src/server/serverRoute.ts @@ -0,0 +1,67 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/node'; +import { addNonEnumerableProperty } from '@sentry/utils'; +import type { RequestEvent } from '@sveltejs/kit'; +import { flushIfServerless, sendErrorToSentry } from './utils'; + +type PatchedServerRouteEvent = RequestEvent & { __sentry_wrapped__?: boolean }; + +/** + * Wraps a server route handler for API or server routes registered in `+server.(js|js)` files. + * + * This function will automatically capture any errors that occur during the execution of the route handler + * and it will start a span for the duration of your route handler. + * + * @example + * ```js + * import { wrapServerRouteWithSentry } from '@sentry/sveltekit'; + * + * const get = async event => { + * return new Response(JSON.stringify({ message: 'hello world' })); + * } + * + * export const GET = wrapServerRouteWithSentry(get); + * ``` + * + * @param originalRouteHandler your server route handler + * @param httpMethod the HTTP method of your route handler + * + * @returns a wrapped version of your server route handler + */ +export function wrapServerRouteWithSentry( + originalRouteHandler: (request: RequestEvent) => Promise, +): (requestEvent: RequestEvent) => Promise { + return new Proxy(originalRouteHandler, { + apply: async (wrappingTarget, thisArg, args) => { + const event = args[0] as PatchedServerRouteEvent; + + if (event.__sentry_wrapped__) { + return wrappingTarget.apply(thisArg, args); + } + + const routeId = event.route && event.route.id; + const httpMethod = event.request.method; + + addNonEnumerableProperty(event as unknown as Record, '__sentry_wrapped__', true); + + try { + return await startSpan( + { + name: `${httpMethod} ${routeId || 'Server Route'}`, + op: `function.sveltekit.server.${httpMethod.toLowerCase()}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.sveltekit', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, + onlyIfParent: true, + }, + () => wrappingTarget.apply(thisArg, args), + ); + } catch (e) { + sendErrorToSentry(e, 'serverRoute'); + throw e; + } finally { + await flushIfServerless(); + } + }, + }); +} diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index 0db8ee893783..8d7f2c649331 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,8 +1,9 @@ -import { flush } from '@sentry/node'; -import { logger } from '@sentry/utils'; +import { captureException, flush } from '@sentry/node'; +import { logger, objectify } from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; import { DEBUG_BUILD } from '../common/debug-build'; +import { isHttpError, isRedirect } from '../common/utils'; /** * Takes a request event and extracts traceparent and DSC data @@ -31,3 +32,41 @@ export async function flushIfServerless(): Promise { } } } + +/** + * Extracts a server-side sveltekit error, filters a couple of known errors we don't want to capture + * and captures the error via `captureException`. + * + * @param e error + * + * @returns an objectified version of @param e + */ +export function sendErrorToSentry(e: unknown, handlerFn: 'handle' | 'load' | 'serverRoute'): object { + // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can + // store a seen flag on it. + const objectifiedErr = objectify(e); + + // The error() helper is commonly used to throw errors in load functions: https://kit.svelte.dev/docs/modules#sveltejs-kit-error + // If we detect a thrown error that is an instance of HttpError, we don't want to capture 4xx errors as they + // could be noisy. + // Also the `redirect(...)` helper is used to redirect users from one page to another. We don't want to capture thrown + // `Redirect`s as they're not errors but expected behaviour + if ( + isRedirect(objectifiedErr) || + (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) + ) { + return objectifiedErr; + } + + captureException(objectifiedErr, { + mechanism: { + type: 'sveltekit', + handled: false, + data: { + function: handlerFn, + }, + }, + }); + + return objectifiedErr; +} diff --git a/packages/sveltekit/test/server/serverRoute.test.ts b/packages/sveltekit/test/server/serverRoute.test.ts new file mode 100644 index 000000000000..de99db5a548e --- /dev/null +++ b/packages/sveltekit/test/server/serverRoute.test.ts @@ -0,0 +1,133 @@ +import * as SentryNode from '@sentry/node'; +import type { NumericRange } from '@sveltejs/kit'; +import { type RequestEvent, error, redirect } from '@sveltejs/kit'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + wrapServerRouteWithSentry, +} from '../../src/server'; + +describe('wrapServerRouteWithSentry', () => { + const originalRouteHandler = vi.fn(); + + const getRequestEventMock = () => + ({ + request: { + method: 'GET', + }, + route: { + id: '/api/users/:id', + }, + }) as RequestEvent; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('wraps a server route span around the original server route handler', () => { + const startSpanSpy = vi.spyOn(SentryNode, 'startSpan'); + + it('assigns the route id as name if available', () => { + const wrappedRouteHandler = wrapServerRouteWithSentry(originalRouteHandler); + + wrappedRouteHandler(getRequestEventMock() as RequestEvent); + + expect(startSpanSpy).toHaveBeenCalledWith( + { + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.sveltekit', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, + name: 'GET /api/users/:id', + onlyIfParent: true, + op: 'function.sveltekit.server.get', + }, + expect.any(Function), + ); + + expect(originalRouteHandler).toHaveBeenCalledTimes(1); + }); + + it('falls back to a generic name if route id is not available', () => { + const wrappedRouteHandler = wrapServerRouteWithSentry(originalRouteHandler); + + wrappedRouteHandler({ ...getRequestEventMock(), route: undefined } as unknown as RequestEvent); + + expect(startSpanSpy).toHaveBeenCalledWith( + { + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.sveltekit', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, + name: 'GET Server Route', + onlyIfParent: true, + op: 'function.sveltekit.server.get', + }, + expect.any(Function), + ); + + expect(originalRouteHandler).toHaveBeenCalledTimes(1); + }); + }); + + const captureExceptionSpy = vi.spyOn(SentryNode, 'captureException'); + describe('captures server route errors', () => { + it('captures and rethrows normal server route error', async () => { + const error = new Error('Server Route Error'); + + const wrappedRouteHandler = wrapServerRouteWithSentry(() => { + throw error; + }); + + await expect(async () => { + await wrappedRouteHandler(getRequestEventMock() as RequestEvent); + }).rejects.toThrowError('Server Route Error'); + + expect(captureExceptionSpy).toHaveBeenCalledWith(error, { + mechanism: { type: 'sveltekit', handled: false, data: { function: 'serverRoute' } }, + }); + }); + + it.each([500, 501, 599])('captures and rethrows %s error() calls', async status => { + const wrappedRouteHandler = wrapServerRouteWithSentry(() => { + error(status as NumericRange<400, 599>, `error(${status}) error`); + }); + + await expect(async () => { + await wrappedRouteHandler(getRequestEventMock() as RequestEvent); + }).rejects.toThrow(); + + expect(captureExceptionSpy).toHaveBeenCalledWith( + { body: { message: `error(${status}) error` }, status }, + { + mechanism: { type: 'sveltekit', handled: false, data: { function: 'serverRoute' } }, + }, + ); + }); + + it.each([400, 401, 499])("doesn't capture but rethrows %s error() calls", async status => { + const wrappedRouteHandler = wrapServerRouteWithSentry(() => { + error(status as NumericRange<400, 599>, `error(${status}) error`); + }); + + await expect(async () => { + await wrappedRouteHandler(getRequestEventMock() as RequestEvent); + }).rejects.toThrow(); + + expect(captureExceptionSpy).not.toHaveBeenCalled(); + }); + + it.each([300, 301, 308])("doesn't capture redirect(%s) calls", async status => { + const wrappedRouteHandler = wrapServerRouteWithSentry(() => { + redirect(status as NumericRange<300, 308>, '/redirect'); + }); + + await expect(async () => { + await wrappedRouteHandler(getRequestEventMock() as RequestEvent); + }).rejects.toThrow(); + + expect(captureExceptionSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index 85133521ef76..a4eae547edb1 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -13,6 +13,7 @@ import { isPlainObject, isString } from './is'; import { logger } from './logger'; import { normalize } from './normalize'; import { stripUrlQueryAndFragment } from './url'; +import { getClientIPAddress, ipHeaderNames } from './vendor/getIpAddress'; const DEFAULT_INCLUDES = { ip: false, @@ -98,7 +99,6 @@ export function extractPathForTransaction( return [name, source]; } -/** JSDoc */ function extractTransaction(req: PolymorphicRequest, type: boolean | TransactionNamingScheme): string { switch (type) { case 'path': { @@ -116,7 +116,6 @@ function extractTransaction(req: PolymorphicRequest, type: boolean | Transaction } } -/** JSDoc */ function extractUserData( user: { [key: string]: unknown; @@ -146,17 +145,16 @@ function extractUserData( */ export function extractRequestData( req: PolymorphicRequest, - options?: { + options: { include?: string[]; - }, + } = {}, ): ExtractedNodeRequestData { - const { include = DEFAULT_REQUEST_INCLUDES } = options || {}; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const requestData: { [key: string]: any } = {}; + const { include = DEFAULT_REQUEST_INCLUDES } = options; + const requestData: { [key: string]: unknown } = {}; // headers: // node, express, koa, nextjs: req.headers - const headers = (req.headers || {}) as { + const headers = (req.headers || {}) as typeof req.headers & { host?: string; cookie?: string; }; @@ -191,6 +189,14 @@ export function extractRequestData( delete (requestData.headers as { cookie?: string }).cookie; } + // Remove IP headers in case IP data should not be included in the event + if (!include.includes('ip')) { + ipHeaderNames.forEach(ipHeaderName => { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete (requestData.headers as Record)[ipHeaderName]; + }); + } + break; } case 'method': { @@ -264,9 +270,12 @@ export function addRequestDataToEvent( }; if (include.request) { - const extractedRequestData = Array.isArray(include.request) - ? extractRequestData(req, { include: include.request }) - : extractRequestData(req); + const includeRequest = Array.isArray(include.request) ? [...include.request] : [...DEFAULT_REQUEST_INCLUDES]; + if (include.ip) { + includeRequest.push('ip'); + } + + const extractedRequestData = extractRequestData(req, { include: includeRequest }); event.request = { ...event.request, @@ -288,8 +297,9 @@ export function addRequestDataToEvent( // client ip: // node, nextjs: req.socket.remoteAddress // express, koa: req.ip + // It may also be sent by proxies as specified in X-Forwarded-For or similar headers if (include.ip) { - const ip = req.ip || (req.socket && req.socket.remoteAddress); + const ip = (req.headers && getClientIPAddress(req.headers)) || req.ip || (req.socket && req.socket.remoteAddress); if (ip) { event.user = { ...event.user, diff --git a/packages/remix/src/utils/vendor/getIpAddress.ts b/packages/utils/src/vendor/getIpAddress.ts similarity index 50% rename from packages/remix/src/utils/vendor/getIpAddress.ts rename to packages/utils/src/vendor/getIpAddress.ts index d63e31779aac..8b96fe2146af 100644 --- a/packages/remix/src/utils/vendor/getIpAddress.ts +++ b/packages/utils/src/vendor/getIpAddress.ts @@ -23,7 +23,21 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import { isIP } from 'net'; +// The headers to check, in priority order +export const ipHeaderNames = [ + 'X-Client-IP', + 'X-Forwarded-For', + 'Fly-Client-IP', + 'CF-Connecting-IP', + 'Fastly-Client-Ip', + 'True-Client-Ip', + 'X-Real-IP', + 'X-Cluster-Client-IP', + 'X-Forwarded', + 'Forwarded-For', + 'Forwarded', + 'X-Vercel-Forwarded-For', +]; /** * Get the IP address of the client sending a request. @@ -31,50 +45,24 @@ import { isIP } from 'net'; * It receives a Request headers object and use it to get the * IP address from one of the following headers in order. * - * - X-Client-IP - * - X-Forwarded-For - * - Fly-Client-IP - * - CF-Connecting-IP - * - Fastly-Client-Ip - * - True-Client-Ip - * - X-Real-IP - * - X-Cluster-Client-IP - * - X-Forwarded - * - Forwarded-For - * - Forwarded - * * If the IP address is valid, it will be returned. Otherwise, null will be * returned. * * If the header values contains more than one IP address, the first valid one * will be returned. */ -export function getClientIPAddress(headers: Headers): string | null { - // The headers to check, in priority order - const headerNames = [ - 'X-Client-IP', - 'X-Forwarded-For', - 'Fly-Client-IP', - 'CF-Connecting-IP', - 'Fastly-Client-Ip', - 'True-Client-Ip', - 'X-Real-IP', - 'X-Cluster-Client-IP', - 'X-Forwarded', - 'Forwarded-For', - 'Forwarded', - ]; - +export function getClientIPAddress(headers: { [key: string]: string | string[] | undefined }): string | null { // This will end up being Array because of the various possible values a header // can take - const headerValues = headerNames.map((headerName: string) => { - const value = headers.get(headerName); + const headerValues = ipHeaderNames.map((headerName: string) => { + const rawValue = headers[headerName]; + const value = Array.isArray(rawValue) ? rawValue.join(';') : rawValue; if (headerName === 'Forwarded') { return parseForwardedHeader(value); } - return value?.split(',').map((v: string) => v.trim()); + return value && value.split(',').map((v: string) => v.trim()); }); // Flatten the array and filter out any falsy entries @@ -92,7 +80,7 @@ export function getClientIPAddress(headers: Headers): string | null { return ipAddress || null; } -function parseForwardedHeader(value: string | null): string | null { +function parseForwardedHeader(value: string | null | undefined): string | null { if (!value) { return null; } @@ -105,3 +93,31 @@ function parseForwardedHeader(value: string | null): string | null { return null; } + +// +/** + * Custom method instead of importing this from `net` package, as this only exists in node + * Accepts: + * 127.0.0.1 + * 192.168.1.1 + * 192.168.1.255 + * 255.255.255.255 + * 10.1.1.1 + * 0.0.0.0 + * 2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5 + * + * Rejects: + * 1.1.1.01 + * 30.168.1.255.1 + * 127.1 + * 192.168.1.256 + * -1.2.3.4 + * 1.1.1.1. + * 3...3 + * 192.168.1.099 + */ +function isIP(str: string): boolean { + const regex = + /(?:^(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}$)|(?:^(?:(?:[a-fA-F\d]{1,4}:){7}(?:[a-fA-F\d]{1,4}|:)|(?:[a-fA-F\d]{1,4}:){6}(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|:[a-fA-F\d]{1,4}|:)|(?:[a-fA-F\d]{1,4}:){5}(?::(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,2}|:)|(?:[a-fA-F\d]{1,4}:){4}(?:(?::[a-fA-F\d]{1,4}){0,1}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,3}|:)|(?:[a-fA-F\d]{1,4}:){3}(?:(?::[a-fA-F\d]{1,4}){0,2}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,4}|:)|(?:[a-fA-F\d]{1,4}:){2}(?:(?::[a-fA-F\d]{1,4}){0,3}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,5}|:)|(?:[a-fA-F\d]{1,4}:){1}(?:(?::[a-fA-F\d]{1,4}){0,4}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,6}|:)|(?::(?:(?::[a-fA-F\d]{1,4}){0,5}:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:\\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3}|(?::[a-fA-F\d]{1,4}){1,7}|:)))(?:%[0-9a-zA-Z]{1,})?$)/; + return regex.test(str); +} diff --git a/packages/utils/test/requestdata.test.ts b/packages/utils/test/requestdata.test.ts index 7e44f703c62a..570f80647b6b 100644 --- a/packages/utils/test/requestdata.test.ts +++ b/packages/utils/test/requestdata.test.ts @@ -1,6 +1,7 @@ import type * as net from 'net'; import type { Event, PolymorphicRequest, TransactionSource, User } from '@sentry/types'; import { addRequestDataToEvent, extractPathForTransaction, extractRequestData } from '@sentry/utils'; +import { getClientIPAddress } from '../src/vendor/getIpAddress'; describe('addRequestDataToEvent', () => { let mockEvent: Event; @@ -107,6 +108,227 @@ describe('addRequestDataToEvent', () => { expect(parsedRequest.user!.ip_address).toEqual('321'); }); + + test.each([ + 'X-Client-IP', + 'X-Forwarded-For', + 'Fly-Client-IP', + 'CF-Connecting-IP', + 'Fastly-Client-Ip', + 'True-Client-Ip', + 'X-Real-IP', + 'X-Cluster-Client-IP', + 'X-Forwarded', + 'Forwarded-For', + 'X-Vercel-Forwarded-For', + ])('can be extracted from %s header', headerName => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + [headerName]: '123.5.6.1', + }, + }; + + const optionsWithIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithIP); + + expect(parsedRequest.user!.ip_address).toEqual('123.5.6.1'); + }); + + it('can be extracted from Forwarded header', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + Forwarded: 'by=111;for=123.5.6.1;for=123.5.6.2;', + }, + }; + + const optionsWithIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithIP); + + expect(parsedRequest.user!.ip_address).toEqual('123.5.6.1'); + }); + + test('it ignores invalid IP in header', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + 'X-Client-IP': 'invalid', + }, + }; + + const optionsWithIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithIP); + + expect(parsedRequest.user!.ip_address).toEqual(undefined); + }); + + test('IP from header takes presedence over socket', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + 'X-Client-IP': '123.5.6.1', + }, + socket: { + remoteAddress: '321', + } as net.Socket, + }; + + const optionsWithIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithIP); + + expect(parsedRequest.user!.ip_address).toEqual('123.5.6.1'); + }); + + test('IP from header takes presedence over req.ip', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + 'X-Client-IP': '123.5.6.1', + }, + ip: '123', + }; + + const optionsWithIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithIP); + + expect(parsedRequest.user!.ip_address).toEqual('123.5.6.1'); + }); + + test('does not add IP if ip=false', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + 'X-Client-IP': '123.5.6.1', + }, + ip: '123', + }; + + const optionsWithoutIP = { + include: { + ip: false, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithoutIP); + + expect(parsedRequest.user!.ip_address).toEqual(undefined); + }); + + test('does not add IP by default', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + 'X-Client-IP': '123.5.6.1', + }, + ip: '123', + }; + + const optionsWithoutIP = {}; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithoutIP); + + expect(parsedRequest.user!.ip_address).toEqual(undefined); + }); + + test('removes IP headers if `ip` is not set in the options', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }, + }; + + const optionsWithoutIP = { + include: {}, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithoutIP); + + expect(parsedRequest.request?.headers).toEqual({ otherHeader: 'hello' }); + }); + + test('keeps IP headers if `ip=true`', () => { + const reqWithIPInHeader = { + ...mockReq, + headers: { + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }, + }; + + const optionsWithoutIP = { + include: { + ip: true, + }, + }; + + const parsedRequest: Event = addRequestDataToEvent(mockEvent, reqWithIPInHeader, optionsWithoutIP); + + expect(parsedRequest.request?.headers).toEqual({ + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }); + }); }); describe('request properties', () => { @@ -269,6 +491,70 @@ describe('extractRequestData', () => { cookies: { foo: 'bar' }, }); }); + + it('removes IP-related headers from requestdata.headers, if `ip` is not set in the options', () => { + const mockReq = { + headers: { + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }, + }; + const options = { include: ['headers'] }; + + expect(extractRequestData(mockReq, options)).toStrictEqual({ + headers: { otherHeader: 'hello' }, + }); + }); + + it('keeps IP-related headers from requestdata.headers, if `ip` is enabled in options', () => { + const mockReq = { + headers: { + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }, + }; + const options = { include: ['headers', 'ip'] }; + + expect(extractRequestData(mockReq, options)).toStrictEqual({ + headers: { + otherHeader: 'hello', + 'X-Client-IP': '123', + 'X-Forwarded-For': '123', + 'Fly-Client-IP': '123', + 'CF-Connecting-IP': '123', + 'Fastly-Client-Ip': '123', + 'True-Client-Ip': '123', + 'X-Real-IP': '123', + 'X-Cluster-Client-IP': '123', + 'X-Forwarded': '123', + 'Forwarded-For': '123', + Forwarded: '123', + 'X-Vercel-Forwarded-For': '123', + }, + }); + }); }); describe('cookies', () => { @@ -502,3 +788,33 @@ describe('extractPathForTransaction', () => { expect(source).toEqual('route'); }); }); + +describe('getClientIPAddress', () => { + it.each([ + [ + '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5,2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35', + '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', + ], + [ + '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35', + '2b01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', + ], + [ + '2a01:cb19:8350:ed00:d0dd:INVALID_IP_ADDR:8be5,141.101.69.35,2a01:cb19:8350:ed00:d0dd:fa5b:de31:8be5', + '141.101.69.35', + ], + [ + '2b01:cb19:8350:ed00:d0dd:fa5b:nope:8be5, 2b01:cb19:NOPE:ed00:d0dd:fa5b:de31:8be5, 141.101.69.35 ', + '141.101.69.35', + ], + ['2b01:cb19:8350:ed00:d0 dd:fa5b:de31:8be5, 141.101.69.35', '141.101.69.35'], + ])('should parse the IP from the X-Forwarded-For header %s', (headerValue, expectedIP) => { + const headers = { + 'X-Forwarded-For': headerValue, + }; + + const ip = getClientIPAddress(headers); + + expect(ip).toEqual(expectedIP); + }); +}); diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index a96fc15e35d2..8be93345fa3d 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -56,6 +56,7 @@ export { getActiveSpan, getRootSpan, getTraceData, + getTraceMetaTags, startSpan, startInactiveSpan, startSpanManual,