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: Add scope.getTransaction, rename scope.setTransaction -> setTransactionName #2668

Merged
merged 16 commits into from Jun 22, 2020
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -9,7 +9,8 @@
- [core] feat: Export `makeMain` (#2665)
- [core] fix: Call `bindClient` when creating new `Hub` to make integrations work automatically (#2665)
- [gatsby] feat: Add @sentry/gatsby package (#2652)
- [apm] feat: Add `Sentry.getSpan` to return the Span on the Scope (#2668)
- [apm] feat: Add `scope.getTransaction` to return a Transaction if it exists (#2668)
HazAT marked this conversation as resolved.
Show resolved Hide resolved
- [apm] ref: Depracate `scope.setTransaction` in favor of `scope.setTransactionName` (#2668)
HazAT marked this conversation as resolved.
Show resolved Hide resolved
HazAT marked this conversation as resolved.
Show resolved Hide resolved
- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` (#2671)

## 5.17.0
Expand Down
24 changes: 16 additions & 8 deletions packages/hub/src/scope.ts
Expand Up @@ -48,8 +48,8 @@ export class Scope implements ScopeInterface {
/** Severity */
protected _level?: Severity;

/** Transaction */
protected _transaction?: string;
/** Transaction Name */
protected _transactionName?: string;

/** Span */
protected _span?: Span;
Expand Down Expand Up @@ -186,12 +186,20 @@ export class Scope implements ScopeInterface {
/**
* @inheritDoc
*/
public setTransaction(transaction?: string): this {
this._transaction = transaction;
public setTransactionName(name?: string): this {
Copy link
Contributor

Choose a reason for hiding this comment

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

offtopic: This whole transaction thing will be very confused (and possibly broken) if someone uses Sentry.Integrations.Transaction in node env.

this._transactionName = name;
this._notifyScopeListeners();
return this;
}

/**
* Can be removed in major version.
HazAT marked this conversation as resolved.
Show resolved Hide resolved
* @deprecated in favor of {@link this.setTransactionName}
*/
public setTransaction(name?: string): this {
Copy link
Contributor

Choose a reason for hiding this comment

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

kilobytes, kilobytes 😶

return this.setTransactionName(name);
}

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -244,7 +252,7 @@ export class Scope implements ScopeInterface {
newScope._user = scope._user;
newScope._level = scope._level;
newScope._span = scope._span;
newScope._transaction = scope._transaction;
newScope._transactionName = scope._transactionName;
newScope._fingerprint = scope._fingerprint;
newScope._eventProcessors = [...scope._eventProcessors];
}
Expand Down Expand Up @@ -307,7 +315,7 @@ export class Scope implements ScopeInterface {
this._user = {};
this._contexts = {};
this._level = undefined;
this._transaction = undefined;
this._transactionName = undefined;
this._fingerprint = undefined;
this._span = undefined;
this._notifyScopeListeners();
Expand Down Expand Up @@ -387,8 +395,8 @@ export class Scope implements ScopeInterface {
if (this._level) {
event.level = this._level;
}
if (this._transaction) {
event.transaction = this._transaction;
if (this._transactionName) {
event.transaction = this._transactionName;
}
// We want to set the trace context for normal events only if there isn't already
// a trace context on the event. There is a product feature in place where we link
Expand Down
18 changes: 9 additions & 9 deletions packages/hub/test/scope.test.ts
Expand Up @@ -73,17 +73,17 @@ describe('Scope', () => {
expect((scope as any)._level).toEqual(Severity.Critical);
});

test('setTransaction', () => {
test('setTransactionName', () => {
const scope = new Scope();
scope.setTransaction('/abc');
expect((scope as any)._transaction).toEqual('/abc');
scope.setTransactionName('/abc');
expect((scope as any)._transactionName).toEqual('/abc');
});

test('setTransaction with no value unsets it', () => {
test('setTransactionName with no value unsets it', () => {
const scope = new Scope();
scope.setTransaction('/abc');
scope.setTransaction();
expect((scope as any)._transaction).toBeUndefined();
scope.setTransactionName('/abc');
scope.setTransactionName();
expect((scope as any)._transactionName).toBeUndefined();
});

test('setContext', () => {
Expand Down Expand Up @@ -157,7 +157,7 @@ describe('Scope', () => {
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.setLevel(Severity.Warning);
scope.setTransaction('/abc');
scope.setTransactionName('/abc');
scope.addBreadcrumb({ message: 'test' }, 100);
scope.setContext('os', { id: '1' });
const event: Event = {};
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('Scope', () => {
test('scope transaction should have priority over event transaction', () => {
expect.assertions(1);
const scope = new Scope();
scope.setTransaction('/abc');
scope.setTransactionName('/abc');
const event: Event = {};
event.transaction = '/cdf';
return scope.applyToEvent(event).then(processedEvent => {
Expand Down
5 changes: 2 additions & 3 deletions packages/types/src/scope.ts
Expand Up @@ -72,10 +72,9 @@ export interface Scope {
setLevel(level: Severity): this;

/**
* Sets the transaction on the scope for future events.
* @param transaction string This will be converted in a tag in Sentry
* Sets the transaction name on the scope for future events.
*/
setTransaction(transaction?: string): this;
setTransactionName(name?: string): this;

/**
* Sets context data with the given name.
Expand Down