Skip to content

Commit 13eea94

Browse files
lobsterkatielobsterkatie
authored
ref(nextjs): Use current hub by default when checking if tracing is enabled (#3530)
1 parent bf3c161 commit 13eea94

File tree

5 files changed

+52
-21
lines changed

5 files changed

+52
-21
lines changed

‎packages/tracing/src/browser/request.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { getCurrentHub } from '@sentry/hub';
21
import { addInstrumentationHandler, isInstanceOf, isMatchingPattern } from '@sentry/utils';
32

43
import { Span } from '../span';
@@ -145,13 +144,7 @@ export function fetchCallback(
145144
shouldCreateSpan: (url: string) => boolean,
146145
spans: Record<string, Span>,
147146
): void {
148-
const currentClientOptions = getCurrentHub()
149-
.getClient()
150-
?.getOptions();
151-
if (
152-
!(currentClientOptions && hasTracingEnabled(currentClientOptions)) ||
153-
!(handlerData.fetchData && shouldCreateSpan(handlerData.fetchData.url))
154-
) {
147+
if (!hasTracingEnabled() || !(handlerData.fetchData && shouldCreateSpan(handlerData.fetchData.url))) {
155148
return;
156149
}
157150

@@ -219,13 +212,10 @@ export function xhrCallback(
219212
shouldCreateSpan: (url: string) => boolean,
220213
spans: Record<string, Span>,
221214
): void {
222-
const currentClientOptions = getCurrentHub()
223-
.getClient()
224-
?.getOptions();
225215
if (
226-
!(currentClientOptions && hasTracingEnabled(currentClientOptions)) ||
227-
!(handlerData.xhr && handlerData.xhr.__sentry_xhr__ && shouldCreateSpan(handlerData.xhr.__sentry_xhr__.url)) ||
228-
handlerData.xhr.__sentry_own_request__
216+
!hasTracingEnabled() ||
217+
handlerData.xhr?.__sentry_own_request__ ||
218+
!(handlerData.xhr?.__sentry_xhr__ && shouldCreateSpan(handlerData.xhr.__sentry_xhr__.url))
229219
) {
230220
return;
231221
}

‎packages/tracing/src/hubextensions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ function traceHeaders(this: Hub): { [key: string]: string } {
4343
*/
4444
function sample<T extends Transaction>(transaction: T, options: Options, samplingContext: SamplingContext): T {
4545
// nothing to do if tracing is not enabled
46-
if (!hasTracingEnabled(options)) {
46+
if (!hasTracingEnabled()) {
4747
transaction.sampled = false;
4848
return transaction;
4949
}

‎packages/tracing/src/utils.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,14 @@ export const TRACEPARENT_REGEXP = new RegExp(
1414
*
1515
* Tracing is enabled when at least one of `tracesSampleRate` and `tracesSampler` is defined in the SDK config.
1616
*/
17-
export function hasTracingEnabled(options: Options): boolean {
17+
export function hasTracingEnabled(
18+
options: Options | undefined = getCurrentHub()
19+
.getClient()
20+
?.getOptions(),
21+
): boolean {
22+
if (!options) {
23+
return false;
24+
}
1825
return 'tracesSampleRate' in options || 'tracesSampler' in options;
1926
}
2027

‎packages/tracing/test/hub.test.ts‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* eslint-disable @typescript-eslint/unbound-method */
22
import { BrowserClient } from '@sentry/browser';
3-
import { Hub } from '@sentry/hub';
4-
import * as hubModule from '@sentry/hub';
3+
import { Hub, makeMain } from '@sentry/hub';
54
import { TransactionSamplingMethod } from '@sentry/types';
65
import * as utilsModule from '@sentry/utils'; // for mocking
76
import { logger } from '@sentry/utils';
@@ -32,6 +31,7 @@ describe('Hub', () => {
3231
describe('getTransaction()', () => {
3332
it('should find a transaction which has been set on the scope if sampled = true', () => {
3433
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
34+
makeMain(hub);
3535
const transaction = hub.startTransaction({ name: 'dogpark' });
3636
transaction.sampled = true;
3737

@@ -44,6 +44,7 @@ describe('Hub', () => {
4444

4545
it('should find a transaction which has been set on the scope if sampled = false', () => {
4646
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
47+
makeMain(hub);
4748
const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
4849

4950
hub.configureScope(scope => {
@@ -55,6 +56,7 @@ describe('Hub', () => {
5556

5657
it("should not find an open transaction if it's not on the scope", () => {
5758
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
59+
makeMain(hub);
5860
hub.startTransaction({ name: 'dogpark' });
5961

6062
expect(hub.getScope()?.getTransaction()).toBeUndefined();
@@ -66,6 +68,8 @@ describe('Hub', () => {
6668
it('should add transaction context data to default sample context', () => {
6769
const tracesSampler = jest.fn();
6870
const hub = new Hub(new BrowserClient({ tracesSampler }));
71+
makeMain(hub);
72+
6973
const transactionContext = {
7074
name: 'dogpark',
7175
parentSpanId: '12312012',
@@ -80,6 +84,7 @@ describe('Hub', () => {
8084
it("should add parent's sampling decision to default sample context", () => {
8185
const tracesSampler = jest.fn();
8286
const hub = new Hub(new BrowserClient({ tracesSampler }));
87+
makeMain(hub);
8388
const parentSamplingDecsion = false;
8489

8590
hub.startTransaction({
@@ -98,20 +103,23 @@ describe('Hub', () => {
98103
it('should set sampled = false when tracing is disabled', () => {
99104
// neither tracesSampleRate nor tracesSampler is defined -> tracing disabled
100105
const hub = new Hub(new BrowserClient({}));
106+
makeMain(hub);
101107
const transaction = hub.startTransaction({ name: 'dogpark' });
102108

103109
expect(transaction.sampled).toBe(false);
104110
});
105111

106112
it('should set sampled = false if tracesSampleRate is 0', () => {
107113
const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 }));
114+
makeMain(hub);
108115
const transaction = hub.startTransaction({ name: 'dogpark' });
109116

110117
expect(transaction.sampled).toBe(false);
111118
});
112119

113120
it('should set sampled = true if tracesSampleRate is 1', () => {
114121
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
122+
makeMain(hub);
115123
const transaction = hub.startTransaction({ name: 'dogpark' });
116124

117125
expect(transaction.sampled).toBe(true);
@@ -120,6 +128,7 @@ describe('Hub', () => {
120128
it("should call tracesSampler if it's defined", () => {
121129
const tracesSampler = jest.fn();
122130
const hub = new Hub(new BrowserClient({ tracesSampler }));
131+
makeMain(hub);
123132
hub.startTransaction({ name: 'dogpark' });
124133

125134
expect(tracesSampler).toHaveBeenCalled();
@@ -128,6 +137,7 @@ describe('Hub', () => {
128137
it('should set sampled = false if tracesSampler returns 0', () => {
129138
const tracesSampler = jest.fn().mockReturnValue(0);
130139
const hub = new Hub(new BrowserClient({ tracesSampler }));
140+
makeMain(hub);
131141
const transaction = hub.startTransaction({ name: 'dogpark' });
132142

133143
expect(tracesSampler).toHaveBeenCalled();
@@ -137,6 +147,7 @@ describe('Hub', () => {
137147
it('should set sampled = true if tracesSampler returns 1', () => {
138148
const tracesSampler = jest.fn().mockReturnValue(1);
139149
const hub = new Hub(new BrowserClient({ tracesSampler }));
150+
makeMain(hub);
140151
const transaction = hub.startTransaction({ name: 'dogpark' });
141152

142153
expect(tracesSampler).toHaveBeenCalled();
@@ -147,6 +158,7 @@ describe('Hub', () => {
147158
// so that the decision otherwise would be false
148159
const tracesSampler = jest.fn().mockReturnValue(0);
149160
const hub = new Hub(new BrowserClient({ tracesSampler }));
161+
makeMain(hub);
150162
const transaction = hub.startTransaction({ name: 'dogpark', sampled: true });
151163

152164
expect(transaction.sampled).toBe(true);
@@ -156,6 +168,7 @@ describe('Hub', () => {
156168
// so that the decision otherwise would be true
157169
const tracesSampler = jest.fn().mockReturnValue(1);
158170
const hub = new Hub(new BrowserClient({ tracesSampler }));
171+
makeMain(hub);
159172
const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
160173

161174
expect(transaction.sampled).toBe(false);
@@ -165,6 +178,7 @@ describe('Hub', () => {
165178
// make the two options do opposite things to prove precedence
166179
const tracesSampler = jest.fn().mockReturnValue(true);
167180
const hub = new Hub(new BrowserClient({ tracesSampleRate: 0, tracesSampler }));
181+
makeMain(hub);
168182
const transaction = hub.startTransaction({ name: 'dogpark' });
169183

170184
expect(tracesSampler).toHaveBeenCalled();
@@ -174,6 +188,7 @@ describe('Hub', () => {
174188
it('should tolerate tracesSampler returning a boolean', () => {
175189
const tracesSampler = jest.fn().mockReturnValue(true);
176190
const hub = new Hub(new BrowserClient({ tracesSampler }));
191+
makeMain(hub);
177192
const transaction = hub.startTransaction({ name: 'dogpark' });
178193

179194
expect(tracesSampler).toHaveBeenCalled();
@@ -183,6 +198,7 @@ describe('Hub', () => {
183198
it('should record sampling method when sampling decision is explicitly set', () => {
184199
const tracesSampler = jest.fn().mockReturnValue(0.1121);
185200
const hub = new Hub(new BrowserClient({ tracesSampler }));
201+
makeMain(hub);
186202
hub.startTransaction({ name: 'dogpark', sampled: true });
187203

188204
expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({
@@ -193,6 +209,7 @@ describe('Hub', () => {
193209
it('should record sampling method and rate when sampling decision comes from tracesSampler', () => {
194210
const tracesSampler = jest.fn().mockReturnValue(0.1121);
195211
const hub = new Hub(new BrowserClient({ tracesSampler }));
212+
makeMain(hub);
196213
hub.startTransaction({ name: 'dogpark' });
197214

198215
expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({
@@ -202,6 +219,7 @@ describe('Hub', () => {
202219

203220
it('should record sampling method when sampling decision is inherited', () => {
204221
const hub = new Hub(new BrowserClient({ tracesSampleRate: 0.1121 }));
222+
makeMain(hub);
205223
hub.startTransaction({ name: 'dogpark', parentSampled: true });
206224

207225
expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({
@@ -211,6 +229,7 @@ describe('Hub', () => {
211229

212230
it('should record sampling method and rate when sampling decision comes from traceSampleRate', () => {
213231
const hub = new Hub(new BrowserClient({ tracesSampleRate: 0.1121 }));
232+
makeMain(hub);
214233
hub.startTransaction({ name: 'dogpark' });
215234

216235
expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({
@@ -222,13 +241,15 @@ describe('Hub', () => {
222241
describe('isValidSampleRate()', () => {
223242
it("should reject tracesSampleRates which aren't numbers or booleans", () => {
224243
const hub = new Hub(new BrowserClient({ tracesSampleRate: 'dogs!' as any }));
244+
makeMain(hub);
225245
hub.startTransaction({ name: 'dogpark' });
226246

227247
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
228248
});
229249

230250
it('should reject tracesSampleRates which are NaN', () => {
231251
const hub = new Hub(new BrowserClient({ tracesSampleRate: 'dogs!' as any }));
252+
makeMain(hub);
232253
hub.startTransaction({ name: 'dogpark' });
233254

234255
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
@@ -237,6 +258,7 @@ describe('Hub', () => {
237258
// the rate might be a boolean, but for our purposes, false is equivalent to 0 and true is equivalent to 1
238259
it('should reject tracesSampleRates less than 0', () => {
239260
const hub = new Hub(new BrowserClient({ tracesSampleRate: -26 }));
261+
makeMain(hub);
240262
hub.startTransaction({ name: 'dogpark' });
241263

242264
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be between 0 and 1'));
@@ -245,6 +267,7 @@ describe('Hub', () => {
245267
// the rate might be a boolean, but for our purposes, false is equivalent to 0 and true is equivalent to 1
246268
it('should reject tracesSampleRates greater than 1', () => {
247269
const hub = new Hub(new BrowserClient({ tracesSampleRate: 26 }));
270+
makeMain(hub);
248271
hub.startTransaction({ name: 'dogpark' });
249272

250273
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be between 0 and 1'));
@@ -253,6 +276,7 @@ describe('Hub', () => {
253276
it("should reject tracesSampler return values which aren't numbers or booleans", () => {
254277
const tracesSampler = jest.fn().mockReturnValue('dogs!');
255278
const hub = new Hub(new BrowserClient({ tracesSampler }));
279+
makeMain(hub);
256280
hub.startTransaction({ name: 'dogpark' });
257281

258282
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
@@ -261,6 +285,7 @@ describe('Hub', () => {
261285
it('should reject tracesSampler return values which are NaN', () => {
262286
const tracesSampler = jest.fn().mockReturnValue(NaN);
263287
const hub = new Hub(new BrowserClient({ tracesSampler }));
288+
makeMain(hub);
264289
hub.startTransaction({ name: 'dogpark' });
265290

266291
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be a boolean or a number'));
@@ -270,6 +295,7 @@ describe('Hub', () => {
270295
it('should reject tracesSampler return values less than 0', () => {
271296
const tracesSampler = jest.fn().mockReturnValue(-12);
272297
const hub = new Hub(new BrowserClient({ tracesSampler }));
298+
makeMain(hub);
273299
hub.startTransaction({ name: 'dogpark' });
274300

275301
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be between 0 and 1'));
@@ -279,6 +305,7 @@ describe('Hub', () => {
279305
it('should reject tracesSampler return values greater than 1', () => {
280306
const tracesSampler = jest.fn().mockReturnValue(31);
281307
const hub = new Hub(new BrowserClient({ tracesSampler }));
308+
makeMain(hub);
282309
hub.startTransaction({ name: 'dogpark' });
283310

284311
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('Sample rate must be between 0 and 1'));
@@ -290,6 +317,7 @@ describe('Hub', () => {
290317
jest.spyOn(client, 'captureEvent');
291318

292319
const hub = new Hub(client);
320+
makeMain(hub);
293321
const transaction = hub.startTransaction({ name: 'dogpark' });
294322

295323
jest.spyOn(transaction, 'finish');
@@ -303,6 +331,7 @@ describe('Hub', () => {
303331
describe('sampling inheritance', () => {
304332
it('should propagate sampling decision to child spans', () => {
305333
const hub = new Hub(new BrowserClient({ tracesSampleRate: Math.random() }));
334+
makeMain(hub);
306335
const transaction = hub.startTransaction({ name: 'dogpark' });
307336
const child = transaction.startChild({ op: 'ball.chase' });
308337

@@ -320,7 +349,7 @@ describe('Hub', () => {
320349
integrations: [new BrowserTracing()],
321350
}),
322351
);
323-
jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
352+
makeMain(hub);
324353

325354
const transaction = hub.startTransaction({ name: 'dogpark' });
326355
hub.configureScope(scope => {
@@ -362,7 +391,7 @@ describe('Hub', () => {
362391
integrations: [new BrowserTracing()],
363392
}),
364393
);
365-
jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub);
394+
makeMain(hub);
366395

367396
const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
368397
hub.configureScope(scope => {
@@ -407,6 +436,7 @@ describe('Hub', () => {
407436
// tracesSampleRate
408437
mathRandom.mockReturnValueOnce(1);
409438
const hub = new Hub(new BrowserClient({ tracesSampleRate: 0.5 }));
439+
makeMain(hub);
410440
const parentSamplingDecsion = true;
411441

412442
const transaction = hub.startTransaction({
@@ -422,6 +452,7 @@ describe('Hub', () => {
422452
// tracesSampleRate = 1 means every transaction should end up with sampled = true, so make parent's decision the
423453
// opposite to prove that inheritance takes precedence over tracesSampleRate
424454
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
455+
makeMain(hub);
425456
const parentSamplingDecsion = false;
426457

427458
const transaction = hub.startTransaction({
@@ -440,6 +471,7 @@ describe('Hub', () => {
440471
const parentSamplingDecsion = false;
441472

442473
const hub = new Hub(new BrowserClient({ tracesSampler }));
474+
makeMain(hub);
443475

444476
const transaction = hub.startTransaction({
445477
name: 'dogpark',
@@ -457,6 +489,7 @@ describe('Hub', () => {
457489
const parentSamplingDecsion = true;
458490

459491
const hub = new Hub(new BrowserClient({ tracesSampler }));
492+
makeMain(hub);
460493

461494
const transaction = hub.startTransaction({
462495
name: 'dogpark',

‎packages/tracing/test/span.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { BrowserClient } from '@sentry/browser';
2-
import { Hub, Scope } from '@sentry/hub';
2+
import { Hub, makeMain, Scope } from '@sentry/hub';
33

44
import { Span, SpanStatus, Transaction } from '../src';
55
import { TRACEPARENT_REGEXP } from '../src/utils';
@@ -10,6 +10,7 @@ describe('Span', () => {
1010
beforeEach(() => {
1111
const myScope = new Scope();
1212
hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }), myScope);
13+
makeMain(hub);
1314
});
1415

1516
describe('new Span', () => {

0 commit comments

Comments
 (0)