Просмотр исходного кода

ref(perf): Accept `MutableQuery` in data loading hooks (#67080)

Hooks like `useSpanMetrics` and `useSpanMetricsSeries` were written
early in Starfish, for simple use cases. The `filters` object is a
simple key-value of fields and the values we want. This served us well
when we were iterating quickly, but it's not a good choice anymore.

1. Lots of complex filter conditions _can't be expressed_ using a simple
key-value object
2. We use `MutableSearch` everywhere else. It's not consistent to have a
different API here, and using an object doesn't provide a clear win now
that `MutableSearch.fromQueryObject` exists and makes it easy to create
simple filters

This PR updates the data loading hooks to accept a `MutableSearch`
instead.
George Gritsouk 11 месяцев назад
Родитель
Сommit
d456c946af

+ 3 - 2
static/app/views/performance/browser/resources/resourceSummaryPage/index.tsx

@@ -8,6 +8,7 @@ import {EnvironmentPageFilter} from 'sentry/components/organizations/environment
 import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
 import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilter';
 import {t} from 'sentry/locale';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {useParams} from 'sentry/utils/useParams';
@@ -45,9 +46,9 @@ function ResourceSummary() {
     query: {transaction},
   } = useLocation();
   const {data} = useSpanMetrics({
-    filters: {
+    search: MutableSearch.fromQueryObject({
       'span.group': groupId,
-    },
+    }),
     fields: [
       `avg(${SPAN_SELF_TIME})`,
       `avg(${HTTP_RESPONSE_CONTENT_LENGTH})`,

+ 3 - 2
static/app/views/performance/browser/resources/resourceSummaryPage/resourceSummaryCharts.tsx

@@ -3,6 +3,7 @@ import type {Series} from 'sentry/types/echarts';
 import {formatBytesBase2} from 'sentry/utils';
 import {formatRate} from 'sentry/utils/formatters';
 import getDynamicText from 'sentry/utils/getDynamicText';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {RESOURCE_THROUGHPUT_UNIT} from 'sentry/views/performance/browser/resources';
 import {useResourceModuleFilters} from 'sentry/views/performance/browser/resources/utils/useResourceFilters';
 import {AVG_COLOR, THROUGHPUT_COLOR} from 'sentry/views/starfish/colours';
@@ -35,12 +36,12 @@ function ResourceSummaryCharts(props: {groupId: string}) {
 
   const {data: spanMetricsSeriesData, isLoading: areSpanMetricsSeriesLoading} =
     useSpanMetricsSeries({
-      filters: {
+      search: MutableSearch.fromQueryObject({
         'span.group': props.groupId,
         ...(filters[RESOURCE_RENDER_BLOCKING_STATUS]
           ? {[RESOURCE_RENDER_BLOCKING_STATUS]: filters[RESOURCE_RENDER_BLOCKING_STATUS]}
           : {}),
-      },
+      }),
       yAxis: [
         `spm()`,
         `avg(${SPAN_SELF_TIME})`,

+ 4 - 3
static/app/views/performance/database/databaseLandingPage.tsx

@@ -15,6 +15,7 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {fromSorts} from 'sentry/utils/discover/eventView';
 import {decodeScalar} from 'sentry/utils/queryString';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
@@ -80,7 +81,7 @@ export function DatabaseLandingPage() {
   const cursor = decodeScalar(location.query?.[QueryParameterNames.SPANS_CURSOR]);
 
   const queryListResponse = useSpanMetrics({
-    filters: tableFilters,
+    search: MutableSearch.fromQueryObject(tableFilters),
     fields: [
       'project.id',
       'span.group',
@@ -101,7 +102,7 @@ export function DatabaseLandingPage() {
     data: throughputData,
     error: throughputError,
   } = useSpanMetricsSeries({
-    filters: chartFilters,
+    search: MutableSearch.fromQueryObject(chartFilters),
     yAxis: ['spm()'],
     referrer: 'api.starfish.span-landing-page-metrics-chart',
   });
@@ -111,7 +112,7 @@ export function DatabaseLandingPage() {
     data: durationData,
     error: durationError,
   } = useSpanMetricsSeries({
-    filters: chartFilters,
+    search: MutableSearch.fromQueryObject(chartFilters),
     yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
     referrer: 'api.starfish.span-landing-page-metrics-chart',
   });

+ 4 - 3
static/app/views/performance/database/databaseSpanSummaryPage.tsx

@@ -11,6 +11,7 @@ import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {DurationUnit, RateUnit, type Sort} from 'sentry/utils/discover/fields';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
@@ -70,7 +71,7 @@ function SpanSummaryPage({params}: Props) {
   const sort = useModuleSort(QueryParameterNames.ENDPOINTS_SORT, DEFAULT_SORT);
 
   const {data, isLoading: areSpanMetricsLoading} = useSpanMetrics({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     fields: [
       SpanMetricsField.SPAN_OP,
       SpanMetricsField.SPAN_DESCRIPTION,
@@ -105,7 +106,7 @@ function SpanSummaryPage({params}: Props) {
     data: throughputData,
     error: throughputError,
   } = useSpanMetricsSeries({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     yAxis: ['spm()'],
     enabled: Boolean(groupId),
     referrer: 'api.starfish.span-summary-page-metrics-chart',
@@ -116,7 +117,7 @@ function SpanSummaryPage({params}: Props) {
     data: durationData,
     error: durationError,
   } = useSpanMetricsSeries({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
     enabled: Boolean(groupId),
     referrer: 'api.starfish.span-summary-page-metrics-chart',

+ 6 - 5
static/app/views/performance/http/httpDomainSummaryPage.tsx

@@ -12,6 +12,7 @@ import {space} from 'sentry/styles/space';
 import {fromSorts} from 'sentry/utils/discover/eventView';
 import {DurationUnit, RateUnit} from 'sentry/utils/discover/fields';
 import {decodeScalar} from 'sentry/utils/queryString';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
@@ -57,7 +58,7 @@ export function HTTPDomainSummaryPage() {
   const cursor = decodeScalar(location.query?.[QueryParameterNames.TRANSACTIONS_CURSOR]);
 
   const {data: domainMetrics, isLoading: areDomainMetricsLoading} = useSpanMetrics({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     fields: [
       SpanMetricsField.SPAN_DOMAIN,
       `${SpanFunction.SPM}()`,
@@ -77,7 +78,7 @@ export function HTTPDomainSummaryPage() {
     data: throughputData,
     error: throughputError,
   } = useSpanMetricsSeries({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     yAxis: ['spm()'],
     enabled: Boolean(domain),
     referrer: 'api.starfish.http-module-domain-summary-throughput-chart',
@@ -88,7 +89,7 @@ export function HTTPDomainSummaryPage() {
     data: durationData,
     error: durationError,
   } = useSpanMetricsSeries({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     yAxis: [`avg(${SpanMetricsField.SPAN_SELF_TIME})`],
     enabled: Boolean(domain),
     referrer: 'api.starfish.http-module-domain-summary-duration-chart',
@@ -99,7 +100,7 @@ export function HTTPDomainSummaryPage() {
     data: responseCodeData,
     error: responseCodeError,
   } = useSpanMetricsSeries({
-    filters: filters,
+    search: MutableSearch.fromQueryObject(filters),
     yAxis: ['http_response_rate(3)', 'http_response_rate(4)', 'http_response_rate(5)'],
     referrer: 'api.starfish.http-module-domain-summary-response-code-chart',
   });
@@ -111,7 +112,7 @@ export function HTTPDomainSummaryPage() {
     error: transactionsListError,
     pageLinks: transactionsListPageLinks,
   } = useSpanMetrics({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     fields: [
       'transaction',
       'spm()',

+ 5 - 4
static/app/views/performance/http/httpLandingPage.tsx

@@ -10,6 +10,7 @@ import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilt
 import {t} from 'sentry/locale';
 import {fromSorts} from 'sentry/utils/discover/eventView';
 import {decodeScalar} from 'sentry/utils/queryString';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
@@ -53,7 +54,7 @@ export function HTTPLandingPage() {
     data: throughputData,
     error: throughputError,
   } = useSpanMetricsSeries({
-    filters: chartFilters,
+    search: MutableSearch.fromQueryObject(chartFilters),
     yAxis: ['spm()'],
     referrer: 'api.starfish.http-module-landing-throughput-chart',
   });
@@ -63,7 +64,7 @@ export function HTTPLandingPage() {
     data: durationData,
     error: durationError,
   } = useSpanMetricsSeries({
-    filters: chartFilters,
+    search: MutableSearch.fromQueryObject(chartFilters),
     yAxis: [`avg(span.self_time)`],
     referrer: 'api.starfish.http-module-landing-duration-chart',
   });
@@ -73,13 +74,13 @@ export function HTTPLandingPage() {
     data: responseCodeData,
     error: responseCodeError,
   } = useSpanMetricsSeries({
-    filters: chartFilters,
+    search: MutableSearch.fromQueryObject(chartFilters),
     yAxis: ['http_response_rate(3)', 'http_response_rate(4)', 'http_response_rate(5)'],
     referrer: 'api.starfish.http-module-landing-response-code-chart',
   });
 
   const domainsListResponse = useSpanMetrics({
-    filters: tableFilters,
+    search: MutableSearch.fromQueryObject(tableFilters),
     fields: [
       'project.id',
       'span.domain',

+ 9 - 1
static/app/views/starfish/queries/useSpanMetrics.spec.tsx

@@ -6,6 +6,7 @@ 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';
@@ -89,7 +90,14 @@ describe('useSpanMetrics', () => {
 
     const {result, waitFor} = reactHooks.renderHook(
       ({filters, fields, sorts, limit, cursor, referrer}) =>
-        useSpanMetrics({filters, fields, sorts, limit, cursor, referrer}),
+        useSpanMetrics({
+          search: MutableSearch.fromQueryObject(filters),
+          fields,
+          sorts,
+          limit,
+          cursor,
+          referrer,
+        }),
       {
         wrapper: Wrapper,
         initialProps: {

+ 7 - 13
static/app/views/starfish/queries/useSpanMetrics.tsx

@@ -2,33 +2,29 @@ import type {PageFilters} from 'sentry/types';
 import EventView from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import usePageFilters from 'sentry/utils/usePageFilters';
-import type {
-  MetricsProperty,
-  MetricsResponse,
-  SpanMetricsQueryFilters,
-} from 'sentry/views/starfish/types';
+import type {MetricsProperty, MetricsResponse} from 'sentry/views/starfish/types';
 import {useWrappedDiscoverQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
 interface UseSpanMetricsOptions<Fields> {
   cursor?: string;
   enabled?: boolean;
   fields?: Fields;
-  filters?: SpanMetricsQueryFilters;
   limit?: number;
   referrer?: string;
+  search?: MutableSearch;
   sorts?: Sort[];
 }
 
 export const useSpanMetrics = <Fields extends MetricsProperty[]>(
   options: UseSpanMetricsOptions<Fields> = {}
 ) => {
-  const {fields = [], filters = {}, sorts = [], limit, cursor, referrer} = options;
+  const {fields = [], search = undefined, sorts = [], limit, cursor, referrer} = options;
 
   const pageFilters = usePageFilters();
 
-  const eventView = getEventView(filters, fields, sorts, pageFilters.selection);
+  const eventView = getEventView(search, fields, sorts, pageFilters.selection);
 
   const result = useWrappedDiscoverQuery({
     eventView,
@@ -51,17 +47,15 @@ export const useSpanMetrics = <Fields extends MetricsProperty[]>(
 };
 
 function getEventView(
-  filters: SpanMetricsQueryFilters = {},
+  search: MutableSearch | undefined,
   fields: string[] = [],
   sorts: Sort[] = [],
   pageFilters: PageFilters
 ) {
-  const query = MutableSearch.fromQueryObject(filters);
-
   const eventView = EventView.fromNewQueryWithPageFilters(
     {
       name: '',
-      query: query.formatString(),
+      query: search?.formatString() ?? '',
       fields,
       dataset: DiscoverDatasets.SPANS_METRICS,
       version: 2,

+ 4 - 2
static/app/views/starfish/queries/useSpanMetricsSeries.spec.tsx

@@ -5,6 +5,7 @@ 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';
@@ -63,7 +64,7 @@ describe('useSpanMetricsSeries', () => {
     const {result} = reactHooks.renderHook(
       ({filters, enabled}) =>
         useSpanMetricsSeries({
-          filters,
+          search: MutableSearch.fromQueryObject(filters),
           enabled,
         }),
       {
@@ -96,7 +97,8 @@ describe('useSpanMetricsSeries', () => {
     });
 
     const {result, waitFor} = reactHooks.renderHook(
-      ({filters, yAxis}) => useSpanMetricsSeries({filters, yAxis}),
+      ({filters, yAxis}) =>
+        useSpanMetricsSeries({search: MutableSearch.fromQueryObject(filters), yAxis}),
       {
         wrapper: Wrapper,
         initialProps: {

+ 7 - 9
static/app/views/starfish/queries/useSpanMetricsSeries.tsx

@@ -7,11 +7,11 @@ 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 {MutableSearch} from 'sentry/utils/tokenizeSearch';
+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 type {MetricsProperty, SpanMetricsQueryFilters} from 'sentry/views/starfish/types';
+import type {MetricsProperty} from 'sentry/views/starfish/types';
 import {useWrappedDiscoverTimeseriesQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
 interface SpanMetricTimeseriesRow {
@@ -21,19 +21,19 @@ interface SpanMetricTimeseriesRow {
 
 interface UseSpanMetricsSeriesOptions<Fields> {
   enabled?: boolean;
-  filters?: SpanMetricsQueryFilters;
   referrer?: string;
+  search?: MutableSearch;
   yAxis?: Fields;
 }
 
 export const useSpanMetricsSeries = <Fields extends MetricsProperty[]>(
   options: UseSpanMetricsSeriesOptions<Fields> = {}
 ) => {
-  const {filters = {}, yAxis = [], referrer = 'span-metrics-series'} = options;
+  const {search = undefined, yAxis = [], referrer = 'span-metrics-series'} = options;
 
   const pageFilters = usePageFilters();
 
-  const eventView = getEventView(filters, pageFilters.selection, yAxis);
+  const eventView = getEventView(search, pageFilters.selection, yAxis);
 
   const result = useWrappedDiscoverTimeseriesQuery<SpanMetricTimeseriesRow[]>({
     eventView,
@@ -61,12 +61,10 @@ export const useSpanMetricsSeries = <Fields extends MetricsProperty[]>(
 };
 
 function getEventView(
-  filters: SpanMetricsQueryFilters,
+  search: MutableSearch | undefined,
   pageFilters: PageFilters,
   yAxis: string[]
 ) {
-  const query = MutableSearch.fromQueryObject(filters);
-
   // 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 => {
@@ -86,7 +84,7 @@ function getEventView(
   return EventView.fromNewQueryWithPageFilters(
     {
       name: '',
-      query: query.formatString(),
+      query: search?.formatString() ?? undefined,
       fields: [],
       yAxis,
       dataset: DiscoverDatasets.SPANS_METRICS,

Некоторые файлы не были показаны из-за большого количества измененных файлов