From e21d3181f4ba13cf2d40c7d307047423a083fdac Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 13 Mar 2024 10:25:21 +0100 Subject: [PATCH 1/6] feat(sveltekit): Switch to Otel-based `@sentry/node` package --- .../scripts/consistentExports.ts | 2 +- packages/sveltekit/package.json | 2 +- packages/sveltekit/src/index.types.ts | 10 ++++++---- packages/sveltekit/src/server/handle.ts | 2 +- packages/sveltekit/src/server/handleError.ts | 2 +- packages/sveltekit/src/server/index.ts | 16 +++------------- packages/sveltekit/src/server/load.ts | 2 +- packages/sveltekit/src/server/sdk.ts | 6 +++--- packages/sveltekit/src/server/utils.ts | 2 +- packages/sveltekit/src/vite/sourceMaps.ts | 2 +- packages/sveltekit/test/server/handle.test.ts | 4 ++-- .../sveltekit/test/server/handleError.test.ts | 2 +- packages/sveltekit/test/server/load.test.ts | 7 ++++--- packages/sveltekit/test/server/sdk.test.ts | 7 ++++--- 14 files changed, 30 insertions(+), 36 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts index c238cf326d68..8c3f3ee0242d 100644 --- a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts +++ b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts @@ -98,7 +98,7 @@ const DEPENDENTS: Dependent[] = [ }, { package: '@sentry/sveltekit', - compareWith: nodeExperimentalExports, + compareWith: nodeExports, exports: Object.keys(SentrySvelteKit), }, ]; diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index e3698b691205..db781682df07 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -39,7 +39,7 @@ "dependencies": { "@sentry-internal/tracing": "8.0.0-alpha.2", "@sentry/core": "8.0.0-alpha.2", - "@sentry/node-experimental": "8.0.0-alpha.2", + "@sentry/node": "8.0.0-alpha.2", "@sentry/svelte": "8.0.0-alpha.2", "@sentry/types": "8.0.0-alpha.2", "@sentry/utils": "8.0.0-alpha.2", diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 42a34e430fd8..970c8593af6e 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -36,16 +36,18 @@ export declare function handleErrorWithSentry any>(origLoad: T): T; -// We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. -// eslint-disable-next-line deprecation/deprecation -export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations; - export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration; export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration; export declare const getDefaultIntegrations: (options: Options) => Integration[]; export declare const defaultStackParser: StackParser; +export declare const getClient: typeof clientSdk.getClient; +// eslint-disable-next-line deprecation/deprecation +export declare const getCurrentHub: typeof clientSdk.getCurrentHub; +// eslint-disable-next-line deprecation/deprecation +export declare const makeMain: typeof clientSdk.makeMain; + export declare function close(timeout?: number | undefined): PromiseLike; export declare function flush(timeout?: number | undefined): PromiseLike; diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 4de371d03ed6..8efbb69fe34c 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -10,7 +10,7 @@ import { withIsolationScope, } from '@sentry/core'; import { startSpan } from '@sentry/core'; -import { captureException } from '@sentry/node-experimental'; +import { captureException } from '@sentry/node'; import type { Span } from '@sentry/types'; import { dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; diff --git a/packages/sveltekit/src/server/handleError.ts b/packages/sveltekit/src/server/handleError.ts index b556342635f3..1289e76a5ee2 100644 --- a/packages/sveltekit/src/server/handleError.ts +++ b/packages/sveltekit/src/server/handleError.ts @@ -1,4 +1,4 @@ -import { captureException } from '@sentry/node-experimental'; +import { captureException } from '@sentry/node'; import type { HandleServerError } from '@sveltejs/kit'; import { flushIfServerless } from './utils'; diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index d843ab8499c7..5bd1be392657 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -1,11 +1,9 @@ // Node SDK exports -// Unfortunately, we cannot `export * from '@sentry/node-experimental'` because in prod builds, +// Unfortunately, we cannot `export * from '@sentry/node'` because in prod builds, // Vite puts these exports into a `default` property (Sentry.default) rather than // on the top - level namespace. // Hence, we export everything from the Node SDK explicitly: export { - // eslint-disable-next-line deprecation/deprecation - addGlobalEventProcessor, addEventProcessor, addBreadcrumb, addIntegration, @@ -15,10 +13,6 @@ export { captureCheckIn, withMonitor, createTransport, - // eslint-disable-next-line deprecation/deprecation - getActiveTransaction, - // eslint-disable-next-line deprecation/deprecation - getCurrentHub, getClient, isInitialized, getCurrentScope, @@ -41,7 +35,6 @@ export { setHttpStatus, withScope, withIsolationScope, - autoDiscoverNodePerformanceMonitoringIntegrations, makeNodeTransport, getDefaultIntegrations, defaultStackParser, @@ -51,7 +44,6 @@ export { addRequestDataToEvent, DEFAULT_USER_INCLUDES, extractRequestData, - Integrations, consoleIntegration, onUncaughtExceptionIntegration, onUnhandledRejectionIntegration, @@ -63,7 +55,6 @@ export { functionToStringIntegration, inboundFiltersIntegration, linkedErrorsIntegration, - Handlers, setMeasurement, getActiveSpan, getRootSpan, @@ -75,16 +66,15 @@ export { cron, parameterize, createGetModuleFromFilename, - hapiErrorPlugin, metrics, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, -} from '@sentry/node-experimental'; +} from '@sentry/node'; // We can still leave this for the carrier init and type exports -export * from '@sentry/node-experimental'; +export * from '@sentry/node'; // ------------------------- // SvelteKit SDK exports: diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 38f9353a18d7..3e289440c0fd 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -4,7 +4,7 @@ import { continueTrace, startSpan, } from '@sentry/core'; -import { captureException } from '@sentry/node-experimental'; +import { captureException } from '@sentry/node'; import { addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; diff --git a/packages/sveltekit/src/server/sdk.ts b/packages/sveltekit/src/server/sdk.ts index faf584b7d0e6..f16220775d3a 100644 --- a/packages/sveltekit/src/server/sdk.ts +++ b/packages/sveltekit/src/server/sdk.ts @@ -1,7 +1,7 @@ import { applySdkMetadata, setTag } from '@sentry/core'; -import type { NodeOptions } from '@sentry/node-experimental'; -import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node-experimental'; -import { init as initNodeSdk } from '@sentry/node-experimental'; +import type { NodeOptions } from '@sentry/node'; +import { getDefaultIntegrations as getDefaultNodeIntegrations } from '@sentry/node'; +import { init as initNodeSdk } from '@sentry/node'; import { rewriteFramesIntegration } from './rewriteFramesIntegration'; diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index a202742a9eea..0db8ee893783 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,4 +1,4 @@ -import { flush } from '@sentry/node-experimental'; +import { flush } from '@sentry/node'; import { logger } from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index 96e288818aae..eb97a3e4a7c6 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -1,7 +1,7 @@ import * as child_process from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { getSentryRelease } from '@sentry/node-experimental'; +import { getSentryRelease } from '@sentry/node'; import { escapeStringForRegex, uuid4 } from '@sentry/utils'; import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; import { sentryVitePlugin } from '@sentry/vite-plugin'; diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index 42326baeade3..431b1efb791e 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -6,8 +6,8 @@ import { spanIsSampled, spanToJSON, } from '@sentry/core'; -import { NodeClient, setCurrentClient } from '@sentry/node-experimental'; -import * as SentryNode from '@sentry/node-experimental'; +import { NodeClient, setCurrentClient } from '@sentry/node'; +import * as SentryNode from '@sentry/node'; import type { Span } from '@sentry/types'; import type { Handle } from '@sveltejs/kit'; import { redirect } from '@sveltejs/kit'; diff --git a/packages/sveltekit/test/server/handleError.test.ts b/packages/sveltekit/test/server/handleError.test.ts index 58d525d9a9db..611fac1f9a4d 100644 --- a/packages/sveltekit/test/server/handleError.test.ts +++ b/packages/sveltekit/test/server/handleError.test.ts @@ -1,4 +1,4 @@ -import * as SentryNode from '@sentry/node-experimental'; +import * as SentryNode from '@sentry/node'; import type { HandleServerError, RequestEvent } from '@sveltejs/kit'; import { vi } from 'vitest'; diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index 00c9d3a34f46..71709d743e3a 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -5,8 +5,8 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions, } from '@sentry/core'; -import { getClient, getCurrentScope, getIsolationScope, init } from '@sentry/node-experimental'; -import * as SentryNode from '@sentry/node-experimental'; +import { getClient, getCurrentScope, getIsolationScope, init } from '@sentry/node'; +import * as SentryNode from '@sentry/node'; import type { Event } from '@sentry/types'; import type { Load, ServerLoad } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit'; @@ -151,7 +151,8 @@ describe.each([ [504, 1], ])('error with status code %s calls captureException %s times', async (code, times) => { async function load({ params }) { - throw error(code, params.id); + // @ts-expect-error - number is not assignable to NumericRange but that's fine here + throw error(code, { message: params.id }); } const wrappedLoad = wrapLoadWithSentry(load); diff --git a/packages/sveltekit/test/server/sdk.test.ts b/packages/sveltekit/test/server/sdk.test.ts index 96aed4103510..90168e3d2b54 100644 --- a/packages/sveltekit/test/server/sdk.test.ts +++ b/packages/sveltekit/test/server/sdk.test.ts @@ -1,7 +1,8 @@ -import * as SentryNode from '@sentry/node-experimental'; -import type { NodeClient } from '@sentry/node-experimental'; -import { SDK_VERSION, getClient } from '@sentry/node-experimental'; +import * as SentryNode from '@sentry/node'; +import type { NodeClient } from '@sentry/node'; +import { SDK_VERSION, getClient } from '@sentry/node'; +import { vi } from 'vitest'; import { init } from '../../src/server/sdk'; const nodeInit = vi.spyOn(SentryNode, 'init'); From e26488681bfc3ac810b482808893d75a91fe7255 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 14 Mar 2024 12:45:38 +0100 Subject: [PATCH 2/6] fix some test fails --- packages/sveltekit/src/index.types.ts | 3 ++- packages/sveltekit/test/server/load.test.ts | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/sveltekit/src/index.types.ts b/packages/sveltekit/src/index.types.ts index 970c8593af6e..96f123ed0d1d 100644 --- a/packages/sveltekit/src/index.types.ts +++ b/packages/sveltekit/src/index.types.ts @@ -47,8 +47,9 @@ export declare const getClient: typeof clientSdk.getClient; export declare const getCurrentHub: typeof clientSdk.getCurrentHub; // eslint-disable-next-line deprecation/deprecation export declare const makeMain: typeof clientSdk.makeMain; - export declare function close(timeout?: number | undefined): PromiseLike; export declare function flush(timeout?: number | undefined): PromiseLike; +export declare const continueTrace: typeof clientSdk.continueTrace; + export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics; diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index 71709d743e3a..976cc0ab79b2 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -283,12 +283,15 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', + status: 'ok', }); expect(transaction.transaction).toEqual('/users/[id]'); expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({ @@ -330,11 +333,14 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, 'http.method': 'GET', + 'otel.kind': 'INTERNAL', }, op: 'function.sveltekit.server.load', span_id: expect.any(String), trace_id: expect.not.stringContaining('1234567890abcdef1234567890abcdef'), + parent_span_id: expect.not.stringContaining('1234567890abcdef'), origin: 'auto.function.sveltekit', + status: 'ok', }); expect(transaction.transaction).toEqual('/users/[id]'); expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({ From 009dfc4a25496950abc9ee7a7071f245b513fa8c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 14 Mar 2024 13:36:55 +0100 Subject: [PATCH 3/6] more test fixes but they still leak :( --- packages/opentelemetry/src/index.ts | 2 ++ packages/sveltekit/package.json | 3 ++- packages/sveltekit/src/server/handle.ts | 6 +++--- packages/sveltekit/src/server/load.ts | 9 ++------- packages/sveltekit/test/server/load.test.ts | 11 +++++++++-- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index abf6640bca5c..493eeaf1f4d1 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -25,6 +25,8 @@ export { isSentryRequestSpan } from './utils/isSentryRequest'; export { getActiveSpan } from './utils/getActiveSpan'; export { startSpan, startSpanManual, startInactiveSpan, withActiveSpan, continueTrace } from './trace'; +export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; + // eslint-disable-next-line deprecation/deprecation export { setupGlobalHub } from './custom/hub'; // eslint-disable-next-line deprecation/deprecation diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index db781682df07..fe4d05fd0456 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -40,6 +40,7 @@ "@sentry-internal/tracing": "8.0.0-alpha.2", "@sentry/core": "8.0.0-alpha.2", "@sentry/node": "8.0.0-alpha.2", + "@sentry/opentelemetry": "8.0.0-alpha.2", "@sentry/svelte": "8.0.0-alpha.2", "@sentry/types": "8.0.0-alpha.2", "@sentry/utils": "8.0.0-alpha.2", @@ -68,7 +69,7 @@ "fix": "eslint . --format stylish --fix", "lint": "eslint . --format stylish", "test": "yarn test:unit", - "test:unit": "vitest run", + "test:unit": "vitest run --outputDiffMaxLines=2000", "test:watch": "vitest --watch", "yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push --sig" }, diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 8efbb69fe34c..3b73fd2da2b1 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -1,20 +1,20 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - continueTrace, getActiveSpan, - getDynamicSamplingContextFromSpan, getRootSpan, setHttpStatus, spanToTraceHeader, withIsolationScope, } from '@sentry/core'; import { startSpan } from '@sentry/core'; -import { captureException } from '@sentry/node'; +import { captureException, continueTrace } from '@sentry/node'; import type { Span } from '@sentry/types'; import { dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; +import { getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry'; + import { isHttpError, isRedirect } from '../common/utils'; import { flushIfServerless, getTracePropagationData } from './utils'; diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 3e289440c0fd..12b887bc76a6 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -1,10 +1,5 @@ -import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - continueTrace, - startSpan, -} from '@sentry/core'; -import { captureException } from '@sentry/node'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/core'; +import { captureException, continueTrace } from '@sentry/node'; import { addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index 976cc0ab79b2..d1edcb0ed014 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -326,6 +326,8 @@ describe('wrapServerLoadWithSentry calls trace', () => { expect(transactions).toHaveLength(1); const transaction = transactions[0]; + console.log(JSON.stringify(transaction.contexts?.trace, null, 2)); + expect(transaction.contexts?.trace).toEqual({ data: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.sveltekit', @@ -338,7 +340,6 @@ describe('wrapServerLoadWithSentry calls trace', () => { op: 'function.sveltekit.server.load', span_id: expect.any(String), trace_id: expect.not.stringContaining('1234567890abcdef1234567890abcdef'), - parent_span_id: expect.not.stringContaining('1234567890abcdef'), origin: 'auto.function.sveltekit', status: 'ok', }); @@ -354,7 +355,7 @@ describe('wrapServerLoadWithSentry calls trace', () => { }); }); - it("doesn't attach the DSC data if the baggage header not available", async () => { + it("doesn't attach the DSC data if the baggage header is not available", async () => { const transactions: Event[] = []; init({ @@ -381,12 +382,15 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', + status: 'ok', }); expect(transaction.transaction).toEqual('/users/[id]'); expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({}); @@ -422,12 +426,15 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', + status: 'ok', }); expect(transaction.transaction).toEqual('/users/123'); }); From 2173eece3e154b0e4512576de64f1bf5a0502188 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 14 Mar 2024 15:55:38 +0100 Subject: [PATCH 4/6] fix test for realz --- packages/sveltekit/src/server/handle.ts | 1 + packages/sveltekit/src/server/load.ts | 9 +- packages/sveltekit/test/server/handle.test.ts | 5 +- packages/sveltekit/test/server/load.test.ts | 121 ++++++++---------- 4 files changed, 63 insertions(+), 73 deletions(-) diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 3b73fd2da2b1..e45dadf2c359 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -181,6 +181,7 @@ async function instrumentHandle( attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.sveltekit', [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: event.route?.id ? 'route' : 'url', + 'http.method': event.request.method, }, name: `${event.request.method} ${event.route?.id || event.url.pathname}`, }, diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 12b887bc76a6..d0d757c31ef6 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -1,5 +1,10 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, startSpan } from '@sentry/core'; -import { captureException, continueTrace } from '@sentry/node'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + captureException, + continueTrace, + startSpan, +} from '@sentry/node'; import { addNonEnumerableProperty, objectify } from '@sentry/utils'; import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index 431b1efb791e..1e210b97eb8c 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -103,7 +103,7 @@ beforeEach(() => { mockCaptureException.mockClear(); }); -describe('handleSentry', () => { +describe('sentryHandle', () => { describe.each([ // isSync, isError, expectedResponse [Type.Sync, true, undefined], @@ -197,7 +197,7 @@ describe('handleSentry', () => { ); }); - it('creates a transaction from sentry-trace header', async () => { + it("creates a transaction from sentry-trace header but doesn't populate a new DSC", async () => { const event = mockEvent({ request: { headers: { @@ -229,6 +229,7 @@ describe('handleSentry', () => { expect(_span!.spanContext().traceId).toEqual('1234567890abcdef1234567890abcdef'); expect(spanToJSON(_span!).parent_span_id).toEqual('1234567890abcdef'); expect(spanIsSampled(_span!)).toEqual(true); + expect(_span!.metadata!.dynamicSamplingContext).toEqual({}); }); it('creates a transaction with dynamic sampling context from baggage header', async () => { diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index d1edcb0ed014..d42615521e7a 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -5,14 +5,15 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions, } from '@sentry/core'; -import { getClient, getCurrentScope, getIsolationScope, init } from '@sentry/node'; +import { NodeClient, getCurrentScope, getIsolationScope, setCurrentClient } from '@sentry/node'; import * as SentryNode from '@sentry/node'; -import type { Event } from '@sentry/types'; +import type { Event, EventEnvelopeHeaders } from '@sentry/types'; import type { Load, ServerLoad } from '@sveltejs/kit'; import { error, redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; import { wrapLoadWithSentry, wrapServerLoadWithSentry } from '../../src/server/load'; +import { getDefaultNodeClientOptions } from '../utils'; const mockCaptureException = vi.spyOn(SentryNode, 'captureException').mockImplementation(() => 'xx'); @@ -192,7 +193,7 @@ describe.each([ }); }); }); -describe('wrapLoadWithSentry calls trace', () => { +describe('wrapLoadWithSentry calls `startSpan`', () => { async function load({ params }): Promise> { return { post: params.id, @@ -243,7 +244,7 @@ describe('wrapLoadWithSentry calls trace', () => { }); }); -describe('wrapServerLoadWithSentry calls trace', () => { +describe('wrapServerLoadWithSentry calls `startSpan`', () => { async function serverLoad({ params }): Promise> { return { post: params.id, @@ -255,27 +256,45 @@ describe('wrapServerLoadWithSentry calls trace', () => { getIsolationScope().clear(); }); - it('attaches trace data if available', async () => { - const transactions: Event[] = []; + let client: NodeClient; - init({ - enableTracing: true, - release: '8.0.0', - dsn: 'https://public@dsn.ingest.sentry.io/1337', - beforeSendTransaction: event => { - transactions.push(event); - return null; + let txnEvents: Event[] = []; + + beforeEach(() => { + txnEvents = []; + + const options = getDefaultNodeClientOptions({ + tracesSampleRate: 1.0, + beforeSendTransaction: evt => { + txnEvents.push(evt); + return evt; }, + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '8.0.0', + debug: true, + }); + + client = new NodeClient(options); + setCurrentClient(client); + client.init(); + + mockCaptureException.mockClear(); + }); + + it('attaches trace data if available', async () => { + let envelopeHeaders: EventEnvelopeHeaders | undefined = undefined; + + client.on('beforeEnvelope', env => { + envelopeHeaders = env[0] as EventEnvelopeHeaders; }); - const client = getClient()!; const wrappedLoad = wrapServerLoadWithSentry(serverLoad); await wrappedLoad(getServerOnlyArgs()); await client.flush(); - expect(transactions).toHaveLength(1); - const transaction = transactions[0]; + expect(txnEvents).toHaveLength(1); + const transaction = txnEvents[0]; expect(transaction.contexts?.trace).toEqual({ data: { @@ -283,18 +302,17 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', - 'otel.kind': 'INTERNAL', - 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', - status: 'ok', }); + expect(transaction.transaction).toEqual('/users/[id]'); - expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({ + + expect(envelopeHeaders!.trace).toEqual({ environment: 'production', public_key: 'dogsarebadatkeepingsecrets', release: '1.0.0', @@ -305,28 +323,19 @@ describe('wrapServerLoadWithSentry calls trace', () => { }); it("doesn't attach trace data if it's not available", async () => { - const transactions: Event[] = []; + let envelopeHeaders: EventEnvelopeHeaders | undefined = undefined; - init({ - enableTracing: true, - release: '8.0.0', - dsn: 'https://public@dsn.ingest.sentry.io/1337', - beforeSendTransaction: event => { - transactions.push(event); - return null; - }, + client.on('beforeEnvelope', env => { + envelopeHeaders = env[0] as EventEnvelopeHeaders; }); - const client = getClient()!; const wrappedLoad = wrapServerLoadWithSentry(serverLoad); await wrappedLoad(getServerArgsWithoutTracingHeaders()); await client.flush(); - expect(transactions).toHaveLength(1); - const transaction = transactions[0]; - - console.log(JSON.stringify(transaction.contexts?.trace, null, 2)); + expect(txnEvents).toHaveLength(1); + const transaction = txnEvents[0]; expect(transaction.contexts?.trace).toEqual({ data: { @@ -335,16 +344,14 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, 'http.method': 'GET', - 'otel.kind': 'INTERNAL', }, op: 'function.sveltekit.server.load', span_id: expect.any(String), trace_id: expect.not.stringContaining('1234567890abcdef1234567890abcdef'), origin: 'auto.function.sveltekit', - status: 'ok', }); expect(transaction.transaction).toEqual('/users/[id]'); - expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({ + expect(envelopeHeaders!.trace).toEqual({ environment: 'production', public_key: 'public', sample_rate: '1', @@ -356,25 +363,19 @@ describe('wrapServerLoadWithSentry calls trace', () => { }); it("doesn't attach the DSC data if the baggage header is not available", async () => { - const transactions: Event[] = []; + let envelopeHeaders: EventEnvelopeHeaders | undefined = undefined; - init({ - enableTracing: true, - dsn: 'https://public@dsn.ingest.sentry.io/1337', - beforeSendTransaction: event => { - transactions.push(event); - return null; - }, + client.on('beforeEnvelope', env => { + envelopeHeaders = env[0] as EventEnvelopeHeaders; }); - const client = getClient()!; const wrappedLoad = wrapServerLoadWithSentry(serverLoad); await wrappedLoad(getServerArgsWithoutBaggageHeader()); await client.flush(); - expect(transactions).toHaveLength(1); - const transaction = transactions[0]; + expect(txnEvents).toHaveLength(1); + const transaction = txnEvents[0]; expect(transaction.contexts?.trace).toEqual({ data: { @@ -382,33 +383,18 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', - 'otel.kind': 'INTERNAL', - 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', - status: 'ok', }); expect(transaction.transaction).toEqual('/users/[id]'); - expect(transaction.sdkProcessingMetadata?.dynamicSamplingContext).toEqual({}); + expect(envelopeHeaders!.trace).toEqual({}); }); it('falls back to the raw url if `event.route.id` is not available', async () => { - const transactions: Event[] = []; - - init({ - enableTracing: true, - dsn: 'https://public@dsn.ingest.sentry.io/1337', - beforeSendTransaction: event => { - transactions.push(event); - return null; - }, - }); - const client = getClient()!; - const event = getServerOnlyArgs(); // @ts-expect-error - this is fine (just tests here) delete event.route; @@ -417,8 +403,8 @@ describe('wrapServerLoadWithSentry calls trace', () => { await client.flush(); - expect(transactions).toHaveLength(1); - const transaction = transactions[0]; + expect(txnEvents).toHaveLength(1); + const transaction = txnEvents[0]; expect(transaction.contexts?.trace).toEqual({ data: { @@ -426,15 +412,12 @@ describe('wrapServerLoadWithSentry calls trace', () => { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'function.sveltekit.server.load', 'http.method': 'GET', - 'otel.kind': 'INTERNAL', - 'sentry.sample_rate': 1, }, op: 'function.sveltekit.server.load', parent_span_id: '1234567890abcdef', span_id: expect.any(String), trace_id: '1234567890abcdef1234567890abcdef', origin: 'auto.function.sveltekit', - status: 'ok', }); expect(transaction.transaction).toEqual('/users/123'); }); From 9cd0647ca3b388fad777a3903c2c814b73cdc219 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 14 Mar 2024 16:05:01 +0100 Subject: [PATCH 5/6] fix duplicated export after rebase --- packages/opentelemetry/src/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 493eeaf1f4d1..abf6640bca5c 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -25,8 +25,6 @@ export { isSentryRequestSpan } from './utils/isSentryRequest'; export { getActiveSpan } from './utils/getActiveSpan'; export { startSpan, startSpanManual, startInactiveSpan, withActiveSpan, continueTrace } from './trace'; -export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; - // eslint-disable-next-line deprecation/deprecation export { setupGlobalHub } from './custom/hub'; // eslint-disable-next-line deprecation/deprecation From 1ce1c7f5da7cd28e62dfd28b19bf8eea9909ac03 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 15 Mar 2024 14:36:15 +0100 Subject: [PATCH 6/6] rewrite handle test dsc logic --- packages/sveltekit/src/server/handle.ts | 7 ++++--- packages/sveltekit/test/server/handle.test.ts | 19 ++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index e45dadf2c359..08ab880c5f27 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -149,9 +149,10 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle { }; const sentryRequestHandler: Handle = input => { - // if there is an active transaction, we know that this handle call is nested and hence - // we don't create a new domain for it. If we created one, nested server calls would - // create new transactions instead of adding a child span to the currently active span. + // if there is an active span, we know that this handle call is nested and hence + // we don't create a new execution context for it. + // If we created one, nested server calls would create new root span instead + // of adding a child span to the currently active span. if (getActiveSpan()) { return instrumentHandle(input, options); } diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index 1e210b97eb8c..1d720046fe48 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -8,7 +8,7 @@ import { } from '@sentry/core'; import { NodeClient, setCurrentClient } from '@sentry/node'; import * as SentryNode from '@sentry/node'; -import type { Span } from '@sentry/types'; +import type { EventEnvelopeHeaders, Span } from '@sentry/types'; import type { Handle } from '@sveltejs/kit'; import { redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; @@ -43,6 +43,7 @@ function mockEvent(override: Record = {}): Parameters[0 isDataRequest: false, ...override, + isSubRequest: false, }; return event; @@ -117,6 +118,7 @@ describe('sentryHandle', () => { response = await sentryHandle()({ event: mockEvent(), resolve: resolve(type, isError) }); } catch (e) { expect(e).toBeInstanceOf(Error); + // @ts-expect-error - this is fine expect(e.message).toEqual(type); } @@ -219,6 +221,11 @@ describe('sentryHandle', () => { } }); + let envelopeHeaders: EventEnvelopeHeaders | undefined = undefined; + client.on('beforeEnvelope', env => { + envelopeHeaders = env[0] as EventEnvelopeHeaders; + }); + try { await sentryHandle()({ event, resolve: resolve(type, isError) }); } catch (e) { @@ -229,7 +236,7 @@ describe('sentryHandle', () => { expect(_span!.spanContext().traceId).toEqual('1234567890abcdef1234567890abcdef'); expect(spanToJSON(_span!).parent_span_id).toEqual('1234567890abcdef'); expect(spanIsSampled(_span!)).toEqual(true); - expect(_span!.metadata!.dynamicSamplingContext).toEqual({}); + expect(envelopeHeaders!.trace).toEqual({}); }); it('creates a transaction with dynamic sampling context from baggage header', async () => { @@ -262,6 +269,11 @@ describe('sentryHandle', () => { } }); + let envelopeHeaders: EventEnvelopeHeaders | undefined = undefined; + client.on('beforeEnvelope', env => { + envelopeHeaders = env[0] as EventEnvelopeHeaders; + }); + try { await sentryHandle()({ event, resolve: resolve(type, isError) }); } catch (e) { @@ -269,7 +281,7 @@ describe('sentryHandle', () => { } expect(_span!).toBeDefined(); - expect(_span.metadata.dynamicSamplingContext).toEqual({ + expect(envelopeHeaders!.trace).toEqual({ environment: 'production', release: '1.0.0', public_key: 'dogsarebadatkeepingsecrets', @@ -313,6 +325,7 @@ describe('sentryHandle', () => { await sentryHandle()({ event, resolve: mockResolve }); } catch (e) { expect(e).toBeInstanceOf(Error); + // @ts-expect-error - this is fine expect(e.message).toEqual(type); }