From 219d1b2a42f2fae947116795aa6c47d8e0036252 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 8 Jun 2020 20:26:40 -0400 Subject: [PATCH 1/4] try to get render working --- packages/react/src/profiler.tsx | 70 ++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 00f404ce5a65..10ea7e4dcaad 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -3,6 +3,9 @@ import { Integration, IntegrationClass } from '@sentry/types'; import { logger } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; +// tslint:disable: no-implicit-dependencies +// @ts-ignore +import * as kap from 'scheduler'; export const UNKNOWN_COMPONENT = 'unknown'; @@ -39,6 +42,35 @@ function afterNextFrame(callback: Function): void { timeout = window.setTimeout(done, 100); } +// This is only active in development mode and in profiling mode +// Learn how to do that here: https://gist.github.com/bvaughn/25e6233aeb1b4f0cdb8d8366e54a3977 +function isProfilingModeOn(): boolean { + // function Hello() { + // return /*#__PURE__*/ React.createElement('div', null); + // } + + // @ts-ignore + console.log(kap); + // const lol = React.createElement(React.Profiler, { id: 'sdf', onRender: () => {} }); + + // @ts-ignore + // console.log(lol); + // function Kappa() { + // return /*#__PURE__*/ React.createElement(Hello, null); + // } + + // I wish React exposed this better + // tslint:disable-next-line: no-unsafe-any + // console.log(Kappa()); + // tslint:disable-next-line: no-unsafe-any + // if (fake._owner && fake._owner.actualDuration) { + // console.log('YES ITS ON'); + // return true; + // } + + return false; +} + const getInitActivity = (name: string): number | null => { const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); @@ -62,20 +94,45 @@ export type ProfilerProps = { class Profiler extends React.Component { public activity: number | null; + public hasProfilingMode: boolean = false; + public constructor(props: ProfilerProps) { super(props); + // TODO: Extract this out into global state + this.hasProfilingMode = isProfilingModeOn(); + this.activity = getInitActivity(this.props.name); } public componentDidMount(): void { - afterNextFrame(this.finishProfile); + if (!this.hasProfilingMode) { + afterNextFrame(this.finishProfile); + } } public componentWillUnmount(): void { - afterNextFrame(this.finishProfile); + if (!this.hasProfilingMode) { + afterNextFrame(this.finishProfile); + } } + // TODO: Figure out how to use these values. + // We should be generating spans from these! + // > React calls this function any time a component within the profiled tree “commits” an update + // See: https://reactjs.org/docs/profiler.html#onrender-callback + // id: string, + // phase: 'mount' | 'update', + // actualDuration: number, + // baseDuration: number, + // startTime: number, + // commitTime: number, + public handleProfilerRender = (..._args: any[]) => { + console.log('SDJFLSJDF'); + console.table(_args); + afterNextFrame(this.finishProfile); + }; + public finishProfile = () => { if (!this.activity) { return; @@ -90,6 +147,15 @@ class Profiler extends React.Component { }; public render(): React.ReactNode { + const { name } = this.props; + if (this.hasProfilingMode) { + return ( + + {this.props.children} + + ); + } + return this.props.children; } } From 9d91107dfeabb82e7d8e62ea8c45cac2c1102dae Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 9 Jun 2020 10:51:33 -0400 Subject: [PATCH 2/4] check active Transaction --- packages/react/src/profiler.tsx | 142 +++++++++++++++++++------------- packages/utils/src/misc.ts | 2 +- 2 files changed, 88 insertions(+), 56 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index 10ea7e4dcaad..a5174bd9c348 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,11 +1,8 @@ import { getCurrentHub } from '@sentry/browser'; -import { Integration, IntegrationClass } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { Integration, IntegrationClass, SpanContext, Transaction } from '@sentry/types'; +import { parseSemver, logger, SemVer } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; -// tslint:disable: no-implicit-dependencies -// @ts-ignore -import * as kap from 'scheduler'; export const UNKNOWN_COMPONENT = 'unknown'; @@ -42,31 +39,22 @@ function afterNextFrame(callback: Function): void { timeout = window.setTimeout(done, 100); } -// This is only active in development mode and in profiling mode -// Learn how to do that here: https://gist.github.com/bvaughn/25e6233aeb1b4f0cdb8d8366e54a3977 +/** + * isProfilingModeOn tells us if the React.Profiler will correctly + * Profile it's components. This is only active in development mode + * and in profiling mode. + * + * Learn how to do that here: https://gist.github.com/bvaughn/25e6233aeb1b4f0cdb8d8366e54a3977 + */ function isProfilingModeOn(): boolean { - // function Hello() { - // return /*#__PURE__*/ React.createElement('div', null); - // } + const fake = React.createElement('div') as any; - // @ts-ignore - console.log(kap); - // const lol = React.createElement(React.Profiler, { id: 'sdf', onRender: () => {} }); - - // @ts-ignore - // console.log(lol); - // function Kappa() { - // return /*#__PURE__*/ React.createElement(Hello, null); - // } - - // I wish React exposed this better - // tslint:disable-next-line: no-unsafe-any - // console.log(Kappa()); - // tslint:disable-next-line: no-unsafe-any - // if (fake._owner && fake._owner.actualDuration) { - // console.log('YES ITS ON'); - // return true; - // } + // tslint:disable-next-line: triple-equals no-unsafe-any + if (fake._owner != null && fake._owner.actualDuration != null) { + // if the component has a valid owner, and that owner has a duration + // React is profiling all it's components + return true; + } return false; } @@ -78,7 +66,7 @@ const getInitActivity = (name: string): number | null => { // tslint:disable-next-line:no-unsafe-any return (tracingIntegration as any).constructor.pushActivity(name, { description: `<${name}>`, - op: 'react', + op: 'react.mount', }); } @@ -94,43 +82,73 @@ export type ProfilerProps = { class Profiler extends React.Component { public activity: number | null; - public hasProfilingMode: boolean = false; + public hasProfilingMode: boolean | null = null; + public reactVersion: SemVer = parseSemver(React.version); public constructor(props: ProfilerProps) { super(props); - // TODO: Extract this out into global state - this.hasProfilingMode = isProfilingModeOn(); - this.activity = getInitActivity(this.props.name); } - public componentDidMount(): void { - if (!this.hasProfilingMode) { - afterNextFrame(this.finishProfile); - } - } + // public componentDidMount(): void { + // if (!this.hasProfilingMode) { + // afterNextFrame(this.finishProfile); + // } + // } public componentWillUnmount(): void { - if (!this.hasProfilingMode) { - afterNextFrame(this.finishProfile); - } + afterNextFrame(this.finishProfile); } - // TODO: Figure out how to use these values. - // We should be generating spans from these! - // > React calls this function any time a component within the profiled tree “commits” an update - // See: https://reactjs.org/docs/profiler.html#onrender-callback - // id: string, - // phase: 'mount' | 'update', - // actualDuration: number, - // baseDuration: number, - // startTime: number, - // commitTime: number, - public handleProfilerRender = (..._args: any[]) => { - console.log('SDJFLSJDF'); - console.table(_args); - afterNextFrame(this.finishProfile); + /** + * + * React calls handleProfilerRender() any time a component within the profiled + * tree “commits” an update. + * + */ + public handleProfilerRender = ( + // The id prop of the Profiler tree that has just committed + id: string, + // Identifies whether the tree has just been mounted for the first time + // or re-rendered due to a change in props, state, or hooks + phase: 'mount' | 'update', + // Time spent rendering the Profiler and its descendants for the current update + actualDuration: number, + // Duration of the most recent render time for each individual component within the Profiler tree + _baseDuration: number, + // Timestamp when React began rendering the current update + // pageload = startTime of 0 + startTime: number, + // Timestamp when React committed the current update + _commitTime: number, + ) => { + const componentName = this.props.name === UNKNOWN_COMPONENT ? id : this.props.name; + + const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); + if (tracingIntegration !== null) { + // tslint:disable-next-line: no-unsafe-any + const activeTransaction = (tracingIntegration as any).constructor._activeTransaction as Transaction; + + console.log(activeTransaction); + + if (activeTransaction) { + console.log('sdfsf'); + + const spanContext: SpanContext = { + description: `<${componentName}>`, + op: `react.${phase}`, + startTimestamp: activeTransaction.startTimestamp + startTime, + }; + + const span = activeTransaction.startChild(spanContext); + + console.log('SLDJFJSF'); + + span.finish(span.startTimestamp + actualDuration); + } + } + // afterNextFrame(this.finishProfile); }; public finishProfile = () => { @@ -148,6 +166,20 @@ class Profiler extends React.Component { public render(): React.ReactNode { const { name } = this.props; + + if ( + // React <= v16.4 + (this.reactVersion.major && this.reactVersion.major <= 15) || + (this.reactVersion.major === 16 && this.reactVersion.minor && this.reactVersion.minor <= 4) + ) { + return this.props.children; + } + + if (this.hasProfilingMode === null) { + // TODO: This should be a global check + this.hasProfilingMode = isProfilingModeOn(); + } + if (this.hasProfilingMode) { return ( diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index cc63caa8f165..02d8690f9a06 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -413,7 +413,7 @@ const SEMVER_REGEXP = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\ /** * Represents Semantic Versioning object */ -interface SemVer { +export interface SemVer { major?: number; minor?: number; patch?: number; From 9420a1ecf51074be42a35ad2caca0b8328a8e901 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 9 Jun 2020 12:54:09 -0400 Subject: [PATCH 3/4] try to get update transaction working --- packages/react/src/profiler.tsx | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index a5174bd9c348..d1b92eb7ff17 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -91,11 +91,11 @@ class Profiler extends React.Component { this.activity = getInitActivity(this.props.name); } - // public componentDidMount(): void { - // if (!this.hasProfilingMode) { - // afterNextFrame(this.finishProfile); - // } - // } + public componentDidMount(): void { + if (!this.hasProfilingMode) { + afterNextFrame(this.finishProfile); + } + } public componentWillUnmount(): void { afterNextFrame(this.finishProfile); @@ -121,34 +121,33 @@ class Profiler extends React.Component { // pageload = startTime of 0 startTime: number, // Timestamp when React committed the current update - _commitTime: number, + commitTime: number, ) => { + if (phase === 'mount') { + afterNextFrame(this.finishProfile); + } + const componentName = this.props.name === UNKNOWN_COMPONENT ? id : this.props.name; const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER); if (tracingIntegration !== null) { // tslint:disable-next-line: no-unsafe-any const activeTransaction = (tracingIntegration as any).constructor._activeTransaction as Transaction; - + console.table({ id, phase, actualDuration, _baseDuration, startTime, commitTime }); console.log(activeTransaction); if (activeTransaction) { - console.log('sdfsf'); - const spanContext: SpanContext = { description: `<${componentName}>`, - op: `react.${phase}`, + op: 'react.update', startTimestamp: activeTransaction.startTimestamp + startTime, }; const span = activeTransaction.startChild(spanContext); - console.log('SLDJFJSF'); - - span.finish(span.startTimestamp + actualDuration); + span.finish(activeTransaction.startTimestamp + actualDuration); } } - // afterNextFrame(this.finishProfile); }; public finishProfile = () => { From 1a2b9a917797e3bdc4d1753eb619b7b16cf83483 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 9 Jun 2020 13:27:58 -0400 Subject: [PATCH 4/4] more experiments --- packages/react/src/profiler.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx index d1b92eb7ff17..ad159a0e5ec2 100644 --- a/packages/react/src/profiler.tsx +++ b/packages/react/src/profiler.tsx @@ -1,6 +1,6 @@ import { getCurrentHub } from '@sentry/browser'; import { Integration, IntegrationClass, SpanContext, Transaction } from '@sentry/types'; -import { parseSemver, logger, SemVer } from '@sentry/utils'; +import { parseSemver, logger, SemVer, timestampWithMs } from '@sentry/utils'; import * as hoistNonReactStatic from 'hoist-non-react-statics'; import * as React from 'react'; @@ -119,9 +119,9 @@ class Profiler extends React.Component { _baseDuration: number, // Timestamp when React began rendering the current update // pageload = startTime of 0 - startTime: number, + _startTime: number, // Timestamp when React committed the current update - commitTime: number, + _commitTime: number, ) => { if (phase === 'mount') { afterNextFrame(this.finishProfile); @@ -133,19 +133,20 @@ class Profiler extends React.Component { if (tracingIntegration !== null) { // tslint:disable-next-line: no-unsafe-any const activeTransaction = (tracingIntegration as any).constructor._activeTransaction as Transaction; - console.table({ id, phase, actualDuration, _baseDuration, startTime, commitTime }); + console.table({ id, phase, actualDuration, _baseDuration, _startTime, _commitTime }); console.log(activeTransaction); if (activeTransaction) { + const now = timestampWithMs(); const spanContext: SpanContext = { description: `<${componentName}>`, op: 'react.update', - startTimestamp: activeTransaction.startTimestamp + startTime, + startTimestamp: now - actualDuration, }; const span = activeTransaction.startChild(spanContext); - span.finish(activeTransaction.startTimestamp + actualDuration); + span.finish(now); } } };