diff --git a/packages/integrations/src/vue.ts b/packages/integrations/src/vue.ts index 6d62d811ca5e..dc9a176ccebf 100644 --- a/packages/integrations/src/vue.ts +++ b/packages/integrations/src/vue.ts @@ -1,14 +1,6 @@ -import { EventProcessor, Hub, Integration, IntegrationClass, Span } from '@sentry/types'; +import { EventProcessor, Hub, Integration, Scope, Span, Transaction } from '@sentry/types'; import { basename, getGlobalObject, logger, timestampWithMs } from '@sentry/utils'; -/** - * Used to extract Tracing integration from the current client, - * without the need to import `Tracing` itself from the @sentry/apm package. - */ -const TRACING_GETTER = ({ - id: 'Tracing', -} as any) as IntegrationClass; - /** Global Vue object limited to the methods/attributes we require */ interface VueInstance { config: { @@ -71,7 +63,7 @@ interface TracingOptions { * Or to an array of specific component names (case-sensitive). */ trackComponents: boolean | string[]; - /** How long to wait until the tracked root activity is marked as finished and sent of to Sentry */ + /** How long to wait until the tracked root span is marked as finished and sent of to Sentry */ timeout: number; /** * List of hooks to keep track of during component lifecycle. @@ -137,7 +129,6 @@ export class Vue implements Integration { private readonly _componentsCache: { [key: string]: string } = {}; private _rootSpan?: Span; private _rootSpanTimer?: ReturnType; - private _tracingActivity?: number; /** * @inheritDoc @@ -221,27 +212,18 @@ export class Vue implements Integration { // On the first handler call (before), it'll be undefined, as `$once` will add it in the future. // However, on the second call (after), it'll be already in place. if (this._rootSpan) { - this._finishRootSpan(now, getCurrentHub); + this._finishRootSpan(now); } else { vm.$once(`hook:${hook}`, () => { - // Create an activity on the first event call. There'll be no second call, as rootSpan will be in place, + // Create an span on the first event call. There'll be no second call, as rootSpan will be in place, // thus new event handler won't be attached. - // We do this whole dance with `TRACING_GETTER` to prevent `@sentry/apm` from becoming a peerDependency. - // We also need to ask for the `.constructor`, as `pushActivity` and `popActivity` are static, not instance methods. - const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); - if (tracingIntegration) { - // tslint:disable-next-line:no-unsafe-any - this._tracingActivity = (tracingIntegration as any).constructor.pushActivity('Vue Application Render'); - // tslint:disable-next-line:no-unsafe-any - const transaction = (tracingIntegration as any).constructor.getTransaction(); - if (transaction) { - // tslint:disable-next-line:no-unsafe-any - this._rootSpan = transaction.startChild({ - description: 'Application Render', - op: 'Vue', - }); - } + const activeTransaction = getActiveTransaction(getCurrentHub()); + if (activeTransaction) { + this._rootSpan = activeTransaction.startChild({ + description: 'Application Render', + op: 'Vue', + }); } }); } @@ -264,7 +246,7 @@ export class Vue implements Integration { // However, on the second call (after), it'll be already in place. if (span) { span.finish(); - this._finishRootSpan(now, getCurrentHub); + this._finishRootSpan(now); } else { vm.$once(`hook:${hook}`, () => { if (this._rootSpan) { @@ -305,24 +287,15 @@ export class Vue implements Integration { }); }; - /** Finish top-level span and activity with a debounce configured using `timeout` option */ - private _finishRootSpan(timestamp: number, getCurrentHub: () => Hub): void { + /** Finish top-level span with a debounce configured using `timeout` option */ + private _finishRootSpan(timestamp: number): void { if (this._rootSpanTimer) { clearTimeout(this._rootSpanTimer); } this._rootSpanTimer = setTimeout(() => { - if (this._tracingActivity) { - // We do this whole dance with `TRACING_GETTER` to prevent `@sentry/apm` from becoming a peerDependency. - // We also need to ask for the `.constructor`, as `pushActivity` and `popActivity` are static, not instance methods. - const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); - if (tracingIntegration) { - // tslint:disable-next-line:no-unsafe-any - (tracingIntegration as any).constructor.popActivity(this._tracingActivity); - if (this._rootSpan) { - this._rootSpan.finish(timestamp); - } - } + if (this._rootSpan) { + this._rootSpan.finish(timestamp); } }, this._options.tracingOptions.timeout); } @@ -333,7 +306,7 @@ export class Vue implements Integration { this._options.Vue.mixin({ beforeCreate(this: ViewModel): void { - if (getCurrentHub().getIntegration(TRACING_GETTER)) { + if (getActiveTransaction(getCurrentHub())) { // `this` points to currently rendered component applyTracingHooks(this, getCurrentHub); } else { @@ -405,3 +378,21 @@ export class Vue implements Integration { } } } + +// tslint:disable-next-line: completed-docs +interface HubType extends Hub { + // tslint:disable-next-line: completed-docs + getScope?(): Scope | undefined; +} + +/** Grabs active transaction off scope */ +export function getActiveTransaction(hub: HubType): T | undefined { + if (hub && hub.getScope) { + const scope = hub.getScope() as Scope; + if (scope) { + return scope.getTransaction() as T | undefined; + } + } + + return undefined; +} diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 2af9dbaab86d..ccfcb4864b9c 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,81 +1,11 @@ -import { getCurrentHub } from '@sentry/browser'; -import { Integration, IntegrationClass, Span } from '@sentry/types'; -import { logger, timestampWithMs } from '@sentry/utils'; +import { getCurrentHub, Hub } from '@sentry/browser'; +import { Span, Transaction } from '@sentry/types'; +import { timestampWithMs } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; export const UNKNOWN_COMPONENT = 'unknown'; -const TRACING_GETTER = ({ - id: 'Tracing', -} as any) as IntegrationClass; - -let globalTracingIntegration: Integration | null = null; -const getTracingIntegration = () => { - if (globalTracingIntegration) { - return globalTracingIntegration; - } - - globalTracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); - return globalTracingIntegration; -}; - -/** - * Warn if tracing integration not configured. Will only warn once. - */ -function warnAboutTracing(name: string): void { - if (globalTracingIntegration === null) { - logger.warn( - `Unable to profile component ${name} due to invalid Tracing Integration. Please make sure the Tracing integration is setup properly.`, - ); - } -} - -/** - * pushActivity creates an new react activity. - * Is a no-op if Tracing integration is not valid - * @param name displayName of component that started activity - */ -function pushActivity(name: string, op: string): number | null { - if (globalTracingIntegration === null) { - return null; - } - - // tslint:disable-next-line:no-unsafe-any - return (globalTracingIntegration as any).constructor.pushActivity(name, { - description: `<${name}>`, - op: `react.${op}`, - }); -} - -/** - * popActivity removes a React activity. - * Is a no-op if Tracing integration is not valid. - * @param activity id of activity that is being popped - */ -function popActivity(activity: number | null): void { - if (activity === null || globalTracingIntegration === null) { - return; - } - - // tslint:disable-next-line:no-unsafe-any - (globalTracingIntegration as any).constructor.popActivity(activity); -} - -/** - * Obtain a span given an activity id. - * Is a no-op if Tracing integration is not valid. - * @param activity activity id associated with obtained span - */ -function getActivitySpan(activity: number | null): Span | undefined { - if (activity === null || globalTracingIntegration === null) { - return undefined; - } - - // tslint:disable-next-line:no-unsafe-any - return (globalTracingIntegration as any).constructor.getActivitySpan(activity) as Span | undefined; -} - export type ProfilerProps = { // The name of the component being profiled. name: string; @@ -95,12 +25,8 @@ export type ProfilerProps = { * spans based on component lifecycles. */ class Profiler extends React.Component { - // The activity representing how long it takes to mount a component. - public mountActivity: number | null = null; - // The span of the mount activity + // The span representing how long it takes to mount a component public mountSpan: Span | undefined = undefined; - // The span of the render - public renderSpan: Span | undefined = undefined; public static defaultProps: Partial = { disabled: false, @@ -116,18 +42,20 @@ class Profiler extends React.Component { return; } - if (getTracingIntegration()) { - this.mountActivity = pushActivity(name, 'mount'); - } else { - warnAboutTracing(name); + const activeTransaction = getActiveTransaction(); + if (activeTransaction) { + this.mountSpan = activeTransaction.startChild({ + description: `<${name}>`, + op: 'react.mount', + }); } } // If a component mounted, we can finish the mount activity. public componentDidMount(): void { - this.mountSpan = getActivitySpan(this.mountActivity); - popActivity(this.mountActivity); - this.mountActivity = null; + if (this.mountSpan) { + this.mountSpan.finish(); + } } public componentDidUpdate({ updateProps, includeUpdates = true }: ProfilerProps): void { @@ -221,22 +149,26 @@ function useProfiler( hasRenderSpan: true, }, ): void { - const [mountActivity] = React.useState(() => { + const [mountSpan] = React.useState(() => { if (options && options.disabled) { - return null; + return undefined; } - if (getTracingIntegration()) { - return pushActivity(name, 'mount'); + const activeTransaction = getActiveTransaction(); + if (activeTransaction) { + return activeTransaction.startChild({ + description: `<${name}>`, + op: 'react.mount', + }); } - warnAboutTracing(name); - return null; + return undefined; }); React.useEffect(() => { - const mountSpan = getActivitySpan(mountActivity); - popActivity(mountActivity); + if (mountSpan) { + mountSpan.finish(); + } return () => { if (mountSpan && options.hasRenderSpan) { @@ -252,3 +184,15 @@ function useProfiler( } export { withProfiler, Profiler, useProfiler }; + +/** Grabs active transaction off scope */ +export function getActiveTransaction(hub: Hub = getCurrentHub()): T | undefined { + if (hub) { + const scope = hub.getScope(); + if (scope) { + return scope.getTransaction() as T | undefined; + } + } + + return undefined; +} diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx index 60c2fd312349..4783934b4e74 100644 --- a/packages/react/test/profiler.test.tsx +++ b/packages/react/test/profiler.test.tsx @@ -5,51 +5,36 @@ import * as React from 'react'; import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler'; -const TEST_SPAN_ID = '518999beeceb49af'; -const TEST_TIMESTAMP = '123456'; - const mockStartChild = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs })); -const mockPushActivity = jest.fn().mockReturnValue(1); -const mockPopActivity = jest.fn(); -const mockLoggerWarn = jest.fn(); -const mockGetActivitySpan = jest.fn().mockReturnValue({ - spanId: TEST_SPAN_ID, - startChild: mockStartChild, -}); +const mockFinish = jest.fn(); -jest.mock('@sentry/utils', () => ({ - logger: { - warn: (message: string) => { - mockLoggerWarn(message); - }, - }, - timestampWithMs: () => TEST_TIMESTAMP, -})); +// @sent +class MockSpan { + public constructor(public readonly ctx: SpanContext) {} + + public startChild(ctx: SpanContext): MockSpan { + mockStartChild(ctx); + return new MockSpan(ctx); + } + + public finish(): void { + mockFinish(); + } +} + +let activeTransaction: Record; jest.mock('@sentry/browser', () => ({ getCurrentHub: () => ({ - getIntegration: (_: string) => { - class MockIntegration { - public constructor(name: string) { - this.name = name; - } - public name: string; - public setupOnce: () => void = jest.fn(); - public static pushActivity: () => void = mockPushActivity; - public static popActivity: () => void = mockPopActivity; - public static getActivitySpan: () => void = mockGetActivitySpan; - } - return new MockIntegration('test'); - }, + getScope: () => ({ + getTransaction: () => activeTransaction, + }), }), })); beforeEach(() => { - mockPushActivity.mockClear(); - mockPopActivity.mockClear(); - mockLoggerWarn.mockClear(); - mockGetActivitySpan.mockClear(); mockStartChild.mockClear(); + activeTransaction = new MockSpan({ op: 'pageload' }); }); describe('withProfiler', () => { @@ -75,30 +60,23 @@ describe('withProfiler', () => { describe('mount span', () => { it('does not get created if Profiler is disabled', () => { const ProfiledComponent = withProfiler(() =>

Testing

, { disabled: true }); - expect(mockPushActivity).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(0); render(); - expect(mockPushActivity).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(0); }); it('is created when a component is mounted', () => { const ProfiledComponent = withProfiler(() =>

Testing

); - expect(mockPushActivity).toHaveBeenCalledTimes(0); - expect(mockGetActivitySpan).toHaveBeenCalledTimes(0); - expect(mockPopActivity).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(0); render(); - expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith(UNKNOWN_COMPONENT, { + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith({ description: `<${UNKNOWN_COMPONENT}>`, op: 'react.mount', }); - expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); - expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); - - expect(mockPopActivity).toHaveBeenCalledTimes(1); - expect(mockPopActivity).toHaveBeenLastCalledWith(1); }); }); @@ -110,13 +88,13 @@ describe('withProfiler', () => { const component = render(); component.unmount(); - expect(mockStartChild).toHaveBeenCalledTimes(1); - expect(mockStartChild).toHaveBeenLastCalledWith( - expect.objectContaining({ - description: `<${UNKNOWN_COMPONENT}>`, - op: 'react.render', - }), - ); + expect(mockStartChild).toHaveBeenCalledTimes(2); + expect(mockStartChild).toHaveBeenLastCalledWith({ + description: `<${UNKNOWN_COMPONENT}>`, + endTimestamp: expect.any(Number), + op: 'react.render', + startTimestamp: undefined, + }); }); it('is not created if hasRenderSpan is false', () => { @@ -126,7 +104,7 @@ describe('withProfiler', () => { const component = render(); component.unmount(); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); }); }); @@ -134,33 +112,33 @@ describe('withProfiler', () => { it('is created when component is updated', () => { const ProfiledComponent = withProfiler((props: { num: number }) =>
{props.num}
); const { rerender } = render(); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); // Dispatch new props rerender(); - expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenCalledTimes(2); expect(mockStartChild).toHaveBeenLastCalledWith({ data: { changedProps: ['num'] }, description: `<${UNKNOWN_COMPONENT}>`, - endTimestamp: TEST_TIMESTAMP, + endTimestamp: expect.any(Number), op: 'react.update', - startTimestamp: TEST_TIMESTAMP, + startTimestamp: expect.any(Number), }); // New props yet again rerender(); - expect(mockStartChild).toHaveBeenCalledTimes(2); + expect(mockStartChild).toHaveBeenCalledTimes(3); expect(mockStartChild).toHaveBeenLastCalledWith({ data: { changedProps: ['num'] }, description: `<${UNKNOWN_COMPONENT}>`, - endTimestamp: TEST_TIMESTAMP, + endTimestamp: expect.any(Number), op: 'react.update', - startTimestamp: TEST_TIMESTAMP, + startTimestamp: expect.any(Number), }); // Should not create spans if props haven't changed rerender(); - expect(mockStartChild).toHaveBeenCalledTimes(2); + expect(mockStartChild).toHaveBeenCalledTimes(3); }); it('does not get created if hasUpdateSpan is false', () => { @@ -168,11 +146,11 @@ describe('withProfiler', () => { includeUpdates: false, }); const { rerender } = render(); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); // Dispatch new props rerender(); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); }); }); }); @@ -182,23 +160,18 @@ describe('useProfiler()', () => { it('does not get created if Profiler is disabled', () => { // tslint:disable-next-line: no-void-expression renderHook(() => useProfiler('Example', { disabled: true })); - expect(mockPushActivity).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(0); }); it('is created when a component is mounted', () => { // tslint:disable-next-line: no-void-expression renderHook(() => useProfiler('Example')); - expect(mockPushActivity).toHaveBeenCalledTimes(1); - expect(mockPushActivity).toHaveBeenLastCalledWith('Example', { + expect(mockStartChild).toHaveBeenCalledTimes(1); + expect(mockStartChild).toHaveBeenLastCalledWith({ description: '', op: 'react.mount', }); - expect(mockGetActivitySpan).toHaveBeenCalledTimes(1); - expect(mockGetActivitySpan).toHaveBeenLastCalledWith(1); - - expect(mockPopActivity).toHaveBeenCalledTimes(1); - expect(mockPopActivity).toHaveBeenLastCalledWith(1); }); }); @@ -206,18 +179,18 @@ describe('useProfiler()', () => { it('does not get created when hasRenderSpan is false', () => { // tslint:disable-next-line: no-void-expression const component = renderHook(() => useProfiler('Example', { hasRenderSpan: false })); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); component.unmount(); - expect(mockStartChild).toHaveBeenCalledTimes(0); + expect(mockStartChild).toHaveBeenCalledTimes(1); }); it('is created by default', () => { // tslint:disable-next-line: no-void-expression const component = renderHook(() => useProfiler('Example')); - expect(mockStartChild).toHaveBeenCalledTimes(0); - component.unmount(); expect(mockStartChild).toHaveBeenCalledTimes(1); + component.unmount(); + expect(mockStartChild).toHaveBeenCalledTimes(2); expect(mockStartChild).toHaveBeenLastCalledWith( expect.objectContaining({ description: '',