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 1 commit
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
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/tracing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"lint": "run-s lint:prettier lint:eslint",
"lint:eslint": "eslint . --format stylish",
"lint:prettier": "prettier --check \"{src,test,scripts}/**/**.ts\"",
"test:unit": "jest",
"test": "jest",
"test:watch": "jest --watch"
},
Expand Down
12 changes: 9 additions & 3 deletions packages/tracing/src/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function traceHeaders(this: Hub): { [key: string]: string } {
*/
function sample<T extends Transaction>(
transaction: T,
options: Pick<Options, 'tracesSampleRate' | 'tracesSampler'>,
options: Pick<Options, 'tracesSampleRate' | 'tracesSampler' | 'enableTracing'>,
samplingContext: SamplingContext,
): T {
// nothing to do if tracing is not enabled
Expand All @@ -61,7 +61,7 @@ function sample<T extends Transaction>(
return transaction;
}

// we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` were defined, so one of these should
// we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` nor `enableTracing` were defined, so one of these should
// work; prefer the hook if so
let sampleRate;
if (typeof options.tracesSampler === 'function') {
Expand All @@ -71,11 +71,17 @@ function sample<T extends Transaction>(
});
} else if (samplingContext.parentSampled !== undefined) {
sampleRate = samplingContext.parentSampled;
} else {
} else if (typeof options.tracesSampleRate !== 'undefined') {
sampleRate = options.tracesSampleRate;
transaction.setMetadata({
sampleRate: Number(sampleRate),
});
} else {
// When `enableTracing === true`, we use a sample rate of 100%
sampleRate = 1;
transaction.setMetadata({
sampleRate,
});
}

// Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The
Expand Down
4 changes: 2 additions & 2 deletions packages/tracing/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ export { TRACEPARENT_REGEXP, extractTraceparentData } from '@sentry/utils';
* 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'> | undefined,
maybeOptions?: Pick<Options, 'tracesSampleRate' | 'tracesSampler' | 'enableTracing'> | undefined,
): boolean {
const client = getCurrentHub().getClient();
const options = maybeOptions || (client && client.getOptions());
return !!options && ('tracesSampleRate' in options || 'tracesSampler' in options);
return !!options && (options.enableTracing || 'tracesSampleRate' in options || 'tracesSampler' in options);
}

/** Grabs active transaction off scope, if any */
Expand Down
51 changes: 51 additions & 0 deletions packages/tracing/test/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,57 @@ describe('Hub', () => {
expect(transaction.sampled).toBe(true);
});

it('should set sampled = true if enableTracing is true', () => {
const options = getDefaultBrowserClientOptions({ enableTracing: true });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

expect(transaction.sampled).toBe(true);
});

it('should set sampled = false if enableTracing is true & tracesSampleRate is 0', () => {
const options = getDefaultBrowserClientOptions({ enableTracing: true, tracesSampleRate: 0 });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

expect(transaction.sampled).toBe(false);
});

it('should set sampled = false if enableTracing is false & tracesSampleRate is 0', () => {
const options = getDefaultBrowserClientOptions({ enableTracing: false, tracesSampleRate: 0 });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

expect(transaction.sampled).toBe(false);
});

it('should prefer tracesSampler returning false to enableTracing', () => {
// make the two options do opposite things to prove precedence
const tracesSampler = jest.fn().mockReturnValue(false);
const options = getDefaultBrowserClientOptions({ enableTracing: true, tracesSampler });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

expect(tracesSampler).toHaveBeenCalled();
expect(transaction.sampled).toBe(false);
});

it('should prefer tracesSampler returning true to enableTracing', () => {
// make the two options do opposite things to prove precedence
const tracesSampler = jest.fn().mockReturnValue(true);
const options = getDefaultBrowserClientOptions({ enableTracing: false, tracesSampler });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
const transaction = hub.startTransaction({ name: 'dogpark' });

expect(tracesSampler).toHaveBeenCalled();
expect(transaction.sampled).toBe(true);
});

it('should not try to override explicitly set positive sampling decision', () => {
// so that the decision otherwise would be false
const tracesSampler = jest.fn().mockReturnValue(0);
Expand Down
16 changes: 15 additions & 1 deletion packages/tracing/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,24 @@ describe('hasTracingEnabled', () => {
const tracesSampleRate = 1;
it.each([
['No options', undefined, false],
['No tracesSampler or tracesSampleRate', {}, 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>) => {
Expand Down
7 changes: 7 additions & 0 deletions packages/types/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
*/
tracesSampleRate?: number;

/**
* If this is enabled, transactions and trace data will be generated and captured.
* This will set the `tracesSampleRate` to the recommended default of `1.0`.
mydea marked this conversation as resolved.
Show resolved Hide resolved
* Note that `tracesSampleRate` and `tracesSampler` take precedence over this option.
*/
enableTracing?: boolean;

/**
* Initial data to populate scope.
*/
Expand Down