Browse Source

feat(alert): Log a transaction when saving alert rules (#25263)

Sends a transaction to sentry when alert rules are saved
David Wang 3 years ago
parent
commit
bfa5b63432

+ 3 - 1
static/app/utils/analytics.tsx

@@ -1,4 +1,5 @@
 import * as Sentry from '@sentry/react';
+import {Transaction} from '@sentry/types';
 
 import HookStore from 'app/stores/hookStore';
 import {Hooks} from 'app/types/hooks';
@@ -128,7 +129,7 @@ type RecordMetric = Hooks['metrics:event'] & {
      * Optional op code
      */
     op?: string;
-  }) => void;
+  }) => Transaction;
 
   endTransaction: (opts: {
     /**
@@ -236,6 +237,7 @@ metric.startTransaction = ({name, traceId, op}) => {
   }
   const transaction = Sentry.startTransaction({name, op, traceId});
   transactionDataStore[name] = transaction;
+  return transaction;
 };
 
 metric.endTransaction = ({name}) => {

+ 2 - 0
static/app/views/settings/incidentRules/create.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 import {RouteComponentProps} from 'react-router';
 
 import {Organization, Project, Team} from 'app/types';
+import {metric} from 'app/utils/analytics';
 import EventView from 'app/utils/discover/eventView';
 import withTeams from 'app/utils/withTeams';
 import {WizardRuleTemplate} from 'app/views/alerts/wizard/options';
@@ -36,6 +37,7 @@ class IncidentRulesCreate extends React.Component<Props> {
     const {router} = this.props;
     const {orgId} = this.props.params;
 
+    metric.endTransaction({name: 'saveAlertRule'});
     router.push(`/organizations/${orgId}/alerts/rules/`);
   };
 

+ 2 - 0
static/app/views/settings/incidentRules/details.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 import {RouteComponentProps} from 'react-router';
 
 import {Organization, Project, Team} from 'app/types';
+import {metric} from 'app/utils/analytics';
 import withTeams from 'app/utils/withTeams';
 import AsyncView from 'app/views/asyncView';
 import RuleForm from 'app/views/settings/incidentRules/ruleForm';
@@ -49,6 +50,7 @@ class IncidentRulesDetails extends AsyncView<Props, State> {
     const {router} = this.props;
     const {orgId} = this.props.params;
 
+    metric.endTransaction({name: 'saveAlertRule'});
     router.push(`/organizations/${orgId}/alerts/rules/`);
   };
 

+ 22 - 4
static/app/views/settings/incidentRules/ruleForm/index.tsx

@@ -22,7 +22,7 @@ import IndicatorStore from 'app/stores/indicatorStore';
 import space from 'app/styles/space';
 import {Organization, Project} from 'app/types';
 import {defined} from 'app/utils';
-import {trackAnalyticsEvent} from 'app/utils/analytics';
+import {metric, trackAnalyticsEvent} from 'app/utils/analytics';
 import {AlertWizardAlertNames} from 'app/views/alerts/wizard/options';
 import {getAlertTypeFromAggregateDataset} from 'app/views/alerts/wizard/utils';
 import Form from 'app/views/settings/components/forms/form';
@@ -198,7 +198,7 @@ class RuleFormContainer extends AsyncComponent<Props, State> {
       this.resetPollingState(loadingSlackIndicator);
 
       if (status === 'failed') {
-        addErrorMessage(error);
+        this.handleRuleSaveFailure(error);
       }
       if (alertRule) {
         addSuccessMessage(ruleId ? t('Updated alert rule') : t('Created alert rule'));
@@ -207,7 +207,7 @@ class RuleFormContainer extends AsyncComponent<Props, State> {
         }
       }
     } catch {
-      addErrorMessage(t('An error occurred'));
+      this.handleRuleSaveFailure(t('An error occurred'));
       this.resetPollingState(loadingSlackIndicator);
     }
   };
@@ -443,6 +443,19 @@ class RuleFormContainer extends AsyncComponent<Props, State> {
       'loading'
     );
     try {
+      const transaction = metric.startTransaction({name: 'saveAlertRule'});
+      transaction.setTag('type', 'metric');
+      transaction.setTag('operation', !rule.id ? 'create' : 'edit');
+      for (const trigger of sanitizedTriggers) {
+        for (const action of trigger.actions) {
+          transaction.setTag(action.type, true);
+          if (action.integrationId) {
+            transaction.setTag(`integrationId:${action.integrationId}`, true);
+          }
+        }
+      }
+      transaction.setData('actions', sanitizedTriggers);
+
       this.setState({loading: true});
       const [resp, , xhr] = await addOrUpdateRule(
         this.api,
@@ -485,7 +498,7 @@ class RuleFormContainer extends AsyncComponent<Props, State> {
           : Object.values(err?.responseJSON)
         : [];
       const apiErrors = errors.length > 0 ? `: ${errors.join(', ')}` : '';
-      addErrorMessage(t('Unable to save alert%s', apiErrors));
+      this.handleRuleSaveFailure(t('Unable to save alert%s', apiErrors));
     }
   };
 
@@ -553,6 +566,11 @@ class RuleFormContainer extends AsyncComponent<Props, State> {
     }
   };
 
+  handleRuleSaveFailure = (msg: React.ReactNode) => {
+    addErrorMessage(msg);
+    metric.endTransaction({name: 'saveAlertRule'});
+  };
+
   handleCancel = () => {
     this.goBack();
   };

+ 25 - 3
static/app/views/settings/projectAlerts/issueRuleEditor/index.tsx

@@ -32,6 +32,7 @@ import {
   IssueAlertRuleConditionTemplate,
   UnsavedIssueAlertRule,
 } from 'app/types/alerts';
+import {metric} from 'app/utils/analytics';
 import {getDisplayName} from 'app/utils/environment';
 import recreateRoute from 'app/utils/recreateRoute';
 import routeTitleGen from 'app/utils/routeTitle';
@@ -198,7 +199,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
           detailedError: {actions: [error ? error : t('An error occurred')]},
           loading: false,
         });
-        addErrorMessage(t('An error occurred'));
+        this.handleRuleSaveFailure(t('An error occurred'));
       }
       if (rule) {
         const ruleId = isSavedAlertRule(origRule) ? `${origRule.id}/` : '';
@@ -206,7 +207,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
         this.handleRuleSuccess(isNew, rule);
       }
     } catch {
-      addErrorMessage(t('An error occurred'));
+      this.handleRuleSaveFailure(t('An error occurred'));
       this.setState({loading: false});
     }
   };
@@ -232,10 +233,17 @@ class IssueRuleEditor extends AsyncView<Props, State> {
       status: 'complete',
     });
 
+    metric.endTransaction({name: 'saveAlertRule'});
+
     router.push(`/organizations/${organization.slug}/alerts/rules/`);
     addSuccessMessage(isNew ? t('Created alert rule') : t('Updated alert rule'));
   };
 
+  handleRuleSaveFailure(msg: React.ReactNode) {
+    addErrorMessage(msg);
+    metric.endTransaction({name: 'saveAlertRule'});
+  }
+
   handleSubmit = async () => {
     const {rule} = this.state;
     const ruleId = isSavedAlertRule(rule) ? `${rule.id}/` : '';
@@ -251,6 +259,20 @@ class IssueRuleEditor extends AsyncView<Props, State> {
     addLoadingMessage();
 
     try {
+      const transaction = metric.startTransaction({name: 'saveAlertRule'});
+      transaction.setTag('type', 'issue');
+      transaction.setTag('operation', isNew ? 'create' : 'edit');
+      if (rule) {
+        for (const action of rule.actions) {
+          // Grab the last part of something like 'sentry.mail.actions.NotifyEmailAction'
+          const splitActionId = action.id.split('.');
+          const actionName = splitActionId[splitActionId.length - 1];
+          if (actionName) {
+            transaction.setTag(actionName, true);
+          }
+        }
+        transaction.setData('actions', rule.actions);
+      }
       const [resp, , xhr] = await this.api.requestPromise(endpoint, {
         includeAllArgs: true,
         method: isNew ? 'POST' : 'PUT',
@@ -271,7 +293,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
         detailedError: err.responseJSON || {__all__: 'Unknown error'},
         loading: false,
       });
-      addErrorMessage(t('An error occurred'));
+      this.handleRuleSaveFailure(t('An error occurred'));
     }
   };
 

+ 12 - 0
tests/js/spec/views/settings/incidentRules/details.spec.jsx

@@ -4,8 +4,19 @@ import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
 
 import GlobalModal from 'app/components/globalModal';
+import {metric} from 'app/utils/analytics';
 import IncidentRulesDetails from 'app/views/settings/incidentRules/details';
 
+jest.mock('app/utils/analytics', () => ({
+  metric: {
+    startTransaction: jest.fn(() => ({
+      setTag: jest.fn(),
+      setData: jest.fn(),
+    })),
+    endTransaction: jest.fn(),
+  },
+}));
+
 describe('Incident Rules Details', function () {
   beforeAll(function () {
     MockApiClient.addMockResponse({
@@ -109,6 +120,7 @@ describe('Incident Rules Details', function () {
     // Save Trigger
     wrapper.find('button[aria-label="Save Rule"]').simulate('submit');
 
+    expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
     expect(editRule).toHaveBeenCalledWith(
       expect.anything(),
       expect.objectContaining({

+ 12 - 0
tests/js/spec/views/settings/incidentRules/ruleForm.spec.jsx

@@ -4,10 +4,20 @@ import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
 
 import {addErrorMessage} from 'app/actionCreators/indicator';
+import {metric} from 'app/utils/analytics';
 import FormModel from 'app/views/settings/components/forms/model';
 import RuleFormContainer from 'app/views/settings/incidentRules/ruleForm';
 
 jest.mock('app/actionCreators/indicator');
+jest.mock('app/utils/analytics', () => ({
+  metric: {
+    startTransaction: jest.fn(() => ({
+      setTag: jest.fn(),
+      setData: jest.fn(),
+    })),
+    endTransaction: jest.fn(),
+  },
+}));
 
 describe('Incident Rules Form', function () {
   const {organization, project, routerContext} = initializeOrg();
@@ -64,6 +74,7 @@ describe('Incident Rules Form', function () {
         url: '/projects/org-slug/project-slug/alert-rules/',
         method: 'POST',
       });
+      metric.startTransaction.mockClear();
     });
 
     /**
@@ -95,6 +106,7 @@ describe('Incident Rules Form', function () {
           }),
         })
       );
+      expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
     });
     describe('Slack async lookup', () => {
       const uuid = 'xxxx-xxxx-xxxx';

+ 15 - 0
tests/js/spec/views/settings/projectAlerts/create.spec.jsx

@@ -7,11 +7,24 @@ import {selectByValue} from 'sentry-test/select-new';
 
 import * as memberActionCreators from 'app/actionCreators/members';
 import ProjectsStore from 'app/stores/projectsStore';
+import {metric} from 'app/utils/analytics';
 import AlertsContainer from 'app/views/alerts';
 import AlertBuilderProjectProvider from 'app/views/alerts/builder/projectProvider';
 import ProjectAlertsCreate from 'app/views/settings/projectAlerts/create';
 
 jest.unmock('app/utils/recreateRoute');
+jest.mock('app/utils/analytics', () => ({
+  metric: {
+    startTransaction: jest.fn(() => ({
+      setTag: jest.fn(),
+      setData: jest.fn(),
+    })),
+    endTransaction: jest.fn(),
+    mark: jest.fn(),
+    measure: jest.fn(),
+  },
+  trackAnalyticsEvent: jest.fn(),
+}));
 
 describe('ProjectAlertsCreate', function () {
   const projectAlertRuleDetailsRoutes = [
@@ -90,6 +103,7 @@ describe('ProjectAlertsCreate', function () {
       url: '/organizations/org-slug/users/',
       body: [TestStubs.User()],
     });
+    metric.startTransaction.mockClear();
   });
 
   afterEach(function () {
@@ -334,6 +348,7 @@ describe('ProjectAlertsCreate', function () {
             },
           })
         );
+        expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
 
         await tick();
         expect(router.push).toHaveBeenCalledWith('/organizations/org-slug/alerts/rules/');

+ 20 - 1
tests/js/spec/views/settings/projectAlerts/issueRuleEditor.spec.jsx

@@ -7,11 +7,23 @@ import {mountGlobalModal} from 'sentry-test/modal';
 import {selectByValue} from 'sentry-test/select-new';
 
 import {updateOnboardingTask} from 'app/actionCreators/onboardingTasks';
+import {metric} from 'app/utils/analytics';
 import ProjectAlerts from 'app/views/settings/projectAlerts';
 import IssueRuleEditor from 'app/views/settings/projectAlerts/issueRuleEditor';
 
 jest.unmock('app/utils/recreateRoute');
 jest.mock('app/actionCreators/onboardingTasks');
+jest.mock('app/utils/analytics', () => ({
+  metric: {
+    startTransaction: jest.fn(() => ({
+      setTag: jest.fn(),
+      setData: jest.fn(),
+    })),
+    endTransaction: jest.fn(),
+    mark: jest.fn(),
+    measure: jest.fn(),
+  },
+}));
 
 describe('ProjectAlerts -> IssueRuleEditor', function () {
   const projectAlertRuleDetailsRoutes = [
@@ -105,6 +117,7 @@ describe('ProjectAlerts -> IssueRuleEditor', function () {
         method: 'PUT',
         body: TestStubs.ProjectAlertRule(),
       });
+      metric.startTransaction.mockClear();
     });
 
     it('gets correct rule name', async function () {
@@ -140,7 +153,7 @@ describe('ProjectAlerts -> IssueRuleEditor', function () {
       );
     });
 
-    it('sends correct environment value', function () {
+    it('sends correct environment value', async function () {
       const {wrapper} = createWrapper();
       selectByValue(wrapper, 'production', {name: 'environment'});
       wrapper.find('form').simulate('submit');
@@ -151,6 +164,8 @@ describe('ProjectAlerts -> IssueRuleEditor', function () {
           data: expect.objectContaining({environment: 'production'}),
         })
       );
+      expect(metric.startTransaction).toHaveBeenCalledTimes(1);
+      expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
     });
 
     it('strips environment value if "All environments" is selected', async function () {
@@ -165,6 +180,8 @@ describe('ProjectAlerts -> IssueRuleEditor', function () {
           data: expect.objectContaining({environment: '__all_environments__'}),
         })
       );
+      expect(metric.startTransaction).toHaveBeenCalledTimes(1);
+      expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
     });
 
     it('updates the alert onboarding task', async function () {
@@ -172,6 +189,8 @@ describe('ProjectAlerts -> IssueRuleEditor', function () {
       wrapper.find('form').simulate('submit');
 
       expect(updateOnboardingTask).toHaveBeenCalled();
+      expect(metric.startTransaction).toHaveBeenCalledTimes(1);
+      expect(metric.startTransaction).toHaveBeenCalledWith({name: 'saveAlertRule'});
     });
   });