Skip to content
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

feat(serverless): Do not include performance integrations by default #11998

Merged
merged 4 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -16,6 +16,7 @@ const NODE_EXPORTS_IGNORE = [
'__esModule',
// Only required from the Node package
'setNodeAsyncContextStrategy',
'getDefaultIntegrationsWithoutPerformance',
];

const nodeExports = Object.keys(SentryNode).filter(e => !NODE_EXPORTS_IGNORE.includes(e));
Expand Down
15 changes: 14 additions & 1 deletion packages/aws-serverless/src/awslambda-auto.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getDefaultIntegrations as getNodeDefaultIntegrations } from '@sentry/node';
import { init, tryPatchHandler } from './sdk';

const lambdaTaskRoot = process.env.LAMBDA_TASK_ROOT;
Expand All @@ -7,7 +8,19 @@ if (lambdaTaskRoot) {
throw Error(`LAMBDA_TASK_ROOT is non-empty(${lambdaTaskRoot}) but _HANDLER is not set`);
}

init();
init({
// We want to load the performance integrations here, if the tracesSampleRate is set for the layer in env vars
// Sentry node's `getDefaultIntegrations` will load them if tracing is enabled,
// which is the case if `tracesSampleRate` is set.
// We can safely add all the node default integrations
integrations: getNodeDefaultIntegrations(
process.env.SENTRY_TRACES_SAMPLE_RATE
? {
tracesSampleRate: parseFloat(process.env.SENTRY_TRACES_SAMPLE_RATE),
}
: {},
),
});

tryPatchHandler(lambdaTaskRoot, handlerString);
} else {
Expand Down
12 changes: 8 additions & 4 deletions packages/aws-serverless/src/sdk.ts
Copy link
Member

Choose a reason for hiding this comment

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

l/suggestion: We could add a sentence to the init function to remind users to initialize additional (database) integrations? (just an idea; we could also wait until we updated docs accordingly and link there)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, we can still tweak this later I'd say! 👍

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
continueTrace,
flush,
getCurrentScope,
getDefaultIntegrations as getNodeDefaultIntegrations,
getDefaultIntegrationsWithoutPerformance,
init as initNode,
startSpanManual,
withScope,
Expand Down Expand Up @@ -60,9 +60,13 @@ export interface WrapperOptions {
startTrace: boolean;
}

/** Get the default integrations for the AWSLambda SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
return [...getNodeDefaultIntegrations(options), awsIntegration(), awsLambdaIntegration()];
/**
* Get the default integrations for the AWSLambda SDK.
*/
// NOTE: in awslambda-auto.ts, we also call the original `getDefaultIntegrations` from `@sentry/node` to load performance integrations.
// If at some point we need to filter a node integration out for good, we need to make sure to also filter it out there.
export function getDefaultIntegrations(_options: Options): Integration[] {
return [...getDefaultIntegrationsWithoutPerformance(), awsIntegration(), awsLambdaIntegration()];
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/google-cloud-serverless/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import type { NodeOptions } from '@sentry/node';
import { SDK_VERSION, getDefaultIntegrations as getDefaultNodeIntegrations, init as initNode } from '@sentry/node';
import { SDK_VERSION, getDefaultIntegrationsWithoutPerformance, init as initNode } from '@sentry/node';
import type { Integration, Options, SdkMetadata } from '@sentry/types';

import { googleCloudGrpcIntegration } from './integrations/google-cloud-grpc';
import { googleCloudHttpIntegration } from './integrations/google-cloud-http';

/** Get the default integrations for the GCP SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
export function getDefaultIntegrations(_options: Options): Integration[] {
return [
...getDefaultNodeIntegrations(options),
...getDefaultIntegrationsWithoutPerformance(),
googleCloudHttpIntegration({ optional: true }), // We mark this integration optional since '@google-cloud/common' module could be missing.
googleCloudGrpcIntegration({ optional: true }), // We mark this integration optional since 'google-gax' module could be missing.
];
Expand Down
6 changes: 5 additions & 1 deletion packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ export { koaIntegration, setupKoaErrorHandler } from './integrations/tracing/koa
export { connectIntegration, setupConnectErrorHandler } from './integrations/tracing/connect';
export { spotlightIntegration } from './integrations/spotlight';

export { init, getDefaultIntegrations } from './sdk/init';
export {
init,
getDefaultIntegrations,
getDefaultIntegrationsWithoutPerformance,
} from './sdk/init';
export { initOpenTelemetry } from './sdk/initOtel';
export { getAutoPerformanceIntegrations } from './integrations/tracing';
export { getSentryRelease, defaultStackParser } from './sdk/api';
Expand Down
24 changes: 18 additions & 6 deletions packages/node/src/sdk/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ function getCjsOnlyIntegrations(): Integration[] {
return isCjs() ? [modulesIntegration()] : [];
}

/** Get the default integrations for the Node Experimental SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
/**
* Get default integrations, excluding performance.
*/
export function getDefaultIntegrationsWithoutPerformance(): Integration[] {
return [
// Common
inboundFiltersIntegration(),
Expand All @@ -69,6 +71,13 @@ export function getDefaultIntegrations(options: Options): Integration[] {
localVariablesIntegration(),
nodeContextIntegration(),
...getCjsOnlyIntegrations(),
];
}

/** Get the default integrations for the Node SDK. */
export function getDefaultIntegrations(options: Options): Integration[] {
return [
...getDefaultIntegrationsWithoutPerformance(),
...(hasTracingEnabled(options) ? getAutoPerformanceIntegrations() : []),
];
}
Expand Down Expand Up @@ -183,10 +192,6 @@ function validateOpenTelemetrySetup(): void {
}

function getClientOptions(options: NodeOptions): NodeClientOptions {
if (options.defaultIntegrations === undefined) {
options.defaultIntegrations = getDefaultIntegrations(options);
}

const release = getRelease(options.release);

const autoSessionTracking =
Expand All @@ -210,6 +215,13 @@ function getClientOptions(options: NodeOptions): NodeClientOptions {
tracesSampleRate,
});

if (options.defaultIntegrations === undefined) {
options.defaultIntegrations = getDefaultIntegrations({
...options,
...overwriteOptions,
});
}

const clientOptions: NodeClientOptions = {
...baseOptions,
...options,
Expand Down
Loading