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/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/.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/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/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/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/test.ts b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts index 22bef9d2b1b9..c42269dd8504 100644 --- a/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/meta-tags/test.ts @@ -16,8 +16,7 @@ describe('getTraceMetaTags', () => { baggage: 'sentry-environment=production', }); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore + // @ts-ignore - response is defined, types just don't reflect it const html = response?.response as unknown as string; expect(html).toMatch(//); 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/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/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/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/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); + }); +});