Skip to content

Commit eb4846e

Browse files
HazATAbhiPrasad
andauthored
feat: Add scope.getTransaction, rename scope.setTransaction -> setTransactionName (#2668)
* feat: Add Sentry.getTransaction * fix: Linter * ref: Introduce Sentry.getSpan * ref: Remove getSpan and use scope.getTransaction * fix: Linter * ref: Rename setTransaction * Update CHANGELOG.md Co-authored-by: Abhijeet Prasad <[email protected]> * ref: CodeReview * fix: Node * ref: CR * ref: Remove unused code * ref: Code Review * ref: Revert tests Co-authored-by: Abhijeet Prasad <[email protected]>
1 parent 7f74693 commit eb4846e

File tree

6 files changed

+84
-32
lines changed

6 files changed

+84
-32
lines changed

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
- [core] feat: Export `makeMain` (#2665)
1010
- [core] fix: Call `bindClient` when creating new `Hub` to make integrations work automatically (#2665)
1111
- [gatsby] feat: Add @sentry/gatsby package (#2652)
12-
- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls`
12+
- [tracing] feat: Add `scope.getTransaction` to return a Transaction if it exists (#2668)
13+
- [tracing] ref: Deprecate `scope.setTransaction` in favor of `scope.setTransactionName` (#2668)
14+
- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` (#2671)
1315
- [react] ref: Refactor Profiler to account for update and render (#2677)
1416
- [apm] feat: Add ability to get span from activity using `getActivitySpan` (#2677)
1517
- [apm] fix: Check if `performance.mark` exists before calling it (#2680)

packages/apm/test/hub.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,28 @@ describe('Hub', () => {
1111
jest.useRealTimers();
1212
});
1313

14+
describe('getTransaction', () => {
15+
test('simple invoke', () => {
16+
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
17+
const transaction = hub.startTransaction({ name: 'foo' });
18+
hub.configureScope(scope => {
19+
scope.setSpan(transaction);
20+
});
21+
hub.configureScope(s => {
22+
expect(s.getTransaction()).toBe(transaction);
23+
});
24+
});
25+
26+
test('not invoke', () => {
27+
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
28+
const transaction = hub.startTransaction({ name: 'foo' });
29+
hub.configureScope(s => {
30+
expect(s.getTransaction()).toBeUndefined();
31+
});
32+
transaction.finish();
33+
});
34+
});
35+
1436
describe('spans', () => {
1537
describe('sampling', () => {
1638
test('set tracesSampleRate 0 on span', () => {

packages/hub/src/scope.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
ScopeContext,
99
Severity,
1010
Span,
11+
Transaction,
1112
User,
1213
} from '@sentry/types';
1314
import { getGlobalObject, isPlainObject, isThenable, SyncPromise, timestampWithMs } from '@sentry/utils';
@@ -47,8 +48,8 @@ export class Scope implements ScopeInterface {
4748
/** Severity */
4849
protected _level?: Severity;
4950

50-
/** Transaction */
51-
protected _transaction?: string;
51+
/** Transaction Name */
52+
protected _transactionName?: string;
5253

5354
/** Span */
5455
protected _span?: Span;
@@ -185,12 +186,20 @@ export class Scope implements ScopeInterface {
185186
/**
186187
* @inheritDoc
187188
*/
188-
public setTransaction(transaction?: string): this {
189-
this._transaction = transaction;
189+
public setTransactionName(name?: string): this {
190+
this._transactionName = name;
190191
this._notifyScopeListeners();
191192
return this;
192193
}
193194

195+
/**
196+
* Can be removed in major version.
197+
* @deprecated in favor of {@link this.setTransactionName}
198+
*/
199+
public setTransaction(name?: string): this {
200+
return this.setTransactionName(name);
201+
}
202+
194203
/**
195204
* @inheritDoc
196205
*/
@@ -210,13 +219,23 @@ export class Scope implements ScopeInterface {
210219
}
211220

212221
/**
213-
* Internal getter for Span, used in Hub.
214-
* @hidden
222+
* @inheritDoc
215223
*/
216224
public getSpan(): Span | undefined {
217225
return this._span;
218226
}
219227

228+
/**
229+
* @inheritDoc
230+
*/
231+
public getTransaction(): Transaction | undefined {
232+
const span = this.getSpan() as Span & { spanRecorder: { spans: Span[] } };
233+
if (span && span.spanRecorder && span.spanRecorder.spans[0]) {
234+
return span.spanRecorder.spans[0] as Transaction;
235+
}
236+
return undefined;
237+
}
238+
220239
/**
221240
* Inherit values from the parent scope.
222241
* @param scope to clone.
@@ -231,7 +250,7 @@ export class Scope implements ScopeInterface {
231250
newScope._user = scope._user;
232251
newScope._level = scope._level;
233252
newScope._span = scope._span;
234-
newScope._transaction = scope._transaction;
253+
newScope._transactionName = scope._transactionName;
235254
newScope._fingerprint = scope._fingerprint;
236255
newScope._eventProcessors = [...scope._eventProcessors];
237256
}
@@ -294,7 +313,7 @@ export class Scope implements ScopeInterface {
294313
this._user = {};
295314
this._contexts = {};
296315
this._level = undefined;
297-
this._transaction = undefined;
316+
this._transactionName = undefined;
298317
this._fingerprint = undefined;
299318
this._span = undefined;
300319
this._notifyScopeListeners();
@@ -374,8 +393,8 @@ export class Scope implements ScopeInterface {
374393
if (this._level) {
375394
event.level = this._level;
376395
}
377-
if (this._transaction) {
378-
event.transaction = this._transaction;
396+
if (this._transactionName) {
397+
event.transaction = this._transactionName;
379398
}
380399
// We want to set the trace context for normal events only if there isn't already
381400
// a trace context on the event. There is a product feature in place where we link

packages/hub/test/scope.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,17 @@ describe('Scope', () => {
7373
expect((scope as any)._level).toEqual(Severity.Critical);
7474
});
7575

76-
test('setTransaction', () => {
76+
test('setTransactionName', () => {
7777
const scope = new Scope();
78-
scope.setTransaction('/abc');
79-
expect((scope as any)._transaction).toEqual('/abc');
78+
scope.setTransactionName('/abc');
79+
expect((scope as any)._transactionName).toEqual('/abc');
8080
});
8181

82-
test('setTransaction with no value unsets it', () => {
82+
test('setTransactionName with no value unsets it', () => {
8383
const scope = new Scope();
84-
scope.setTransaction('/abc');
85-
scope.setTransaction();
86-
expect((scope as any)._transaction).toBeUndefined();
84+
scope.setTransactionName('/abc');
85+
scope.setTransactionName();
86+
expect((scope as any)._transactionName).toBeUndefined();
8787
});
8888

8989
test('setContext', () => {
@@ -157,7 +157,7 @@ describe('Scope', () => {
157157
scope.setUser({ id: '1' });
158158
scope.setFingerprint(['abcd']);
159159
scope.setLevel(Severity.Warning);
160-
scope.setTransaction('/abc');
160+
scope.setTransactionName('/abc');
161161
scope.addBreadcrumb({ message: 'test' }, 100);
162162
scope.setContext('os', { id: '1' });
163163
const event: Event = {};
@@ -256,7 +256,7 @@ describe('Scope', () => {
256256
test('scope transaction should have priority over event transaction', () => {
257257
expect.assertions(1);
258258
const scope = new Scope();
259-
scope.setTransaction('/abc');
259+
scope.setTransactionName('/abc');
260260
const event: Event = {};
261261
event.transaction = '/cdf';
262262
return scope.applyToEvent(event).then(processedEvent => {

packages/node/src/integrations/http.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,14 @@ function createHandlerWrapper(
8282
let transaction: Transaction | undefined;
8383

8484
const scope = getCurrentHub().getScope();
85-
if (scope) {
86-
transaction = scope.getSpan() as Transaction;
87-
}
88-
89-
if (tracingEnabled && transaction) {
90-
span = transaction.startChild({
91-
description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
92-
op: 'request',
93-
});
85+
if (scope && tracingEnabled) {
86+
transaction = scope.getTransaction();
87+
if (transaction) {
88+
span = transaction.startChild({
89+
description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
90+
op: 'request',
91+
});
92+
}
9493
}
9594

9695
return originalHandler

packages/types/src/scope.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Breadcrumb } from './breadcrumb';
22
import { EventProcessor } from './eventprocessor';
33
import { Severity } from './severity';
44
import { Span } from './span';
5+
import { Transaction } from './transaction';
56
import { User } from './user';
67

78
/** JSDocs */
@@ -71,10 +72,9 @@ export interface Scope {
7172
setLevel(level: Severity): this;
7273

7374
/**
74-
* Sets the transaction on the scope for future events.
75-
* @param transaction string This will be converted in a tag in Sentry
75+
* Sets the transaction name on the scope for future events.
7676
*/
77-
setTransaction(transaction?: string): this;
77+
setTransactionName(name?: string): this;
7878

7979
/**
8080
* Sets context data with the given name.
@@ -89,6 +89,16 @@ export interface Scope {
8989
*/
9090
setSpan(span?: Span): this;
9191

92+
/**
93+
* Returns the `Span` if there is one
94+
*/
95+
getSpan(): Span | undefined;
96+
97+
/**
98+
* Returns the `Transaction` if there is one
99+
*/
100+
getTransaction(): Transaction | undefined;
101+
92102
/**
93103
* Updates the scope with provided data. Can work in three variations:
94104
* - plain object containing updatable attributes

0 commit comments

Comments
 (0)