Skip to content
Merged
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,7 +9,9 @@
- [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)
- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls`
- [tracing] feat: Add `scope.getTransaction` to return a Transaction if it exists (#2668)
- [tracing] ref: Deprecate `scope.setTransaction` in favor of `scope.setTransactionName` (#2668)
- [core] ref: Rename `whitelistUrls/blacklistUrls` to `allowUrls/denyUrls` (#2671)
- [react] ref: Refactor Profiler to account for update and render (#2677)
- [tracing] feat: Add ability to get span from activity using `getActivitySpan` (#2677)

Expand Down
22 changes: 22 additions & 0 deletions packages/apm/test/hub.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,28 @@ describe('Hub', () => {
jest.useRealTimers();
});

describe('getTransaction', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These tests are not technically "correct". defined/undefined test should be moved to scope.test.ts, as this functionality is owned by the Scope, and tests below should only assert that a given method was called on the scope. expect(s.getTransaction).toBeCalled().

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Moved to scope.test.ts

test('simple invoke', () => {
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
const transaction = hub.startTransaction({ name: 'foo' });
hub.configureScope(scope => {
scope.setSpan(transaction);
});
hub.configureScope(s => {
expect(s.getTransaction()).toBe(transaction);
});
});

test('not invoke', () => {
const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
const transaction = hub.startTransaction({ name: 'foo' });
hub.configureScope(s => {
expect(s.getTransaction()).toBeUndefined();
});
transaction.finish();
});
});

describe('spans', () => {
describe('sampling', () => {
test('set tracesSampleRate 0 on span', () => {
Expand Down
39 changes: 29 additions & 10 deletions packages/hub/src/scope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ import {
ScopeContext,
Severity,
Span,
Transaction,
User,
} from '@sentry/types';
import { getGlobalObject, isPlainObject, isThenable, SyncPromise, timestampWithMs } from '@sentry/utils';
Expand DownExpand Up@@ -47,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 DownExpand Up@@ -185,12 +186,20 @@ export class Scope implements ScopeInterface {
/**
* @inheritDoc
*/
public setTransaction(transaction?: string): this {
this._transaction = transaction;
public setTransactionName(name?: string): this {

Copy link
Copy Markdown
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.
Comment thread
HazAT marked this conversation as resolved.
* @deprecated in favor of {@link this.setTransactionName}
*/
public setTransaction(name?: string): this {

Copy link
Copy Markdown
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 All@@ -210,13 +219,23 @@ export class Scope implements ScopeInterface {
}

/**
* Internal getter for Span, used in Hub.
* @hidden
* @inheritDoc
*/
public getSpan(): Span | undefined {
return this._span;
}

/**
* @inheritDoc
*/
public getTransaction(): Transaction | undefined {
Comment thread
HazAT marked this conversation as resolved.
const span = this.getSpan() as Span & { spanRecorder: { spans: Span[] } };
if (span && span.spanRecorder && span.spanRecorder.spans[0]) {
return span.spanRecorder.spans[0] as Transaction;
}
return undefined;
}

/**
* Inherit values from the parent scope.
* @param scope to clone.
Expand All@@ -231,7 +250,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 DownExpand Up@@ -294,7 +313,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 DownExpand Up@@ -374,8 +393,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
Original file line numberDiff line numberDiff line change
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 DownExpand 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 DownExpand 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
17 changes: 8 additions & 9 deletions packages/node/src/integrations/http.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -82,15 +82,14 @@ function createHandlerWrapper(
let transaction: Transaction | undefined;

const scope = getCurrentHub().getScope();
if (scope) {
transaction = scope.getSpan() as Transaction;
}

if (tracingEnabled && transaction) {
span = transaction.startChild({
description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
op: 'request',
});
if (scope && tracingEnabled) {
transaction = scope.getTransaction();
if (transaction) {
span = transaction.startChild({
description: `${typeof options === 'string' || !options.method ? 'GET' : options.method} ${requestUrl}`,
op: 'request',
});
}
}

return originalHandler
Expand Down
16 changes: 13 additions & 3 deletions packages/types/src/scope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { Breadcrumb } from './breadcrumb';
import { EventProcessor } from './eventprocessor';
import { Severity } from './severity';
import { Span } from './span';
import { Transaction } from './transaction';
import { User } from './user';

/** JSDocs */
Expand DownExpand Up@@ -71,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 All@@ -89,6 +89,16 @@ export interface Scope {
*/
setSpan(span?: Span): this;

/**
* Returns the `Span` if there is one
*/
getSpan(): Span | undefined;
Comment thread
HazAT marked this conversation as resolved.

/**
* Returns the `Transaction` if there is one
*/
getTransaction(): Transaction | undefined;

/**
* Updates the scope with provided data. Can work in three variations:
* - plain object containing updatable attributes
Expand Down