diff --git a/CHANGELOG.md b/CHANGELOG.md index abcab70edde8..890baeb4161f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,8 @@ # Changelog - -> [!IMPORTANT] -> If you are upgrading to the `9.x` versions of the SDK from `8.x` or below, make sure you follow our -> [migration guide](https://docs.sentry.io/platforms/javascript/migration/) first. - +## 9.25.1 -## Unreleased - -- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +- fix(otel): Don't ignore child spans after the root is sent ([#16416](https://github.com/getsentry/sentry-javascript/pull/16416)) ## 9.25.0 diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts index 8448829443d6..c675d003853a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/playwright.config.ts @@ -5,15 +5,9 @@ if (!testEnv) { throw new Error('No test env defined'); } -const config = getPlaywrightConfig( - { - startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030', - port: 3030, - }, - { - // This comes with the risk of tests leaking into each other but the tests run quite slow so we should parallelize - workers: '100%', - }, -); +const config = getPlaywrightConfig({ + startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030', + port: 3030, +}); export default config; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.mjs index 8448829443d6..c675d003853a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.mjs +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/playwright.config.mjs @@ -5,15 +5,9 @@ if (!testEnv) { throw new Error('No test env defined'); } -const config = getPlaywrightConfig( - { - startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030', - port: 3030, - }, - { - // This comes with the risk of tests leaking into each other but the tests run quite slow so we should parallelize - workers: '100%', - }, -); +const config = getPlaywrightConfig({ + startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030', + port: 3030, +}); export default config; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/.gitignore b/dev-packages/e2e-tests/test-applications/nextjs-orpc/.gitignore new file mode 100644 index 000000000000..e799cc33c4e7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/.gitignore @@ -0,0 +1,45 @@ +# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. + +# dependencies +/node_modules +/.pnp +.pnp.js + +# testing +/coverage + +# next.js +/.next/ +/out/ + +# production +/build + +# misc +.DS_Store +*.pem + +# debug +npm-debug.log* +yarn-debug.log* +yarn-error.log* +.pnpm-debug.log* + +# local env files +.env*.local + +# vercel +.vercel + +# typescript +*.tsbuildinfo +next-env.d.ts + +!*.d.ts + +# Sentry +.sentryclirc + +.vscode + +test-results diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/.npmrc b/dev-packages/e2e-tests/test-applications/nextjs-orpc/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/next-env.d.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/next-env.d.ts new file mode 100644 index 000000000000..40c3d68096c2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/next-env.d.ts @@ -0,0 +1,5 @@ +/// +/// + +// NOTE: This file should not be edited +// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information. diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/next.config.js b/dev-packages/e2e-tests/test-applications/nextjs-orpc/next.config.js new file mode 100644 index 000000000000..ade813b1cde3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/next.config.js @@ -0,0 +1,8 @@ +/** @type {import("next").NextConfig} */ +const config = {}; + +import { withSentryConfig } from '@sentry/nextjs'; + +export default withSentryConfig(config, { + disableLogger: true, +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/package.json b/dev-packages/e2e-tests/test-applications/nextjs-orpc/package.json new file mode 100644 index 000000000000..7fcad2ab0e64 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/package.json @@ -0,0 +1,48 @@ +{ + "name": "next-orpc", + "version": "0.1.0", + "private": true, + "type": "module", + "scripts": { + "build": "next build", + "dev": "next dev -p 3030", + "start": "next start -p 3030", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test:prod": "TEST_ENV=production playwright test", + "test:dev": "TEST_ENV=development playwright test", + "test:build": "pnpm install && pnpm build", + "test:build-canary": "pnpm install && pnpm add next@canary && pnpm add react@beta && pnpm add react-dom@beta && pnpm build", + "test:build-latest": "pnpm install && pnpm add next@rc && pnpm add react@beta && pnpm add react-dom@beta && pnpm build", + "test:assert": "pnpm test:prod && pnpm test:dev" + }, + "dependencies": { + "@sentry/nextjs": "latest || *", + "@orpc/server": "latest", + "@orpc/client": "latest", + "next": "14.2.29", + "react": "18.3.1", + "react-dom": "18.3.1", + "server-only": "^0.0.1" + }, + "devDependencies": { + "@playwright/test": "~1.50.0", + "@sentry-internal/test-utils": "link:../../../test-utils", + "@types/eslint": "^8.56.10", + "@types/node": "^18.19.1", + "@types/react": "18.3.1", + "@types/react-dom": "^18.3.0", + "@typescript-eslint/eslint-plugin": "^8.1.0", + "@typescript-eslint/parser": "^8.1.0", + "eslint": "^8.57.0", + "eslint-config-next": "^14.2.4", + "postcss": "^8.4.39", + "prettier": "^3.3.2", + "typescript": "^5.5.3" + }, + "volta": { + "extends": "../../package.json" + }, + "sentryTest": { + "optional": true + } +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/nextjs-orpc/playwright.config.mjs new file mode 100644 index 000000000000..c675d003853a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/playwright.config.mjs @@ -0,0 +1,13 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; +const testEnv = process.env.TEST_ENV; + +if (!testEnv) { + throw new Error('No test env defined'); +} + +const config = getPlaywrightConfig({ + startCommand: testEnv === 'development' ? 'pnpm next dev -p 3030' : 'pnpm next start -p 3030', + port: 3030, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.edge.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.edge.config.ts new file mode 100644 index 000000000000..4f1cb3e93e9c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.edge.config.ts @@ -0,0 +1,13 @@ +// This file configures the initialization of Sentry for edge features (middleware, edge routes, and so on). +// The config you add here will be used whenever one of the edge features is loaded. +// Note that this config is unrelated to the Vercel Edge Runtime and is also required when running locally. +// https://docs.sentry.io/platforms/javascript/guides/nextjs/ + +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.server.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.server.config.ts new file mode 100644 index 000000000000..ad780407a5b7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/sentry.server.config.ts @@ -0,0 +1,8 @@ +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1.0, +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client-error/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client-error/page.tsx new file mode 100644 index 000000000000..ff25388b59c4 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client-error/page.tsx @@ -0,0 +1,9 @@ +import { FindPlanet } from '~/components/FindPlanet'; + +export default async function ClientErrorPage() { + return ( +
+ +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client/page.tsx new file mode 100644 index 000000000000..8c1d5a7607f6 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/client/page.tsx @@ -0,0 +1,9 @@ +import { FindPlanet } from '~/components/FindPlanet'; + +export default async function ClientPage() { + return ( +
+ +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/global-error.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/global-error.tsx new file mode 100644 index 000000000000..912ad3606a61 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/global-error.tsx @@ -0,0 +1,27 @@ +'use client'; + +import * as Sentry from '@sentry/nextjs'; +import NextError from 'next/error'; +import { useEffect } from 'react'; + +export default function GlobalError({ + error, +}: { + error: Error & { digest?: string }; +}) { + useEffect(() => { + Sentry.captureException(error); + }, [error]); + + return ( + + + {/* `NextError` is the default Next.js error page component. Its type + definition requires a `statusCode` prop. However, since the App Router + does not expose status codes for errors, we simply pass 0 to render a + generic error message. */} + + + + ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/layout.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/layout.tsx new file mode 100644 index 000000000000..97fff2740ace --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/layout.tsx @@ -0,0 +1,20 @@ +import '../orpc/server'; +import * as Sentry from '@sentry/nextjs'; + +import { type Metadata } from 'next'; + +export function generateMetadata(): Metadata { + return { + other: { + ...Sentry.getTraceData(), + }, + }; +} + +export default function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) { + return ( + + {children} + + ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/page.tsx new file mode 100644 index 000000000000..d26349dcf47e --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/page.tsx @@ -0,0 +1,19 @@ +import Link from 'next/link'; +import { client } from '~/orpc/client'; + +export default async function Home() { + const planets = await client.planet.list({ limit: 10 }); + + return ( +
+

Planets

+
    + {planets.map(planet => ( +
  • {planet.name}
    • + ))} +
+ Client + Error +
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/rpc/[[...rest]]/route.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/rpc/[[...rest]]/route.ts new file mode 100644 index 000000000000..e8602b1bd55b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/app/rpc/[[...rest]]/route.ts @@ -0,0 +1,22 @@ +import { RPCHandler } from '@orpc/server/fetch'; +import { router } from '~/orpc/router'; + +const handler = new RPCHandler(router); + +async function handleRequest(request: Request) { + const { response } = await handler.handle(request, { + prefix: '/rpc', + context: { + headers: Object.fromEntries(request.headers.entries()), + }, + }); + + return response ?? new Response('Not found', { status: 404 }); +} + +export const HEAD = handleRequest; +export const GET = handleRequest; +export const POST = handleRequest; +export const PUT = handleRequest; +export const PATCH = handleRequest; +export const DELETE = handleRequest; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/components/FindPlanet.tsx b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/components/FindPlanet.tsx new file mode 100644 index 000000000000..eb559e74dadf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/components/FindPlanet.tsx @@ -0,0 +1,42 @@ +'use client'; + +import { client } from '~/orpc/client'; +import { useEffect, useState } from 'react'; + +type Planet = { + id: number; + name: string; + description?: string; +}; + +export function FindPlanet({ withError = false }: { withError?: boolean }) { + const [planet, setPlanet] = useState(); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + + useEffect(() => { + async function fetchPlanet() { + const data = withError ? await client.planet.findWithError({ id: 1 }) : await client.planet.find({ id: 1 }); + setPlanet(data); + } + + setLoading(true); + fetchPlanet(); + setLoading(false); + }, []); + + if (loading) { + return
Loading planet...
; + } + + if (error) { + return
Error: {error}
; + } + + return ( +
+

Planet

+
{planet?.name}
+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation-client.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation-client.ts new file mode 100644 index 000000000000..d85e1cb17cbf --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation-client.ts @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/nextjs'; + +Sentry.init({ + dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN, + tunnel: `http://localhost:3031/`, // proxy server + tracesSampleRate: 1, + debug: false, +}); + +export const onRouterTransitionStart = Sentry.captureRouterTransitionStart; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation.ts new file mode 100644 index 000000000000..8aff09f087d0 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/instrumentation.ts @@ -0,0 +1,13 @@ +import * as Sentry from '@sentry/nextjs'; + +export async function register() { + if (process.env.NEXT_RUNTIME === 'nodejs') { + await import('../sentry.server.config'); + } + + if (process.env.NEXT_RUNTIME === 'edge') { + await import('../sentry.edge.config'); + } +} + +export const onRequestError = Sentry.captureRequestError; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/client.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/client.ts new file mode 100644 index 000000000000..2c6b4f7a3d1f --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/client.ts @@ -0,0 +1,20 @@ +import { createORPCClient } from '@orpc/client'; +import { RPCLink } from '@orpc/client/fetch'; +import { RouterClient } from '@orpc/server'; +import type { headers } from 'next/headers'; +import { router } from './router'; + +declare global { + var $headers: typeof headers; +} + +const link = new RPCLink({ + url: new URL('/rpc', typeof window !== 'undefined' ? window.location.href : 'http://localhost:3030'), + headers: async () => { + return globalThis.$headers + ? Object.fromEntries(await globalThis.$headers()) // ssr + : {}; // browser + }, +}); + +export const client: RouterClient = createORPCClient(link); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/router.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/router.ts new file mode 100644 index 000000000000..7081e3ed2ad2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/router.ts @@ -0,0 +1,45 @@ +import { ORPCError, os } from '@orpc/server'; +import { z } from 'zod'; +import { sentryTracingMiddleware } from './sentry-middleware'; + +const PlanetSchema = z.object({ + id: z.number().int().min(1), + name: z.string(), + description: z.string().optional(), +}); + +export const base = os.use(sentryTracingMiddleware); + +export const listPlanet = base + .input( + z.object({ + limit: z.number().int().min(1).max(100).optional(), + cursor: z.number().int().min(0).default(0), + }), + ) + .handler(async ({ input }) => { + return [ + { id: 1, name: 'name' }, + { id: 2, name: 'another name' }, + ]; + }); + +export const findPlanet = base.input(PlanetSchema.pick({ id: true })).handler(async ({ input }) => { + await new Promise(resolve => setTimeout(resolve, 500)); + return { id: 1, name: 'name' }; +}); + +export const throwingFindPlanet = base.input(PlanetSchema.pick({ id: true })).handler(async ({ input }) => { + throw new ORPCError('OH_OH', { + message: 'You are hitting an error', + data: { some: 'data' }, + }); +}); + +export const router = { + planet: { + list: listPlanet, + find: findPlanet, + findWithError: throwingFindPlanet, + }, +}; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/sentry-middleware.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/sentry-middleware.ts new file mode 100644 index 000000000000..fdfcc9b7cd98 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/sentry-middleware.ts @@ -0,0 +1,16 @@ +import { os } from '@orpc/server'; +import * as Sentry from '@sentry/nextjs'; + +export const sentryTracingMiddleware = os.$context<{}>().middleware(async ({ context, next }) => { + return Sentry.startSpan( + { name: 'ORPC Middleware', op: 'middleware.orpc', attributes: { 'sentry.origin': 'auto' } }, + async () => { + try { + return await next(); + } catch (error) { + Sentry.captureException(error); + throw error; + } + }, + ); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/server.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/server.ts new file mode 100644 index 000000000000..3d53175dafe1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/src/orpc/server.ts @@ -0,0 +1,5 @@ +'server only'; + +import { headers } from 'next/headers'; + +globalThis.$headers = headers; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/nextjs-orpc/start-event-proxy.mjs new file mode 100644 index 000000000000..472e6a6098ce --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'nextjs-orpc', +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-error.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-error.test.ts new file mode 100644 index 000000000000..8a9f371972c0 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-error.test.ts @@ -0,0 +1,22 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('should capture orpc error', async ({ page }) => { + const orpcErrorPromise = waitForError('nextjs-orpc', errorEvent => { + return errorEvent.exception?.values?.[0]?.value === 'You are hitting an error'; + }); + + await page.goto('/'); + await page.waitForTimeout(500); + await page.getByRole('link', { name: 'Error' }).click(); + + const orpcError = await orpcErrorPromise; + + expect(orpcError.exception).toMatchObject({ + values: [ + expect.objectContaining({ + value: 'You are hitting an error', + }), + ], + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-tracing.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-tracing.test.ts new file mode 100644 index 000000000000..f2863b4e5095 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tests/orpc-tracing.test.ts @@ -0,0 +1,111 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + +test('should trace orpc server component', async ({ page }) => { + const pageloadPromise = waitForTransaction('nextjs-orpc', transactionEvent => { + return transactionEvent.transaction === '/'; + }); + + const orpcTxPromise = waitForTransaction('nextjs-orpc', transactionEvent => { + return transactionEvent.transaction === 'POST /rpc/[[...rest]]'; + }); + + await page.goto('/'); + const pageloadTx = await pageloadPromise; + const orpcTx = await orpcTxPromise; + + expect(pageloadTx.contexts?.trace).toMatchObject({ + parent_span_id: expect.any(String), + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.pageload.nextjs.app_router_instrumentation', + 'sentry.op': 'pageload', + 'sentry.source': 'url', + }, + op: 'pageload', + origin: 'auto.pageload.nextjs.app_router_instrumentation', + }); + + expect(orpcTx.contexts?.trace).toMatchObject({ + parent_span_id: expect.any(String), + span_id: expect.any(String), + trace_id: pageloadTx.contexts?.trace?.trace_id, + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto', + 'sentry.source': 'route', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'next.span_name': 'POST /rpc/[[...rest]]/route', + 'next.span_type': 'BaseServer.handleRequest', + 'http.method': 'POST', + 'http.target': '/rpc/planet/list', + 'next.rsc': false, + 'http.route': '/rpc/[[...rest]]/route', + 'next.route': '/rpc/[[...rest]]', + 'http.status_code': 200, + }, + op: 'http.server', + origin: 'auto', + }); + + expect(orpcTx.spans?.map(span => span.description)).toContain('ORPC Middleware'); +}); + +test('should trace orpc client component', async ({ page }) => { + const navigationPromise = waitForTransaction('nextjs-orpc', transactionEvent => { + return transactionEvent.transaction === '/client'; + }); + + const orpcTxPromise = waitForTransaction('nextjs-orpc', transactionEvent => { + return ( + transactionEvent.transaction === 'POST /rpc/[[...rest]]' && + transactionEvent.contexts?.trace?.data?.['http.target'] === '/rpc/planet/find' + ); + }); + + await page.goto('/'); + await page.waitForTimeout(500); + await page.getByRole('link', { name: 'Client' }).click(); + const navigationTx = await navigationPromise; + const orpcTx = await orpcTxPromise; + + expect(navigationTx.contexts?.trace).toMatchObject({ + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.op': 'navigation', + 'sentry.origin': 'auto.navigation.nextjs.app_router_instrumentation', + 'sentry.source': 'url', + 'sentry.previous_trace': expect.any(String), + }, + op: 'navigation', + origin: 'auto.navigation.nextjs.app_router_instrumentation', + }); + + expect(orpcTx?.contexts?.trace).toMatchObject({ + parent_span_id: expect.any(String), + span_id: expect.any(String), + trace_id: navigationTx?.contexts?.trace?.trace_id, + data: { + 'sentry.op': 'http.server', + 'sentry.origin': 'auto', + 'sentry.source': 'route', + 'otel.kind': 'SERVER', + 'http.response.status_code': 200, + 'next.span_name': 'POST /rpc/[[...rest]]/route', + 'next.span_type': 'BaseServer.handleRequest', + 'http.method': 'POST', + 'http.target': '/rpc/planet/find', + 'next.rsc': false, + 'http.route': '/rpc/[[...rest]]/route', + 'next.route': '/rpc/[[...rest]]', + 'http.status_code': 200, + }, + op: 'http.server', + origin: 'auto', + }); + + expect(orpcTx.spans?.map(span => span.description)).toContain('ORPC Middleware'); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-orpc/tsconfig.json b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tsconfig.json new file mode 100644 index 000000000000..905062ded60c --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-orpc/tsconfig.json @@ -0,0 +1,42 @@ +{ + "compilerOptions": { + /* Base Options: */ + "esModuleInterop": true, + "skipLibCheck": true, + "target": "es2022", + "allowJs": true, + "resolveJsonModule": true, + "moduleDetection": "force", + "isolatedModules": true, + + /* Strictness */ + "strict": true, + "noUncheckedIndexedAccess": true, + "checkJs": true, + + /* Bundled projects */ + "lib": ["dom", "dom.iterable", "ES2022"], + "noEmit": true, + "module": "ESNext", + "moduleResolution": "Bundler", + "jsx": "preserve", + "plugins": [{ "name": "next" }], + "incremental": true, + + /* Path Aliases */ + "baseUrl": ".", + "paths": { + "~/*": ["./src/*"] + } + }, + "include": [ + ".eslintrc.cjs", + "next-env.d.ts", + "**/*.ts", + "**/*.tsx", + "**/*.cjs", + "**/*.js", + ".next/types/**/*.ts" + ], + "exclude": ["node_modules"] +} diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json index 4c6f9f281406..94da7baed3ab 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json @@ -21,7 +21,7 @@ "@trpc/react-query": "^11.0.0-rc.446", "@trpc/server": "^11.0.0-rc.446", "geist": "^1.3.0", - "next": "14.2.10", + "next": "14.2.29", "react": "18.3.1", "react-dom": "18.3.1", "server-only": "^0.0.1", diff --git a/dev-packages/opentelemetry-v2-tests/test/integration/transactions.test.ts b/dev-packages/opentelemetry-v2-tests/test/integration/transactions.test.ts index fc2702b4e390..3bdf6c113555 100644 --- a/dev-packages/opentelemetry-v2-tests/test/integration/transactions.test.ts +++ b/dev-packages/opentelemetry-v2-tests/test/integration/transactions.test.ts @@ -1,7 +1,6 @@ import type { SpanContext } from '@opentelemetry/api'; import { context, ROOT_CONTEXT, trace, TraceFlags } from '@opentelemetry/api'; import { TraceState } from '@opentelemetry/core'; -import type { SpanProcessor } from '@opentelemetry/sdk-trace-base'; import type { Event, TransactionEvent } from '@sentry/core'; import { addBreadcrumb, @@ -15,7 +14,6 @@ import { } from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { SENTRY_TRACE_STATE_DSC } from '../../../../packages/opentelemetry/src/constants'; -import { SentrySpanProcessor } from '../../../../packages/opentelemetry/src/spanProcessor'; import { startInactiveSpan, startSpan } from '../../../../packages/opentelemetry/src/trace'; import { makeTraceState } from '../../../../packages/opentelemetry/src/utils/makeTraceState'; import { cleanupOtel, getProvider, getSpanProcessor, mockSdkInit } from '../helpers/mockSdkInit'; @@ -550,7 +548,60 @@ describe('Integration | Transactions', () => { expect(finishedSpans.length).toBe(0); }); - it('discards child spans that are finished after their parent span', async () => { +it('collects child spans that are finished within 5 minutes their parent span has been sent', async () => { + const timeout = 5 * 60 * 1000; + const now = Date.now(); + vi.useFakeTimers(); + vi.setSystemTime(now); + + const logs: unknown[] = []; + vi.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg)); + + const transactions: Event[] = []; + + mockSdkInit({ + tracesSampleRate: 1, + beforeSendTransaction: event => { + transactions.push(event); + return null; + }, + }); + + const provider = getProvider(); + const spanProcessor = getSpanProcessor(); + + const exporter = spanProcessor ? spanProcessor['_exporter'] : undefined; + + if (!exporter) { + throw new Error('No exporter found, aborting test...'); + } + + startSpanManual({ name: 'test name' }, async span => { + const subSpan = startInactiveSpan({ name: 'inner span 1' }); + subSpan.end(); + + const subSpan2 = startInactiveSpan({ name: 'inner span 2' }); + + span.end(); + + setTimeout(() => { + subSpan2.end(); + }, timeout - 2); + }); + + vi.advanceTimersByTime(timeout - 1); + + expect(transactions).toHaveLength(2); + expect(transactions[0]?.spans).toHaveLength(1); + + const finishedSpans: any = exporter['_finishedSpanBuckets'].flatMap(bucket => + bucket ? Array.from(bucket.spans) : [], + ); + expect(finishedSpans.length).toBe(0); +}); + + it('discards child spans that are finished after 5 minutes their parent span has been sent', async () => { + const timeout = 5 * 60 * 1000; const now = Date.now(); vi.useFakeTimers(); vi.setSystemTime(now); @@ -587,10 +638,10 @@ describe('Integration | Transactions', () => { setTimeout(() => { subSpan2.end(); - }, 1); + }, timeout + 1); }); - vi.advanceTimersByTime(2); + vi.advanceTimersByTime(timeout + 2); expect(transactions).toHaveLength(1); expect(transactions[0]?.spans).toHaveLength(1); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index f9c403a47dfc..fee780def708 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -71,6 +71,9 @@ export class SentrySpanExporter { private _finishedSpanBucketSize: number; private _spansToBucketEntry: WeakMap; private _lastCleanupTimestampInS: number; + // Essentially a a set of span ids that are already sent. The values are expiration + // times in this cache so we don't hold onto them indefinitely. + private _sentSpans: Map; public constructor(options?: { /** Lower bound of time in seconds until spans that are buffered but have not been sent as part of a transaction get cleared from memory. */ @@ -80,6 +83,48 @@ export class SentrySpanExporter { this._finishedSpanBuckets = new Array(this._finishedSpanBucketSize).fill(undefined); this._lastCleanupTimestampInS = Math.floor(Date.now() / 1000); this._spansToBucketEntry = new WeakMap(); + this._sentSpans = new Map(); + } + + /** + * Check if a span with the given ID has already been sent using the `_sentSpans` as a cache. + * Purges "expired" spans from the cache upon checking. + * @param spanId The span id to check. + * @returns Whether the span is already sent in the past X seconds. + */ + public isSpanAlreadySent(spanId: string): boolean { + const expirationTime = this._sentSpans.get(spanId); + if (expirationTime) { + if (Date.now() >= expirationTime) { + this._sentSpans.delete(spanId); // Remove expired span + } else { + return true; + } + } + return false; + } + + /** Remove "expired" span id entries from the _sentSpans cache. */ + public flushSentSpanCache(): void { + const currentTimestamp = Date.now(); + // Note, it is safe to delete items from the map as we go: https://stackoverflow.com/a/35943995/90297 + for (const [spanId, expirationTime] of this._sentSpans.entries()) { + if (expirationTime <= currentTimestamp) { + this._sentSpans.delete(spanId); + } + } + } + + /** Check if a node is a completed root node or a node whose parent has already been sent */ + public nodeIsCompletedRootNode(node: SpanNode): node is SpanNodeCompleted { + return !!node.span && (!node.parentNode || this.isSpanAlreadySent(node.parentNode.id)); + } + + /** Get all completed root nodes from a list of nodes */ + public getCompletedRootNodes(nodes: SpanNode[]): SpanNodeCompleted[] { + // TODO: We should be able to remove the explicit `node is SpanNodeCompleted` type guard + // once we stop supporting TS < 5.5 + return nodes.filter((node): node is SpanNodeCompleted => this.nodeIsCompletedRootNode(node)); } /** Export a single span. */ @@ -113,7 +158,8 @@ export class SentrySpanExporter { this._spansToBucketEntry.set(span, currentBucket); // If the span doesn't have a local parent ID (it's a root span), we're gonna flush all the ended spans - if (!getLocalParentId(span)) { + const localParentId = getLocalParentId(span); + if (!localParentId || this.isSpanAlreadySent(localParentId)) { this._clearTimeout(); // If we got a parent span, we try to send the span tree @@ -128,30 +174,29 @@ export class SentrySpanExporter { public flush(): void { this._clearTimeout(); - const finishedSpans: ReadableSpan[] = []; - this._finishedSpanBuckets.forEach(bucket => { - if (bucket) { - finishedSpans.push(...bucket.spans); - } - }); + const finishedSpans: ReadableSpan[] = this._finishedSpanBuckets.flatMap(bucket => + bucket ? Array.from(bucket.spans) : [], + ); - const sentSpans = maybeSend(finishedSpans); + this.flushSentSpanCache(); + const sentSpans = this._maybeSend(finishedSpans); + for (const span of finishedSpans) { + this._sentSpans.set(span.spanContext().spanId, Date.now() + DEFAULT_TIMEOUT * 1000); + } const sentSpanCount = sentSpans.size; - const remainingOpenSpanCount = finishedSpans.length - sentSpanCount; - DEBUG_BUILD && logger.log( `SpanExporter exported ${sentSpanCount} spans, ${remainingOpenSpanCount} spans are waiting for their parent spans to finish`, ); - sentSpans.forEach(span => { + for (const span of sentSpans) { const bucketEntry = this._spansToBucketEntry.get(span); if (bucketEntry) { bucketEntry.spans.delete(span); } - }); + } } /** Clear the exporter. */ @@ -167,59 +212,51 @@ export class SentrySpanExporter { this._flushTimeout = undefined; } } -} - -/** - * Send the given spans, but only if they are part of a finished transaction. - * - * Returns the sent spans. - * Spans remain unsent when their parent span is not yet finished. - * This will happen regularly, as child spans are generally finished before their parents. - * But it _could_ also happen because, for whatever reason, a parent span was lost. - * In this case, we'll eventually need to clean this up. - */ -function maybeSend(spans: ReadableSpan[]): Set { - const grouped = groupSpansWithParents(spans); - const sentSpans = new Set(); - const rootNodes = getCompletedRootNodes(grouped); + /** + * Send the given spans, but only if they are part of a finished transaction. + * + * Returns the sent spans. + * Spans remain unsent when their parent span is not yet finished. + * This will happen regularly, as child spans are generally finished before their parents. + * But it _could_ also happen because, for whatever reason, a parent span was lost. + * In this case, we'll eventually need to clean this up. + */ + private _maybeSend(spans: ReadableSpan[]): Set { + const grouped = groupSpansWithParents(spans); + const sentSpans = new Set(); - rootNodes.forEach(root => { - const span = root.span; - sentSpans.add(span); - const transactionEvent = createTransactionForOtelSpan(span); + const rootNodes = this.getCompletedRootNodes(grouped); - // We'll recursively add all the child spans to this array - const spans = transactionEvent.spans || []; + for (const root of rootNodes) { + const span = root.span; + sentSpans.add(span); + const transactionEvent = createTransactionForOtelSpan(span); - root.children.forEach(child => { - createAndFinishSpanForOtelSpan(child, spans, sentSpans); - }); + // We'll recursively add all the child spans to this array + const spans = transactionEvent.spans || []; - // spans.sort() mutates the array, but we do not use this anymore after this point - // so we can safely mutate it here - transactionEvent.spans = - spans.length > MAX_SPAN_COUNT - ? spans.sort((a, b) => a.start_timestamp - b.start_timestamp).slice(0, MAX_SPAN_COUNT) - : spans; + for (const child of root.children) { + createAndFinishSpanForOtelSpan(child, spans, sentSpans); + } - const measurements = timedEventsToMeasurements(span.events); - if (measurements) { - transactionEvent.measurements = measurements; - } + // spans.sort() mutates the array, but we do not use this anymore after this point + // so we can safely mutate it here + transactionEvent.spans = + spans.length > MAX_SPAN_COUNT + ? spans.sort((a, b) => a.start_timestamp - b.start_timestamp).slice(0, MAX_SPAN_COUNT) + : spans; - captureEvent(transactionEvent); - }); - - return sentSpans; -} + const measurements = timedEventsToMeasurements(span.events); + if (measurements) { + transactionEvent.measurements = measurements; + } -function nodeIsCompletedRootNode(node: SpanNode): node is SpanNodeCompleted { - return !!node.span && !node.parentNode; -} + captureEvent(transactionEvent); + } -function getCompletedRootNodes(nodes: SpanNode[]): SpanNodeCompleted[] { - return nodes.filter(nodeIsCompletedRootNode); + return sentSpans; + } } function parseSpan(span: ReadableSpan): { op?: string; origin?: SpanOrigin; source?: TransactionSource } { diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 97d075ef5071..165871df69ca 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -561,7 +561,8 @@ describe('Integration | Transactions', () => { expect(finishedSpans.length).toBe(0); }); - it('discards child spans that are finished after their parent span', async () => { + it('collects child spans that are finished within 5 minutes their parent span has been sent', async () => { + const timeout = 5 * 60 * 1000; const now = Date.now(); vi.useFakeTimers(); vi.setSystemTime(now); @@ -603,10 +604,67 @@ describe('Integration | Transactions', () => { setTimeout(() => { subSpan2.end(); - }, 1); + }, timeout - 2); }); - vi.advanceTimersByTime(2); + vi.advanceTimersByTime(timeout - 1); + + expect(transactions).toHaveLength(2); + expect(transactions[0]?.spans).toHaveLength(1); + + const finishedSpans: any = exporter['_finishedSpanBuckets'].flatMap(bucket => + bucket ? Array.from(bucket.spans) : [], + ); + expect(finishedSpans.length).toBe(0); + }); + + it('discards child spans that are finished after 5 minutes their parent span has been sent', async () => { + const timeout = 5 * 60 * 1000; + const now = Date.now(); + vi.useFakeTimers(); + vi.setSystemTime(now); + + const logs: unknown[] = []; + vi.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg)); + + const transactions: Event[] = []; + + mockSdkInit({ + tracesSampleRate: 1, + beforeSendTransaction: event => { + transactions.push(event); + return null; + }, + }); + + const provider = getProvider(); + const multiSpanProcessor = provider?.activeSpanProcessor as + | (SpanProcessor & { _spanProcessors?: SpanProcessor[] }) + | undefined; + const spanProcessor = multiSpanProcessor?.['_spanProcessors']?.find( + spanProcessor => spanProcessor instanceof SentrySpanProcessor, + ) as SentrySpanProcessor | undefined; + + const exporter = spanProcessor ? spanProcessor['_exporter'] : undefined; + + if (!exporter) { + throw new Error('No exporter found, aborting test...'); + } + + startSpanManual({ name: 'test name' }, async span => { + const subSpan = startInactiveSpan({ name: 'inner span 1' }); + subSpan.end(); + + const subSpan2 = startInactiveSpan({ name: 'inner span 2' }); + + span.end(); + + setTimeout(() => { + subSpan2.end(); + }, timeout + 1); + }); + + vi.advanceTimersByTime(timeout + 2); expect(transactions).toHaveLength(1); expect(transactions[0]?.spans).toHaveLength(1);