Browse Source

ref(perf): Update `useSpanMetrics` and `useSpanMetricsSeries` API (#62564)

- Update `useSpanMetrics` API to accept an object
- Update `useSpanMetricsSeries` API to accept an object

This is a lot tidier, especially since `useSpanMetrics` has so many
optional options, positional arguments don't make sense there.
George Gritsouk 1 year ago
parent
commit
affcb9611f

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

@@ -41,11 +41,11 @@ function ResourceSummary() {
   const {
     query: {transaction},
   } = useLocation();
-  const {data} = useSpanMetrics(
-    {
+  const {data} = useSpanMetrics({
+    filters: {
       'span.group': groupId,
     },
-    [
+    fields: [
       `avg(${SPAN_SELF_TIME})`,
       `avg(${HTTP_RESPONSE_CONTENT_LENGTH})`,
       `avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`,
@@ -54,8 +54,8 @@ function ResourceSummary() {
       'spm()',
       SPAN_DESCRIPTION,
       'time_spent_percentage()',
-    ]
-  );
+    ],
+  });
   const spanMetrics = data[0] ?? {};
 
   const isImage = IMAGE_FILE_EXTENSIONS.includes(

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

@@ -34,21 +34,21 @@ function ResourceSummaryCharts(props: {groupId: string}) {
   // });
 
   const {data: spanMetricsSeriesData, isLoading: areSpanMetricsSeriesLoading} =
-    useSpanMetricsSeries(
-      {
+    useSpanMetricsSeries({
+      filters: {
         'span.group': props.groupId,
         ...(filters[RESOURCE_RENDER_BLOCKING_STATUS]
           ? {[RESOURCE_RENDER_BLOCKING_STATUS]: filters[RESOURCE_RENDER_BLOCKING_STATUS]}
           : {}),
       },
-      [
+      yAxis: [
         `spm()`,
         `avg(${SPAN_SELF_TIME})`,
         `avg(${HTTP_RESPONSE_CONTENT_LENGTH})`,
         `avg(${HTTP_DECODED_RESPONSE_CONTENT_LENGTH})`,
         `avg(${HTTP_RESPONSE_TRANSFER_SIZE})`,
-      ]
-    );
+      ],
+    });
 
   if (spanMetricsSeriesData) {
     spanMetricsSeriesData[`avg(${HTTP_RESPONSE_TRANSFER_SIZE})`].lineStyle = {

+ 17 - 15
static/app/views/performance/database/databaseLandingPage.tsx

@@ -79,9 +79,9 @@ export function DatabaseLandingPage() {
 
   const cursor = decodeScalar(location.query?.[QueryParameterNames.SPANS_CURSOR]);
 
-  const queryListResponse = useSpanMetrics(
-    pickBy(tableFilters, value => value !== undefined),
-    [
+  const queryListResponse = useSpanMetrics({
+    filters: pickBy(tableFilters, value => value !== undefined),
+    fields: [
       'project.id',
       'span.group',
       'span.description',
@@ -90,23 +90,25 @@ export function DatabaseLandingPage() {
       'sum(span.self_time)',
       'time_spent_percentage()',
     ],
-    [sort],
-    LIMIT,
+    sorts: [sort],
+    limit: LIMIT,
     cursor,
-    'api.starfish.use-span-list'
-  );
+    referrer: 'api.starfish.use-span-list',
+  });
 
   const {isLoading: isThroughputDataLoading, data: throughputData} = useSpanMetricsSeries(
-    chartFilters,
-    ['spm()'],
-    'api.starfish.span-landing-page-metrics-chart'
+    {
+      filters: chartFilters,
+      yAxis: ['spm()'],
+      referrer: 'api.starfish.span-landing-page-metrics-chart',
+    }
   );
 
-  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries(
-    chartFilters,
-    [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
-    'api.starfish.span-landing-page-metrics-chart'
-  );
+  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries({
+    filters: chartFilters,
+    yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
+    referrer: 'api.starfish.span-landing-page-metrics-chart',
+  });
 
   const isCriticalDataLoading =
     isThroughputDataLoading || isDurationDataLoading || queryListResponse.isLoading;

+ 13 - 14
static/app/views/performance/database/databaseSpanSummaryPage.tsx

@@ -67,9 +67,9 @@ function SpanSummaryPage({params}: Props) {
 
   const sort = useModuleSort(QueryParameterNames.ENDPOINTS_SORT, DEFAULT_SORT);
 
-  const {data} = useSpanMetrics(
+  const {data} = useSpanMetrics({
     filters,
-    [
+    fields: [
       SpanMetricsField.SPAN_OP,
       SpanMetricsField.SPAN_DESCRIPTION,
       SpanMetricsField.SPAN_ACTION,
@@ -81,11 +81,8 @@ function SpanSummaryPage({params}: Props) {
       `${SpanFunction.TIME_SPENT_PERCENTAGE}()`,
       `${SpanFunction.HTTP_ERROR_COUNT}()`,
     ],
-    undefined,
-    undefined,
-    undefined,
-    'api.starfish.span-summary-page-metrics'
-  );
+    referrer: 'api.starfish.span-summary-page-metrics',
+  });
 
   const spanMetrics = data[0] ?? {};
 
@@ -101,16 +98,18 @@ function SpanSummaryPage({params}: Props) {
   };
 
   const {isLoading: isThroughputDataLoading, data: throughputData} = useSpanMetricsSeries(
-    filters,
-    ['spm()'],
-    'api.starfish.span-summary-page-metrics-chart'
+    {
+      filters,
+      yAxis: ['spm()'],
+      referrer: 'api.starfish.span-summary-page-metrics-chart',
+    }
   );
 
-  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries(
+  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries({
     filters,
-    [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
-    'api.starfish.span-summary-page-metrics-chart'
-  );
+    yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
+    referrer: 'api.starfish.span-summary-page-metrics-chart',
+  });
 
   useSynchronizeCharts([!isThroughputDataLoading && !isDurationDataLoading]);
 

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

@@ -67,7 +67,7 @@ describe('useSpanMetrics', () => {
 
     const {result, waitForNextUpdate} = reactHooks.renderHook(
       ({filters, fields, sorts, limit, cursor, referrer}) =>
-        useSpanMetrics(filters, fields, sorts, limit, cursor, referrer),
+        useSpanMetrics({filters, fields, sorts, limit, cursor, referrer}),
       {
         wrapper: Wrapper,
         initialProps: {

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

@@ -13,14 +13,20 @@ import {
 import {useWrappedDiscoverQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 import {EMPTY_OPTION_VALUE} from 'sentry/views/starfish/views/spans/selectors/emptyOption';
 
-export const useSpanMetrics = <T extends MetricsProperty[]>(
-  filters: SpanMetricsQueryFilters,
-  fields: T,
-  sorts?: Sort[],
-  limit?: number,
-  cursor?: string,
-  referrer: string = 'api.starfish.use-span-metrics'
+interface UseSpanMetricsOptions<Fields> {
+  cursor?: string;
+  fields?: Fields;
+  filters?: SpanMetricsQueryFilters;
+  limit?: number;
+  referrer?: string;
+  sorts?: Sort[];
+}
+
+export const useSpanMetrics = <Fields extends MetricsProperty[]>(
+  options: UseSpanMetricsOptions<Fields> = {}
 ) => {
+  const {fields = [], filters = {}, sorts = [], limit, cursor, referrer} = options;
+
   const location = useLocation();
 
   const eventView = getEventView(filters, fields, sorts, location);
@@ -38,7 +44,7 @@ export const useSpanMetrics = <T extends MetricsProperty[]>(
 
   // 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
-  const data = (result?.data ?? []) as Pick<MetricsResponse, T[number]>[] | [];
+  const data = (result?.data ?? []) as Pick<MetricsResponse, Fields[number]>[] | [];
 
   return {
     ...result,
@@ -48,7 +54,7 @@ export const useSpanMetrics = <T extends MetricsProperty[]>(
 };
 
 function getEventView(
-  filters: SpanMetricsQueryFilters,
+  filters: SpanMetricsQueryFilters = {},
   fields: string[] = [],
   sorts: Sort[] = [],
   location: Location

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

@@ -67,7 +67,7 @@ describe('useSpanMetricsSeries', () => {
     });
 
     const {result, waitForNextUpdate} = reactHooks.renderHook(
-      ({filters, yAxis}) => useSpanMetricsSeries(filters, yAxis),
+      ({filters, yAxis}) => useSpanMetricsSeries({filters, yAxis}),
       {
         wrapper: Wrapper,
         initialProps: {
@@ -121,7 +121,7 @@ describe('useSpanMetricsSeries', () => {
     });
 
     const {rerender, waitForNextUpdate} = reactHooks.renderHook(
-      ({yAxis}) => useSpanMetricsSeries({}, yAxis),
+      ({yAxis}) => useSpanMetricsSeries({yAxis}),
       {
         wrapper: Wrapper,
         initialProps: {

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

@@ -23,11 +23,15 @@ export type SpanMetrics = {
   'time_spent_percentage()': number;
 };
 
-export const useSpanMetricsSeries = (
-  filters: SpanMetricsQueryFilters,
-  yAxis: string[] = [],
-  referrer = 'span-metrics-series'
-) => {
+interface UseSpanMetricsSeriesOptions {
+  filters?: SpanMetricsQueryFilters;
+  referrer?: string;
+  yAxis?: string[];
+}
+
+export const useSpanMetricsSeries = (options: UseSpanMetricsSeriesOptions) => {
+  const {filters = {}, yAxis = [], referrer = 'span-metrics-series'} = options;
+
   const pageFilters = usePageFilters();
 
   const eventView = getEventView(filters, pageFilters.selection, yAxis);

+ 5 - 5
static/app/views/starfish/queries/useSpanSamples.tsx

@@ -76,11 +76,11 @@ export const useSpanSamples = (options: Options) => {
 
   const dateCondtions = getDateConditions(pageFilter.selection);
 
-  const {isLoading: isLoadingSeries, data: spanMetricsSeriesData} = useSpanMetricsSeries(
-    {'span.group': groupId, ...filters},
-    [`avg(${SPAN_SELF_TIME})`],
-    'api.starfish.sidebar-span-metrics'
-  );
+  const {isLoading: isLoadingSeries, data: spanMetricsSeriesData} = useSpanMetricsSeries({
+    filters: {'span.group': groupId, ...filters},
+    yAxis: [`avg(${SPAN_SELF_TIME})`],
+    referrer: 'api.starfish.sidebar-span-metrics',
+  });
 
   const maxYValue = computeAxisMax([spanMetricsSeriesData?.[`avg(${SPAN_SELF_TIME})`]]);
 

+ 4 - 7
static/app/views/starfish/views/screens/screenLoadSpans/samples/samplesContainer.tsx

@@ -65,14 +65,11 @@ export function ScreenLoadSampleContainer({
     filters.release = release;
   }
 
-  const {data} = useSpanMetrics(
+  const {data} = useSpanMetrics({
     filters,
-    [`avg(${SPAN_SELF_TIME})`, 'count()', SPAN_OP],
-    undefined,
-    undefined,
-    undefined,
-    'api.starfish.span-summary-panel-samples-table-avg'
-  );
+    fields: [`avg(${SPAN_SELF_TIME})`, 'count()', SPAN_OP],
+    referrer: 'api.starfish.span-summary-panel-samples-table-avg',
+  });
 
   const spanMetrics = data[0] ?? {};
 

Some files were not shown because too many files changed in this diff