Browse Source

feat(profiling): display empty sample warning (#37707)

* fix(profiling): add empty state warning

* fix(profiling): add export to empty state

* feat(profile): track discarded samples (#37710)
Jonas 2 years ago
parent
commit
8ec840900d

+ 58 - 0
static/app/components/profiling/FlamegraphWarnings.tsx

@@ -0,0 +1,58 @@
+import styled from '@emotion/styled';
+
+import {ExportProfileButton} from 'sentry/components/profiling/exportProfileButton';
+import {t} from 'sentry/locale';
+import {Flamegraph} from 'sentry/utils/profiling/flamegraph';
+import {useParams} from 'sentry/utils/useParams';
+
+interface FlamegraphWarningProps {
+  flamegraph: Flamegraph;
+}
+
+export function FlamegraphWarnings(props: FlamegraphWarningProps) {
+  const params = useParams();
+
+  if (props.flamegraph.profile.samples.length === 0) {
+    return (
+      <Overlay>
+        <p>
+          {t(
+            'This profile either has no samples or the total duration of frames in the profile is 0.'
+          )}
+        </p>
+        <div>
+          <ExportProfileButton
+            variant="default"
+            eventId={params.eventId}
+            orgId={params.orgId}
+            size="sm"
+            projectId={params.projectId}
+            title={undefined}
+            disabled={
+              params.eventId === undefined ||
+              params.orgId === undefined ||
+              params.projectId === undefined
+            }
+          >
+            {t('Export Raw Profile')}
+          </ExportProfileButton>
+        </div>
+      </Overlay>
+    );
+  }
+
+  return null;
+}
+
+const Overlay = styled('div')`
+  position: absolute;
+  left: 0;
+  top: 0;
+  width: 100%;
+  height: 100%;
+  display: grid;
+  grid: auto/50%;
+  place-content: center;
+  z-index: ${p => p.theme.zIndex.modal};
+  text-align: center;
+`;

+ 1 - 0
static/app/components/profiling/FrameStack/frameStack.tsx

@@ -171,6 +171,7 @@ const FrameStack = memo(function FrameStack(props: FrameStackProps) {
         />
         <ListItem margin="none">
           <ExportProfileButton
+            variant="xs"
             eventId={params.eventId}
             orgId={params.orgId}
             projectId={params.projectId}

+ 23 - 11
static/app/components/profiling/exportProfileButton.tsx

@@ -14,6 +14,8 @@ interface ExportProfileButtonProps
   eventId: string | undefined;
   orgId: string | undefined;
   projectId: string | undefined;
+  children?: React.ReactNode;
+  variant?: 'xs' | 'default';
 }
 
 export function ExportProfileButton(props: ExportProfileButtonProps) {
@@ -24,22 +26,32 @@ export function ExportProfileButton(props: ExportProfileButtonProps) {
     return p.slug === props.projectId;
   });
 
-  return (
-    <StyledButton
+  const href = `${api.baseUrl}/projects/${props.orgId}/${props.projectId}/profiling/raw_profiles/${props.eventId}/`;
+  const download = `${organization.slug}_${
+    project?.slug ?? props.projectId ?? 'unknown_project'
+  }_${props.eventId}.profile.json`;
+
+  const title = t('Export Profile');
+
+  return props.variant === 'xs' ? (
+    <StyledButtonSmall size="xs" title={title} href={href} download={download} {...props}>
+      {props.children}
+      <IconDownload size="xs" />
+    </StyledButtonSmall>
+  ) : (
+    <Button
+      icon={<IconDownload />}
+      title={title}
+      href={href}
+      download={download}
       {...props}
-      size="xs"
-      title={t('Export Profile')}
-      href={`${api.baseUrl}/projects/${props.orgId}/${props.projectId}/profiling/raw_profiles/${props.eventId}/`}
-      download={`${organization.slug}_${
-        project?.slug ?? props.projectId ?? 'unknown_project'
-      }_${props.eventId}.profile.json`}
     >
-      <IconDownload size="xs" />
-    </StyledButton>
+      {props.children}
+    </Button>
   );
 }
 
-const StyledButton = styled(Button)`
+const StyledButtonSmall = styled(Button)`
   border: none;
   background-color: transparent;
   box-shadow: none;

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

@@ -41,6 +41,7 @@ import {FlamegraphRenderer} from 'sentry/utils/profiling/renderers/flamegraphRen
 import {useDevicePixelRatio} from 'sentry/utils/useDevicePixelRatio';
 import {useMemoWithPrevious} from 'sentry/utils/useMemoWithPrevious';
 
+import {FlamegraphWarnings} from './FlamegraphWarnings';
 import {ProfilingFlamechartLayout} from './profilingFlamechartLayout';
 
 function getTransactionConfigSpace(profiles: Profile[]): Rect {
@@ -326,6 +327,7 @@ function Flamegraph(props: FlamegraphProps): ReactElement {
         }
         flamechart={
           <ProfileDragDropImport onImport={props.onImport}>
+            <FlamegraphWarnings flamegraph={flamegraph} />
             <FlamegraphZoomView
               flamegraphRenderer={flamegraphRenderer}
               canvasBounds={canvasBounds}

+ 48 - 50
static/app/components/profiling/flamegraphZoomView.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react';
+import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
 import styled from '@emotion/styled';
 import {mat3, vec2} from 'gl-matrix';
 
@@ -734,55 +734,53 @@ function FlamegraphZoomView({
   const contextMenu = useContextMenu({container: flamegraphCanvasRef});
 
   return (
-    <Fragment>
-      <CanvasContainer>
-        <Canvas
-          ref={canvas => setFlamegraphCanvasRef(canvas)}
-          onMouseDown={onCanvasMouseDown}
-          onMouseUp={onCanvasMouseUp}
-          onMouseMove={onCanvasMouseMove}
-          onMouseLeave={onCanvasMouseLeave}
-          onContextMenu={contextMenu.handleContextMenu}
-          style={{cursor: lastInteraction === 'pan' ? 'grab' : 'default'}}
-        />
-        <Canvas
-          ref={canvas => setFlamegraphOverlayCanvasRef(canvas)}
-          style={{
-            pointerEvents: 'none',
-          }}
-        />
-        <FlamegraphOptionsContextMenu contextMenu={contextMenu} />
-        {flamegraphCanvas &&
-        flamegraphRenderer &&
-        flamegraphView &&
-        configSpaceCursor &&
-        hoveredNode?.frame?.name ? (
-          <BoundTooltip
-            bounds={canvasBounds}
-            cursor={configSpaceCursor}
-            flamegraphCanvas={flamegraphCanvas}
-            flamegraphView={flamegraphView}
-          >
-            <HoveredFrameMainInfo>
-              <FrameColorIndicator
-                backgroundColor={formatColorForFrame(hoveredNode, flamegraphRenderer)}
-              />
-              {flamegraphRenderer.flamegraph.formatter(hoveredNode.node.totalWeight)}{' '}
-              {formatWeightToProfileDuration(
-                hoveredNode.node,
-                flamegraphRenderer.flamegraph
-              )}{' '}
-              {hoveredNode.frame.name}
-            </HoveredFrameMainInfo>
-            <HoveredFrameTimelineInfo>
-              {flamegraphRenderer.flamegraph.timelineFormatter(hoveredNode.start)}{' '}
-              {' \u2014 '}
-              {flamegraphRenderer.flamegraph.timelineFormatter(hoveredNode.end)}
-            </HoveredFrameTimelineInfo>
-          </BoundTooltip>
-        ) : null}
-      </CanvasContainer>
-    </Fragment>
+    <CanvasContainer>
+      <Canvas
+        ref={canvas => setFlamegraphCanvasRef(canvas)}
+        onMouseDown={onCanvasMouseDown}
+        onMouseUp={onCanvasMouseUp}
+        onMouseMove={onCanvasMouseMove}
+        onMouseLeave={onCanvasMouseLeave}
+        onContextMenu={contextMenu.handleContextMenu}
+        style={{cursor: lastInteraction === 'pan' ? 'grab' : 'default'}}
+      />
+      <Canvas
+        ref={canvas => setFlamegraphOverlayCanvasRef(canvas)}
+        style={{
+          pointerEvents: 'none',
+        }}
+      />
+      <FlamegraphOptionsContextMenu contextMenu={contextMenu} />
+      {flamegraphCanvas &&
+      flamegraphRenderer &&
+      flamegraphView &&
+      configSpaceCursor &&
+      hoveredNode?.frame?.name ? (
+        <BoundTooltip
+          bounds={canvasBounds}
+          cursor={configSpaceCursor}
+          flamegraphCanvas={flamegraphCanvas}
+          flamegraphView={flamegraphView}
+        >
+          <HoveredFrameMainInfo>
+            <FrameColorIndicator
+              backgroundColor={formatColorForFrame(hoveredNode, flamegraphRenderer)}
+            />
+            {flamegraphRenderer.flamegraph.formatter(hoveredNode.node.totalWeight)}{' '}
+            {formatWeightToProfileDuration(
+              hoveredNode.node,
+              flamegraphRenderer.flamegraph
+            )}{' '}
+            {hoveredNode.frame.name}
+          </HoveredFrameMainInfo>
+          <HoveredFrameTimelineInfo>
+            {flamegraphRenderer.flamegraph.timelineFormatter(hoveredNode.start)}{' '}
+            {' \u2014 '}
+            {flamegraphRenderer.flamegraph.timelineFormatter(hoveredNode.end)}
+          </HoveredFrameTimelineInfo>
+        </BoundTooltip>
+      ) : null}
+    </CanvasContainer>
   );
 }
 

+ 1 - 0
static/app/utils/profiling/profile/eventedProfile.tsx

@@ -137,6 +137,7 @@ export class EventedProfile extends Profile {
   leaveFrame(_event: Frame, at: number): void {
     this.addWeightToFrames(at);
     this.addWeightsToNodes(at);
+    this.trackSampleStats(at);
 
     const leavingStackTop = this.appendOrderStack.pop();
 

+ 2 - 0
static/app/utils/profiling/profile/jsSelfProfile.tsx

@@ -95,6 +95,8 @@ export class JSSelfProfile extends Profile {
   }
 
   appendSample(stack: Frame[], weight: number): void {
+    this.trackSampleStats(weight);
+
     let node = this.appendOrderTree;
     const framesInStack: CallTreeNode[] = [];
 

+ 21 - 1
static/app/utils/profiling/profile/profile.tsx

@@ -3,7 +3,12 @@ import {lastOfArray} from 'sentry/utils';
 import {CallTreeNode} from '../callTreeNode';
 import {Frame} from '../frame';
 
-// This is a simplified port of speedscope's profile with a few simplifications and some removed functionality.
+interface ProfileStats {
+  discardedSamplesCount: number;
+  negativeSamplesCount: number;
+}
+
+// This is a simplified port of speedscope's profile with a few simplifications and some removed functionality + some added functionality.
 // head at commit e37f6fa7c38c110205e22081560b99cb89ce885e
 
 // We should try and remove these as we adopt our own profile format and only rely on the sampled format.
@@ -32,6 +37,11 @@ export class Profile {
   samples: CallTreeNode[] = [];
   weights: number[] = [];
 
+  stats: ProfileStats = {
+    discardedSamplesCount: 0,
+    negativeSamplesCount: 0,
+  };
+
   constructor(
     duration: number,
     startedAt: number,
@@ -52,6 +62,16 @@ export class Profile {
     return new Profile(1000, 0, 1000, '', 'milliseconds', 0).build();
   }
 
+  trackSampleStats(duration: number) {
+    // Keep track of discarded samples and ones that may have negative weights
+    if (duration === 0) {
+      this.stats.discardedSamplesCount++;
+    }
+    if (duration < 0) {
+      this.stats.negativeSamplesCount++;
+    }
+  }
+
   forEach(
     openFrame: (node: CallTreeNode, value: number) => void,
     closeFrame: (node: CallTreeNode, value: number) => void

+ 3 - 0
static/app/utils/profiling/profile/sampledProfile.tsx

@@ -49,6 +49,9 @@ export class SampledProfile extends Profile {
   }
 
   appendSampleWithWeight(stack: Frame[], weight: number): void {
+    // Keep track of discarded samples and ones that may have negative weights
+    this.trackSampleStats(weight);
+
     // Ignore samples with 0 weight
     if (weight === 0) {
       return;

+ 37 - 0
tests/js/spec/utils/profiling/profile/eventedProfile.spec.tsx

@@ -24,6 +24,43 @@ describe('EventedProfile', () => {
     expect(profile.endedAt).toBe(1000);
   });
 
+  it('tracks discarded samples', () => {
+    const trace: Profiling.EventedProfile = {
+      name: 'profile',
+      startValue: 0,
+      endValue: 1000,
+      unit: 'milliseconds',
+      threadID: 0,
+      type: 'evented',
+      events: [
+        {type: 'O', at: 0, frame: 0},
+        {type: 'C', at: 0, frame: 0},
+      ],
+    };
+
+    const profile = EventedProfile.FromProfile(trace, createFrameIndex([{name: 'f0'}]));
+
+    expect(profile.stats.discardedSamplesCount).toBe(1);
+  });
+
+  it('tracks negative samples', () => {
+    const trace: Profiling.EventedProfile = {
+      name: 'profile',
+      startValue: 0,
+      endValue: 1000,
+      unit: 'milliseconds',
+      threadID: 0,
+      type: 'evented',
+      events: [
+        {type: 'O', at: 0, frame: 0},
+        {type: 'C', at: -1, frame: 0},
+      ],
+    };
+
+    const profile = EventedProfile.FromProfile(trace, createFrameIndex([{name: 'f0'}]));
+    expect(profile.stats.negativeSamplesCount).toBe(1);
+  });
+
   it('rebuilds the stack', () => {
     const trace: Profiling.EventedProfile = {
       name: 'profile',

Some files were not shown because too many files changed in this diff