Browse Source

feat(starfish): use metrics on top throughput and p75 graphs in db module (#48708)

Were going to use metrics data instead of experimental data now for
transaction throughput and p75 and its more statistically accurate.

Two things to note here,
1. Not everything was fully supported using the discover hook +
eventView
2. I had to make some small changes to discover types to make things
work properly.
Dominik Buszowiecki 1 year ago
parent
commit
bc443f70c8

+ 2 - 2
static/app/utils/discover/discoverQuery.tsx

@@ -33,13 +33,13 @@ export type EventsTableData = {
 
 export type TableDataWithTitle = TableData & {title: string};
 
-type DiscoverQueryPropsWithThresholds = DiscoverQueryProps & {
+export type DiscoverQueryPropsWithThresholds = DiscoverQueryProps & {
   transactionName?: string;
   transactionThreshold?: number;
   transactionThresholdMetric?: TransactionThresholdMetric;
 };
 
-type DiscoverQueryComponentProps = DiscoverQueryPropsWithThresholds & {
+export type DiscoverQueryComponentProps = DiscoverQueryPropsWithThresholds & {
   children: (props: GenericChildrenProps<TableData>) => React.ReactNode;
 };
 

+ 1 - 1
static/app/utils/discover/genericDiscoverQuery.tsx

@@ -84,7 +84,7 @@ type BaseDiscoverQueryProps = {
   /**
    * Extra query parameters to be added.
    */
-  queryExtras?: Record<string, string>;
+  queryExtras?: Record<string, string | string[] | undefined>;
   /**
    * Sets referrer parameter in the API Payload. Set of allowed referrers are defined
    * on the OrganizationDiscoverEndpoint view.

+ 1 - 1
static/app/views/starfish/modules/databaseModule/databaseChartView.tsx

@@ -98,7 +98,7 @@ export default function DatabaseChartView({table, onChange}: Props) {
   const tpmTransactionSeries = queryToSeries(
     topTransactionsData,
     'transaction',
-    'count',
+    'epm',
     startTime,
     endTime
   );

+ 1 - 1
static/app/views/starfish/modules/databaseModule/index.tsx

@@ -77,7 +77,7 @@ function DatabaseModule() {
       document.removeEventListener('keydown', handleKeyDown);
     };
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [tableData]);
 
   const toggleFilterNew = () => {
     setFilterNew(!filterNew);

+ 111 - 26
static/app/views/starfish/modules/databaseModule/queries.ts

@@ -1,8 +1,14 @@
 import {Moment, unix} from 'moment';
 
-import {EventTransaction} from 'sentry/types';
-import {useDiscoverQuery} from 'sentry/utils/discover/discoverQuery';
+import {EventTransaction, NewQuery} from 'sentry/types';
+import {
+  DiscoverQueryComponentProps,
+  DiscoverQueryPropsWithThresholds,
+  useDiscoverQuery,
+} from 'sentry/utils/discover/discoverQuery';
 import EventView from 'sentry/utils/discover/eventView';
+import {useGenericDiscoverQuery} from 'sentry/utils/discover/genericDiscoverQuery';
+import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {DefinedUseQueryResult, useQuery} from 'sentry/utils/queryClient';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -156,40 +162,88 @@ export const useQueryTopDbOperationsChart = (
   });
 };
 
-export const useGetTransactionsForTables = (tableNames: string[], interval: number) => {
+type TopTransactionData = {
+  interval: string;
+  transaction: string;
+  epm?: number;
+  p75?: number;
+};
+
+export const useGetTransactionsForTables = (
+  tableNames: string[],
+  interval: number
+): DefinedUseQueryResult<TopTransactionData[]> => {
   const pageFilter = usePageFilters();
+  const location = useLocation();
   const {startTime, endTime} = getDateFilters(pageFilter);
   const dateFilters = getDateQueryFilter(startTime, endTime);
-  const transactionFilter = `transaction IN (${getTransactionsFromTableSubquery(
-    tableNames,
-    dateFilters
-  )})`;
-
-  const filters = [transactionFilter];
 
-  const query = `
-    SELECT
-      transaction,
-      floor(quantile(0.75)(exclusive_time), 5) as p75,
-      count() as count,
-      toStartOfInterval(start_timestamp, INTERVAL ${interval} hour) as interval
-    FROM default.spans_experimental_starfish
-    WHERE
-      ${filters.join(' AND ')}
-      ${dateFilters}
-    GROUP BY interval, transaction
-    ORDER BY interval
-  `;
+  const transactionNameQuery = getTransactionsFromTableSubquery(tableNames, dateFilters);
 
-  const {start, end} = pageFilter.selection.datetime;
+  const {start, end, period} = pageFilter.selection.datetime;
 
-  return useQuery({
+  const result1 = useQuery<{transaction: string}[]>({
     enabled: !!tableNames?.length,
-    queryKey: ['topTable', tableNames.join(','), start, end],
-    queryFn: () => fetch(`${HOST}/?query=${query}`).then(res => res.json()),
+    queryKey: ['topTransactionNames', tableNames.join(','), start, end],
+    queryFn: () =>
+      fetch(`${HOST}/?query=${transactionNameQuery}`).then(res => res.json()),
     retry: false,
     initialData: [],
   });
+
+  const query: NewQuery = {
+    id: undefined,
+    name: 'Db module - epm/p75 for top transactions',
+    query: `transaction:[${result1.data?.map(d => d.transaction).join(',')}]`,
+    projects: [1],
+    fields: ['transaction', 'epm()', 'p75(transaction.duration)'],
+    version: 1,
+    topEvents: '5',
+    start: start?.toString(),
+    end: end?.toString(),
+    dataset: DiscoverDatasets.METRICS_ENHANCED,
+    interval: `${interval}h`,
+    yAxis: ['epm()', 'p75(transaction.duration)'],
+  };
+
+  const eventView = EventView.fromNewQueryWithLocation(query, location);
+  eventView.statsPeriod = period ?? undefined;
+
+  const result2 = useDiscoverEventsStatsQuery({
+    eventView,
+    referrer: 'api.starfish.database.charts',
+    location,
+    orgSlug: 'sentry',
+    queryExtras: {
+      interval: `${interval}h`, // This interval isn't being propogated from eventView
+      yAxis: ['epm()', 'p75(transaction.duration)'], // workaround - eventView actually doesn't support multiple yAxis
+      excludeOther: '1',
+      topEvents: '5',
+      per_page: undefined,
+    },
+  });
+
+  const data: TopTransactionData[] = [];
+  if (!result2.isLoading && result2.data) {
+    Object.entries(result2.data).forEach(([transactionName, result]: [string, any]) => {
+      result['epm()'].data.forEach(entry => {
+        data.push({
+          transaction: transactionName,
+          interval: unix(entry[0]).format('YYYY-MM-DDTHH:mm:ss'),
+          epm: entry[1][0].count,
+        });
+      });
+      result['p75(transaction.duration)'].data.forEach(entry => {
+        data.push({
+          transaction: transactionName,
+          interval: unix(entry[0]).format('YYYY-MM-DDTHH:mm:ss'),
+          p75: entry[1][0].count,
+        });
+      });
+    });
+  }
+
+  return {...result2, data} as DefinedUseQueryResult<TopTransactionData[]>;
 };
 
 type TopTableQuery = {
@@ -558,3 +612,34 @@ export const getDateQueryFilter = (startTime: Moment, endTime: Moment) => {
   ${end_timestamp ? `AND lessOrEquals(start_timestamp, '${end_timestamp}')` : ''}
   `;
 };
+
+const shouldRefetchData = (
+  prevProps: DiscoverQueryPropsWithThresholds,
+  nextProps: DiscoverQueryPropsWithThresholds
+) => {
+  return (
+    prevProps.transactionName !== nextProps.transactionName ||
+    prevProps.transactionThreshold !== nextProps.transactionThreshold ||
+    prevProps.transactionThresholdMetric !== nextProps.transactionThresholdMetric
+  );
+};
+
+// We should find a way to use this in discover
+export function useDiscoverEventsStatsQuery(
+  props: Omit<DiscoverQueryComponentProps, 'children'>
+) {
+  const afterFetch = (data, _) => {
+    const {fields, ...otherMeta} = data.meta ?? {};
+    return {
+      ...data,
+      meta: {...fields, ...otherMeta},
+    };
+  };
+
+  return useGenericDiscoverQuery({
+    route: 'events-stats',
+    shouldRefetchData,
+    afterFetch,
+    ...props,
+  });
+}

+ 3 - 1
static/app/views/starfish/modules/databaseModule/utils.ts

@@ -22,7 +22,9 @@ export const queryToSeries = (
         data: [],
       };
     }
-    seriesMap[row[groupByProperty]].data.push(dataEntry);
+    if (dataEntry.value) {
+      seriesMap[row[groupByProperty]].data.push(dataEntry);
+    }
   });
   return Object.values(seriesMap).map(series =>
     zeroFillSeries(series, moment.duration(INTERVAL, 'hours'), startTime, endTime)