feat(metrics): Add doMetricsRequest action (#31704)

static/app/actionCreators/metrics.tsx

@@ -0,0 +1,71 @@
+import {Client} from 'sentry/api';
+import {getInterval} from 'sentry/components/charts/utils';
+import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
+import {DateString, MetricsApiResponse, Organization} from 'sentry/types';
+import {defined} from 'sentry/utils';
+
+export type DoMetricsRequestOptions = {
+  field: string[];
+  orgSlug: Organization['slug'];
+  cursor?: string;
+  end?: DateString;
+  environment?: Readonly<string[]>;
+  groupBy?: string[];
+  includeAllArgs?: boolean;
+  interval?: string;
+  limit?: number;
+  orderBy?: string;
+  project?: Readonly<number[]>;
+  query?: string;
+  start?: DateString;
+  statsPeriod?: string | null;
+  statsPeriodEnd?: string;
+  statsPeriodStart?: string;
+};
+
+export const doMetricsRequest = (
+  api: Client,
+  {
+    field,
+    orgSlug,
+    cursor,
+    environment,
+    groupBy,
+    interval,
+    limit,
+    orderBy,
+    project,
+    query,
+    includeAllArgs = false,
+    statsPeriodStart,
+    statsPeriodEnd,
+    ...dateTime
+  }: DoMetricsRequestOptions
+): Promise<MetricsApiResponse> => {
+  const {start, end, statsPeriod} = normalizeDateTimeParams(dateTime, {
+    allowEmptyPeriod: true,
+  });
+
+  const urlQuery = Object.fromEntries(
+    Object.entries({
+      field,
+      cursor,
+      end,
+      environment,
+      groupBy,
+      interval: interval || getInterval({start, end, period: statsPeriod}),
+      query: query || undefined,
+      per_page: limit,
+      project,
+      orderBy,
+      start,
+      statsPeriod,
+      statsPeriodStart,
+      statsPeriodEnd,
+    }).filter(([, value]) => defined(value))
+  );
+
+  const pathname = `/organizations/${orgSlug}/metrics/data/`;
+
+  return api.requestPromise(pathname, {includeAllArgs, query: urlQuery});
+};

static/app/utils/metrics/metricsRequest.tsx

@@ -3,11 +3,12 @@ import isEqual from 'lodash/isEqual';
 import omitBy from 'lodash/omitBy';
 
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
+import {doMetricsRequest, DoMetricsRequestOptions} from 'sentry/actionCreators/metrics';
 import {Client, ResponseMeta} from 'sentry/api';
-import {getInterval, shouldFetchPreviousPeriod} from 'sentry/components/charts/utils';
+import {shouldFetchPreviousPeriod} from 'sentry/components/charts/utils';
 import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
 import {t} from 'sentry/locale';
-import {DateString, MetricsApiResponse, Organization} from 'sentry/types';
+import {MetricsApiResponse} from 'sentry/types';
 import {getPeriod} from 'sentry/utils/getPeriod';
 
 import {TableData} from '../discover/discoverQuery';
@@ -35,31 +36,25 @@ type DefaultProps = {
    * Include data for previous period
    */
   includePrevious: boolean;
-};
-
-type Props = DefaultProps & {
-  api: Client;
-  field: string[];
-  orgSlug: Organization['slug'];
-  children?: (renderProps: MetricsRequestRenderProps) => React.ReactNode;
-  cursor?: string;
-  end?: DateString;
-  environment?: Readonly<string[]>;
-  groupBy?: string[];
   /**
    * Transform the response data to be something ingestible by GridEditable tables
    */
   includeTabularData?: boolean;
-  interval?: string;
+  /**
+   * If true, no request will be made
+   */
   isDisabled?: boolean;
-  limit?: number;
-  orderBy?: string;
-  project?: Readonly<number[]>;
-  query?: string;
-  start?: DateString;
-  statsPeriod?: string | null;
 };
 
+type Props = DefaultProps &
+  Omit<
+    DoMetricsRequestOptions,
+    'includeAllArgs' | 'statsPeriodStart' | 'statsPeriodEnd'
+  > & {
+    api: Client;
+    children?: (renderProps: MetricsRequestRenderProps) => React.ReactNode;
+  };
+
 type State = {
   error: string | null;
   errored: boolean;
@@ -72,6 +67,8 @@ type State = {
 class MetricsRequest extends React.Component<Props, State> {
   static defaultProps: DefaultProps = {
     includePrevious: false,
+    includeTabularData: false,
+    isDisabled: false,
   };
 
   state: State = {
@@ -101,13 +98,7 @@ class MetricsRequest extends React.Component<Props, State> {
 
   private unmounting: boolean = false;
 
-  get path() {
-    const {orgSlug} = this.props;
-
-    return `/organizations/${orgSlug}/metrics/data/`;
-  }
-
-  baseQueryParams({previousPeriod = false} = {}) {
+  getQueryParams({previousPeriod = false} = {}) {
     const {
       project,
       environment,
@@ -118,24 +109,23 @@ class MetricsRequest extends React.Component<Props, State> {
       limit,
       interval,
       cursor,
+      statsPeriod,
+      start,
+      end,
+      orgSlug,
     } = this.props;
 
-    const {start, end, statsPeriod} = getPeriod({
-      period: this.props.statsPeriod,
-      start: this.props.start,
-      end: this.props.end,
-    });
-
     const commonQuery = {
-      project,
-      environment,
       field,
-      query: query || undefined,
+      cursor,
+      environment,
       groupBy,
+      interval,
+      query,
+      limit,
+      project,
       orderBy,
-      per_page: limit,
-      cursor,
-      interval: interval ? interval : getInterval({start, end, period: statsPeriod}),
+      orgSlug,
     };
 
     if (!previousPeriod) {
@@ -174,18 +164,12 @@ class MetricsRequest extends React.Component<Props, State> {
     }));
 
     const promises = [
-      api.requestPromise(this.path, {
-        includeAllArgs: true,
-        query: this.baseQueryParams(),
-      }),
+      doMetricsRequest(api, {includeAllArgs: true, ...this.getQueryParams()}),
     ];
 
+    // TODO(metrics): this could be merged into one request by doubling the statsPeriod and then splitting the response in half
     if (shouldFetchPreviousPeriod({start, end, period: statsPeriod, includePrevious})) {
-      promises.push(
-        api.requestPromise(this.path, {
-          query: this.baseQueryParams({previousPeriod: true}),
-        })
-      );
+      promises.push(doMetricsRequest(api, this.getQueryParams({previousPeriod: true})));
     }
 
     try {

static/app/views/dashboardsV2/widgetBuilder/metricWidget/statsRequest.tsx

@@ -128,6 +128,7 @@ function StatsRequest({
         });
       }
 
+      // TODO(metrics): replace with doMetricsRequest
       return api.requestPromise(metricDataEndpoint, {
         query,
       });

tests/js/spec/actionCreators/metrics.spec.tsx

@@ -0,0 +1,82 @@
+import {doMetricsRequest} from 'sentry/actionCreators/metrics';
+import {Client} from 'sentry/api';
+import {SessionMetric} from 'sentry/utils/metrics/fields';
+
+describe('Metrics ActionCreator', function () {
+  const api = new Client();
+  const orgSlug = TestStubs.Organization().slug;
+  const options = {
+    field: [SessionMetric.SENTRY_SESSIONS_SESSION],
+    orgSlug,
+    cursor: undefined,
+    environment: [],
+    groupBy: ['session.status'],
+    interval: '1h',
+    limit: 5,
+    orderBy: SessionMetric.SENTRY_SESSIONS_SESSION,
+    project: [TestStubs.Project().id],
+    query: 'release:123',
+    statsPeriod: '14d',
+  };
+
+  let mock;
+
+  beforeEach(function () {
+    MockApiClient.clearMockResponses();
+    mock = MockApiClient.addMockResponse({
+      url: `/organizations/${orgSlug}/metrics/data/`,
+      body: {
+        data: [],
+      },
+    });
+  });
+
+  it('requests metrics data', function () {
+    doMetricsRequest(api, options);
+    expect(mock).toHaveBeenCalledTimes(1);
+    expect(mock).toHaveBeenLastCalledWith(
+      `/organizations/${orgSlug}/metrics/data/`,
+      expect.objectContaining({
+        query: {
+          environment: [],
+          field: ['sentry.sessions.session'],
+          groupBy: ['session.status'],
+          interval: '1h',
+          orderBy: 'sentry.sessions.session',
+          per_page: 5,
+          project: ['2'],
+          query: 'release:123',
+          statsPeriod: '14d',
+        },
+      })
+    );
+  });
+
+  it('fills in interval based on start and end', function () {
+    doMetricsRequest(api, {
+      ...options,
+      statsPeriod: undefined,
+      interval: undefined,
+      start: '2022-01-01T00:00:00',
+      end: '2022-03-01T00:00:00',
+    });
+    expect(mock).toHaveBeenCalledTimes(1);
+    expect(mock).toHaveBeenLastCalledWith(
+      `/organizations/${orgSlug}/metrics/data/`,
+      expect.objectContaining({
+        query: {
+          environment: [],
+          field: ['sentry.sessions.session'],
+          groupBy: ['session.status'],
+          interval: '4h',
+          orderBy: 'sentry.sessions.session',
+          per_page: 5,
+          project: ['2'],
+          query: 'release:123',
+          start: '2022-01-01T00:00:00.000',
+          end: '2022-03-01T00:00:00.000',
+        },
+      })
+    );
+  });
+});

tests/js/spec/utils/metrics/metricsRequest.spec.tsx

@@ -49,7 +49,6 @@ describe('MetricsRequest', () => {
       expect.anything(),
       expect.objectContaining({
         query: {
-          end: undefined,
           environment: ['prod'],
           field: ['fieldA'],
           groupBy: ['status'],
@@ -58,7 +57,6 @@ describe('MetricsRequest', () => {
           orderBy: 'fieldA',
           project: ['2'],
           query: 'abc',
-          start: undefined,
           statsPeriod: '14d',
         },
       })
@@ -161,7 +159,6 @@ describe('MetricsRequest', () => {
       expect.anything(),
       expect.objectContaining({
         query: {
-          end: undefined,
           environment: ['prod'],
           field: ['fieldA'],
           groupBy: ['status'],
@@ -170,7 +167,6 @@ describe('MetricsRequest', () => {
           orderBy: 'fieldA',
           project: ['2'],
           query: 'abc',
-          start: undefined,
           statsPeriod: '14d',
         },
       })
@@ -241,7 +237,7 @@ describe('MetricsRequest', () => {
       expect.anything(),
       expect.objectContaining({
         query: {
-          end: '2021-12-17T00:59:59',
+          end: '2021-12-17T00:59:59.000',
           environment: ['prod'],
           field: ['fieldA'],
           groupBy: ['status'],
@@ -250,8 +246,7 @@ describe('MetricsRequest', () => {
           orderBy: 'fieldA',
           project: ['2'],
           query: 'abc',
-          start: '2021-12-01T01:00:00',
-          statsPeriod: undefined,
+          start: '2021-12-01T01:00:00.000',
         },
       })
     );

tests/js/spec/views/performance/landing/widgets/widgetContainer.spec.jsx

@@ -732,14 +732,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -750,10 +744,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -813,14 +803,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -831,10 +815,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -894,14 +874,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -912,10 +886,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -1022,14 +992,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -1040,10 +1004,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -1103,14 +1063,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -1121,10 +1075,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -1184,14 +1134,8 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -1202,10 +1146,6 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
-            groupBy: undefined,
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',
@@ -1265,14 +1205,9 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
             groupBy: ['transaction.status'],
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriod: '7d',
-            start: undefined,
-            end: undefined,
           }),
         })
       );
@@ -1283,10 +1218,7 @@ describe('Performance > Widgets > WidgetContainer', function () {
             project: [-42],
             environment: ['prod'],
             field: [field],
-            query: undefined,
             groupBy: ['transaction.status'],
-            orderBy: undefined,
-            per_page: undefined,
             interval: '1h',
             statsPeriodStart: '14d',
             statsPeriodEnd: '7d',

tests/js/spec/views/performance/transactionSummary.spec.tsx

@@ -460,12 +460,8 @@ describe('Performance > TransactionSummary', function () {
           ],
           query: 'transaction:/organizations/:orgId/issues/',
           groupBy: ['measurement_rating'],
-          orderBy: undefined,
-          per_page: undefined,
           interval: '1h',
           statsPeriod: '14d',
-          start: undefined,
-          end: undefined,
         }),
       })
     );
@@ -565,16 +561,12 @@ describe('Performance > TransactionSummary', function () {
       '/organizations/org-slug/metrics/data/',
       expect.objectContaining({
         query: {
-          end: undefined,
           environment: [],
           field: ['count(sentry.transactions.transaction.duration)'],
           groupBy: ['transaction.status'],
           interval: '1h',
-          per_page: undefined,
-          orderBy: undefined,
           project: [2],
           query: 'transaction:/performance',
-          start: undefined,
           statsPeriod: '14d',
         },
       })
@@ -589,16 +581,11 @@ describe('Performance > TransactionSummary', function () {
       '/organizations/org-slug/metrics/data/',
       expect.objectContaining({
         query: {
-          end: undefined,
           environment: [],
           field: ['count(sentry.transactions.transaction.duration)'],
-          groupBy: undefined,
           interval: '1h',
-          per_page: undefined,
-          orderBy: undefined,
           project: [2],
           query: 'transaction:/performance',
-          start: undefined,
           statsPeriod: '14d',
         },
       })