Skip to content

feat(sveltekit): Switch to Otel-based @sentry/node package #11075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const DEPENDENTS: Dependent[] = [
},
{
package: '@sentry/sveltekit',
compareWith: nodeExperimentalExports,
compareWith: nodeExports,
exports: Object.keys(SentrySvelteKit),
},
];
Expand Down
5 changes: 3 additions & 2 deletions packages/sveltekit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"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/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",
Expand Down Expand Up @@ -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"
},
Expand Down
11 changes: 7 additions & 4 deletions packages/sveltekit/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,20 @@ export declare function handleErrorWithSentry<T extends HandleClientError | Hand
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export declare function wrapLoadWithSentry<T extends (...args: any) => 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<boolean>;
export declare function flush(timeout?: number | undefined): PromiseLike<boolean>;

export declare const continueTrace: typeof clientSdk.continueTrace;

export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics;
7 changes: 4 additions & 3 deletions packages/sveltekit/src/server/handle.ts
Original file line number Diff line number Diff line change
@@ -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-experimental';
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';

Expand Down Expand Up @@ -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}`,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/src/server/handleError.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
16 changes: 3 additions & 13 deletions packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand All @@ -41,7 +35,6 @@ export {
setHttpStatus,
withScope,
withIsolationScope,
autoDiscoverNodePerformanceMonitoringIntegrations,
makeNodeTransport,
getDefaultIntegrations,
defaultStackParser,
Expand All @@ -51,7 +44,6 @@ export {
addRequestDataToEvent,
DEFAULT_USER_INCLUDES,
extractRequestData,
Integrations,
consoleIntegration,
onUncaughtExceptionIntegration,
onUnhandledRejectionIntegration,
Expand All @@ -63,7 +55,6 @@ export {
functionToStringIntegration,
inboundFiltersIntegration,
linkedErrorsIntegration,
Handlers,
setMeasurement,
getActiveSpan,
getRootSpan,
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions packages/sveltekit/src/server/load.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
captureException,
continueTrace,
startSpan,
} from '@sentry/core';
import { captureException } from '@sentry/node-experimental';
} from '@sentry/node';
import { addNonEnumerableProperty, objectify } from '@sentry/utils';
import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit';

Expand Down
6 changes: 3 additions & 3 deletions packages/sveltekit/src/server/sdk.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/src/server/utils.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/src/vite/sourceMaps.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
9 changes: 5 additions & 4 deletions packages/sveltekit/test/server/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -103,7 +103,7 @@ beforeEach(() => {
mockCaptureException.mockClear();
});

describe('handleSentry', () => {
describe('sentryHandle', () => {
describe.each([
// isSync, isError, expectedResponse
[Type.Sync, true, undefined],
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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({});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this actually works? 😅 I thought this should fail as this would not be set?

Can we do instead getDynamicSamplingContextForSpan(_span) here to test this, maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was as surprised as you that it works 😅 hmm I don't wanna use getDynamicSamplingContextForSpan here because I don't think this is necessarily all the logic that contributes to what we ultimately propagate/put into the envelope. Maybe I just refactor these tests similarly to the ones in load.test.ts to use the beforeEnvelope hook to check for the trace header.

});

it('creates a transaction with dynamic sampling context from baggage header', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/test/server/handleError.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
Loading