feat(cache): plot and highlight transaction samples (#70521)

1. plots the transaction samples on the cache sidebar 2. Allows samples to be highlighted (similar to other starfish modules) <img width="611" alt="image" src="https://github.com/getsentry/sentry/assets/44422760/5195ce1d-800e-4b69-bd75-708707ff666a">
getsentry · May 9, 2024 · 36980c8 · 36980c8
1 parent cb038e9
commit 36980c8
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 14 deletions.
diff --git a/static/app/views/performance/cache/samplePanel/charts/transactionDurationChart.tsx b/static/app/views/performance/cache/samplePanel/charts/transactionDurationChart.tsx
@@ -1,16 +1,31 @@
+import type {EChartHighlightHandler} from 'sentry/types/echarts';
 import {decodeScalar} from 'sentry/utils/queryString';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import {Referrer} from 'sentry/views/performance/cache/referrers';
 import {CHART_HEIGHT} from 'sentry/views/performance/cache/settings';
+import type {DataRow} from 'sentry/views/performance/cache/tables/spanSamplesTable';
 import {AVG_COLOR} from 'sentry/views/starfish/colors';
 import Chart, {ChartType} from 'sentry/views/starfish/components/chart';
 import ChartPanel from 'sentry/views/starfish/components/chartPanel';
 import {useMetricsSeries} from 'sentry/views/starfish/queries/useDiscoverSeries';
 import type {MetricsQueryFilters} from 'sentry/views/starfish/types';
 import {DataTitles} from 'sentry/views/starfish/views/spans/types';
+import {useSampleScatterPlotSeries} from 'sentry/views/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';
 
-export function TransactionDurationChart() {
+type Props = {
+  averageTransactionDuration: number;
+  onHighlight: EChartHighlightHandler;
+  samples: DataRow[];
+  highlightedSpanId?: string;
+};
+
+export function TransactionDurationChart({
+  samples,
+  averageTransactionDuration,
+  onHighlight,
+  highlightedSpanId,
+}: Props) {
   const {transaction} = useLocationQuery({
     fields: {
       project: decodeScalar,
@@ -28,6 +43,33 @@ export function TransactionDurationChart() {
     referrer: Referrer.SAMPLES_CACHE_TRANSACTION_DURATION,
   });
 
+  const sampledSpanDataSeries = useSampleScatterPlotSeries(
+    samples,
+    averageTransactionDuration,
+    highlightedSpanId,
+    'transaction.duration'
+  );
+
+  // TODO: This is duplicated from `DurationChart` in `SampleList`. Resolve the duplication
+  const handleChartHighlight: EChartHighlightHandler = function (event) {
+    // TODO: Gross hack. Even though `scatterPlot` is a separate prop, it's just an array of `Series` that gets appended to the main series. To find the point that was hovered, we re-construct the correct series order. It would have been cleaner to just pass the scatter plot as its own, single series
+    const allSeries = [
+      data['avg(transaction.duration)'],
+      ...(sampledSpanDataSeries ?? []),
+    ];
+
+    const highlightedDataPoints = event.batch.map(batch => {
+      const {seriesIndex, dataIndex} = batch;
+
+      const highlightedSeries = allSeries?.[seriesIndex];
+      const highlightedDataPoint = highlightedSeries.data?.[dataIndex];
+
+      return {series: highlightedSeries, dataPoint: highlightedDataPoint};
+    });
+
+    onHighlight?.(highlightedDataPoints, event);
+  };
+
   return (
     <ChartPanel title={DataTitles.transactionDuration}>
       <Chart
@@ -38,8 +80,10 @@ export function TransactionDurationChart() {
           top: '8px',
           bottom: '0',
         }}
+        scatterPlot={sampledSpanDataSeries}
         data={[data['avg(transaction.duration)']]}
         loading={isLoading}
+        onHighlight={handleChartHighlight}
         chartColors={[AVG_COLOR]}
         type={ChartType.LINE}
       />

diff --git a/static/app/views/performance/cache/samplePanel/samplePanel.tsx b/static/app/views/performance/cache/samplePanel/samplePanel.tsx
@@ -21,6 +21,7 @@ import {Referrer} from 'sentry/views/performance/cache/referrers';
 import {TransactionDurationChart} from 'sentry/views/performance/cache/samplePanel/charts/transactionDurationChart';
 import {BASE_FILTERS} from 'sentry/views/performance/cache/settings';
 import {SpanSamplesTable} from 'sentry/views/performance/cache/tables/spanSamplesTable';
+import {useDebouncedState} from 'sentry/views/performance/http/useDebouncedState';
 import {MetricReadout} from 'sentry/views/performance/metricReadout';
 import * as ModuleLayout from 'sentry/views/performance/moduleLayout';
 import DetailPanel from 'sentry/views/starfish/components/detailPanel';
@@ -37,6 +38,7 @@ import {
   SpanMetricsField,
   type SpanMetricsQueryFilters,
 } from 'sentry/views/starfish/types';
+import {findSampleFromDataPoint} from 'sentry/views/starfish/utils/chart/findDataPoint';
 import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
 
 // This is similar to http sample table, its difficult to use the generic span samples sidebar as we require a bunch of custom things.
@@ -51,6 +53,12 @@ export function CacheSamplePanel() {
     },
   });
 
+  const [highlightedSpanId, setHighlightedSpanId] = useDebouncedState<string | undefined>(
+    undefined,
+    [],
+    10
+  );
+
   // `detailKey` controls whether the panel is open. If all required properties are ailable, concat them to make a key, otherwise set to `undefined` and hide the panel
   const detailKey = query.transaction
     ? [query.transaction].filter(Boolean).join(':')
@@ -240,7 +248,30 @@ export function CacheSamplePanel() {
 
           <Fragment>
             <ModuleLayout.Full>
-              <TransactionDurationChart />
+              <TransactionDurationChart
+                samples={spansWithDuration}
+                averageTransactionDuration={
+                  transactionDurationData?.[0]?.[
+                    `avg(${MetricsFields.TRANSACTION_DURATION})`
+                  ]
+                }
+                highlightedSpanId={highlightedSpanId}
+                onHighlight={highlights => {
+                  const firstHighlight = highlights[0];
+
+                  if (!firstHighlight) {
+                    setHighlightedSpanId(undefined);
+                    return;
+                  }
+
+                  const sample = findSampleFromDataPoint<(typeof spansWithDuration)[0]>(
+                    firstHighlight.dataPoint,
+                    spansWithDuration,
+                    'transaction.duration'
+                  );
+                  setHighlightedSpanId(sample?.span_id);
+                }}
+              />
             </ModuleLayout.Full>
           </Fragment>
           <Fragment>
@@ -255,6 +286,9 @@ export function CacheSamplePanel() {
                   units: {[SpanIndexedField.CACHE_ITEM_SIZE]: 'byte'},
                 }}
                 isLoading={isCacheSpanSamplesFetching || isFetchingTransactions}
+                highlightedSpanId={highlightedSpanId}
+                onSampleMouseOver={sample => setHighlightedSpanId(sample.span_id)}
+                onSampleMouseOut={() => setHighlightedSpanId(undefined)}
                 error={transactionError}
               />
             </ModuleLayout.Full>

diff --git a/static/app/views/performance/cache/tables/spanSamplesTable.tsx b/static/app/views/performance/cache/tables/spanSamplesTable.tsx
@@ -35,7 +35,9 @@ type ColumnKeys =
   | SpanIndexedField.CACHE_ITEM_SIZE
   | 'transaction.duration';
 
-type DataRow = Pick<IndexedResponse, DataRowKeys> & {'transaction.duration': number};
+export type DataRow = Pick<IndexedResponse, DataRowKeys> & {
+  'transaction.duration': number;
+};
 
 type Column = GridColumnHeader<ColumnKeys>;
 

diff --git a/static/app/views/performance/http/data/useSpanSamples.tsx b/static/app/views/performance/http/data/useSpanSamples.tsx
@@ -58,6 +58,7 @@ export const useSpanSamples = <Fields extends IndexedProperty[]>(
           | SpanIndexedField.TIMESTAMP
           | SpanIndexedField.ID
           | SpanIndexedField.PROFILE_ID
+          | SpanIndexedField.SPAN_SELF_TIME
         >[]
       // This type is a little awkward but it explicitly states that the response could be empty. This doesn't enable unchecked access errors, but it at least indicates that it's possible that there's no data
       // eslint-disable-next-line @typescript-eslint/ban-types

diff --git a/static/app/views/performance/http/httpSamplesPanel.tsx b/static/app/views/performance/http/httpSamplesPanel.tsx
@@ -49,6 +49,7 @@ import {
   SpanMetricsField,
   type SpanMetricsQueryFilters,
 } from 'sentry/views/starfish/types';
+import {findSampleFromDataPoint} from 'sentry/views/starfish/utils/chart/findDataPoint';
 import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
 import {useSampleScatterPlotSeries} from 'sentry/views/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';
 
@@ -242,12 +243,6 @@ export function HTTPSamplesPanel() {
     highlightedSpanId
   );
 
-  const findSampleFromDataPoint = (dataPoint: {name: string | number; value: number}) => {
-    return durationSamplesData.find(
-      s => s.timestamp === dataPoint.name && s['span.self_time'] === dataPoint.value
-    );
-  };
-
   const handleClose = () => {
     router.replace({
       pathname: router.location.pathname,
@@ -402,7 +397,14 @@ export function HTTPSamplesPanel() {
                       return;
                     }
 
-                    const sample = findSampleFromDataPoint(firstHighlight.dataPoint);
+                    const sample = findSampleFromDataPoint<
+                      (typeof durationSamplesData)[0]
+                    >(
+                      firstHighlight.dataPoint,
+                      durationSamplesData,
+                      SpanIndexedField.SPAN_SELF_TIME
+                    );
+
                     setHighlightedSpanId(sample?.span_id);
                   }}
                   isLoading={isDurationDataFetching}

diff --git a/static/app/views/starfish/utils/chart/findDataPoint.ts b/static/app/views/starfish/utils/chart/findDataPoint.ts
@@ -0,0 +1,9 @@
+export function findSampleFromDataPoint<T extends {timestamp: string}>(
+  dataPoint: {name: string | number; value: number},
+  data: T[],
+  matchKey: keyof T
+) {
+  return data?.find(
+    s => s.timestamp === dataPoint.name && s[matchKey] === dataPoint.value
+  );
+}
diff --git a/...ws/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries.tsx b/...ws/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries.tsx
@@ -12,15 +12,16 @@ import {getSampleChartSymbol} from 'sentry/views/starfish/views/spanSummaryPage/
 export function useSampleScatterPlotSeries(
   spans: Partial<IndexedResponse>[],
   average?: number,
-  highlightedSpanId?: string
+  highlightedSpanId?: string,
+  key: string = 'span.self_time'
 ): Series[] {
   const theme = useTheme();
 
   return spans.map(span => {
     let symbol, color;
 
-    if (span['span.self_time'] && defined(average)) {
-      ({symbol, color} = getSampleChartSymbol(span['span.self_time'], average, theme));
+    if (span[key] && defined(average)) {
+      ({symbol, color} = getSampleChartSymbol(span[key], average, theme));
     } else {
       symbol = 'circle';
       color = AVG_COLOR;
@@ -30,7 +31,7 @@ export function useSampleScatterPlotSeries(
       data: [
         {
           name: span?.timestamp ?? span.span_id ?? t('Span'),
-          value: span?.['span.self_time'] ?? 0,
+          value: span?.[key] ?? 0,
         },
       ],
       symbol,