Browse Source

feat(profiling) correctly implement negation (#61366)

In a negated view, new functions actually mean the function was removed,
as it is not present in the after list of frames
Jonas 1 year ago
parent
commit
6e1cd9be39

+ 185 - 95
static/app/components/events/eventStatisticalDetector/eventDifferentialFlamegraph.tsx

@@ -7,6 +7,7 @@ import {Button} from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import Link from 'sentry/components/links/link';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
+import Panel from 'sentry/components/panels/panel';
 import PerformanceDuration from 'sentry/components/performanceDuration';
 import Placeholder from 'sentry/components/placeholder';
 import {DifferentialFlamegraph} from 'sentry/components/profiling/flamegraph/differentialFlamegraph';
@@ -21,6 +22,7 @@ import {
   CanvasPoolManager,
   useCanvasScheduler,
 } from 'sentry/utils/profiling/canvasScheduler';
+import {colorComponentsToRGBA} from 'sentry/utils/profiling/colors/utils';
 import {DifferentialFlamegraph as DifferentialFlamegraphModel} from 'sentry/utils/profiling/differentialFlamegraph';
 import {Flamegraph} from 'sentry/utils/profiling/flamegraph';
 import {FlamegraphStateProvider} from 'sentry/utils/profiling/flamegraph/flamegraphStateProvider/flamegraphContextProvider';
@@ -222,21 +224,12 @@ function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewP
       return DifferentialFlamegraphModel.Empty();
     }
 
-    if (negated) {
-      return DifferentialFlamegraphModel.FromDiff(
-        {
-          before: afterFlamegraph,
-          after: beforeFlamegraph,
-        },
-        theme
-      );
-    }
-
     return DifferentialFlamegraphModel.FromDiff(
       {
         before: beforeFlamegraph,
         after: afterFlamegraph,
       },
+      {negated},
       theme
     );
   }, [beforeFlamegraph, afterFlamegraph, theme, negated]);
@@ -264,64 +257,116 @@ function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewP
 
   return (
     <Fragment>
-      <DifferentialFlamegraphTransactionToolbar
-        transaction={props.transaction}
-        onNextTransactionClick={props.onNextTransactionClick}
-        onPreviousTransactionClick={props.onPreviousTransactionClick}
-      />
-      <DifferentialFlamegraphToolbar
-        frameFilter={frameFilterSetting}
-        onFrameFilterChange={setFrameFilterSetting}
-        negated={negated}
-        onNegatedChange={setNegated}
-        flamegraph={differentialFlamegraph}
-        canvasPoolManager={canvasPoolManager}
-      />
-      <DifferentialFlamegraphContainer>
-        {props.after.isLoading || props.before.isLoading ? (
-          <LoadingIndicatorContainer>
-            <LoadingIndicator />
-          </LoadingIndicatorContainer>
-        ) : props.before.isError && props.after.isError ? (
-          <ErrorMessageContainer>
-            {t('Failed to load flamegraph for before and after regression time range.')}
-          </ErrorMessageContainer>
-        ) : props.before.isError ? (
-          <ErrorMessageContainer>
-            {t('Failed to load flamegraph for before regression time range.')}
-          </ErrorMessageContainer>
-        ) : props.after.isError ? (
-          <ErrorMessageContainer>
-            {t('Failed to load flamegraph for after regression time range.')}
-          </ErrorMessageContainer>
-        ) : null}
-        <DifferentialFlamegraph
-          profileGroup={afterProfileGroup ?? LOADING_PROFILE_GROUP}
-          differentialFlamegraph={differentialFlamegraph}
-          canvasPoolManager={canvasPoolManager}
-          scheduler={scheduler}
+      <Panel>
+        <DifferentialFlamegraphTransactionToolbar
+          transaction={props.transaction}
+          onNextTransactionClick={props.onNextTransactionClick}
+          onPreviousTransactionClick={props.onPreviousTransactionClick}
         />
-      </DifferentialFlamegraphContainer>
-      <DifferentialFlamegraphExplanationBar negated={negated} />
-
-      <DifferentialFlamegraphFunctionsContainer>
-        <DifferentialFlamegraphChangedFunctions
-          loading={props.after.isLoading || props.before.isLoading}
-          title={t('Largest Increase')}
-          subtitle={negated ? t('before regression') : t('after regression')}
-          functions={differentialFlamegraph.increasedFrames}
+        <DifferentialFlamegraphToolbar
+          frameFilter={frameFilterSetting}
+          onFrameFilterChange={setFrameFilterSetting}
+          negated={negated}
+          onNegatedChange={setNegated}
           flamegraph={differentialFlamegraph}
-          makeFunctionLink={makeFunctionFlamechartLink}
-        />
-        <DifferentialFlamegraphChangedFunctions
-          loading={props.after.isLoading || props.before.isLoading}
-          title={t('Largest Decrease')}
-          subtitle={negated ? t('before regression') : t('after regression')}
-          functions={differentialFlamegraph.decreasedFrames}
-          flamegraph={differentialFlamegraph}
-          makeFunctionLink={makeFunctionFlamechartLink}
+          canvasPoolManager={canvasPoolManager}
         />
-      </DifferentialFlamegraphFunctionsContainer>
+        <DifferentialFlamegraphContainer>
+          {props.after.isLoading || props.before.isLoading ? (
+            <LoadingIndicatorContainer>
+              <LoadingIndicator />
+            </LoadingIndicatorContainer>
+          ) : props.before.isError && props.after.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for before and after regression time range.')}
+            </ErrorMessageContainer>
+          ) : props.before.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for before regression time range.')}
+            </ErrorMessageContainer>
+          ) : props.after.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for after regression time range.')}
+            </ErrorMessageContainer>
+          ) : null}
+          <DifferentialFlamegraph
+            profileGroup={afterProfileGroup ?? LOADING_PROFILE_GROUP}
+            differentialFlamegraph={differentialFlamegraph}
+            canvasPoolManager={canvasPoolManager}
+            scheduler={scheduler}
+          />
+        </DifferentialFlamegraphContainer>
+        <DifferentialFlamegraphExplanationBar negated={negated} />
+
+        <DifferentialFlamegraphContainer>
+          {props.after.isLoading || props.before.isLoading ? (
+            <LoadingIndicatorContainer>
+              <LoadingIndicator />
+            </LoadingIndicatorContainer>
+          ) : props.before.isError && props.after.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for before and after regression time range.')}
+            </ErrorMessageContainer>
+          ) : props.before.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for before regression time range.')}
+            </ErrorMessageContainer>
+          ) : props.after.isError ? (
+            <ErrorMessageContainer>
+              {t('Failed to load flamegraph for after regression time range.')}
+            </ErrorMessageContainer>
+          ) : null}
+          <DifferentialFlamegraph
+            profileGroup={afterProfileGroup ?? LOADING_PROFILE_GROUP}
+            differentialFlamegraph={differentialFlamegraph}
+            canvasPoolManager={canvasPoolManager}
+            scheduler={scheduler}
+          />
+        </DifferentialFlamegraphContainer>
+        <DifferentialFlamegraphExplanationBar negated={negated} />
+      </Panel>
+
+      <Panel>
+        <DifferentialFlamegraphFunctionsContainer>
+          <DifferentialFlamegraphChangedFunctions
+            loading={props.after.isLoading || props.before.isLoading}
+            title={t('Slower functions')}
+            subtitle={t('after regression')}
+            functions={differentialFlamegraph.increasedFrames}
+            flamegraph={differentialFlamegraph}
+            makeFunctionLink={makeFunctionFlamechartLink}
+          />
+          <DifferentialFlamegraphChangedFunctions
+            loading={props.after.isLoading || props.before.isLoading}
+            title={t('Faster functions')}
+            subtitle={t('after regression')}
+            functions={differentialFlamegraph.decreasedFrames}
+            flamegraph={differentialFlamegraph}
+            makeFunctionLink={makeFunctionFlamechartLink}
+          />
+        </DifferentialFlamegraphFunctionsContainer>
+      </Panel>
+
+      <Panel>
+        <DifferentialFlamegraphFunctionsContainer>
+          <DifferentialFlamegraphChangedFunctions
+            loading={props.after.isLoading || props.before.isLoading}
+            title={t('New functions')}
+            subtitle={t('after regression')}
+            functions={differentialFlamegraph.newFrames}
+            flamegraph={differentialFlamegraph}
+            makeFunctionLink={makeFunctionFlamechartLink}
+          />
+          <DifferentialFlamegraphChangedFunctions
+            loading={props.after.isLoading || props.before.isLoading}
+            title={t('Removed functions')}
+            subtitle={t('after regression')}
+            functions={differentialFlamegraph.removedFrames}
+            flamegraph={differentialFlamegraph}
+            makeFunctionLink={makeFunctionFlamechartLink}
+          />
+        </DifferentialFlamegraphFunctionsContainer>
+      </Panel>
     </Fragment>
   );
 }
@@ -445,7 +490,9 @@ function paginationReducer(
 
 interface DifferentialFlamegraphChangedFunctionsProps {
   flamegraph: DifferentialFlamegraphModel;
-  functions: DifferentialFlamegraphModel['increasedFrames'];
+  functions:
+    | DifferentialFlamegraphModel['increasedFrames']
+    | DifferentialFlamegraphModel['newFrames'];
   loading: boolean;
   makeFunctionLink: (frame: FlamegraphFrame) => LocationDescriptor;
   subtitle: string;
@@ -454,6 +501,7 @@ interface DifferentialFlamegraphChangedFunctionsProps {
 function DifferentialFlamegraphChangedFunctions(
   props: DifferentialFlamegraphChangedFunctionsProps
 ) {
+  const theme = useFlamegraphTheme();
   const [state, dispatch] = useReducer(paginationReducer, {
     page: 0,
     pageSize: 0,
@@ -483,7 +531,7 @@ function DifferentialFlamegraphChangedFunctions(
   }, [state.page, state.pageCount]);
 
   return (
-    <div>
+    <DifferentialFlamegraphFunctionsWrapper>
       <DifferentialFlamegraphChangedFunctionsTitle
         title={props.title}
         subtitle={props.subtitle}
@@ -518,17 +566,20 @@ function DifferentialFlamegraphChangedFunctions(
               state.page * state.pageSize,
               state.page * state.pageSize + state.pageSize
             )
-            .map((func, idx) => {
+            .map((func: [number, FlamegraphFrame] | FlamegraphFrame, idx) => {
+              const frame = Array.isArray(func) ? func[1] : func;
+              const changeValue = Array.isArray(func) ? func[0] : null;
+
               const countAfter =
                 props.flamegraph.afterCounts.get(
-                  DifferentialFlamegraphModel.FrameKey(func[1])
+                  DifferentialFlamegraphModel.FrameKey(frame)
                 ) ?? 0;
               const countBefore =
                 props.flamegraph.beforeCounts.get(
-                  DifferentialFlamegraphModel.FrameKey(func[1])
+                  DifferentialFlamegraphModel.FrameKey(frame)
                 ) ?? 0;
 
-              const linkToFlamechart = props.makeFunctionLink(func[1]);
+              const linkToFlamechart = props.makeFunctionLink(frame);
               return (
                 <DifferentialFlamegraphChangedFunctionContainer key={idx}>
                   <div>
@@ -536,33 +587,63 @@ function DifferentialFlamegraphChangedFunctions(
                       disabled={!linkToFlamechart}
                       to={linkToFlamechart}
                     >
-                      {func[1].frame.name}
+                      <DifferentialFlamegraphFunctionColorIndicator
+                        style={{
+                          backgroundColor: colorComponentsToRGBA(
+                            props.flamegraph.colors.get(
+                              DifferentialFlamegraphModel.FrameKey(frame)
+                            ) ?? theme.COLORS.FRAME_FALLBACK_COLOR
+                          ),
+                        }}
+                      />
+                      <span>{frame.frame.name}</span>
                     </DifferentialFlamegraphChangedFunctionNameLink>
                     <DifferentialFlamegraphChangedFunctionModule>
-                      {func[1].frame.module ||
-                        func[1].frame.package ||
-                        func[1].frame.file}
+                      {frame.frame.module || frame.frame.package || frame.frame.file}
                     </DifferentialFlamegraphChangedFunctionModule>
                   </div>
 
-                  <DifferentialFlamegraphChangedFunctionStats>
-                    <div>
-                      {countAfter > countBefore ? '+' : ''}
-                      {formatPercentage(relativeChange(countAfter, countBefore))}
-                      {/* diff % */}
-                      {/* n samples, x weight */}
-                    </div>
-                    <DifferentialFlamegraphFunctionSecondaryStats>
-                      {formatAbbreviatedNumber(func[1].node.selfWeight)} {t('samples')}
-                    </DifferentialFlamegraphFunctionSecondaryStats>
-                  </DifferentialFlamegraphChangedFunctionStats>
+                  {typeof changeValue === 'number' ? (
+                    <DifferentialFlamegraphChangedFunctionStats>
+                      <div>
+                        {countAfter > countBefore ? '+' : ''}
+                        {formatPercentage(relativeChange(countAfter, countBefore))}
+                        {/* diff % */}
+                        {/* n samples, x weight */}
+                      </div>
+                      <DifferentialFlamegraphFunctionSecondaryStats>
+                        {formatAbbreviatedNumber(frame.node.selfWeight)} {t('samples')}
+                      </DifferentialFlamegraphFunctionSecondaryStats>
+                    </DifferentialFlamegraphChangedFunctionStats>
+                  ) : null}
                 </DifferentialFlamegraphChangedFunctionContainer>
               );
             })}
-    </div>
+    </DifferentialFlamegraphFunctionsWrapper>
   );
 }
 
+const DifferentialFlamegraphFunctionsWrapper = styled('div')`
+  flex: 1;
+  width: 50%;
+  &:first-child {
+    padding-right: ${space(0.5)};
+  }
+  &:nth-child(2) {
+    padding-left: ${space(0.5)};
+  }
+`;
+
+const DifferentialFlamegraphFunctionColorIndicator = styled('div')`
+  width: 10px;
+  height: 10px;
+  border-radius: 2px;
+  display: inline-block;
+  border: 1px solid ${p => p.theme.border};
+  margin-right: ${space(0.25)};
+  background-color: ${p => p.theme.green300};
+`;
+
 const RIGHT_ALIGN_PLACEHOLDER_STYLES: React.CSSProperties = {
   marginBottom: '4px',
   marginLeft: 'auto',
@@ -575,6 +656,7 @@ const MARGIN_BOTTOM_PLACEHOLDER_STYLES: React.CSSProperties = {
 
 const DifferentialFlamegraphChangedFunctionStats = styled('div')`
   text-align: right;
+  flex-shrink: 0;
 `;
 
 const DifferentialFlamegraphFunctionSecondaryStats = styled('div')`
@@ -583,12 +665,23 @@ const DifferentialFlamegraphFunctionSecondaryStats = styled('div')`
 `;
 
 const DifferentialFlamegraphChangedFunctionNameLink = styled(Link)`
-  overflow: hidden;
-  text-overflow: ellipsis;
+  display: flex;
+  flex-direction: row;
+  align-items: center;
+  white-space: nowrap;
+
+  > span {
+    overflow: hidden;
+    text-overflow: ellipsis;
+    min-width: 0;
+  }
 `;
 
 const DifferentialFlamegraphChangedFunctionModule = styled('div')`
   color: ${p => p.theme.subText};
+  min-width: 0;
+  text-overflow: ellipsis;
+  overflow: hidden;
 `;
 
 const DifferentialFlamegraphChangedFunctionContainer = styled('div')`
@@ -598,7 +691,9 @@ const DifferentialFlamegraphChangedFunctionContainer = styled('div')`
   justify-content: space-between;
   gap: ${space(1)};
   padding: ${space(0.5)} ${space(0)};
+
   > *:first-child {
+    min-width: 0;
     flex: 1;
   }
 `;
@@ -624,6 +719,7 @@ function DifferentialFlamegraphExplanationBar(
 const DifferentialFlamegraphExplanationBarContainer = styled('div')`
   display: flex;
   justify-content: space-between;
+  border-radius: 0 0 ${p => p.theme.borderRadius} ${p => p.theme.borderRadius};
   padding: ${space(0.5)} ${space(1)};
   font-size: ${p => p.theme.fontSizeExtraSmall};
   color: ${p => p.theme.subText};
@@ -726,15 +822,9 @@ const DifferentialFlamegraphChangedFunctionsSubtitleText = styled('div')`
 `;
 
 const DifferentialFlamegraphFunctionsContainer = styled('div')`
-  border-top: 1px solid ${p => p.theme.border};
   display: flex;
   flex-direction: row;
-  gap: ${space(2)};
   padding: ${space(1)};
-
-  > div {
-    flex: 0.5;
-  }
 `;
 
 const DifferentialFlamegraphPaginationButton = styled(Button)`

+ 4 - 4
static/app/components/profiling/flamegraph/flamegraphToolbar/differentialFlamegraphNegationSwitch.tsx

@@ -27,14 +27,14 @@ export function DifferentialFlamegraphNegationSwitch(
         value={props.negated ? 'negated' : 'regular'}
         onChange={onWrapChange}
       >
-        <SegmentedControl.Item key="negated">{t('Before → After')}</SegmentedControl.Item>
-        <SegmentedControl.Item key="regular">{t('After → Before')}</SegmentedControl.Item>
+        <SegmentedControl.Item key="negated">{t('Baseline')}</SegmentedControl.Item>
+        <SegmentedControl.Item key="regular">{t('Regressed')}</SegmentedControl.Item>
       </SegmentedControl>
     </DifferentialFlamegraphNegationSwitchContainer>
   );
 }
 
 const DifferentialFlamegraphNegationSwitchContainer = styled('div')`
-  /* after this size, the text is quickly truncated */
-  min-width: 210px;
+  /* ensure text is not truncated */
+  flex-shrink: 0;
 `;

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

@@ -110,6 +110,8 @@ function DifferentialFlamegraphTooltip(props: DifferentialFlamegraphTooltipProps
   const change = shouldShowChange ? relativeChange(countAfter, countBefore) : 0;
   const formattedChange = shouldShowChange
     ? `${countAfter > countBefore ? '+' : ''}${formatPercentage(change)}`
+    : props.flamegraph.negated
+    ? t('removed function')
     : t(`new function`);
 
   return (

+ 175 - 4
static/app/utils/profiling/differentialFlamegraph.spec.tsx

@@ -77,7 +77,11 @@ describe('differentialFlamegraph', () => {
       ],
     });
 
-    const flamegraph = DifferentialFlamegraph.FromDiff({before, after}, THEME);
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: false},
+      THEME
+    );
 
     expect(flamegraph.colors?.get('new function')).toEqual([
       ...THEME.COLORS.DIFFERENTIAL_INCREASE,
@@ -85,6 +89,45 @@ describe('differentialFlamegraph', () => {
     ]);
   });
 
+  it('tracks removed frames and creates color entry', () => {
+    const before = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}, {name: 'removed function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [1]],
+          weights: [1, 1],
+        },
+      ],
+    });
+    const after = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [1, 1],
+        },
+      ],
+    });
+
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: false},
+      THEME
+    );
+
+    expect(flamegraph.colors?.get('removed function')).toEqual([
+      ...THEME.COLORS.DIFFERENTIAL_DECREASE,
+      1 * DifferentialFlamegraph.ALPHA_SCALING,
+    ]);
+    expect(flamegraph.removedFrames?.[0].frame.name).toBe('removed function');
+  });
+
   it('sums weight across all stacks', () => {
     const before = makeFlamegraph({
       shared: {
@@ -111,7 +154,11 @@ describe('differentialFlamegraph', () => {
       ],
     });
 
-    const flamegraph = DifferentialFlamegraph.FromDiff({before, after}, THEME);
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: false},
+      THEME
+    );
 
     expect(flamegraph.colors?.get('function')).toEqual([
       ...THEME.COLORS.DIFFERENTIAL_INCREASE,
@@ -149,7 +196,11 @@ describe('differentialFlamegraph', () => {
       ],
     });
 
-    const flamegraph = DifferentialFlamegraph.FromDiff({before, after}, THEME);
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: false},
+      THEME
+    );
 
     expect(flamegraph.colors?.get('function')).toEqual([
       ...THEME.COLORS.DIFFERENTIAL_INCREASE,
@@ -187,7 +238,11 @@ describe('differentialFlamegraph', () => {
       ],
     });
 
-    const flamegraph = DifferentialFlamegraph.FromDiff({before, after}, THEME);
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: false},
+      THEME
+    );
 
     expect(flamegraph.colors?.get('function')).toEqual([
       ...THEME.COLORS.DIFFERENTIAL_DECREASE,
@@ -199,3 +254,119 @@ describe('differentialFlamegraph', () => {
     ]);
   });
 });
+
+describe('negation', () => {
+  it('color encodes removed frames blue', () => {
+    const before = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}, {name: 'other function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [1]],
+          weights: [11, 4],
+        },
+      ],
+    });
+    const after = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}, {name: 'other function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [1, 1],
+        },
+      ],
+    });
+
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: true},
+      THEME
+    );
+
+    expect(flamegraph.colors?.get('other function')).toEqual([
+      ...THEME.COLORS.DIFFERENTIAL_DECREASE,
+      1 * DifferentialFlamegraph.ALPHA_SCALING,
+    ]);
+  });
+
+  it('increase: color encodes functions that got slower as red', () => {
+    const before = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [1, 1],
+        },
+      ],
+    });
+    const after = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [5, 5],
+        },
+      ],
+    });
+
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: true},
+      THEME
+    );
+
+    expect(flamegraph.colors?.get('function')).toEqual([
+      ...THEME.COLORS.DIFFERENTIAL_INCREASE,
+      1 * DifferentialFlamegraph.ALPHA_SCALING,
+    ]);
+  });
+
+  it('decrease: color encodes functions that got faster as blue', () => {
+    const before = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [10, 10],
+        },
+      ],
+    });
+    const after = makeFlamegraph({
+      shared: {
+        frames: [{name: 'function'}],
+      },
+      profiles: [
+        {
+          ...baseProfile,
+          samples: [[0], [0]],
+          weights: [1, 1],
+        },
+      ],
+    });
+
+    const flamegraph = DifferentialFlamegraph.FromDiff(
+      {before, after},
+      {negated: true},
+      THEME
+    );
+
+    expect(flamegraph.colors?.get('function')).toEqual([
+      ...THEME.COLORS.DIFFERENTIAL_DECREASE,
+      1 * DifferentialFlamegraph.ALPHA_SCALING,
+    ]);
+  });
+});

+ 69 - 21
static/app/utils/profiling/differentialFlamegraph.tsx

@@ -19,15 +19,16 @@ function makeFrameMap(frames: ReadonlyArray<FlamegraphFrame>): Map<string, numbe
 export class DifferentialFlamegraph extends Flamegraph {
   colors: Map<string, ColorChannels> = new Map();
   colorBuffer: number[] = [];
-
   beforeCounts: Map<string, number> = new Map();
   afterCounts: Map<string, number> = new Map();
 
   newFrames: FlamegraphFrame[] = [];
+  removedFrames: FlamegraphFrame[] = [];
   increasedFrames: [number, FlamegraphFrame][] = [];
   decreasedFrames: [number, FlamegraphFrame][] = [];
 
   static ALPHA_SCALING = 0.8;
+  public negated: boolean = false;
 
   static FrameKey(frame: FlamegraphFrame): string {
     return (
@@ -48,9 +49,15 @@ export class DifferentialFlamegraph extends Flamegraph {
 
   static FromDiff(
     {before, after}: {after: Flamegraph; before: Flamegraph},
+    // When drawing a negated view, the differential flamegraph renders the flamegraph
+    // of the previous state, with the colors of the after state. This way we can see
+    // how the exectution of the program changed, i.e. see into the future.
+    {negated}: {negated: boolean},
     theme: FlamegraphTheme
   ): DifferentialFlamegraph {
-    const differentialFlamegraph = new DifferentialFlamegraph(after.profile, {
+    const sourceFlamegraph = negated ? before : after;
+
+    const differentialFlamegraph = new DifferentialFlamegraph(sourceFlamegraph.profile, {
       inverted: after.inverted,
       sort: after.sort,
     });
@@ -61,78 +68,119 @@ export class DifferentialFlamegraph extends Flamegraph {
     const afterCounts = makeFrameMap(after.frames);
 
     const newFrames: FlamegraphFrame[] = [];
+    const removedFrames: FlamegraphFrame[] = [];
     const increasedFrames: [number, FlamegraphFrame][] = [];
     const decreasedFrames: [number, FlamegraphFrame][] = [];
 
-    // @TODO do we want to show removed frames?
-    // This would require iterating over the entire
-    // before frame list and checking if the frame is
-    // still present in the after frame list
+    const INCREASED_FRAME_COLOR = theme.COLORS.DIFFERENTIAL_INCREASE;
+    const DECREASED_FRAME_COLOR = theme.COLORS.DIFFERENTIAL_DECREASE;
+    const NEW_FRAME_COLOR = INCREASED_FRAME_COLOR.concat(
+      1 * DifferentialFlamegraph.ALPHA_SCALING
+    );
+    const REMOVED_FRAME_COLOR = DECREASED_FRAME_COLOR.concat(
+      1 * DifferentialFlamegraph.ALPHA_SCALING
+    );
 
     // Keep track of max increase and decrease so that we can
     // scale the colors accordingly to the max value
     let maxIncrease = 0;
     let maxDecrease = 0;
 
+    // Find frames that are in the before state, but not in the after state
+    for (const frame of before.frames) {
+      const key = DifferentialFlamegraph.FrameKey(frame);
+
+      !afterCounts.has(key) &&
+        removedFrames.push(frame) &&
+        colorMap.set(key, REMOVED_FRAME_COLOR as ColorChannels);
+    }
+
     for (const frame of after.frames) {
       const key = DifferentialFlamegraph.FrameKey(frame);
 
       const beforeCount = beforeCounts.get(key);
       const afterCount = afterCounts.get(key);
 
+      // In a negated view, frames missing in before state are new frames
+      if (beforeCount === undefined && negated) {
+        newFrames.push(frame);
+        continue;
+      }
+
       if (afterCount === undefined) {
         throw new Error(`Missing count for frame ${key}, this should never happen`);
       }
 
+      // In a non-negated view, frames missing in the before state are new frames
       if (beforeCount === undefined) {
         newFrames.push(frame);
-      } else if (afterCount > beforeCount) {
+        continue;
+      }
+
+      // If frames have same count, we don't need to color them
+      if (beforeCount === afterCount) {
+        continue;
+      }
+
+      // If the frame count increased, color it red
+      if (afterCount > beforeCount) {
         if (afterCount - beforeCount > maxIncrease) {
           maxIncrease = afterCount - beforeCount;
         }
         increasedFrames.push([afterCount - beforeCount, frame]);
-      } else if (beforeCount > afterCount) {
+        continue;
+      }
+
+      // If the frame count decreased, color it blue
+      if (beforeCount > afterCount) {
         if (beforeCount - afterCount > maxDecrease) {
           maxDecrease = beforeCount - afterCount;
         }
         decreasedFrames.push([beforeCount - afterCount, frame]);
+        continue;
       }
     }
 
     for (const frame of newFrames) {
-      colorMap.set(DifferentialFlamegraph.FrameKey(frame), [
-        ...theme.COLORS.DIFFERENTIAL_INCREASE,
-        1 * DifferentialFlamegraph.ALPHA_SCALING,
-      ] as ColorChannels);
+      colorMap.set(
+        DifferentialFlamegraph.FrameKey(frame),
+        NEW_FRAME_COLOR as ColorChannels
+      );
     }
 
     for (const frame of increasedFrames) {
-      colorMap.set(DifferentialFlamegraph.FrameKey(frame[1]), [
-        ...theme.COLORS.DIFFERENTIAL_INCREASE,
-        (frame[0] / maxIncrease) * DifferentialFlamegraph.ALPHA_SCALING,
-      ] as ColorChannels);
+      colorMap.set(
+        DifferentialFlamegraph.FrameKey(frame[1]),
+        INCREASED_FRAME_COLOR.concat(
+          (frame[0] / maxIncrease) * DifferentialFlamegraph.ALPHA_SCALING
+        ) as ColorChannels
+      );
     }
 
     for (const frame of decreasedFrames) {
-      colorMap.set(DifferentialFlamegraph.FrameKey(frame[1]), [
-        ...theme.COLORS.DIFFERENTIAL_DECREASE,
-        (frame[0] / maxDecrease) * DifferentialFlamegraph.ALPHA_SCALING,
-      ] as ColorChannels);
+      colorMap.set(
+        DifferentialFlamegraph.FrameKey(frame[1]),
+        DECREASED_FRAME_COLOR.concat(
+          (frame[0] / maxDecrease) * DifferentialFlamegraph.ALPHA_SCALING
+        ) as ColorChannels
+      );
     }
 
     differentialFlamegraph.colors = colorMap;
     differentialFlamegraph.colorBuffer = makeColorBuffer(
-      after.frames,
+      sourceFlamegraph.frames,
       colorMap,
       theme.COLORS.FRAME_FALLBACK_COLOR as unknown as ColorChannels,
       DifferentialFlamegraph.FrameKey
     );
 
     differentialFlamegraph.newFrames = newFrames;
+    differentialFlamegraph.removedFrames = removedFrames;
     differentialFlamegraph.increasedFrames = increasedFrames;
     differentialFlamegraph.decreasedFrames = decreasedFrames;
     differentialFlamegraph.beforeCounts = beforeCounts;
     differentialFlamegraph.afterCounts = afterCounts;
+    differentialFlamegraph.negated = negated;
 
     return differentialFlamegraph;
   }

+ 2 - 4
static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx

@@ -35,7 +35,6 @@ import {EventRRWebIntegration} from 'sentry/components/events/rrwebIntegration';
 import {DataSection} from 'sentry/components/events/styles';
 import {SuspectCommits} from 'sentry/components/events/suspectCommits';
 import {EventUserFeedback} from 'sentry/components/events/userFeedback';
-import Panel from 'sentry/components/panels/panel';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {Event, Group, IssueCategory, IssueType, Project} from 'sentry/types';
@@ -235,9 +234,8 @@ function ProfilingDurationRegressionIssueDetailsContent({
                 frame with the largest increase in call stack population likely
                 contributed to the cause for the duration regression.`)}
               </p>
-              <Panel>
-                <EventDifferentialFlamegraph event={event} />
-              </Panel>
+
+              <EventDifferentialFlamegraph event={event} />
             </DataSection>
           </ErrorBoundary>
         </Feature>