Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions static/app/utils/analytics/alertsAnalyticsEvents.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
import type {MetricRule} from 'sentry/views/alerts/rules/metric/types';
import type {UptimeMonitorMode} from 'sentry/views/alerts/rules/uptime/types';
import type {MonitorConfig} from 'sentry/views/insights/crons/types';

export type AlertsEventParameters = {
'anomaly-detection.feedback-submitted': {
choice_selected: boolean;
incident_id: string;
};
'cron_monitor.created': {
cron_schedule_type: MonitorConfig['schedule_type'];
};
'issue_alert_rule.created': Record<string, unknown>;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out another typing solution for issue_alert_rule.created where there aren't additional parameters

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think this should be the correct way

'metric_alert_rule.created': {
aggregate: MetricRule['aggregate'];
dataset: MetricRule['dataset'];
};
'uptime_monitor.created': {
uptime_mode: UptimeMonitorMode;
};
};

type AlertsEventKey = keyof AlertsEventParameters;

export const alertsEventMap: Record<AlertsEventKey, string | null> = {
'anomaly-detection.feedback-submitted': 'Anomaly Detection Feedback Submitted',
'issue_alert_rule.created': 'Issue Alert Rule Created',
'metric_alert_rule.created': 'Metric Alert Rule Created',
'cron_monitor.created': 'Cron Monitor Created',
'uptime_monitor.created': 'Uptime Monitor Created',
};
9 changes: 8 additions & 1 deletion static/app/utils/analytics/monitorsAnalyticsEvents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ type DetectorAnalyticsEventPayload = ReturnType<typeof getDetectorAnalyticsPaylo

type AutomationAnalyticsEventPayload = ReturnType<typeof getAutomationAnalyticsPayload>;

type DetectorCreateAnalyticsEventPayload =
| (DetectorAnalyticsEventPayload & {
success: true;
})
| {detector_type: string; success: false};

export type MonitorsEventParameters = {
'automation.created': AutomationAnalyticsEventPayload & {
organization: Organization;
success: boolean;
};
'automation.updated': AutomationAnalyticsEventPayload & {
organization: Organization;
Expand All @@ -17,7 +24,7 @@ export type MonitorsEventParameters = {
guide: string;
platform: string;
};
'monitor.created': DetectorAnalyticsEventPayload;
'monitor.created': DetectorCreateAnalyticsEventPayload;
'monitor.updated': DetectorAnalyticsEventPayload;
};

Expand Down
11 changes: 8 additions & 3 deletions static/app/views/alerts/create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {t} from 'sentry/locale';
import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
import type {Member, Organization} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import {trackAnalytics} from 'sentry/utils/analytics';
import EventView from 'sentry/utils/discover/eventView';
import {uniqueId} from 'sentry/utils/guid';
import {decodeScalar} from 'sentry/utils/queryString';
Expand Down Expand Up @@ -166,14 +167,18 @@ function Create(props: Props) {
<MonitorForm
apiMethod="POST"
apiEndpoint={`/organizations/${organization.slug}/monitors/`}
onSubmitSuccess={(data: Monitor) =>
onSubmitSuccess={(data: Monitor) => {
trackAnalytics('cron_monitor.created', {
organization,
cron_schedule_type: data.config.schedule_type,
});
navigate(
makeAlertsPathname({
path: `/rules/crons/${data.project.slug}/${data.slug}/details/`,
organization,
})
)
}
);
}}
submitLabel={t('Create')}
/>
) : !hasMetricAlerts || alertType === AlertRuleType.ISSUE ? (
Expand Down
6 changes: 6 additions & 0 deletions static/app/views/alerts/rules/issue/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,12 @@ class IssueRuleEditor extends DeprecatedAsyncComponent<Props, State> {

metric.endSpan({name: 'saveAlertRule'});

if (isNew) {
trackAnalytics('issue_alert_rule.created', {
organization,
});
}

router.push(
makeAlertsPathname({
path: `/rules/${project.slug}/${rule.id}/details/`,
Expand Down
18 changes: 18 additions & 0 deletions static/app/views/alerts/rules/metric/ruleForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,15 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
}
if (alertRule) {
addSuccessMessage(ruleId ? t('Updated alert rule') : t('Created alert rule'));

if (!ruleId) {
trackAnalytics('metric_alert_rule.created', {
organization,
aggregate: alertRule.aggregate,
dataset: alertRule.dataset,
});
}

if (onSubmitSuccess) {
onSubmitSuccess(alertRule, model);
}
Expand Down Expand Up @@ -849,6 +858,15 @@ class RuleFormContainer extends DeprecatedAsyncComponent<Props, State> {
IndicatorStore.remove(loadingIndicator);
this.setState({loading: false});
addSuccessMessage(ruleId ? t('Updated alert rule') : t('Created alert rule'));

if (!ruleId) {
trackAnalytics('metric_alert_rule.created', {
organization,
aggregate: data.aggregate,
dataset: data.dataset,
});
}

if (onSubmitSuccess) {
onSubmitSuccess(data, model);
}
Expand Down
9 changes: 9 additions & 0 deletions static/app/views/alerts/rules/uptime/uptimeAlertForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import ListItem from 'sentry/components/list/listItem';
import Panel from 'sentry/components/panels/panel';
import {t, tct} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {trackAnalytics} from 'sentry/utils/analytics';
import getDuration from 'sentry/utils/duration/getDuration';
import {useQueryClient} from 'sentry/utils/queryClient';
import {useNavigate} from 'sentry/utils/useNavigate';
Expand Down Expand Up @@ -129,6 +130,14 @@ export function UptimeAlertForm({handleDelete, rule}: Props) {
exact: true,
});
}

if (!rule) {
trackAnalytics('uptime_monitor.created', {
organization,
uptime_mode: response.mode,
});
}

navigate(
makeAlertsPathname({
path: `/rules/uptime/${projectSlug}/${response.id}/details/`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {Automation} from 'sentry/types/workflowEngine/automations';
import type {NewAutomation} from 'sentry/types/workflowEngine/automations';

export function getAutomationAnalyticsPayload(automation: Automation): {
export function getAutomationAnalyticsPayload(automation: NewAutomation): {
actions_count: number;
detectors_count: number;
environment: string | null;
Expand Down
18 changes: 9 additions & 9 deletions static/app/views/automations/new.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('AutomationNewSettings', () => {
});

beforeEach(() => {
jest.clearAllMocks();
MockApiClient.clearMockResponses();

// Available actions (include Slack with a default integration)
Expand Down Expand Up @@ -205,15 +206,14 @@ describe('AutomationNewSettings', () => {
);

// Verify analytics was called with correct event and payload structure
await waitFor(() => {
expect(trackAnalytics).toHaveBeenCalledWith('automation.created', {
organization,
frequency_minutes: expect.any(Number),
environment: expect.anything(),
detectors_count: expect.any(Number),
trigger_conditions_count: expect.any(Number),
actions_count: expect.any(Number),
});
expect(trackAnalytics).toHaveBeenCalledWith('automation.created', {
organization,
frequency_minutes: expect.any(Number),
environment: null,
detectors_count: expect.any(Number),
trigger_conditions_count: expect.any(Number),
success: true,
actions_count: expect.any(Number),
});
});
});
24 changes: 19 additions & 5 deletions static/app/views/automations/new.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,30 @@ export default function AutomationNewSettings() {
async (data, _, __, ___, ____) => {
const errors = validateAutomationBuilderState(state);
setAutomationBuilderErrors(errors);
const newAutomationData = getNewAutomationData(data as AutomationFormData, state);

if (Object.keys(errors).length === 0) {
const automation = await createAutomation(
getNewAutomationData(data as AutomationFormData, state)
);
try {
const automation = await createAutomation(newAutomationData);
trackAnalytics('automation.created', {
organization,
...getAutomationAnalyticsPayload(newAutomationData),
success: true,
});
navigate(makeAutomationDetailsPathname(organization.slug, automation.id));
} catch {
trackAnalytics('automation.created', {
organization,
...getAutomationAnalyticsPayload(newAutomationData),
success: false,
});
}
} else {
trackAnalytics('automation.created', {
organization,
...getAutomationAnalyticsPayload(automation),
...getAutomationAnalyticsPayload(newAutomationData),
success: false,
});
navigate(makeAutomationDetailsPathname(organization.slug, automation.id));
}
},
[createAutomation, state, navigate, organization]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export function NewDetectorLayout<
const maxWidth = theme.breakpoints.xl;

const formSubmitHandler = useCreateDetectorFormSubmit({
detectorType,
formDataToEndpointPayload,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {t} from 'sentry/locale';
import type {
BaseDetectorUpdatePayload,
Detector,
DetectorType,
} from 'sentry/types/workflowEngine/detectors';
import {trackAnalytics} from 'sentry/utils/analytics';
import {useNavigate} from 'sentry/utils/useNavigate';
Expand All @@ -15,6 +16,10 @@ import {useCreateDetector} from 'sentry/views/detectors/hooks';
import {makeMonitorDetailsPathname} from 'sentry/views/detectors/pathnames';

interface UseCreateDetectorFormSubmitOptions<TFormData, TUpdatePayload> {
/**
* Detector type for analytics tracking when validation fails
*/
detectorType: DetectorType;
/**
* Function to transform form data to API payload
*/
Expand All @@ -27,6 +32,7 @@ export function useCreateDetectorFormSubmit<
TFormData extends Data,
TUpdatePayload extends BaseDetectorUpdatePayload,
>({
detectorType,
formDataToEndpointPayload,
onError,
onSuccess,
Expand All @@ -39,16 +45,23 @@ export function useCreateDetectorFormSubmit<
async (data, onSubmitSuccess, onSubmitError, _, formModel) => {
const isValid = formModel.validateForm();
if (!isValid) {
trackAnalytics('monitor.created', {
organization,
detector_type: detectorType,
success: false,
});
return;
}

const payload = formDataToEndpointPayload(data as TFormData);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Payload creation moved outside try-catch loses error handling

The formDataToEndpointPayload call was moved from inside the try-catch block to outside it. Previously, if this function threw an exception, it would be caught and the error handling code would run (addErrorMessage, onError callback, onSubmitError). Now if it throws, the exception propagates uncaught, bypassing all error handling and potentially crashing the UI without user feedback.

Fix in Cursor Fix in Web


try {
const payload = formDataToEndpointPayload(data as TFormData);
const resultDetector = await createDetector(payload);

trackAnalytics('monitor.created', {
organization,
...getDetectorAnalyticsPayload(resultDetector),
success: true,
});

addSuccessMessage(t('Monitor created successfully'));
Expand All @@ -61,6 +74,12 @@ export function useCreateDetectorFormSubmit<

onSubmitSuccess?.(resultDetector);
} catch (error) {
trackAnalytics('monitor.created', {
organization,
detector_type: payload.type,
success: false,
});

addErrorMessage(t('Unable to create monitor'));

if (onError) {
Expand All @@ -71,6 +90,7 @@ export function useCreateDetectorFormSubmit<
}
},
[
detectorType,
formDataToEndpointPayload,
createDetector,
organization,
Expand Down
Loading