feat(perf): Add `useSpanMetricsTopNSeries` data hook (#68619)

`useSpanMetricsTopNSeries` allows for `topEvents` queries on the spans
metrics dataset. This is a very similar hook to `useSpanMetricsSeries`
but with one important distinction: The contents of the response are
_unknown_. Unlike a regular timeseries query with a `yAxis`, the shape
of the response depends on the data!

This means that two things are different:

- the data is processed differently!
- the typing makes fewer guarantees about what comes back

That would be awkward to merge into the existing hook, so I didn't
bother.

I had to fix some logic in `useWrappedDiscoverTimeseriesQuery` to make
this work. Somewhere along the way, we lost support for `topEvents`
queries in there. The reason, in short:

- the code makes a decision on whether the response data will have one
axis or multi-axis responses based on the contents of `yAxis`
- if the response is single-axis, immediately returns processed data
- if the response is multi-axis, iterates the data and returns processed
data as an object

The trick is, `topEvents` queries can be single-axis, but _still require
iterating the data_! This was missing. The code change is that instead
of inferring how many axes there are from `yAxis`, it's done on _the
data itself_. If there are multiple keys, it iterates.

static/app/views/starfish/queries/getSeriesEventView.tsx

@@ -0,0 +1,48 @@
+import sortBy from 'lodash/sortBy';
+
+import type {PageFilters} from 'sentry/types';
+import {intervalToMilliseconds} from 'sentry/utils/dates';
+import EventView from 'sentry/utils/discover/eventView';
+import {parseFunction} from 'sentry/utils/discover/fields';
+import {DiscoverDatasets} from 'sentry/utils/discover/types';
+import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import {getIntervalForMetricFunction} from 'sentry/views/performance/database/getIntervalForMetricFunction';
+import {DEFAULT_INTERVAL} from 'sentry/views/performance/database/settings';
+
+export function getSeriesEventView(
+  search: MutableSearch | undefined,
+  fields: string[] = [],
+  pageFilters: PageFilters,
+  yAxis: string[],
+  topEvents?: number
+) {
+  // Pick the highest possible interval for the given yAxis selection. Find the ideal interval for each function, then choose the largest one. This results in the lowest granularity, but best performance.
+  const interval = sortBy(
+    yAxis.map(yAxisFunctionName => {
+      const parseResult = parseFunction(yAxisFunctionName);
+
+      if (!parseResult) {
+        return DEFAULT_INTERVAL;
+      }
+
+      return getIntervalForMetricFunction(parseResult.name, pageFilters.datetime);
+    }),
+    result => {
+      return intervalToMilliseconds(result);
+    }
+  ).at(-1);
+
+  return EventView.fromNewQueryWithPageFilters(
+    {
+      name: '',
+      query: search?.formatString() ?? undefined,
+      fields,
+      yAxis,
+      dataset: DiscoverDatasets.SPANS_METRICS,
+      interval,
+      topEvents: topEvents?.toString(),
+      version: 2,
+    },
+    pageFilters
+  );
+}

static/app/views/starfish/queries/useSpanMetricsSeries.spec.tsx

@@ -132,15 +132,6 @@ describe('useSpanMetricsSeries', () => {
     );
 
     await waitFor(() => expect(result.current.isLoading).toEqual(false));
-    expect(result.current.data).toEqual({
-      'spm()': {
-        data: [
-          {name: '2023-11-13T20:35:00+00:00', value: 7810.2},
-          {name: '2023-11-13T20:40:00+00:00', value: 1216.8},
-        ],
-        seriesName: 'spm()',
-      },
-    });
   });
 
   it('adjusts interval based on the yAxis', async () => {
@@ -188,4 +179,89 @@ describe('useSpanMetricsSeries', () => {
       )
     );
   });
+
+  it('rolls single-axis responses up into a series', async () => {
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events-stats/`,
+      method: 'GET',
+      body: {
+        data: [
+          [1699907700, [{count: 7810.2}]],
+          [1699908000, [{count: 1216.8}]],
+        ],
+      },
+    });
+
+    const {result, waitFor} = reactHooks.renderHook(
+      ({yAxis}) => useSpanMetricsSeries({yAxis}),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          yAxis: ['spm()'] as MetricsProperty[],
+        },
+      }
+    );
+
+    await waitFor(() => expect(result.current.isLoading).toEqual(false));
+
+    expect(result.current.data).toEqual({
+      'spm()': {
+        data: [
+          {name: '2023-11-13T20:35:00+00:00', value: 7810.2},
+          {name: '2023-11-13T20:40:00+00:00', value: 1216.8},
+        ],
+        seriesName: 'spm()',
+      },
+    });
+  });
+
+  it('rolls multi-axis responses up into multiple series', async () => {
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events-stats/`,
+      method: 'GET',
+      body: {
+        'http_response_rate(3)': {
+          data: [
+            [1699907700, [{count: 10.1}]],
+            [1699908000, [{count: 11.2}]],
+          ],
+        },
+        'http_response_rate(4)': {
+          data: [
+            [1699907700, [{count: 12.6}]],
+            [1699908000, [{count: 13.8}]],
+          ],
+        },
+      },
+    });
+
+    const {result, waitFor} = reactHooks.renderHook(
+      ({yAxis}) => useSpanMetricsSeries({yAxis}),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          yAxis: ['http_response_rate(3)', 'http_response_rate(4)'] as MetricsProperty[],
+        },
+      }
+    );
+
+    await waitFor(() => expect(result.current.isLoading).toEqual(false));
+
+    expect(result.current.data).toEqual({
+      'http_response_rate(3)': {
+        data: [
+          {name: '2023-11-13T20:35:00+00:00', value: 10.1},
+          {name: '2023-11-13T20:40:00+00:00', value: 11.2},
+        ],
+        seriesName: 'http_response_rate(3)',
+      },
+      'http_response_rate(4)': {
+        data: [
+          {name: '2023-11-13T20:35:00+00:00', value: 12.6},
+          {name: '2023-11-13T20:40:00+00:00', value: 13.8},
+        ],
+        seriesName: 'http_response_rate(4)',
+      },
+    });
+  });
 });

static/app/views/starfish/queries/useSpanMetricsSeries.tsx

@@ -1,16 +1,9 @@
 import keyBy from 'lodash/keyBy';
-import sortBy from 'lodash/sortBy';
 
-import type {PageFilters} from 'sentry/types';
 import type {Series} from 'sentry/types/echarts';
-import {intervalToMilliseconds} from 'sentry/utils/dates';
-import EventView from 'sentry/utils/discover/eventView';
-import {parseFunction} from 'sentry/utils/discover/fields';
-import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import usePageFilters from 'sentry/utils/usePageFilters';
-import {getIntervalForMetricFunction} from 'sentry/views/performance/database/getIntervalForMetricFunction';
-import {DEFAULT_INTERVAL} from 'sentry/views/performance/database/settings';
+import {getSeriesEventView} from 'sentry/views/starfish/queries/getSeriesEventView';
 import type {MetricsProperty} from 'sentry/views/starfish/types';
 import {useWrappedDiscoverTimeseriesQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
@@ -33,7 +26,13 @@ export const useSpanMetricsSeries = <Fields extends MetricsProperty[]>(
 
   const pageFilters = usePageFilters();
 
-  const eventView = getEventView(search, pageFilters.selection, yAxis);
+  const eventView = getSeriesEventView(
+    search,
+    undefined,
+    pageFilters.selection,
+    yAxis,
+    undefined
+  );
 
   const result = useWrappedDiscoverTimeseriesQuery<SpanMetricTimeseriesRow[]>({
     eventView,
@@ -59,38 +58,3 @@ export const useSpanMetricsSeries = <Fields extends MetricsProperty[]>(
 
   return {...result, data: parsedData};
 };
-
-function getEventView(
-  search: MutableSearch | undefined,
-  pageFilters: PageFilters,
-  yAxis: string[]
-) {
-  // Pick the highest possible interval for the given yAxis selection. Find the ideal interval for each function, then choose the largest one. This results in the lowest granularity, but best performance.
-  const interval = sortBy(
-    yAxis.map(yAxisFunctionName => {
-      const parseResult = parseFunction(yAxisFunctionName);
-
-      if (!parseResult) {
-        return DEFAULT_INTERVAL;
-      }
-
-      return getIntervalForMetricFunction(parseResult.name, pageFilters.datetime);
-    }),
-    result => {
-      return intervalToMilliseconds(result);
-    }
-  ).at(-1);
-
-  return EventView.fromNewQueryWithPageFilters(
-    {
-      name: '',
-      query: search?.formatString() ?? undefined,
-      fields: [],
-      yAxis,
-      dataset: DiscoverDatasets.SPANS_METRICS,
-      interval,
-      version: 2,
-    },
-    pageFilters
-  );
-}

static/app/views/starfish/queries/useSpanMetricsTopNSeries.spec.tsx

@@ -0,0 +1,134 @@
+import type {ReactNode} from 'react';
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {makeTestQueryClient} from 'sentry-test/queryClient';
+import {reactHooks} from 'sentry-test/reactTestingLibrary';
+
+import {QueryClientProvider} from 'sentry/utils/queryClient';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import {useSpanMetricsTopNSeries} from 'sentry/views/starfish/queries/useSpanMetricsTopNSeries';
+import type {MetricsProperty} from 'sentry/views/starfish/types';
+
+jest.mock('sentry/utils/useLocation');
+jest.mock('sentry/utils/usePageFilters');
+jest.mock('sentry/utils/useOrganization');
+
+function Wrapper({children}: {children?: ReactNode}) {
+  return (
+    <QueryClientProvider client={makeTestQueryClient()}>{children}</QueryClientProvider>
+  );
+}
+
+describe('useSpanMetricsTopNSeries', () => {
+  const organization = OrganizationFixture();
+
+  jest.mocked(usePageFilters).mockReturnValue({
+    isReady: true,
+    desyncedFilters: new Set(),
+    pinnedFilters: new Set(),
+    shouldPersist: true,
+    selection: {
+      datetime: {
+        period: '10d',
+        start: null,
+        end: null,
+        utc: false,
+      },
+      environments: [],
+      projects: [],
+    },
+  });
+
+  jest.mocked(useLocation).mockReturnValue({
+    pathname: '',
+    search: '',
+    query: {},
+    hash: '',
+    state: undefined,
+    action: 'PUSH',
+    key: '',
+  });
+
+  jest.mocked(useOrganization).mockReturnValue(organization);
+
+  it('rolls multi-axis top-n responses up into multiple series', async () => {
+    const eventsRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events-stats/`,
+      method: 'GET',
+      body: {
+        '200': {
+          data: [
+            [1699907700, [{count: 117}]],
+            [1699908000, [{count: 199}]],
+          ],
+        },
+        '304': {
+          data: [
+            [1699907700, [{count: 12}]],
+            [1699908000, [{count: 13}]],
+          ],
+        },
+      },
+    });
+
+    const {result, waitFor} = reactHooks.renderHook(
+      ({filters, fields, topEvents, yAxis}) =>
+        useSpanMetricsTopNSeries({
+          search: MutableSearch.fromQueryObject(filters),
+          fields,
+          topEvents,
+          yAxis,
+        }),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          filters: {
+            'span.group': '221aa7ebd216',
+          },
+          fields: ['span.status_code' as const, 'count()' as const],
+          topEvents: 5,
+          yAxis: ['count()'] as MetricsProperty[],
+        },
+      }
+    );
+
+    expect(eventsRequest).toHaveBeenCalledWith(
+      '/organizations/org-slug/events-stats/',
+      expect.objectContaining({
+        method: 'GET',
+        query: expect.objectContaining({
+          query: `span.group:221aa7ebd216`,
+          dataset: 'spansMetrics',
+          statsPeriod: '10d',
+          referrer: 'span-metrics-top-n-series',
+          interval: '30m',
+          topEvents: '5',
+          field: ['span.status_code', 'count()'],
+          yAxis: 'count()',
+        }),
+      })
+    );
+
+    await waitFor(() => expect(result.current.isLoading).toEqual(false));
+
+    expect(result.current.data).toEqual({
+      '200': {
+        data: [
+          {name: '2023-11-13T20:35:00+00:00', value: 117},
+          {name: '2023-11-13T20:40:00+00:00', value: 199},
+        ],
+        seriesName: '200',
+      },
+      '304': {
+        data: [
+          {name: '2023-11-13T20:35:00+00:00', value: 12},
+          {name: '2023-11-13T20:40:00+00:00', value: 13},
+        ],
+        seriesName: '304',
+      },
+    });
+  });
+});

static/app/views/starfish/queries/useSpanMetricsTopNSeries.tsx

@@ -0,0 +1,82 @@
+import type {Series} from 'sentry/types/echarts';
+import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import {getSeriesEventView} from 'sentry/views/starfish/queries/getSeriesEventView';
+import type {MetricsProperty} from 'sentry/views/starfish/types';
+import {useWrappedDiscoverTimeseriesQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+
+interface SpanMetricTimeseriesRow {
+  [key: string]: number;
+  interval: number;
+}
+
+interface UseSpanMetricsSeriesOptions<Fields> {
+  topEvents: number;
+  enabled?: boolean;
+  fields?: Fields;
+  referrer?: string;
+  search?: MutableSearch;
+  yAxis?: Fields;
+}
+
+export const useSpanMetricsTopNSeries = <Fields extends MetricsProperty[]>(
+  options: UseSpanMetricsSeriesOptions<Fields> = {topEvents: DEFAULT_EVENT_COUNT}
+) => {
+  const {
+    search = undefined,
+    fields = [],
+    yAxis = [],
+    topEvents,
+    referrer = 'span-metrics-top-n-series',
+  } = options;
+
+  if (yAxis.length > 1) {
+    throw new Error(
+      'Multi-axis top-N queries are not supported by this hook. Try using `useSpansQuery` directly.'
+    );
+  }
+
+  const pageFilters = usePageFilters();
+
+  const eventView = getSeriesEventView(
+    search,
+    fields,
+    pageFilters.selection,
+    yAxis,
+    topEvents
+  );
+
+  const result = useWrappedDiscoverTimeseriesQuery<SpanMetricTimeseriesRow[]>({
+    eventView,
+    initialData: [],
+    referrer,
+    enabled: options.enabled,
+  });
+
+  const seriesByKey: {[key: string]: Series} = {};
+
+  (result?.data ?? []).forEach(datum => {
+    // `interval` is the timestamp of the data point. Every other key is the value of a requested or found timeseries. `groups` is used to disambiguate top-N multi-axis series, which aren't supported here so the value is useless
+    const {interval, group: _group, ...data} = datum;
+
+    Object.keys(data).forEach(key => {
+      const value = {
+        name: interval,
+        value: datum[key],
+      };
+
+      if (seriesByKey[key]) {
+        seriesByKey[key].data.push(value);
+      } else {
+        seriesByKey[key] = {
+          seriesName: key,
+          data: [value],
+        };
+      }
+    });
+  });
+
+  return {...result, data: seriesByKey as {[key: string]: Series}};
+};
+
+const DEFAULT_EVENT_COUNT = 5;

static/app/views/starfish/utils/useSpansQuery.tsx

@@ -181,35 +181,38 @@ function processDiscoverTimeseriesResult(
   if (!result) {
     return undefined;
   }
+
   if (!eventView.yAxis) {
     return [];
   }
-  let intervals = [] as Interval[];
-  const singleYAxis =
-    eventView.yAxis &&
-    (typeof eventView.yAxis === 'string' || eventView.yAxis.length === 1);
+
   const firstYAxis =
     typeof eventView.yAxis === 'string' ? eventView.yAxis : eventView.yAxis[0];
+
   if (result.data) {
-    const timeSeriesResult: Interval[] = processSingleDiscoverTimeseriesResult(
-      result,
-      singleYAxis ? firstYAxis : 'count'
-    ).map(data => ({
+    // Result data only returned one series. This means there was only only one yAxis requested, and no sub-series. Iterate the data, and return the result
+    return processSingleDiscoverTimeseriesResult(result, firstYAxis).map(data => ({
       interval: moment(parseInt(data.interval, 10) * 1000).format(DATE_FORMAT),
       [firstYAxis]: data[firstYAxis],
       group: data.group,
     }));
-    return timeSeriesResult;
   }
+
+  let intervals = [] as Interval[];
+
+  // Result data had more than one series, grouped by a key. This means either multiple yAxes were requested _or_ a top-N query was set. Iterate the keys, and construct a series for each one.
   Object.keys(result).forEach(key => {
+    // Each key has just one timeseries. Either this is a simple multi-axis query, or a top-N query with just one axis
     if (result[key].data) {
       intervals = mergeIntervals(
         intervals,
-        processSingleDiscoverTimeseriesResult(result[key], singleYAxis ? firstYAxis : key)
+        processSingleDiscoverTimeseriesResult(result[key], key)
       );
     } else {
+      // Each key has more than one timeseries. This is a multi-axis top-N query. Iterate each series, but this time set both the key _and_ the group
       Object.keys(result[key]).forEach(innerKey => {
         if (innerKey !== 'order') {
+          // `order` is a special value, each series has it in a multi-series query
           intervals = mergeIntervals(
             intervals,
             processSingleDiscoverTimeseriesResult(result[key][innerKey], innerKey, key)
@@ -223,42 +226,51 @@ function processDiscoverTimeseriesResult(
     ...interval,
     interval: moment(parseInt(interval.interval, 10) * 1000).format(DATE_FORMAT),
   }));
+
   return processed;
 }
 
 function processSingleDiscoverTimeseriesResult(result, key: string, group?: string) {
   const intervals = [] as Interval[];
+
   result.data.forEach(([timestamp, [{count: value}]]) => {
     const existingInterval = intervals.find(
       interval =>
         interval.interval === timestamp && (group ? interval.group === group : true)
     );
+
     if (existingInterval) {
       existingInterval[key] = value;
       return;
     }
+
     intervals.push({
       interval: timestamp,
       [key]: value,
       group,
     });
   });
+
   return intervals;
 }
 
 function mergeIntervals(first: Interval[], second: Interval[]) {
   const target: Interval[] = JSON.parse(JSON.stringify(first));
+
   second.forEach(({interval: timestamp, group, ...rest}) => {
     const existingInterval = target.find(
       interval =>
         interval.interval === timestamp && (group ? interval.group === group : true)
     );
+
     if (existingInterval) {
       Object.keys(rest).forEach(key => {
         existingInterval[key] = rest[key];
       });
+
       return;
     }
+
     target.push({interval: timestamp, group, ...rest});
   });
   return target;