Browse Source

chore(flamegraph): Remove transactions from differential flamegraphs (#80807)

Due to continuous profiling, we can no longer associate functions to
transactions. Only transactions to functions. This removes any reference
to transactions in differential flamegraphs so we are only working with
functions.
Tony Xiao 3 months ago
parent
commit
07c88ec1a5

+ 14 - 173
static/app/components/events/eventStatisticalDetector/eventDifferentialFlamegraph.tsx

@@ -8,7 +8,6 @@ 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';
 import {DifferentialFlamegraphToolbar} from 'sentry/components/profiling/flamegraph/flamegraphToolbar/differentialFlamegraphToolbar';
@@ -18,6 +17,7 @@ import ProjectsStore from 'sentry/stores/projectsStore';
 import {space} from 'sentry/styles/space';
 import type {Event} from 'sentry/types/event';
 import type {Project} from 'sentry/types/project';
+import {defined} from 'sentry/utils';
 import {formatAbbreviatedNumber} from 'sentry/utils/formatters';
 import {formatPercentage} from 'sentry/utils/number/formatPercentage';
 import {
@@ -31,7 +31,6 @@ import {FlamegraphThemeProvider} from 'sentry/utils/profiling/flamegraph/flamegr
 import {useFlamegraphTheme} from 'sentry/utils/profiling/flamegraph/useFlamegraphTheme';
 import type {FlamegraphFrame} from 'sentry/utils/profiling/flamegraphFrame';
 import type {Frame} from 'sentry/utils/profiling/frame';
-import type {EventsResultsDataRow} from 'sentry/utils/profiling/hooks/types';
 import {useDifferentialFlamegraphModel} from 'sentry/utils/profiling/hooks/useDifferentialFlamegraphModel';
 import type {DifferentialFlamegraphQueryResult} from 'sentry/utils/profiling/hooks/useDifferentialFlamegraphQuery';
 import {useDifferentialFlamegraphQuery} from 'sentry/utils/profiling/hooks/useDifferentialFlamegraphQuery';
@@ -41,8 +40,6 @@ import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {LOADING_PROFILE_GROUP} from 'sentry/views/profiling/profileGroupProvider';
 
-import {useTransactionsDelta} from './transactionsDeltaProvider';
-
 interface EventDifferentialFlamegraphProps {
   event: Event;
 }
@@ -75,53 +72,13 @@ export function EventDifferentialFlamegraph(props: EventDifferentialFlamegraphPr
     });
   }, [isValid, fingerprint, breakpoint]);
 
-  const transactions = useTransactionsDelta();
-  const [transaction, setTransaction] = useState<
-    EventsResultsDataRow<string> | undefined
-  >(undefined);
-
-  if (transaction === undefined) {
-    const firstTransaction = transactions?.data?.data?.[0];
-    if (firstTransaction) {
-      setTransaction(firstTransaction);
-    }
-  }
-
   const {before, after} = useDifferentialFlamegraphQuery({
     projectID: parseInt(props.event.projectID, 10),
     breakpoint,
     environments: selection.selection.environments,
     fingerprint: props.event.occurrence?.evidenceData?.fingerprint,
-    transaction: (transaction?.transaction as string) ?? '',
   });
 
-  const onNextTransactionClick = useMemo(() => {
-    if (!transaction) {
-      return undefined;
-    }
-    const idx = transactions?.data?.data?.indexOf?.(transaction) ?? -1;
-    if (idx === -1 || idx === (transactions?.data?.data?.length ?? 0) - 1) {
-      return undefined;
-    }
-
-    return () => {
-      setTransaction(transactions?.data?.data?.[idx + 1] ?? transaction);
-    };
-  }, [transaction, transactions?.data?.data]);
-
-  const onPreviousTransactionClick = useMemo(() => {
-    if (!transaction) {
-      return undefined;
-    }
-    const idx = transactions?.data?.data?.indexOf?.(transaction) ?? -1;
-    if (idx === -1 || idx === 0) {
-      return undefined;
-    }
-    return () => {
-      setTransaction(transactions?.data?.data?.[idx - 1] ?? transaction);
-    };
-  }, [transaction, transactions?.data?.data]);
-
   return (
     <Fragment>
       <FlamegraphThemeProvider>
@@ -135,9 +92,6 @@ export function EventDifferentialFlamegraph(props: EventDifferentialFlamegraphPr
         >
           <EventDifferentialFlamegraphView
             project={project}
-            onNextTransactionClick={onNextTransactionClick}
-            onPreviousTransactionClick={onPreviousTransactionClick}
-            transaction={transaction}
             before={before}
             after={after}
           />
@@ -158,10 +112,7 @@ function systemFrameOnly(frame: Frame): boolean {
 interface EventDifferentialFlamegraphViewProps {
   after: DifferentialFlamegraphQueryResult['before'];
   before: DifferentialFlamegraphQueryResult['after'];
-  onNextTransactionClick: (() => void) | undefined;
-  onPreviousTransactionClick: (() => void) | undefined;
   project: Project | undefined;
-  transaction: EventsResultsDataRow<string> | undefined;
 }
 function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewProps) {
   const organization = useOrganization();
@@ -196,27 +147,21 @@ function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewP
       if (!frame.profileIds?.length) {
         return '';
       }
-      const profileId = frame.profileIds[0];
-
-      if (typeof profileId !== 'undefined') {
-        return (
-          generateProfileRouteFromProfileReference({
-            orgSlug: organization.slug,
-            projectSlug: props.project.slug,
-            reference:
-              typeof profileId === 'string'
-                ? profileId
-                : 'profiler_id' in profileId
-                  ? profileId.profiler_id
-                  : profileId.profile_id,
-            framePackage: frame.frame.package,
-            frameName: frame.frame.name,
-          }) ?? ''
-        );
+      const profile = frame.profileIds?.[0];
+
+      if (!defined(profile)) {
+        return '';
       }
 
-      // Regression issues do not work with continuous profiles
-      return '';
+      return (
+        generateProfileRouteFromProfileReference({
+          orgSlug: organization.slug,
+          projectSlug: props.project.slug,
+          reference: profile,
+          framePackage: frame.frame.package,
+          frameName: frame.frame.name,
+        }) ?? ''
+      );
     },
     [organization.slug, props.project]
   );
@@ -224,11 +169,6 @@ function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewP
   return (
     <FlamegraphContainer>
       <StyledPanel>
-        <DifferentialFlamegraphTransactionToolbar
-          transaction={props.transaction}
-          onNextTransactionClick={props.onNextTransactionClick}
-          onPreviousTransactionClick={props.onPreviousTransactionClick}
-        />
         <DifferentialFlamegraphToolbar
           frameFilter={frameFilterSetting}
           onFrameFilterChange={setFrameFilterSetting}
@@ -310,85 +250,6 @@ function EventDifferentialFlamegraphView(props: EventDifferentialFlamegraphViewP
   );
 }
 
-const numberFormatter = Intl.NumberFormat(undefined, {
-  maximumFractionDigits: 2,
-});
-
-interface DifferentialFlamegraphTransactionToolbarProps {
-  onNextTransactionClick: (() => void) | undefined;
-  onPreviousTransactionClick: (() => void) | undefined;
-  transaction: EventsResultsDataRow<string> | undefined;
-}
-function DifferentialFlamegraphTransactionToolbar(
-  props: DifferentialFlamegraphTransactionToolbarProps
-) {
-  const [before, after] = useMemo(() => {
-    if (!props.transaction) {
-      return [0, 0];
-    }
-
-    const keys = Object.keys(props.transaction);
-
-    let beforePercentile = 0;
-    let afterPercentile = 0;
-
-    for (const key of keys) {
-      if (key.startsWith('percentile_after')) {
-        afterPercentile = props.transaction[key] as number;
-      }
-      if (key.startsWith('percentile_before')) {
-        beforePercentile = props.transaction[key] as number;
-      }
-    }
-
-    return [beforePercentile, afterPercentile];
-  }, [props.transaction]);
-
-  return (
-    <DifferentialFlamegraphTransactionToolbarContainer>
-      {props.transaction?.transaction ? (
-        <DifferentialFlamegraphTransactionName>
-          {props.transaction.transaction}
-        </DifferentialFlamegraphTransactionName>
-      ) : (
-        <Placeholder height="20px" width="66%" />
-      )}
-
-      {props.transaction ? (
-        <span>
-          <PerformanceDuration nanoseconds={before} abbreviation />
-          <DifferentialFlamegraphRegressionChange>
-            {after === 0 || before === 0
-              ? ''
-              : '+' + numberFormatter.format(relativeChange(after, before) * 100) + '%'}
-          </DifferentialFlamegraphRegressionChange>
-        </span>
-      ) : (
-        <Fragment>
-          <Placeholder height="20px" width="60px" />
-          <Placeholder height="20px" width="60px" />
-        </Fragment>
-      )}
-      <ButtonBar merged>
-        <DifferentialFlamegraphPaginationButton
-          icon={<IconChevron direction="left" />}
-          aria-label={t('Previous Transaction')}
-          size="xs"
-          disabled={!props.onPreviousTransactionClick}
-          onClick={props.onPreviousTransactionClick}
-        />
-        <DifferentialFlamegraphPaginationButton
-          icon={<IconChevron direction="right" />}
-          aria-label={t('Next Transaction')}
-          size="xs"
-          disabled={!props.onNextTransactionClick}
-          onClick={props.onNextTransactionClick}
-        />
-      </ButtonBar>
-    </DifferentialFlamegraphTransactionToolbarContainer>
-  );
-}
-
 interface PaginationReducerState {
   page: number;
   pageCount: number;
@@ -762,26 +623,6 @@ const DifferentialFlamegraphPaginationButton = styled(Button)`
   padding-left: ${space(0.75)};
   padding-right: ${space(0.75)};
 `;
-const DifferentialFlamegraphTransactionName = styled('div')`
-  font-weight: ${p => p.theme.fontWeightBold};
-  flex: 1;
-  overflow: hidden;
-  text-overflow: ellipsis;
-`;
-
-const DifferentialFlamegraphRegressionChange = styled('span')`
-  margin-left: ${space(1)};
-  color: ${p => p.theme.red300};
-`;
-
-const DifferentialFlamegraphTransactionToolbarContainer = styled('div')`
-  display: flex;
-  justify-content: space-between;
-  align-items: center;
-  padding: ${space(1)};
-  gap: ${space(1)};
-  border-bottom: 1px solid ${p => p.theme.border};
-`;
 
 const ErrorMessageContainer = styled('div')`
   position: absolute;

+ 0 - 80
static/app/components/events/eventStatisticalDetector/transactionsDeltaProvider.tsx

@@ -1,80 +0,0 @@
-import type React from 'react';
-import {createContext, useContext} from 'react';
-
-import {RELATIVE_DAYS_WINDOW} from 'sentry/components/events/eventStatisticalDetector/consts';
-import type {Event} from 'sentry/types/event';
-import type {Project} from 'sentry/types/project';
-import type {EventsResults} from 'sentry/utils/profiling/hooks/types';
-import {useProfileFunctions} from 'sentry/utils/profiling/hooks/useProfileFunctions';
-import {useRelativeDateTime} from 'sentry/utils/profiling/hooks/useRelativeDateTime';
-import type {UseApiQueryResult} from 'sentry/utils/queryClient';
-import type RequestError from 'sentry/utils/requestError/requestError';
-
-const TransactionsDeltaProviderContext = createContext<UseApiQueryResult<
-  EventsResults<string>,
-  RequestError
-> | null>(null);
-
-const TRANSACTIONS_LIMIT = 10;
-
-export function useTransactionsDelta(): UseApiQueryResult<
-  EventsResults<string>,
-  RequestError
-> {
-  const ctx = useContext(TransactionsDeltaProviderContext);
-  if (!ctx) {
-    throw new Error(
-      'useTransactionsDelta called outside of TransactionsDeltaProviderProvider'
-    );
-  }
-  return ctx;
-}
-
-interface TransactionsDeltaProviderProps {
-  children: React.ReactNode;
-  event: Event;
-  project: Project;
-}
-
-export function TransactionsDeltaProvider(props: TransactionsDeltaProviderProps) {
-  const evidenceData = props.event.occurrence?.evidenceData;
-  const fingerprint = evidenceData?.fingerprint;
-  const breakpoint = evidenceData?.breakpoint;
-
-  const datetime = useRelativeDateTime({
-    anchor: breakpoint,
-    relativeDays: RELATIVE_DAYS_WINDOW,
-  });
-
-  const regressionScore = `regression_score(function.duration, 0.95, ${breakpoint})`;
-  const percentileBefore = `percentile_before(function.duration, 0.95, ${breakpoint})`;
-  const percentileAfter = `percentile_after(function.duration, 0.95, ${breakpoint})`;
-  const throughputBefore = `cpm_before(${breakpoint})`;
-  const throughputAfter = `cpm_after(${breakpoint})`;
-
-  const transactionsDeltaQuery = useProfileFunctions({
-    datetime,
-    fields: [
-      'transaction',
-      percentileBefore,
-      percentileAfter,
-      throughputBefore,
-      throughputAfter,
-      regressionScore,
-    ],
-    sort: {
-      key: regressionScore,
-      order: 'desc',
-    },
-    query: `fingerprint:${fingerprint}`,
-    projects: [props.project.id],
-    limit: TRANSACTIONS_LIMIT,
-    referrer: 'api.profiling.functions.regression.transactions',
-  });
-
-  return (
-    <TransactionsDeltaProviderContext.Provider value={transactionsDeltaQuery}>
-      {props.children}
-    </TransactionsDeltaProviderContext.Provider>
-  );
-}

+ 2 - 19
static/app/utils/profiling/hooks/useDifferentialFlamegraphQuery.ts

@@ -2,7 +2,6 @@ import {useMemo} from 'react';
 
 import {RELATIVE_DAYS_WINDOW} from 'sentry/components/events/eventStatisticalDetector/consts';
 import {useRelativeDateTime} from 'sentry/utils/profiling/hooks/useRelativeDateTime';
-import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 
 import type {AggregateFlamegraphQueryParameters} from './useAggregateFlamegraphQuery';
 import {useAggregateFlamegraphQuery} from './useAggregateFlamegraphQuery';
@@ -12,7 +11,6 @@ interface DifferentialFlamegraphQueryParameters {
   environments: AggregateFlamegraphQueryParameters['environments'];
   fingerprint: string | undefined;
   projectID: number | null;
-  transaction: string;
 }
 
 export interface DifferentialFlamegraphQueryResult {
@@ -23,32 +21,17 @@ export interface DifferentialFlamegraphQueryResult {
 export function useDifferentialFlamegraphQuery(
   params: DifferentialFlamegraphQueryParameters
 ): DifferentialFlamegraphQueryResult {
-  const query = useMemo(() => {
-    // TODO: this should contain the user query
-    // wait util we fully switch over to the transactions dataset
-    const conditions = new MutableSearch('');
-    conditions.setFilterValues('transaction', [params.transaction]);
-    return conditions.formatString();
-  }, [params.transaction]);
-
   const sharedAggregateQueryParams: AggregateFlamegraphQueryParameters = useMemo(() => {
     const p: Exclude<AggregateFlamegraphQueryParameters, 'datetime'> = {
-      query,
+      query: '',
       environments: params.environments,
       fingerprint: params.fingerprint,
       projects:
         params.projectID === null || isNaN(params.projectID) ? [] : [params.projectID],
-      enabled: !!params.transaction,
     };
 
     return p;
-  }, [
-    query,
-    params.environments,
-    params.projectID,
-    params.fingerprint,
-    params.transaction,
-  ]);
+  }, [params.environments, params.projectID, params.fingerprint]);
 
   const regressionDateRange = useRelativeDateTime({
     anchor: params.breakpoint,

+ 17 - 20
static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx

@@ -23,7 +23,6 @@ import EventComparison from 'sentry/components/events/eventStatisticalDetector/e
 import {EventDifferentialFlamegraph} from 'sentry/components/events/eventStatisticalDetector/eventDifferentialFlamegraph';
 import {EventRegressionSummary} from 'sentry/components/events/eventStatisticalDetector/eventRegressionSummary';
 import {EventFunctionBreakpointChart} from 'sentry/components/events/eventStatisticalDetector/functionBreakpointChart';
-import {TransactionsDeltaProvider} from 'sentry/components/events/eventStatisticalDetector/transactionsDeltaProvider';
 import {EventTagsAndScreenshot} from 'sentry/components/events/eventTagsAndScreenshot';
 import {ScreenshotDataSection} from 'sentry/components/events/eventTagsAndScreenshot/screenshot/screenshotDataSection';
 import EventTagsDataSection from 'sentry/components/events/eventTagsAndScreenshot/tags';
@@ -331,25 +330,23 @@ export function EventDetailsContent({
       )}
       {issueTypeConfig.profilingDurationRegression.enabled && (
         <Fragment>
-          <TransactionsDeltaProvider event={event} project={project}>
-            <ErrorBoundary mini>
-              <EventFunctionBreakpointChart event={event} />
-            </ErrorBoundary>
-            <ErrorBoundary mini>
-              <InterimSection
-                type={SectionKey.REGRESSION_FLAMEGRAPH}
-                title={t('Regression Flamegraph')}
-              >
-                <b>{t('Largest Changes in Call Stack Frequency')}</b>
-                <p>
-                  {t(`See which functions changed the most before and after the regression. The
-              frame with the largest increase in call stack population likely
-              contributed to the cause for the duration regression.`)}
-                </p>
-                <EventDifferentialFlamegraph event={event} />
-              </InterimSection>
-            </ErrorBoundary>
-          </TransactionsDeltaProvider>
+          <ErrorBoundary mini>
+            <EventFunctionBreakpointChart event={event} />
+          </ErrorBoundary>
+          <ErrorBoundary mini>
+            <InterimSection
+              type={SectionKey.REGRESSION_FLAMEGRAPH}
+              title={t('Regression Flamegraph')}
+            >
+              <b>{t('Largest Changes in Call Stack Frequency')}</b>
+              <p>
+                {t(`See which functions changed the most before and after the regression. The
+            frame with the largest increase in call stack population likely
+            contributed to the cause for the duration regression.`)}
+              </p>
+              <EventDifferentialFlamegraph event={event} />
+            </InterimSection>
+          </ErrorBoundary>
         </Fragment>
       )}
       <EventHydrationDiff event={event} group={group} />

+ 0 - 1
static/app/views/profiling/differentialFlamegraph.tsx

@@ -81,7 +81,6 @@ function DifferentialFlamegraphView() {
     breakpoint: location.query.breakpoint as unknown as number,
     environments: selection.selection.environments,
     fingerprint: location.query.fingerprint as unknown as string,
-    transaction: location.query.transaction as unknown as string,
   });
 
   const differentialFlamegraph = useDifferentialFlamegraphModel({