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(tracing): Add enableTracing option #7238

Merged
merged 3 commits into from
Feb 22, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

This release adds a new `enableTracing` option, which can be used instead of `tracesSampleRate` for an easier setup.
Related to this, the `hasTracingEnabled` utility function was moved from `@sentry/tracing` to `@sentry/core`.
The old export from `@sentry/tracing` has been deprecated and will be removed in v8.

## 7.37.2

This release includes changes and fixes around text masking and blocking in Replay's `rrweb` dependency. See versions [1.102.0](https://github.com/getsentry/rrweb/releases/tag/1.102.0) and [1.103.0](https://github.com/getsentry/rrweb/releases/tag/1.103.0).
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"lint:eslint": "lerna run lint:eslint",
"postpublish": "lerna run --stream --concurrency 1 postpublish",
"test": "lerna run --ignore @sentry-internal/* test",
"test:unit": "lerna run --ignore @sentry-internal/* test:unit",
"test-ci-browser": "lerna run test --ignore \"@sentry/{node,opentelemetry-node,serverless,nextjs,remix,gatsby}\" --ignore @sentry-internal/*",
"test-ci-node": "ts-node ./scripts/node-unit-tests.ts",
"test:update-snapshots": "lerna run test:update-snapshots"
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export { SDK_VERSION } from './version';
export { getIntegrationsToSetup } from './integration';
export { FunctionToString, InboundFilters } from './integrations';
export { prepareEvent } from './utils/prepareEvent';
export { hasTracingEnabled } from './utils/hasTracingEnabled';

import * as Integrations from './integrations';

Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/utils/hasTracingEnabled.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { Options } from '@sentry/types';

import { getCurrentHub } from '../hub';

// Treeshakable guard to remove all code related to tracing
declare const __SENTRY_TRACING__: boolean | undefined;

/**
* Determines if tracing is currently enabled.
*
* Tracing is enabled when at least one of `tracesSampleRate` and `tracesSampler` is defined in the SDK config.
*/
export function hasTracingEnabled(
maybeOptions?: Pick<Options, 'tracesSampleRate' | 'tracesSampler' | 'enableTracing'> | undefined,
): boolean {
if (typeof __SENTRY_TRACING__ === 'boolean' && !__SENTRY_TRACING__) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I wonder if this is worth it 🤔 WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure but theoretically, rollup should be able to basically reduce this function to return false if users configure this guard in their bundler setup 🤔 So I'd argue yes

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 think the "downside" is that users that do not configure this will/may still have this code in their bundle, right?

return false;
}

const client = getCurrentHub().getClient();
const options = maybeOptions || (client && client.getOptions());
return !!options && (options.enableTracing || 'tracesSampleRate' in options || 'tracesSampler' in options);
}
32 changes: 32 additions & 0 deletions packages/core/test/lib/utils/hasTracingEnabled.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { hasTracingEnabled } from '../../../src';

describe('hasTracingEnabled', () => {
const tracesSampler = () => 1;
const tracesSampleRate = 1;
it.each([
['No options', undefined, false],
['No tracesSampler or tracesSampleRate or enableTracing', {}, false],
['With tracesSampler', { tracesSampler }, true],
['With tracesSampleRate', { tracesSampleRate }, true],
['With enableTracing=true', { enableTracing: true }, true],
['With enableTracing=false', { enableTracing: false }, false],
['With tracesSampler && enableTracing=false', { tracesSampler, enableTracing: false }, true],
['With tracesSampleRate && enableTracing=false', { tracesSampler, enableTracing: false }, true],
['With tracesSampler and tracesSampleRate', { tracesSampler, tracesSampleRate }, true],
[
'With tracesSampler and tracesSampleRate and enableTracing=true',
{ tracesSampler, tracesSampleRate, enableTracing: true },
true,
],
[
'With tracesSampler and tracesSampleRate and enableTracing=false',
{ tracesSampler, tracesSampleRate, enableTracing: false },
true,
],
])(
'%s',
(_: string, input: Parameters<typeof hasTracingEnabled>[0], output: ReturnType<typeof hasTracingEnabled>) => {
expect(hasTracingEnabled(input)).toBe(output);
},
);
});
1 change: 1 addition & 0 deletions packages/gatsby/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"access": "public"
},
"dependencies": {
"@sentry/core": "7.38.0",
"@sentry/react": "7.38.0",
"@sentry/tracing": "7.38.0",
"@sentry/types": "7.38.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/src/utils/integrations.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { hasTracingEnabled } from '@sentry/core';
import * as Tracing from '@sentry/tracing';
import type { Integration } from '@sentry/types';

Expand All @@ -13,7 +14,7 @@ export type UserIntegrations = Integration[] | UserFnIntegrations;
* @param options The options users have defined.
*/
export function getIntegrationsFromOptions(options: GatsbyOptions): UserIntegrations {
const isTracingEnabled = Tracing.hasTracingEnabled(options);
const isTracingEnabled = hasTracingEnabled(options);
if (options.integrations === undefined) {
return getIntegrationsFromArray([], isTracingEnabled);
} else if (Array.isArray(options.integrations)) {
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { hasTracingEnabled } from '@sentry/core';
import { RewriteFrames } from '@sentry/integrations';
import type { BrowserOptions } from '@sentry/react';
import { configureScope, init as reactInit, Integrations } from '@sentry/react';
import { BrowserTracing, defaultRequestInstrumentationOptions, hasTracingEnabled } from '@sentry/tracing';
import { BrowserTracing, defaultRequestInstrumentationOptions } from '@sentry/tracing';
import type { EventProcessor } from '@sentry/types';

import { getVercelEnv } from '../common/getVercelEnv';
Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/src/edge/utils/edgeWrapperUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { captureException, getCurrentHub, startTransaction } from '@sentry/core';
import { hasTracingEnabled } from '@sentry/tracing';
import { captureException, getCurrentHub, hasTracingEnabled, startTransaction } from '@sentry/core';
import type { Span } from '@sentry/types';
import {
addExceptionMechanism,
Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/src/server/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import type { Carrier } from '@sentry/core';
import { getHubFromCarrier, getMainCarrier } from '@sentry/core';
import { getHubFromCarrier, getMainCarrier, 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 { hasTracingEnabled } from '@sentry/tracing';
import type { EventProcessor } from '@sentry/types';
import { escapeStringForRegex, logger } from '@sentry/utils';
import * as domainModule from 'domain';
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/server/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { hasTracingEnabled } from '@sentry/core';
import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
import { extractTraceparentData, hasTracingEnabled } from '@sentry/tracing';
import { extractTraceparentData } from '@sentry/tracing';
import type { Transaction } from '@sentry/types';
import {
addExceptionMechanism,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type App from 'next/app';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import type Document from 'next/document';

import { isBuild } from './utils/isBuild';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { NextPageContext } from 'next';
import type { ErrorProps } from 'next/error';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { NextPage } from 'next';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
import type { GetServerSideProps } from 'next';

Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hasTracingEnabled } from '@sentry/core';
import { getCurrentHub } from '@sentry/node';
import { hasTracingEnabled } from '@sentry/tracing';
import type { GetStaticProps } from 'next';

import { isBuild } from './utils/isBuild';
Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/test/config/wrappers.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +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';

import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/server';
Expand All @@ -17,7 +16,7 @@ describe('data-fetching function wrappers', () => {
req = { headers: {}, url: 'http://dogs.are.great/tricks/kangaroo' } as IncomingMessage;
res = { end: jest.fn() } as unknown as ServerResponse;

jest.spyOn(SentryTracing, 'hasTracingEnabled').mockReturnValueOnce(true);
jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValueOnce(true);
jest.spyOn(SentryNode, 'getCurrentHub').mockReturnValueOnce({
getClient: () =>
({
Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/test/edge/edgeWrapperUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as coreSdk from '@sentry/core';
import * as sentryTracing from '@sentry/tracing';

import { withEdgeWrapping } from '../../src/edge/utils/edgeWrapperUtils';

Expand Down Expand Up @@ -30,7 +29,7 @@ afterAll(() => {
beforeEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true);
jest.spyOn(coreSdk, 'hasTracingEnabled').mockImplementation(() => true);
});

describe('withEdgeWrapping', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/nextjs/test/edge/withSentryAPI.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as coreSdk from '@sentry/core';
import * as sentryTracing from '@sentry/tracing';

import { wrapApiHandlerWithSentry } from '../../src/edge';

Expand Down Expand Up @@ -32,7 +31,7 @@ afterAll(() => {
beforeEach(() => {
jest.resetAllMocks();
jest.restoreAllMocks();
jest.spyOn(sentryTracing, 'hasTracingEnabled').mockImplementation(() => true);
jest.spyOn(coreSdk, 'hasTracingEnabled').mockImplementation(() => true);
});

describe('wrapApiHandlerWithSentry', () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core';
import { captureException, getCurrentHub, hasTracingEnabled, startTransaction, withScope } from '@sentry/core';
import type { Span } from '@sentry/types';
import type { AddRequestDataToEventOptions } from '@sentry/utils';
import {
Expand Down Expand Up @@ -46,9 +46,7 @@ export function tracingHandler(): (
return next();
}

// TODO: This is the `hasTracingEnabled` check, but we're doing it manually since `@sentry/tracing` isn't a
// dependency of `@sentry/node`. Long term, that function should probably move to `@sentry/hub.
if (!('tracesSampleRate' in options) && !('tracesSampler' in options)) {
if (!hasTracingEnabled(options)) {
__DEBUG_BUILD__ &&
logger.warn(
'Sentry `tracingHandler` is being used, but tracing is disabled. Please enable tracing by setting ' +
Expand Down
39 changes: 19 additions & 20 deletions packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as sentryCore from '@sentry/core';
import { Hub, makeMain, Scope } from '@sentry/core';
import { Transaction } from '@sentry/tracing';
import type { Event } from '@sentry/types';
import { SentryError } from '@sentry/utils';
Expand Down Expand Up @@ -47,7 +46,7 @@ describe('requestHandler', () => {
it('autoSessionTracking is enabled, sets requestSession status to ok, when handling a request', () => {
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.2' });
client = new NodeClient(options);
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);

jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

Expand All @@ -60,7 +59,7 @@ describe('requestHandler', () => {
it('autoSessionTracking is disabled, does not set requestSession, when handling a request', () => {
const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '1.2' });
client = new NodeClient(options);
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);

jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

Expand All @@ -73,7 +72,7 @@ describe('requestHandler', () => {
it('autoSessionTracking is enabled, calls _captureRequestSession, on response finish', done => {
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.2' });
client = new NodeClient(options);
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);

jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

Expand All @@ -94,7 +93,7 @@ describe('requestHandler', () => {
it('autoSessionTracking is disabled, does not call _captureRequestSession, on response finish', done => {
const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '1.2' });
client = new NodeClient(options);
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

const captureRequestSession = jest.spyOn<any, any>(client, '_captureRequestSession');
Expand Down Expand Up @@ -138,7 +137,7 @@ describe('requestHandler', () => {
});

it('stores request and request data options in `sdkProcessingMetadata`', () => {
const hub = new Hub(new NodeClient(getDefaultNodeClientOptions()));
const hub = new sentryCore.Hub(new NodeClient(getDefaultNodeClientOptions()));
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

const requestHandlerOptions = { include: { ip: false } };
Expand All @@ -165,15 +164,15 @@ describe('tracingHandler', () => {

const sentryTracingMiddleware = tracingHandler();

let hub: Hub, req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined;
let hub: sentryCore.Hub, req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined;

function createNoOpSpy() {
const noop = { noop: () => undefined }; // this is wrapped in an object so jest can spy on it
return jest.spyOn(noop, 'noop') as any;
}

beforeEach(() => {
hub = new Hub(new NodeClient(getDefaultNodeClientOptions({ tracesSampleRate: 1.0 })));
hub = new sentryCore.Hub(new NodeClient(getDefaultNodeClientOptions({ tracesSampleRate: 1.0 })));
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
req = {
headers,
Expand Down Expand Up @@ -271,7 +270,7 @@ describe('tracingHandler', () => {
it('extracts request data for sampling context', () => {
const tracesSampler = jest.fn();
const options = getDefaultNodeClientOptions({ tracesSampler });
const hub = new Hub(new NodeClient(options));
const hub = new sentryCore.Hub(new NodeClient(options));
hub.run(() => {
sentryTracingMiddleware(req, res, next);

Expand All @@ -291,7 +290,7 @@ describe('tracingHandler', () => {

it('puts its transaction on the scope', () => {
const options = getDefaultNodeClientOptions({ tracesSampleRate: 1.0 });
const hub = new Hub(new NodeClient(options));
const hub = new sentryCore.Hub(new NodeClient(options));

jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

Expand Down Expand Up @@ -411,7 +410,7 @@ describe('tracingHandler', () => {

it('stores request in transaction metadata', () => {
const options = getDefaultNodeClientOptions({ tracesSampleRate: 1.0 });
const hub = new Hub(new NodeClient(options));
const hub = new sentryCore.Hub(new NodeClient(options));

jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);

Expand Down Expand Up @@ -465,7 +464,7 @@ describe('errorHandler()', () => {
client.initSessionFlusher();

const scope = sentryCore.getCurrentHub().getScope();
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);

jest.spyOn<any, any>(client, '_captureRequestSession');
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
Expand All @@ -481,7 +480,7 @@ describe('errorHandler()', () => {
client = new NodeClient(options);

const scope = sentryCore.getCurrentHub().getScope();
const hub = new Hub(client);
const hub = new sentryCore.Hub(client);

jest.spyOn<any, any>(client, '_captureRequestSession');
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
Expand All @@ -498,8 +497,8 @@ describe('errorHandler()', () => {
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
// by the`requestHandler`)
client.initSessionFlusher();
const scope = new Scope();
const hub = new Hub(client, scope);
const scope = new sentryCore.Scope();
const hub = new sentryCore.Hub(client, scope);

jest.spyOn<any, any>(client, '_captureRequestSession');

Expand All @@ -517,8 +516,8 @@ describe('errorHandler()', () => {
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
// by the`requestHandler`)
client.initSessionFlusher();
const scope = new Scope();
const hub = new Hub(client, scope);
const scope = new sentryCore.Scope();
const hub = new sentryCore.Hub(client, scope);

jest.spyOn<any, any>(client, '_captureRequestSession');
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
Expand All @@ -532,12 +531,12 @@ describe('errorHandler()', () => {
const options = getDefaultNodeClientOptions({});
client = new NodeClient(options);

const hub = new Hub(client);
makeMain(hub);
const hub = new sentryCore.Hub(client);
sentryCore.makeMain(hub);

// `sentryErrorMiddleware` uses `withScope`, and we need access to the temporary scope it creates, so monkeypatch
// `captureException` in order to examine the scope as it exists inside the `withScope` callback
hub.captureException = function (this: Hub, _exception: any) {
hub.captureException = function (this: sentryCore.Hub, _exception: any) {
const scope = this.getScope();
expect((scope as any)._sdkProcessingMetadata.request).toEqual(req);
} as any;
Expand Down