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(node): Migrate to domains used through AsyncContextStrategy #7779

Merged
merged 18 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 10 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
47 changes: 1 addition & 46 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,7 @@ import type {
TransactionContext,
User,
} from '@sentry/types';
import {
consoleSandbox,
dateTimestampInSeconds,
getGlobalSingleton,
GLOBAL_OBJ,
isNodeEnv,
logger,
uuid4,
} from '@sentry/utils';
import { consoleSandbox, dateTimestampInSeconds, getGlobalSingleton, GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils';

import { DEFAULT_ENVIRONMENT } from './constants';
import { Scope } from './scope';
Expand Down Expand Up @@ -95,10 +87,6 @@ export interface Carrier {
*/
integrations?: Integration[];
extensions?: {
/** Hack to prevent bundlers from breaking our usage of the domain package in the cross-platform Hub package */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
domain?: { [key: string]: any };
} & {
/** Extension methods for the hub, which are bound to the current Hub instance */
// eslint-disable-next-line @typescript-eslint/ban-types
[key: string]: Function;
Expand Down Expand Up @@ -551,11 +539,6 @@ export function getCurrentHub(): Hub {
}
}

// Prefer domains over global if they are there (applicable only to Node environment)
if (isNodeEnv()) {
return getHubFromActiveDomain(registry);
}

// Return hub that lives on a global object
return getGlobalHub(registry);
}
Expand Down Expand Up @@ -611,34 +594,6 @@ export function runWithAsyncContext<T>(callback: (hub: Hub) => T, options: RunWi
return callback(getCurrentHub());
}

/**
* Try to read the hub from an active domain, and fallback to the registry if one doesn't exist
* @returns discovered hub
*/
function getHubFromActiveDomain(registry: Carrier): Hub {
try {
const sentry = getMainCarrier().__SENTRY__;
const activeDomain = sentry && sentry.extensions && sentry.extensions.domain && sentry.extensions.domain.active;

// If there's no active domain, just return global hub
if (!activeDomain) {
return getHubFromCarrier(registry);
}

// If there's no hub on current domain, or it's an old API, assign a new one
if (!hasHubOnCarrier(activeDomain) || getHubFromCarrier(activeDomain).isOlderThan(API_VERSION)) {
const registryHubTopStack = getHubFromCarrier(registry).getStackTop();
setHubOnCarrier(activeDomain, new Hub(registryHubTopStack.client, Scope.clone(registryHubTopStack.scope)));
}

// Return hub that lives on a domain
return getHubFromCarrier(activeDomain);
} catch (_Oo) {
// Return hub that lives on a global object
return getHubFromCarrier(registry);
}
}

/**
* This will tell whether a carrier has a hub on it or not
* @param carrier object
Expand Down
30 changes: 1 addition & 29 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import type { Carrier } from '@sentry/core';
import { getHubFromCarrier, getMainCarrier, hasTracingEnabled } from '@sentry/core';
import { hasTracingEnabled } from '@sentry/core';
import { RewriteFrames } from '@sentry/integrations';
import type { NodeOptions } from '@sentry/node';
import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
import type { EventProcessor } from '@sentry/types';
import type { IntegrationWithExclusionOption } from '@sentry/utils';
import { addOrUpdateIntegration, escapeStringForRegex, logger } from '@sentry/utils';
import * as domainModule from 'domain';
import * as path from 'path';

import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
Expand Down Expand Up @@ -55,8 +53,6 @@ const globalWithInjectedValues = global as typeof global & {
__rewriteFramesDistDir__: string;
};

const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };

// TODO (v8): Remove this
/**
* @deprecated This constant will be removed in the next major update.
Expand Down Expand Up @@ -87,16 +83,6 @@ export function init(options: NodeOptions): void {
// Right now we only capture frontend sessions for Next.js
options.autoSessionTracking = false;

// In an ideal world, this init function would be called before any requests are handled. That way, every domain we
// use to wrap a request would inherit its scope and client from the global hub. In practice, however, handling the
// first request is what causes us to initialize the SDK, as the init code is injected into `_app` and all API route
// handlers, and those are only accessed in the course of handling a request. As a result, we're already in a domain
// when `init` is called. In order to compensate for this and mimic the ideal world scenario, we stash the active
// domain, run `init` as normal, and then restore the domain afterwards, copying over data from the main hub as if we
// really were inheriting.
const activeDomain = domain.active;
domain.active = null;

nodeInit(options);

const filterTransactions: EventProcessor = event => {
Expand All @@ -118,20 +104,6 @@ export function init(options: NodeOptions): void {
}
});

if (activeDomain) {
const globalHub = getHubFromCarrier(getMainCarrier());
const domainHub = getHubFromCarrier(activeDomain);

// apply the changes made by `nodeInit` to the domain's hub also
domainHub.bindClient(globalHub.getClient());
domainHub.getScope()?.update(globalHub.getScope());
// `scope.update()` doesn’t copy over event processors, so we have to add it manually
domainHub.getScope()?.addEventProcessor(filterTransactions);

// restore the domain hub as the current one
domain.active = activeDomain;
}

__DEBUG_BUILD__ && logger.log('SDK successfully initialized');
}

Expand Down
9 changes: 4 additions & 5 deletions packages/nextjs/src/server/utils/wrapperUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { captureException, getActiveTransaction, getCurrentHub, startTransaction } from '@sentry/core';
import { captureException, getActiveTransaction, runWithAsyncContext, startTransaction } from '@sentry/core';
import type { Transaction } from '@sentry/types';
import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
import * as domain from 'domain';
import type { IncomingMessage, ServerResponse } from 'http';

import { platformSupportsStreaming } from './platformSupportsStreaming';
Expand Down Expand Up @@ -75,7 +74,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
},
): (...params: Parameters<F>) => Promise<ReturnType<F>> {
return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
return domain.create().bind(async () => {
return runWithAsyncContext(async hub => {
let requestTransaction: Transaction | undefined = getTransactionFromRequest(req);
let dataFetcherSpan;

Expand Down Expand Up @@ -134,7 +133,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
});
}

const currentScope = getCurrentHub().getScope();
const currentScope = hub.getScope();
if (currentScope) {
currentScope.setSpan(dataFetcherSpan);
currentScope.setSDKProcessingMetadata({ request: req });
Expand All @@ -154,7 +153,7 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
await flushQueue();
}
}
})();
});
};
}

Expand Down
Loading