Browse Source

fix(flamegraph): Offset profile by delta from transaction (#53655)

We were not adjusting the profile by the delta from the transaction if
the 2 start timestamps are not equal causing the flamegraph to be
misaligned.
Tony Xiao 1 year ago
parent
commit
b20f58aaa8

+ 19 - 2
static/app/components/profiling/flamegraph/flamegraph.tsx

@@ -326,6 +326,23 @@ function Flamegraph(): ReactElement {
         return null;
       }
 
+      let offset = flamegraph.profile.startedAt;
+
+      const transactionStart =
+        profiledTransaction.type === 'resolved'
+          ? profiledTransaction.data?.startTimestamp ?? null
+          : null;
+
+      const profileStart = flamegraph.profile.timestamp;
+
+      if (defined(transactionStart) && defined(profileStart)) {
+        offset += formatTo(
+          profileStart - transactionStart,
+          'second',
+          flamegraph.profile.unit
+        );
+      }
+
       const newView = new CanvasView({
         canvas: flamegraphCanvas,
         model: flamegraph,
@@ -334,7 +351,7 @@ function Flamegraph(): ReactElement {
           minWidth: flamegraph.profile.minFrameDuration,
           barHeight: flamegraphTheme.SIZES.BAR_HEIGHT,
           depthOffset: flamegraphTheme.SIZES.FLAMEGRAPH_DEPTH_OFFSET,
-          configSpaceTransform: new Rect(flamegraph.profile.startedAt, 0, 0, 0),
+          configSpaceTransform: new Rect(offset, 0, 0, 0),
         },
       });
 
@@ -410,7 +427,7 @@ function Flamegraph(): ReactElement {
 
     // We skip position.view dependency because it will go into an infinite loop
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [flamegraph, flamegraphCanvas, flamegraphTheme]
+    [flamegraph, flamegraphCanvas, flamegraphTheme, profiledTransaction]
   );
 
   const uiFramesView = useMemoWithPrevious<CanvasView<UIFrames> | null>(

+ 9 - 4
static/app/utils/profiling/profile/profile.tsx

@@ -7,13 +7,15 @@ interface ProfileStats {
 }
 
 export class Profile {
+  // The epoch time at which this profile was started. All relative timestamp should be
+  // relative to this.
+  // Some older formats may not have a timestamp defined.
+  timestamp: number | null;
   // Duration of the profile
   duration: number;
-  // Started at ts of the profile - varies between implementations of the profiler.
-  // For JS self profiles, this is the time origin (https://www.w3.org/TR/hr-time-2/#dfn-time-origin), for others it's epoch time
+  // Releative timestamp of the first sample in the timestamp.
   startedAt: number;
-  // Ended at ts of the profile - varies between implementations of the profiler.
-  // For JS self profiles, this is the time origin (https://www.w3.org/TR/hr-time-2/#dfn-time-origin), for others it's epoch time
+  // Releative timestamp of the last sample in the timestamp.
   endedAt: number;
   threadId: number;
   type: string;
@@ -47,6 +49,7 @@ export class Profile {
     name,
     unit,
     threadId,
+    timestamp,
     type,
   }: {
     duration: number;
@@ -56,6 +59,7 @@ export class Profile {
     threadId: number;
     type: string;
     unit: string;
+    timestamp?: number;
   }) {
     this.threadId = threadId;
     this.duration = duration;
@@ -64,6 +68,7 @@ export class Profile {
     this.name = name;
     this.unit = unit;
     this.type = type ?? '';
+    this.timestamp = timestamp ?? null;
   }
 
   static Empty = new Profile({

+ 4 - 0
static/app/utils/profiling/profile/sentrySampledProfile.tsx

@@ -1,3 +1,5 @@
+import moment from 'moment';
+
 import {defined, lastOfArray} from 'sentry/utils';
 import {CallTreeNode} from 'sentry/utils/profiling/callTreeNode';
 
@@ -146,6 +148,8 @@ export class SentrySampledProfile extends Profile {
     const {threadId, threadName} = getThreadData(sampledProfile);
 
     const profile = new SentrySampledProfile({
+      // .unix() only has second resolution
+      timestamp: moment(sampledProfile.timestamp).valueOf() / 1000,
       duration: endedAt - startedAt,
       startedAt,
       endedAt,