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.

Sign up for GitHub

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

Jump to bottom

ref: Convert React and Vue Tracing to use active transaction #2741

Merged
merged 2 commits into from Jul 14, 2020
Merged

ref: Convert React and Vue Tracing to use active transaction #2741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a16e81a
ref: Make React Profiler use active transaction
AbhiPrasad Jul 14, 2020
07e6e16
ref: Make Vue Tracing using active transaction
AbhiPrasad Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
77 changes: 34 additions & 43 deletions packages/integrations/src/vue.ts
View file
Open in desktop
@@ -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<Integration>;

/** Global Vue object limited to the methods/attributes we require */
interface VueInstance {
config: {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -137,7 +129,6 @@ export class Vue implements Integration {
private readonly _componentsCache: { [key: string]: string } = {};
private _rootSpan?: Span;
private _rootSpanTimer?: ReturnType<typeof setTimeout>;
private _tracingActivity?: number;

/**
* @inheritDoc
Expand Down Expand Up @@ -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',
});
}
});
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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<T extends Transaction>(hub: HubType): T | undefined {
if (hub && hub.getScope) {
const scope = hub.getScope() as Scope;
if (scope) {
return scope.getTransaction() as T | undefined;
}
}

return undefined;
}
Comment on lines +389 to +398
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't like that we have this duplicated everywhere :/

Copy link
Member

Choose a reason for hiding this comment

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

Isn't doing hub.getScope().getTransaction() the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

public getScope(): Scope | undefined

sentry-javascript/packages/hub/src/hub.ts

Line 140 in e8a37d2

public getScope(): Scope | undefined {

Copy link
Member Author

Choose a reason for hiding this comment

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

So it is more like hub?.getScope?.getTransaction()

Copy link
Member Author

Choose a reason for hiding this comment

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

We can always refactor this if we figure out a better way to do this.

130 changes: 37 additions & 93 deletions packages/react/src/profiler.tsx
View file
Open in desktop
@@ -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<Integration>;

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;
Expand All @@ -95,12 +25,8 @@ export type ProfilerProps = {
* spans based on component lifecycles.
*/
class Profiler extends React.Component<ProfilerProps> {
// 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<ProfilerProps> = {
disabled: false,
Expand All @@ -116,18 +42,20 @@ class Profiler extends React.Component<ProfilerProps> {
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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -252,3 +184,15 @@ function useProfiler(
}

export { withProfiler, Profiler, useProfiler };

/** Grabs active transaction off scope */
export function getActiveTransaction<T extends Transaction>(hub: Hub = getCurrentHub()): T | undefined {
if (hub) {
const scope = hub.getScope();
if (scope) {
return scope.getTransaction() as T | undefined;
}
}

return undefined;
}