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

fix(dashboards): Add includeSeries and includeTotals in RH requests (#34964)

This is so that the Metrics API does not attempt to return
series or totals data when it's not required. Additionally,
pass a limit of 1 when requesting non-grouped (top-n type)
series data because otherwise, requesting data from .../metrics/data
without a limit, the backend would apply a default limit of 50
resulting series/groups. This does not apply to the number of results
in that series (that's determined by the interval requested).

To reproduce the bug in prod:

Go to the widget builder and select Releases as your data set and a Line chart
Check that your page filter is set to 24hrs
Change the chart type to Area
You should see an error saying:
Requested interval of timedelta of 0:05:00 with statsPeriod timedelta of 1 day, 0:00:00 is too granular for a per_page of 51 elements. Increase your interval, decrease your statsPeriod, or decrease your per_page parameter.
You will see similar errors when changing the page filter time range.
Shruthi 2 лет назад
Родитель
Сommit
cbad78ad51

+ 6 - 0
static/app/actionCreators/metrics.tsx

@@ -12,6 +12,8 @@ export type DoMetricsRequestOptions = {
   environment?: Readonly<string[]>;
   groupBy?: string[];
   includeAllArgs?: boolean;
+  includeSeries?: number;
+  includeTotals?: number;
   interval?: string;
   limit?: number;
   orderBy?: string;
@@ -31,6 +33,8 @@ export const doMetricsRequest = (
     cursor,
     environment,
     groupBy,
+    includeSeries,
+    includeTotals,
     interval,
     limit,
     orderBy,
@@ -53,6 +57,8 @@ export const doMetricsRequest = (
       end,
       environment,
       groupBy: groupBy?.filter(g => !!g),
+      includeSeries,
+      includeTotals,
       interval: interval || getInterval({start, end, period: statsPeriod}),
       query: query || undefined,
       per_page: limit,

+ 2 - 2
static/app/types/metrics.tsx

@@ -15,8 +15,8 @@ export type MetricsApiResponse = {
   end: string;
   groups: {
     by: Record<string, string>;
-    series: Record<string, Array<number | null>>;
-    totals: Record<string, number | null>;
+    series?: Record<string, Array<number | null>>;
+    totals?: Record<string, number | null>;
   }[];
   intervals: string[];
   query: string;

+ 8 - 5
static/app/utils/metrics/transformMetricsResponseToSeries.tsx

@@ -6,8 +6,11 @@ export function transformMetricsResponseToSeries(
   queryAlias?: string
 ): Series[] {
   return (
-    response?.groups.flatMap(group =>
-      Object.keys(group.series).map(field => ({
+    response?.groups.flatMap(group => {
+      if (group.series === undefined) {
+        return [];
+      }
+      return Object.keys(group.series).map(field => ({
         seriesName: `${queryAlias ? `${queryAlias}: ` : ''}${field}${Object.entries(
           group.by
         )
@@ -15,9 +18,9 @@ export function transformMetricsResponseToSeries(
           .join('')}`,
         data: response.intervals.map((interval, index) => ({
           name: interval,
-          value: group.series[field][index] ?? 0,
+          value: group.series ? group.series[field][index] ?? 0 : 0,
         })),
-      }))
-    ) ?? []
+      }));
+    }) ?? []
   );
 }

+ 41 - 24
static/app/views/dashboardsV2/widgetCard/releaseWidgetQueries.tsx

@@ -270,6 +270,14 @@ class ReleaseWidgetQueries extends Component<Props, State> {
     );
     const columns = widget.queries[0].columns;
 
+    const includeSeries = widget.displayType !== DisplayType.TABLE ? 1 : 0;
+    const includeTotals =
+      widget.displayType === DisplayType.TABLE ||
+      widget.displayType === DisplayType.BIG_NUMBER ||
+      columns.length > 0
+        ? 1
+        : 0;
+
     widget.queries.forEach(query => {
       let requestData;
       let requester;
@@ -302,7 +310,7 @@ class ReleaseWidgetQueries extends Component<Props, State> {
           end,
           environment: environments,
           groupBy: columns.map(fieldsToDerivedMetrics),
-          limit: unsupportedOrderby || rawOrderby === '' ? undefined : this.limit,
+          limit: columns.length === 0 ? 1 : this.limit,
           orderBy: unsupportedOrderby
             ? ''
             : isDescending
@@ -315,6 +323,8 @@ class ReleaseWidgetQueries extends Component<Props, State> {
           statsPeriod: period,
           includeAllArgs,
           cursor,
+          includeSeries,
+          includeTotals,
         };
         requester = doMetricsRequest;
 
@@ -356,32 +366,39 @@ class ReleaseWidgetQueries extends Component<Props, State> {
           }
 
           // Transform to fit the table format
-          const tableData = transformSessionsResponseToTable(
-            data,
-            derivedStatusFields,
-            injectedFields
-          ) as TableDataWithTitle; // Cast so we can add the title.
-          tableData.title = widget.queries[requestIndex]?.name ?? '';
-          const tableResults = [...(prevState.tableResults ?? []), tableData];
+          let tableResults: TableDataWithTitle[] | undefined;
+          if (includeTotals) {
+            const tableData = transformSessionsResponseToTable(
+              data,
+              derivedStatusFields,
+              injectedFields
+            ) as TableDataWithTitle; // Cast so we can add the title.
+            tableData.title = widget.queries[requestIndex]?.name ?? '';
+            tableResults = [...(prevState.tableResults ?? []), tableData];
+          } else {
+            tableResults = undefined;
+          }
 
           // Transform to fit the chart format
           const timeseriesResults = [...(prevState.timeseriesResults ?? [])];
-          const transformedResult = transformSessionsResponseToSeries(
-            data,
-            derivedStatusFields,
-            injectedFields,
-            widget.queries[requestIndex].name
-          );
-
-          // When charting timeseriesData on echarts, color association to a timeseries result
-          // is order sensitive, ie series at index i on the timeseries array will use color at
-          // index i on the color array. This means that on multi series results, we need to make
-          // sure that the order of series in our results do not change between fetches to avoid
-          // coloring inconsistencies between renders.
-          transformedResult.forEach((result, resultIndex) => {
-            timeseriesResults[requestIndex * transformedResult.length + resultIndex] =
-              result;
-          });
+          if (includeSeries) {
+            const transformedResult = transformSessionsResponseToSeries(
+              data,
+              derivedStatusFields,
+              injectedFields,
+              widget.queries[requestIndex].name
+            );
+
+            // When charting timeseriesData on echarts, color association to a timeseries result
+            // is order sensitive, ie series at index i on the timeseries array will use color at
+            // index i on the color array. This means that on multi series results, we need to make
+            // sure that the order of series in our results do not change between fetches to avoid
+            // coloring inconsistencies between renders.
+            transformedResult.forEach((result, resultIndex) => {
+              timeseriesResults[requestIndex * transformedResult.length + resultIndex] =
+                result;
+            });
+          }
 
           onDataFetched?.({timeseriesResults, tableResults});
 

+ 1 - 1
static/app/views/performance/vitalDetail/vitalChartMetrics.tsx

@@ -102,7 +102,7 @@ function VitalChartMetrics({
               seriesName: field,
               data: response.intervals.map((intervalValue, intervalIndex) => ({
                 name: moment(intervalValue).valueOf(),
-                value: group.series[field][intervalIndex],
+                value: group.series ? group.series[field][intervalIndex] : 0,
               })),
             })) as Series[] | undefined;
 

+ 6 - 0
tests/js/spec/views/dashboardsV2/releaseWidgetQueries.spec.tsx

@@ -536,6 +536,9 @@ describe('Dashboards > ReleaseWidgetQueries', function () {
           interval: '30m',
           project: [1],
           statsPeriod: '14d',
+          per_page: 1,
+          includeSeries: 1,
+          includeTotals: 0,
         },
       })
     );
@@ -551,6 +554,9 @@ describe('Dashboards > ReleaseWidgetQueries', function () {
           project: [1],
           query: 'environment:prod',
           statsPeriod: '14d',
+          per_page: 1,
+          includeSeries: 1,
+          includeTotals: 0,
         },
       })
     );