From f1ed0e9cd7bed420987bf6f1126a19620a058ea9 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 14 Nov 2023 14:47:46 +0100 Subject: [PATCH 01/16] ref: Deprecate `extractTraceParentData` from `@sentry/core` & downstream packages (#9158) Instead, users should import this from `@sentry/utils`. This was also re-exported from all downstream packages (e.g. `@sentry/browser`, `@sentry/node`). I don't think this is something users need to import from there...? If you need this, installing `@sentry/utils` is probably fine. --- MIGRATION.md | 6 +++++ packages/astro/src/index.server.ts | 1 + packages/browser/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/core/src/tracing/index.ts | 1 + packages/core/src/tracing/utils.ts | 27 ++++++++++--------- packages/deno/src/index.ts | 2 ++ packages/node-experimental/src/index.ts | 1 + packages/node/src/index.ts | 1 + packages/remix/src/index.server.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + .../tracing-internal/src/exports/index.ts | 1 + packages/tracing/src/index.ts | 2 +- packages/vercel-edge/src/index.ts | 1 + 14 files changed, 33 insertions(+), 14 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 7f626efe7e23..33a6c0f6ea09 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -8,6 +8,12 @@ npx @sentry/migr8@latest This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes! +## Deprecate `extractTraceParentData` export from `@sentry/core` & downstream packages + +Instead, import this directly from `@sentry/utils`. + +Generally, in most cases you should probably use `continueTrace` instead, which abstracts this away from you and handles scope propagation for you. + ## Deprecate `timestampWithMs` export - #7878 The `timestampWithMs` util is deprecated in favor of using `timestampInSeconds`. diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 5ec649c81584..9dce9689a7eb 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -16,6 +16,7 @@ export { withMonitor, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, getActiveTransaction, getHubFromCarrier, diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 60a2bef7a2f9..75286d3da7df 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -42,6 +42,7 @@ export type { RequestInstrumentationOptions } from '@sentry-internal/tracing'; export { addTracingExtensions, setMeasurement, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, getActiveTransaction, spanStatusfromHttpCode, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 1f10a0d265e8..9750a2ba7815 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -33,6 +33,7 @@ export { close, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, flush, getActiveTransaction, diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index f0356a528c2d..fcb208a797aa 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -3,6 +3,7 @@ export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction'; export type { BeforeFinishCallback } from './idletransaction'; export { Span, spanStatusfromHttpCode } from './span'; export { Transaction } from './transaction'; +// eslint-disable-next-line deprecation/deprecation export { extractTraceparentData, getActiveTransaction } from './utils'; // eslint-disable-next-line deprecation/deprecation export { SpanStatus } from './spanstatus'; diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index 624fff153270..f1b4c0f1ae06 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -1,8 +1,19 @@ import type { Transaction } from '@sentry/types'; +import { extractTraceparentData as _extractTraceparentData } from '@sentry/utils'; import type { Hub } from '../hub'; import { getCurrentHub } from '../hub'; +/** Grabs active transaction off scope, if any */ +export function getActiveTransaction(maybeHub?: Hub): T | undefined { + const hub = maybeHub || getCurrentHub(); + const scope = hub.getScope(); + return scope.getTransaction() as T | undefined; +} + +// so it can be used in manual instrumentation without necessitating a hard dependency on @sentry/utils +export { stripUrlQueryAndFragment } from '@sentry/utils'; + /** * The `extractTraceparentData` function and `TRACEPARENT_REGEXP` constant used * to be declared in this file. It was later moved into `@sentry/utils` as part of a @@ -11,18 +22,8 @@ import { getCurrentHub } from '../hub'; * * These exports are kept here for backwards compatability's sake. * - * TODO(v7): Reorganize these exports - * * See https://github.com/getsentry/sentry-javascript/issues/4642 for more details. + * + * @deprecated Import this function from `@sentry/utils` instead */ -export { TRACEPARENT_REGEXP, extractTraceparentData } from '@sentry/utils'; - -/** Grabs active transaction off scope, if any */ -export function getActiveTransaction(maybeHub?: Hub): T | undefined { - const hub = maybeHub || getCurrentHub(); - const scope = hub.getScope(); - return scope.getTransaction() as T | undefined; -} - -// so it can be used in manual instrumentation without necessitating a hard dependency on @sentry/utils -export { stripUrlQueryAndFragment } from '@sentry/utils'; +export const extractTraceparentData = _extractTraceparentData; diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 52e881528866..3301e50db6c2 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -31,7 +31,9 @@ export { close, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, + continueTrace, flush, getActiveTransaction, getHubFromCarrier, diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index 798a474702c1..740c52b92bb8 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -32,6 +32,7 @@ export { close, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, flush, getActiveTransaction, diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index ff49247ce337..3c4a28489aa8 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -33,6 +33,7 @@ export { close, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, flush, getActiveTransaction, diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 7686a08782c8..80c6602e4938 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -19,6 +19,7 @@ export { captureMessage, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, getActiveTransaction, getHubFromCarrier, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index e3a6f0fc2be7..b595324c2e4c 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -14,6 +14,7 @@ export { withMonitor, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, getActiveTransaction, getHubFromCarrier, diff --git a/packages/tracing-internal/src/exports/index.ts b/packages/tracing-internal/src/exports/index.ts index 010e35d4ec6a..7b7f81a9caa1 100644 --- a/packages/tracing-internal/src/exports/index.ts +++ b/packages/tracing-internal/src/exports/index.ts @@ -1,4 +1,5 @@ export { + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, getActiveTransaction, hasTracingEnabled, diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index ff52e3d31466..680a36f7a68f 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -20,7 +20,6 @@ import { Postgres, Prisma, Span as SpanT, - // eslint-disable-next-line deprecation/deprecation SpanStatus as SpanStatusT, spanStatusfromHttpCode as spanStatusfromHttpCodeT, startIdleTransaction as startIdleTransactionT, @@ -70,6 +69,7 @@ export const getActiveTransaction = getActiveTransactionT; * * `extractTraceparentData` can be imported from `@sentry/node`, `@sentry/browser`, or your framework SDK */ +// eslint-disable-next-line deprecation/deprecation export const extractTraceparentData = extractTraceparentDataT; /** diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index 47e1b9fd7924..8b8247639ae5 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -32,6 +32,7 @@ export { close, configureScope, createTransport, + // eslint-disable-next-line deprecation/deprecation extractTraceparentData, flush, getActiveTransaction, From 11e0c2dcaf1cb780180a6dacc642078bda4b14b4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 14 Nov 2023 15:33:26 +0100 Subject: [PATCH 02/16] feat(nextjs): Add instrumentation utility for server actions (#9553) --- .../nextjs-app-dir/app/server-action/page.tsx | 24 +++ .../nextjs-app-dir/next.config.js | 1 + .../nextjs-app-dir/tests/transactions.test.ts | 25 ++++ packages/nextjs/src/common/index.ts | 2 + .../common/withServerActionInstrumentation.ts | 139 ++++++++++++++++++ 5 files changed, 191 insertions(+) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/app/server-action/page.tsx create mode 100644 packages/nextjs/src/common/withServerActionInstrumentation.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-action/page.tsx b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-action/page.tsx new file mode 100644 index 000000000000..4137fafd9c3c --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/app/server-action/page.tsx @@ -0,0 +1,24 @@ +import * as Sentry from '@sentry/nextjs'; +import { headers } from 'next/headers'; + +export default function ServerComponent() { + async function myServerAction(formData: FormData) { + 'use server'; + return await Sentry.withServerActionInstrumentation( + 'myServerAction', + { formData, headers: headers(), recordResponse: true }, + async () => { + await fetch('http://example.com/'); + return { city: 'Vienna' }; + }, + ); + } + + return ( + // @ts-ignore +
+ + +
+ ); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/next.config.js b/packages/e2e-tests/test-applications/nextjs-app-dir/next.config.js index b4110295ace3..2c0d391e87dc 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/next.config.js +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/next.config.js @@ -8,6 +8,7 @@ const { withSentryConfig } = require('@sentry/nextjs'); const moduleExports = { experimental: { appDir: true, + serverActions: true, }, }; diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index b57538e3b90c..597b9ad66072 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -2,6 +2,8 @@ import { test, expect } from '@playwright/test'; import { waitForTransaction } from '../event-proxy-server'; import axios, { AxiosError } from 'axios'; +const packageJson = require('../package.json'); + const authToken = process.env.E2E_TEST_AUTH_TOKEN; const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG; const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT; @@ -112,3 +114,26 @@ if (process.env.TEST_ENV === 'production') { expect((await serverComponentTransactionPromise).contexts?.trace?.status).toBe('not_found'); }); } + +test('Should send a transaction for instrumented server actions', async ({ page }) => { + const nextjsVersion = packageJson.dependencies.next; + const nextjsMajor = Number(nextjsVersion.split('.')[0]); + test.skip(!isNaN(nextjsMajor) && nextjsMajor < 14, 'only applies to nextjs apps >= version 14'); + + const serverComponentTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'serverAction/myServerAction'; + }); + + await page.goto('/server-action'); + await page.getByText('Run Action').click(); + + expect(await serverComponentTransactionPromise).toBeDefined(); + expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_form_data']).toEqual( + expect.objectContaining({ 'some-text-value': 'some-default-value' }), + ); + expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_result']).toEqual({ + city: 'Vienna', + }); + + expect(Object.keys((await serverComponentTransactionPromise).request?.headers || {}).length).toBeGreaterThan(0); +}); diff --git a/packages/nextjs/src/common/index.ts b/packages/nextjs/src/common/index.ts index 2f166b3e4b59..063c11bff62a 100644 --- a/packages/nextjs/src/common/index.ts +++ b/packages/nextjs/src/common/index.ts @@ -43,3 +43,5 @@ export { wrapApiHandlerWithSentryVercelCrons } from './wrapApiHandlerWithSentryV export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; export { wrapPageComponentWithSentry } from './wrapPageComponentWithSentry'; + +export { withServerActionInstrumentation } from './withServerActionInstrumentation'; diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts new file mode 100644 index 000000000000..1ec28f372cab --- /dev/null +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -0,0 +1,139 @@ +import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core'; +import { addExceptionMechanism, logger, tracingContextFromHeaders } from '@sentry/utils'; + +import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; + +interface Options { + formData?: FormData; + // TODO: Whenever we decide to drop support for Next.js <= 12 we can automatically pick up the headers becauase "next/headers" will be resolvable. + headers?: Headers; + recordResponse?: boolean; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function withServerActionInstrumentation any>( + serverActionName: string, + callback: A, +): Promise>; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function withServerActionInstrumentation any>( + serverActionName: string, + options: Options, + callback: A, +): Promise>; + +/** + * Wraps a Next.js Server Action implementation with Sentry Error and Performance instrumentation. + */ +export function withServerActionInstrumentation unknown>( + ...args: [string, Options, A] | [string, A] +): Promise> { + if (typeof args[1] === 'function') { + const [serverActionName, callback] = args; + return withServerActionInstrumentationImplementation(serverActionName, {}, callback); + } else { + const [serverActionName, options, callback] = args; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return withServerActionInstrumentationImplementation(serverActionName, options, callback!); + } +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +async function withServerActionInstrumentationImplementation any>( + serverActionName: string, + options: Options, + callback: A, +): Promise> { + addTracingExtensions(); + return runWithAsyncContext(async () => { + const hub = getCurrentHub(); + const sendDefaultPii = hub.getClient()?.getOptions().sendDefaultPii; + + let sentryTraceHeader; + let baggageHeader; + const fullHeadersObject: Record = {}; + try { + sentryTraceHeader = options.headers?.get('sentry-trace') ?? undefined; + baggageHeader = options.headers?.get('baggage'); + options.headers?.forEach((value, key) => { + fullHeadersObject[key] = value; + }); + } catch (e) { + __DEBUG_BUILD__ && + logger.warn( + "Sentry wasn't able to extract the tracing headers for a server action. Will not trace this request.", + ); + } + + const currentScope = hub.getScope(); + const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders( + sentryTraceHeader, + baggageHeader, + ); + currentScope.setPropagationContext(propagationContext); + + let res; + try { + res = await trace( + { + op: 'function.server_action', + name: `serverAction/${serverActionName}`, + status: 'ok', + ...traceparentData, + metadata: { + source: 'route', + dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, + request: { + headers: fullHeadersObject, + }, + }, + }, + async span => { + const result = await callback(); + + if (options.recordResponse !== undefined ? options.recordResponse : sendDefaultPii) { + span?.setData('server_action_result', result); + } + + if (options.formData) { + const formDataObject: Record = {}; + options.formData.forEach((value, key) => { + if (typeof value === 'string') { + formDataObject[key] = value; + } else { + formDataObject[key] = '[non-string value]'; + } + }); + span?.setData('server_action_form_data', formDataObject); + } + + return result; + }, + error => { + captureException(error, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + handled: false, + }); + return event; + }); + + return scope; + }); + }, + ); + } finally { + if (!platformSupportsStreaming()) { + // Lambdas require manual flushing to prevent execution freeze before the event is sent + await flush(1000); + } + + if (process.env.NEXT_RUNTIME === 'edge') { + void flush(); + } + } + + return res; + }); +} From db127a35b4bed6419b28a06bc08eb57657db0fde Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Tue, 14 Nov 2023 12:09:41 -0500 Subject: [PATCH 03/16] chore(feedback): Readme note beta and ea org (#9558) Co-authored-by: Catherine Lee <55311782+c298lee@users.noreply.github.com> Co-authored-by: Billy Vong --- packages/feedback/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/feedback/README.md b/packages/feedback/README.md index 382b31139dd7..b1228eb5f0d7 100644 --- a/packages/feedback/README.md +++ b/packages/feedback/README.md @@ -6,9 +6,11 @@ # Sentry Integration for Feedback -This SDK is **considered experimental and in an alpha state**. It may experience breaking changes, and may be discontinued at any time. Please reach out on +This SDK is **considered experimental and in a beta state**. It may experience breaking changes, and may be discontinued at any time. Please reach out on [GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback/concerns. +To view Feedback in Sentry, your [Sentry organization must be an early adopter](https://docs.sentry.io/product/accounts/early-adopter-features/). + ## Pre-requisites `@sentry-internal/feedback` currently can only be used by browsers with [Shadow DOM](https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_shadow_DOM) support. From 04a41ee962747a6824dd4e69a2a88af018392c88 Mon Sep 17 00:00:00 2001 From: mdtro Date: Tue, 14 Nov 2023 11:09:46 -0600 Subject: [PATCH 04/16] codeql: use extended query pack --- .github/workflows/codeql-analysis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index a6ca6542ec28..a74adca29afd 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -32,7 +32,7 @@ concurrency: jobs: analyze: name: Analyze - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest strategy: fail-fast: false @@ -51,6 +51,7 @@ jobs: uses: github/codeql-action/init@v2 with: config-file: ./.github/codeql/codeql-config.yml + queries: security-extended languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. # By default, queries listed here will override any specified in a config file. From 6d424571e3cd5e99991b711f4e23d773e321e294 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 14 Nov 2023 15:23:25 -0330 Subject: [PATCH 05/16] docs(feedback): Example docs on `sendFeedback` (#9560) --- packages/feedback/README.md | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/feedback/README.md b/packages/feedback/README.md index b1228eb5f0d7..53a4578d40b3 100644 --- a/packages/feedback/README.md +++ b/packages/feedback/README.md @@ -212,7 +212,7 @@ import {BrowserClient, getCurrentHub} from '@sentry/react'; import {Feedback} from '@sentry-internal/feedback'; function MyFeedbackButton() { - const client = hub && getCurrentHub().getClient(); + const client = getCurrentHub().getClient(); const feedback = client?.getIntegration(Feedback); // Don't render custom feedback button if Feedback integration not installed @@ -230,7 +230,21 @@ function MyFeedbackButton() { ### Bring Your Own Widget -You can also bring your own widget and UI and simply pass a feedback object to the `sendFeedback()` function. +You can also bring your own widget and UI and simply pass a feedback object to the `sendFeedback()` function. The `sendFeedback` function accepts two parameters: +* a feedback object with a required `message` property, and additionally, optional `name` and `email` properties +* an options object + +```javascript +sendFeedback({ + name: 'Jane Doe', // optional + email: 'email@example.org', // optional + message: 'This is an example feedback', // required +}, { + includeReplay: true, // optional +}) +``` + +Here is a simple example ```html
@@ -238,14 +252,18 @@ You can also bring your own widget and UI and simply pass a feedback object to t