Browse Source

feat(dam): Metrics tag store (#32035)

Store metrics tags in reflux store instead of
making an API request every time to retrieve
the data.
Shruthi 3 years ago
parent
commit
b0a4ab90d0

+ 9 - 1
static/app/actionCreators/metrics.tsx

@@ -1,8 +1,10 @@
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
+import MetricsTagActions from 'sentry/actions/metricTagActions';
 import {Client} from 'sentry/api';
 import {getInterval} from 'sentry/components/charts/utils';
 import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilters/parse';
 import {t} from 'sentry/locale';
+import MetricsTagStore from 'sentry/stores/metricsTagStore';
 import {
   DateString,
   MetricMeta,
@@ -79,12 +81,18 @@ export const doMetricsRequest = (
   return api.requestPromise(pathname, {includeAllArgs, query: urlQuery});
 };
 
+function tagFetchSuccess(tags: MetricTag[]) {
+  MetricsTagActions.loadMetricsTagsSuccess(tags);
+}
+
 export function fetchMetricsTags(
   api: Client,
   orgSlug: Organization['slug'],
   projects?: number[],
   fields?: string[]
 ): Promise<MetricTag[]> {
+  MetricsTagStore.reset();
+
   const promise = api.requestPromise(`/organizations/${orgSlug}/metrics/tags/`, {
     query: {
       project: projects,
@@ -92,7 +100,7 @@ export function fetchMetricsTags(
     },
   });
 
-  promise.catch(response => {
+  promise.then(tagFetchSuccess).catch(response => {
     const errorResponse = response?.responseJSON ?? t('Unable to fetch metric tags');
     addErrorMessage(errorResponse);
     handleXhrErrorResponse(errorResponse)(response);

+ 5 - 0
static/app/actions/metricTagActions.tsx

@@ -0,0 +1,5 @@
+import Reflux from 'reflux';
+
+const MetricsTagActions = Reflux.createActions(['loadMetricsTagsSuccess']);
+
+export default MetricsTagActions;

+ 10 - 18
static/app/components/modals/addDashboardWidgetModal.tsx

@@ -9,7 +9,7 @@ import set from 'lodash/set';
 
 import {validateWidget} from 'sentry/actionCreators/dashboards';
 import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
-import {fetchMetricsFields, fetchMetricsTags} from 'sentry/actionCreators/metrics';
+import {fetchMetricsFields} from 'sentry/actionCreators/metrics';
 import {ModalRenderProps} from 'sentry/actionCreators/modal';
 import {Client} from 'sentry/api';
 import Button from 'sentry/components/button';
@@ -27,7 +27,7 @@ import space from 'sentry/styles/space';
 import {
   DateString,
   MetricMeta,
-  MetricTag,
+  MetricTagCollection,
   Organization,
   PageFilters,
   SelectValue,
@@ -38,6 +38,7 @@ import Measurements from 'sentry/utils/measurements/measurements';
 import {SessionMetric} from 'sentry/utils/metrics/fields';
 import {SPAN_OP_BREAKDOWN_FIELDS} from 'sentry/utils/performance/spanOperationBreakdowns/constants';
 import withApi from 'sentry/utils/withApi';
+import withMetricsTags from 'sentry/utils/withMetricsTags';
 import withPageFilters from 'sentry/utils/withPageFilters';
 import withTags from 'sentry/utils/withTags';
 import {DISPLAY_TYPE_CHOICES} from 'sentry/views/dashboardsV2/data';
@@ -90,6 +91,7 @@ export type DashboardWidgetModalOptions = {
 type Props = ModalRenderProps &
   DashboardWidgetModalOptions & {
     api: Client;
+    metricsTags: MetricTagCollection;
     organization: Organization;
     selection: PageFilters;
     tags: TagCollection;
@@ -105,7 +107,6 @@ type State = {
   interval: Widget['interval'];
   loading: boolean;
   metricFields: MetricMeta[];
-  metricTags: MetricTag[];
   queries: Widget['queries'];
   title: string;
   userHasModified: boolean;
@@ -162,7 +163,6 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
         errors: undefined,
         loading: !!this.omitDashboardProp,
         dashboards: [],
-        metricTags: [],
         metricFields: [],
         userHasModified: false,
         widgetType: WidgetType.DISCOVER,
@@ -178,7 +178,6 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
       errors: undefined,
       loading: false,
       dashboards: [],
-      metricTags: [],
       metricFields: [],
       userHasModified: false,
       widgetType: widget.widgetType ?? WidgetType.DISCOVER,
@@ -190,7 +189,6 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
       this.fetchDashboards();
     }
     if (this.props.organization.features.includes('dashboards-metrics')) {
-      this.fetchMetricsTags();
       this.fetchMetricsFields();
     }
   }
@@ -527,15 +525,6 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
     this.setState({loading: false});
   }
 
-  async fetchMetricsTags() {
-    const {api, organization, selection} = this.props;
-    const projects = !selection.projects.length ? undefined : selection.projects;
-    const metricTags = await fetchMetricsTags(api, organization.slug, projects);
-    this.setState({
-      metricTags,
-    });
-  }
-
   async fetchMetricsFields() {
     const {api, organization, selection} = this.props;
     const projects = !selection.projects.length ? undefined : selection.projects;
@@ -607,7 +596,8 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
   }
 
   renderWidgetQueryForm() {
-    const {organization, selection, tags, start, end, statsPeriod} = this.props;
+    const {organization, selection, tags, metricsTags, start, end, statsPeriod} =
+      this.props;
     const state = this.state;
     const errors = state.errors;
 
@@ -622,7 +612,7 @@ class AddDashboardWidgetModal extends React.Component<Props, State> {
     const issueWidgetFieldOptions = generateIssueWidgetFieldOptions();
     const metricsWidgetFieldOptions = generateMetricsWidgetFieldOptions(
       state.metricFields.length ? state.metricFields : DEFAULT_METRICS_FIELDS,
-      Object.values(state.metricTags).map(({key}) => key)
+      Object.values(metricsTags).map(({key}) => key)
     );
     const fieldOptions = (measurementKeys: string[]) =>
       generateFieldOptions({
@@ -930,4 +920,6 @@ const StyledFieldLabel = styled(FieldLabel)`
   display: inline-flex;
 `;
 
-export default withApi(withPageFilters(withTags(AddDashboardWidgetModal)));
+export default withApi(
+  withPageFilters(withTags(withMetricsTags(AddDashboardWidgetModal)))
+);

+ 47 - 0
static/app/stores/metricsTagStore.tsx

@@ -0,0 +1,47 @@
+import Reflux from 'reflux';
+
+import MetricsTagActions from 'sentry/actions/metricTagActions';
+import {MetricTag, MetricTagCollection} from 'sentry/types';
+
+type MetricsTagStoreInterface = {
+  getAllTags(): MetricTagCollection;
+  onLoadTagsSuccess(data: MetricTag[]): void;
+  reset(): void;
+  state: MetricTagCollection;
+};
+
+const storeConfig: Reflux.StoreDefinition & MetricsTagStoreInterface = {
+  state: {},
+
+  init() {
+    this.state = {};
+    this.listenTo(MetricsTagActions.loadMetricsTagsSuccess, this.onLoadTagsSuccess);
+  },
+
+  reset() {
+    this.state = {};
+    this.trigger(this.state);
+  },
+
+  getAllTags() {
+    return this.state;
+  },
+
+  onLoadTagsSuccess(data) {
+    const newTags = data.reduce<MetricTagCollection>((acc, tag) => {
+      acc[tag.key] = {
+        ...tag,
+      };
+
+      return acc;
+    }, {});
+
+    this.state = {...this.state, ...newTags};
+    this.trigger(this.state);
+  },
+};
+
+const MetricsTagStore = Reflux.createStore(storeConfig) as Reflux.Store &
+  MetricsTagStoreInterface;
+
+export default MetricsTagStore;

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

@@ -12,6 +12,8 @@ export type MetricsApiResponse = {
   start: string;
 };
 
+export type MetricTagCollection = Record<string, MetricTag>;
+
 export type MetricTag = {
   key: string;
 };

+ 49 - 0
static/app/utils/withMetricsTags.tsx

@@ -0,0 +1,49 @@
+import * as React from 'react';
+
+import MetricsTagStore from 'sentry/stores/metricsTagStore';
+import {MetricTagCollection} from 'sentry/types';
+import getDisplayName from 'sentry/utils/getDisplayName';
+
+export type InjectedMetricsTagsProps = {
+  metricsTags: MetricTagCollection;
+};
+
+type State = {
+  metricsTags: MetricTagCollection;
+};
+
+function withMetricsTags<P extends InjectedMetricsTagsProps>(
+  WrappedComponent: React.ComponentType<P>
+) {
+  class WithMetricTags extends React.Component<
+    Omit<P, keyof InjectedMetricsTagsProps>,
+    State
+  > {
+    static displayName = `withMetricsTags(${getDisplayName(WrappedComponent)})`;
+
+    state: State = {
+      metricsTags: MetricsTagStore.getAllTags(),
+    };
+
+    componentWillUnmount() {
+      this.unsubscribe();
+    }
+    unsubscribe = MetricsTagStore.listen(
+      (metricsTags: MetricTagCollection) => this.setState({metricsTags}),
+      undefined
+    );
+
+    render() {
+      const {metricsTags, ...props} = this.props as P;
+      return (
+        <WrappedComponent
+          {...({metricsTags: metricsTags ?? this.state.metricsTags, ...props} as P)}
+        />
+      );
+    }
+  }
+
+  return WithMetricTags;
+}
+
+export default withMetricsTags;

+ 6 - 1
static/app/views/dashboardsV2/dashboard.tsx

@@ -16,6 +16,7 @@ import isEqual from 'lodash/isEqual';
 import {validateWidget} from 'sentry/actionCreators/dashboards';
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {fetchOrgMembers} from 'sentry/actionCreators/members';
+import {fetchMetricsTags} from 'sentry/actionCreators/metrics';
 import {openAddDashboardWidgetModal} from 'sentry/actionCreators/modal';
 import {loadOrganizationTags} from 'sentry/actionCreators/tags';
 import {Client} from 'sentry/api';
@@ -137,10 +138,14 @@ class Dashboard extends Component<Props, State> {
   }
 
   async componentDidMount() {
-    const {isEditing, organization} = this.props;
+    const {isEditing, organization, api} = this.props;
     if (organization.features.includes('dashboard-grid-layout')) {
       window.addEventListener('resize', this.debouncedHandleResize);
     }
+
+    if (organization.features.includes('dashboards-metrics')) {
+      fetchMetricsTags(api, organization.slug);
+    }
     // Load organization tags when in edit mode.
     if (isEditing) {
       this.fetchTags();

+ 8 - 7
tests/js/spec/components/modals/addDashboardWidgetModal.spec.jsx

@@ -13,6 +13,7 @@ import {getOptionByLabel, openMenu, selectByLabel} from 'sentry-test/select-new'
 import {openDashboardWidgetLibraryModal} from 'sentry/actionCreators/modal';
 import AddDashboardWidgetModal from 'sentry/components/modals/addDashboardWidgetModal';
 import {t} from 'sentry/locale';
+import MetricsTagStore from 'sentry/stores/metricsTagStore';
 import TagStore from 'sentry/stores/tagStore';
 import {SessionMetric} from 'sentry/utils/metrics/fields';
 import * as types from 'sentry/views/dashboardsV2/types';
@@ -116,16 +117,18 @@ describe('Modals -> AddDashboardWidgetModal', function () {
     {name: 'browser.name', key: 'browser.name'},
     {name: 'custom-field', key: 'custom-field'},
   ];
+  const metricsTags = [{key: 'environment'}, {key: 'release'}, {key: 'session.status'}];
   const dashboard = TestStubs.Dashboard([], {
     id: '1',
     title: 'Test Dashboard',
     widgetDisplay: ['area'],
   });
 
-  let eventsStatsMock, metricsTagsMock, metricsMetaMock, metricsDataMock;
+  let eventsStatsMock, metricsMetaMock, metricsDataMock;
 
   beforeEach(function () {
     TagStore.onLoadTagsSuccess(tags);
+    MetricsTagStore.onLoadTagsSuccess(metricsTags);
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/dashboards/widgets/',
       method: 'POST',
@@ -156,7 +159,7 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       url: '/organizations/org-slug/issues/',
       body: [],
     });
-    metricsTagsMock = MockApiClient.addMockResponse({
+    MockApiClient.addMockResponse({
       url: '/organizations/org-slug/metrics/tags/',
       body: [{key: 'environment'}, {key: 'release'}, {key: 'session.status'}],
     });
@@ -1222,7 +1225,6 @@ describe('Modals -> AddDashboardWidgetModal', function () {
         source: types.DashboardWidgetSource.DASHBOARDS,
       });
 
-      expect(metricsTagsMock).not.toHaveBeenCalled();
       expect(metricsMetaMock).not.toHaveBeenCalled();
       expect(metricsDataMock).not.toHaveBeenCalled();
 
@@ -1233,7 +1235,7 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       expect(
         screen.getByText('Issues (States, Assignment, Time, etc.)')
       ).toBeInTheDocument();
-      // Hide without the dashboard-metrics feature flag
+      // Hide without the dashboards-metrics feature flag
       expect(screen.queryByText('Metrics (Release Health)')).not.toBeInTheDocument();
       wrapper.unmount();
     });
@@ -1327,7 +1329,7 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       wrapper.unmount();
     });
 
-    it('retrieves tags when modal is opened', async function () {
+    it('displays metrics tags', async function () {
       initialData.organization.features = [
         'performance-view',
         'discover-query',
@@ -1341,7 +1343,6 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       });
 
       await tick();
-      expect(metricsTagsMock).toHaveBeenCalledTimes(1);
       expect(metricsMetaMock).toHaveBeenCalledTimes(1);
 
       await act(async () =>
@@ -1382,7 +1383,6 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       });
 
       await tick();
-      expect(metricsTagsMock).toHaveBeenCalledTimes(1);
       expect(metricsMetaMock).toHaveBeenCalledTimes(1);
 
       await act(async () =>
@@ -1402,6 +1402,7 @@ describe('Modals -> AddDashboardWidgetModal', function () {
       expect(screen.getByText('sentry.sessions.user')).toBeInTheDocument();
       wrapper.unmount();
     });
+
     it('makes the appropriate metrics call', async function () {
       initialData.organization.features = [
         'performance-view',

+ 31 - 0
tests/js/spec/stores/metricsTagStore.spec.jsx

@@ -0,0 +1,31 @@
+import MetricsTagStore from 'sentry/stores/metricsTagStore';
+
+describe('MetricsTagStore', function () {
+  beforeEach(() => {
+    MetricsTagStore.reset();
+  });
+
+  describe('onLoadTagsSuccess()', () => {
+    it('should add a new tags and trigger the new addition', () => {
+      jest.spyOn(MetricsTagStore, 'trigger');
+
+      const tags = MetricsTagStore.getAllTags();
+      expect(tags).toEqual({});
+
+      MetricsTagStore.onLoadTagsSuccess([
+        {key: 'environment'},
+        {key: 'release'},
+        {key: 'session.status'},
+      ]);
+
+      const updatedTags = MetricsTagStore.getAllTags();
+      expect(updatedTags).toEqual({
+        environment: {key: 'environment'},
+        release: {key: 'release'},
+        'session.status': {key: 'session.status'},
+      });
+
+      expect(MetricsTagStore.trigger).toHaveBeenCalledTimes(1);
+    });
+  });
+});

+ 50 - 0
tests/js/spec/utils/withMetricsTags.spec.tsx

@@ -0,0 +1,50 @@
+import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
+
+import MetricsTagStore from 'sentry/stores/metricsTagStore';
+import withMetricsTags, {InjectedMetricsTagsProps} from 'sentry/utils/withMetricsTags';
+
+interface MyComponentProps extends InjectedMetricsTagsProps {
+  other: string;
+}
+
+describe('withMetricsTags HoC', function () {
+  beforeEach(() => {
+    MetricsTagStore.reset();
+  });
+
+  it('works', function () {
+    jest.spyOn(MetricsTagStore, 'trigger');
+    const MyComponent = (props: MyComponentProps) => {
+      return (
+        <div>
+          <span>{props.other}</span>
+          {props.metricsTags &&
+            Object.entries(props.metricsTags).map(([key, tag]) => (
+              <em key={key}>{tag.key}</em>
+            ))}
+        </div>
+      );
+    };
+
+    const Container = withMetricsTags(MyComponent);
+    mountWithTheme(<Container other="value" />);
+
+    // Should forward props.
+    expect(screen.getByText('value')).toBeInTheDocument();
+
+    MetricsTagStore.onLoadTagsSuccess([
+      {key: 'environment'},
+      {key: 'release'},
+      {key: 'session.status'},
+    ]);
+
+    // Should forward prop
+    expect(screen.getByText('value')).toBeInTheDocument();
+
+    expect(MetricsTagStore.trigger).toHaveBeenCalledTimes(1);
+
+    // includes custom metricsTags
+    const renderedTag = screen.getByText('session.status');
+    expect(renderedTag).toBeInTheDocument();
+  });
+});