From 7d43873a0fc4654baae0ed89bc7fa4dd40727ca5 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 17 Jan 2023 13:16:50 +0100 Subject: [PATCH] fix(nextjs): Disable NextJS perf monitoring when using otel (#6820) --- .../nextjs/src/server/wrapApiHandlerWithSentry.ts | 7 ++++--- .../server/wrapAppGetInitialPropsWithSentry.ts | 4 +++- .../wrapDocumentGetInitialPropsWithSentry.ts | 4 +++- .../server/wrapErrorGetInitialPropsWithSentry.ts | 4 +++- .../src/server/wrapGetInitialPropsWithSentry.ts | 4 +++- .../server/wrapGetServerSidePropsWithSentry.ts | 4 +++- .../src/server/wrapGetStaticPropsWithSentry.ts | 15 +++++++++++---- packages/nextjs/test/config/withSentry.test.ts | 6 +++--- packages/nextjs/test/config/wrappers.test.ts | 7 +++++++ 9 files changed, 40 insertions(+), 15 deletions(-) diff --git a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts index 44cad73e6e81..897b362cc550 100644 --- a/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/server/wrapApiHandlerWithSentry.ts @@ -87,12 +87,14 @@ export function withSentry(origHandler: NextApiHandler, parameterizedRoute?: str // eslint-disable-next-line complexity const boundHandler = local.bind(async () => { let transaction: Transaction | undefined; - const currentScope = getCurrentHub().getScope(); + const hub = getCurrentHub(); + const currentScope = hub.getScope(); + const options = hub.getClient()?.getOptions(); if (currentScope) { currentScope.setSDKProcessingMetadata({ request: req }); - if (hasTracingEnabled()) { + if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { @@ -137,7 +139,6 @@ export function withSentry(origHandler: NextApiHandler, parameterizedRoute?: str { request: req }, ); currentScope.setSpan(transaction); - if (platformSupportsStreaming() && !origHandler.__sentry_test_doesnt_support_streaming__) { autoEndTransactionOnResponseEnd(transaction, res); } else { diff --git a/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts index d21ecfc0a44e..09b9d7070e9f 100644 --- a/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts @@ -1,3 +1,4 @@ +import { getCurrentHub } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type App from 'next/app'; @@ -29,12 +30,13 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI const { req, res } = context.ctx; const errorWrappedAppGetInitialProps = withErrorInstrumentation(origAppGetInitialProps); + const options = getCurrentHub().getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher // span with each other when there are no req or res objects, we simply do not trace them at all here. - if (hasTracingEnabled() && req && res) { + if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') { const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedAppGetInitialProps, req, res, { dataFetcherRouteName: '/_app', requestedRouteName: context.ctx.pathname, diff --git a/packages/nextjs/src/server/wrapDocumentGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapDocumentGetInitialPropsWithSentry.ts index 7052250275d7..614516af1e47 100644 --- a/packages/nextjs/src/server/wrapDocumentGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapDocumentGetInitialPropsWithSentry.ts @@ -1,3 +1,4 @@ +import { getCurrentHub } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import type Document from 'next/document'; @@ -29,12 +30,13 @@ export function wrapDocumentGetInitialPropsWithSentry( const { req, res } = context; const errorWrappedGetInitialProps = withErrorInstrumentation(origDocumentGetInitialProps); + const options = getCurrentHub().getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher // span with each other when there are no req or res objects, we simply do not trace them at all here. - if (hasTracingEnabled() && req && res) { + if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') { const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, { dataFetcherRouteName: '/_document', requestedRouteName: context.pathname, diff --git a/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts index 54f52ef8dfc9..5c1987b26416 100644 --- a/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts @@ -1,3 +1,4 @@ +import { getCurrentHub } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { NextPageContext } from 'next'; @@ -32,12 +33,13 @@ export function wrapErrorGetInitialPropsWithSentry( const { req, res } = context; const errorWrappedGetInitialProps = withErrorInstrumentation(origErrorGetInitialProps); + const options = getCurrentHub().getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher // span with each other when there are no req or res objects, we simply do not trace them at all here. - if (hasTracingEnabled() && req && res) { + if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') { const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, { dataFetcherRouteName: '/_error', requestedRouteName: context.pathname, diff --git a/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts b/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts index aed6829a6cb4..72521f10c4f2 100644 --- a/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts @@ -1,3 +1,4 @@ +import { getCurrentHub } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { NextPage } from 'next'; @@ -28,12 +29,13 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro const { req, res } = context; const errorWrappedGetInitialProps = withErrorInstrumentation(origGetInitialProps); + const options = getCurrentHub().getClient()?.getOptions(); // Generally we can assume that `req` and `res` are always defined on the server: // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher // span with each other when there are no req or res objects, we simply do not trace them at all here. - if (hasTracingEnabled() && req && res) { + if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') { const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, { dataFetcherRouteName: context.pathname, requestedRouteName: context.pathname, diff --git a/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts b/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts index dc3640b6dfe9..691b890e4723 100644 --- a/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts @@ -1,3 +1,4 @@ +import { getCurrentHub } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { GetServerSideProps } from 'next'; @@ -29,8 +30,9 @@ export function wrapGetServerSidePropsWithSentry( const { req, res } = context; const errorWrappedGetServerSideProps = withErrorInstrumentation(origGetServerSideProps); + const options = getCurrentHub().getClient()?.getOptions(); - if (hasTracingEnabled()) { + if (hasTracingEnabled() && options?.instrumenter === 'sentry') { const tracedGetServerSideProps = withTracedServerSideDataFetcher(errorWrappedGetServerSideProps, req, res, { dataFetcherRouteName: parameterizedRoute, requestedRouteName: parameterizedRoute, diff --git a/packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts b/packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts index 727e4a69c224..380ea38949fc 100644 --- a/packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts +++ b/packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts @@ -1,3 +1,5 @@ +import { getCurrentHub } from '@sentry/node'; +import { hasTracingEnabled } from '@sentry/tracing'; import type { GetStaticProps } from 'next'; import { isBuild } from './utils/isBuild'; @@ -24,11 +26,16 @@ export function wrapGetStaticPropsWithSentry( } const errorWrappedGetStaticProps = withErrorInstrumentation(origGetStaticProps); + const options = getCurrentHub().getClient()?.getOptions(); - return callDataFetcherTraced(errorWrappedGetStaticProps, getStaticPropsArguments, { - parameterizedRoute, - dataFetchingMethodName: 'getStaticProps', - }); + if (hasTracingEnabled() && options?.instrumenter === 'sentry') { + return callDataFetcherTraced(errorWrappedGetStaticProps, getStaticPropsArguments, { + parameterizedRoute, + dataFetchingMethodName: 'getStaticProps', + }); + } + + return errorWrappedGetStaticProps(...getStaticPropsArguments); }; } diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts index 75709d3f0bea..e622df357a61 100644 --- a/packages/nextjs/test/config/withSentry.test.ts +++ b/packages/nextjs/test/config/withSentry.test.ts @@ -66,9 +66,9 @@ describe('withSentry', () => { describe('tracing', () => { it('starts a transaction and sets metadata when tracing is enabled', async () => { - jest - .spyOn(hub.Hub.prototype, 'getClient') - .mockReturnValueOnce({ getOptions: () => ({ tracesSampleRate: 1 } as ClientOptions) } as Client); + jest.spyOn(hub.Hub.prototype, 'getClient').mockReturnValueOnce({ + getOptions: () => ({ tracesSampleRate: 1, instrumenter: 'sentry' } as ClientOptions), + } as Client); await callWrappedHandler(wrappedHandlerNoError, req, res); diff --git a/packages/nextjs/test/config/wrappers.test.ts b/packages/nextjs/test/config/wrappers.test.ts index 4824d6dc1573..de7fe48b5d18 100644 --- a/packages/nextjs/test/config/wrappers.test.ts +++ b/packages/nextjs/test/config/wrappers.test.ts @@ -1,4 +1,5 @@ import * as SentryCore from '@sentry/core'; +import * as SentryNode from '@sentry/node'; import * as SentryTracing from '@sentry/tracing'; import type { IncomingMessage, ServerResponse } from 'http'; @@ -17,6 +18,12 @@ describe('data-fetching function wrappers', () => { res = { end: jest.fn() } as unknown as ServerResponse; jest.spyOn(SentryTracing, 'hasTracingEnabled').mockReturnValueOnce(true); + jest.spyOn(SentryNode, 'getCurrentHub').mockReturnValueOnce({ + getClient: () => + ({ + getOptions: () => ({ instrumenter: 'sentry' }), + } as any), + } as any); }); afterEach(() => {