Skip to content

Commit

Permalink
fix(nextjs): Disable NextJS perf monitoring when using otel (#6820)
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad authored Jan 17, 2023
1 parent 952d866 commit 7d43873
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 15 deletions.
7 changes: 4 additions & 3 deletions packages/nextjs/src/server/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import type Document from 'next/document';

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { NextPageContext } from 'next';
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { NextPage } from 'next';
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { GetServerSideProps } from 'next';
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 11 additions & 4 deletions packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import type { GetStaticProps } from 'next';

import { isBuild } from './utils/isBuild';
Expand All @@ -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);
};
}

Expand Down
6 changes: 3 additions & 3 deletions packages/nextjs/test/config/withSentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions packages/nextjs/test/config/wrappers.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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(() => {
Expand Down

0 comments on commit 7d43873

Please sign in to comment.