ref(aci): Fully separate detector types (#95182)

malwilley · web-flow · commit e47337187537 · 2025-07-09T16:51:09.000-07:00
We were using a shared `Detector` type everywhere, which meant that
there were a lot of type guards sprinkled around. This introduces more
concrete types for each kind of detector to make type checking more
useful.
diff --git a/static/app/types/workflowEngine/detectors.tsx b/static/app/types/workflowEngine/detectors.tsx
@@ -25,6 +25,9 @@ interface SnubaQuery {
   environment?: string;
 }
 
+/**
+ * See DataSourceSerializer
+ */
 interface BaseDataSource {
   id: string;
   organizationId: string;
@@ -59,17 +62,10 @@ interface UptimeSubscriptionDataSource extends BaseDataSource {
     timeoutMs: number;
     traceSampling: boolean;
     url: string;
-    urlDomain: string;
-    urlDomainSuffix: string;
   };
   type: 'uptime_subscription';
 }
 
-/**
- * See DataSourceSerializer
- */
-export type DataSource = SnubaQueryDataSource | UptimeSubscriptionDataSource;
-
 export type DetectorType =
   | 'error'
   | 'metric_issue'
@@ -114,28 +110,45 @@ interface UptimeDetectorConfig {
   environment: string;
 }
 
-export type DetectorConfig = MetricDetectorConfig | UptimeDetectorConfig;
-
-interface NewDetector {
-  conditionGroup: DataConditionGroup | null;
-  config: DetectorConfig;
-  dataSources: DataSource[] | null;
+type BaseDetector = Readonly<{
+  createdBy: string | null;
+  dateCreated: string;
+  dateUpdated: string;
   disabled: boolean;
+  id: string;
+  lastTriggered: string;
   name: string;
+  owner: string | null;
   projectId: string;
   type: DetectorType;
   workflowIds: string[];
+}>;
+
+export interface MetricDetector extends BaseDetector {
+  readonly conditionGroup: DataConditionGroup | null;
+  readonly config: MetricDetectorConfig;
+  readonly dataSources: SnubaQueryDataSource[];
+  readonly type: 'metric_issue';
+}
+
+export interface UptimeDetector extends BaseDetector {
+  readonly config: UptimeDetectorConfig;
+  readonly dataSources: UptimeSubscriptionDataSource[];
+  readonly type: 'uptime_domain_failure';
 }
 
-export interface Detector extends Readonly<NewDetector> {
-  readonly createdBy: string | null;
-  readonly dateCreated: string;
-  readonly dateUpdated: string;
-  readonly id: string;
-  readonly lastTriggered: string;
-  readonly owner: string | null;
+interface CronDetector extends BaseDetector {
+  // TODO: Add cron detector type fields
+  readonly type: 'uptime_subscription';
 }
 
+export interface ErrorDetector extends BaseDetector {
+  // TODO: Add error detector type fields
+  readonly type: 'error';
+}
+
+export type Detector = MetricDetector | UptimeDetector | CronDetector | ErrorDetector;
+
 interface UpdateConditionGroupPayload {
   conditions: Array<Omit<DataCondition, 'id'>>;
   logicType: DataConditionGroup['logicType'];
@@ -172,7 +185,7 @@ export interface UptimeDetectorUpdatePayload extends BaseDetectorUpdatePayload {
 
 export interface MetricDetectorUpdatePayload extends BaseDetectorUpdatePayload {
   conditionGroup: UpdateConditionGroupPayload;
-  config: DetectorConfig;
+  config: MetricDetectorConfig;
   dataSource: UpdateSnubaDataSourcePayload;
   type: 'metric_issue';
 }
diff --git a/static/app/views/automations/list.spec.tsx b/static/app/views/automations/list.spec.tsx
@@ -1,5 +1,5 @@
 import {AutomationFixture} from 'sentry-fixture/automations';
-import {DetectorFixture} from 'sentry-fixture/detectors';
+import {MetricDetectorFixture} from 'sentry-fixture/detectors';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 import {PageFiltersFixture} from 'sentry-fixture/pageFilters';
 import {ProjectFixture} from 'sentry-fixture/project';
@@ -32,7 +32,7 @@ describe('AutomationsList', function () {
     });
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/detectors/1/',
-      body: [DetectorFixture({name: 'Detector 1'})],
+      body: [MetricDetectorFixture({name: 'Detector 1'})],
     });
     PageFiltersStore.onInitializeUrlState(PageFiltersFixture({projects: [1]}), new Set());
   });
@@ -59,7 +59,7 @@ describe('AutomationsList', function () {
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/detectors/',
       body: [
-        DetectorFixture({
+        MetricDetectorFixture({
           id: '1',
           name: 'Detector 1',
           workflowIds: ['100'],
diff --git a/static/app/views/detectors/components/detailsPanel.tsx b/static/app/views/detectors/components/detailsPanel.tsx
@@ -41,9 +41,11 @@ function SnubaQueryDetails({dataSource}: {dataSource: SnubaQueryDataSource}) {
 }
 
 function DetailsPanel({detector}: DetailsPanelProps) {
-  const dataSource = detector.dataSources?.[0];
-  if (dataSource?.type === 'snuba_query_subscription') {
-    return <SnubaQueryDetails dataSource={dataSource} />;
+  if (detector.type === 'metric_issue') {
+    const dataSource = detector.dataSources?.[0];
+    if (dataSource?.type === 'snuba_query_subscription') {
+      return <SnubaQueryDetails dataSource={dataSource} />;
+    }
   }
 
   return (
diff --git a/static/app/views/detectors/components/detectorDetailsSidebar.tsx b/static/app/views/detectors/components/detectorDetailsSidebar.tsx
@@ -26,6 +26,11 @@ import {getResolutionDescription} from 'sentry/views/detectors/utils/getDetector
 import {getMetricDetectorSuffix} from 'sentry/views/detectors/utils/metricDetectorSuffix';
 
 function getDetectorEnvironment(detector: Detector) {
+  // TODO: Add support for other detector types
+  if (detector.type !== 'metric_issue') {
+    return '<placeholder>';
+  }
+
   return (
     detector.dataSources?.find(ds => ds.type === 'snuba_query_subscription')?.queryObj
       ?.snubaQuery.environment ?? t('All environments')
@@ -67,6 +72,10 @@ function AssignToUser({userId}: {userId: string}) {
 }
 
 function DetectorPriorities({detector}: {detector: Detector}) {
+  if (detector.type !== 'metric_issue') {
+    return null;
+  }
+
   // TODO: Add support for other detector types
   if (!('detectionType' in detector.config)) {
     return null;
@@ -126,7 +135,7 @@ function DetectorPriorities({detector}: {detector: Detector}) {
 
 function DetectorResolve({detector}: {detector: Detector}) {
   // TODO: Add support for other detector types
-  if (!('detectionType' in detector.config)) {
+  if (detector.type !== 'metric_issue') {
     return null;
   }
 
diff --git a/static/app/views/detectors/components/detectorLink.tsx b/static/app/views/detectors/components/detectorLink.tsx
@@ -11,7 +11,11 @@ import {
   DETECTOR_PRIORITY_LEVEL_TO_PRIORITY_LEVEL,
   DetectorPriorityLevel,
 } from 'sentry/types/workflowEngine/dataConditions';
-import type {DataSource, Detector} from 'sentry/types/workflowEngine/detectors';
+import type {
+  Detector,
+  MetricDetector,
+  UptimeDetector,
+} from 'sentry/types/workflowEngine/detectors';
 import {defined} from 'sentry/utils';
 import getDuration from 'sentry/utils/duration/getDuration';
 import {middleEllipsis} from 'sentry/utils/string/middleEllipsis';
@@ -76,12 +80,7 @@ function DetailItem({children}: {children: React.ReactNode}) {
   );
 }
 
-function ConfigDetails({detector}: {detector: Detector}) {
-  // TODO: Use a MetricDetector type to avoid checking for this
-  if (!('detectionType' in detector.config)) {
-    return null;
-  }
-
+function MetricDetectorConfigDetails({detector}: {detector: MetricDetector}) {
   const type = detector.config.detectionType;
   const conditions = detector.conditionGroup?.conditions;
   if (!conditions?.length) {
@@ -118,54 +117,60 @@ function ConfigDetails({detector}: {detector: Detector}) {
   }
 }
 
-function DataSourceDetails({dataSource}: {dataSource: DataSource}) {
-  const type = dataSource.type;
-  switch (type) {
-    case 'snuba_query_subscription':
-      if (!dataSource.queryObj) {
-        return <DetailItem>{t('Query not found.')}</DetailItem>;
-      }
-      return (
-        <Fragment>
-          <DetailItem>{dataSource.queryObj.snubaQuery.environment}</DetailItem>
-          <DetailItem>{dataSource.queryObj.snubaQuery.aggregate}</DetailItem>
-          <DetailItem>
-            {middleEllipsis(dataSource.queryObj.snubaQuery.query, 40)}
-          </DetailItem>
-        </Fragment>
-      );
-    case 'uptime_subscription':
-      return (
-        <Fragment>
-          <DetailItem>
-            {dataSource.queryObj.urlDomain + '.' + dataSource.queryObj.urlDomainSuffix}
-          </DetailItem>
-          <DetailItem>{getDuration(dataSource.queryObj.intervalSeconds)}</DetailItem>
-        </Fragment>
-      );
-    default:
-      unreachable(type);
-      return null;
-  }
+function MetricDetectorDetails({detector}: {detector: MetricDetector}) {
+  return (
+    <Fragment>
+      {detector.dataSources.map(dataSource => {
+        if (!dataSource.queryObj) {
+          return null;
+        }
+        return (
+          <Fragment key={dataSource.id}>
+            <DetailItem>{dataSource.queryObj.snubaQuery.environment}</DetailItem>
+            <DetailItem>{dataSource.queryObj.snubaQuery.aggregate}</DetailItem>
+            <DetailItem>
+              {middleEllipsis(dataSource.queryObj.snubaQuery.query, 40)}
+            </DetailItem>
+          </Fragment>
+        );
+      })}
+      <MetricDetectorConfigDetails detector={detector} />
+    </Fragment>
+  );
 }
 
-function Details({detector}: {detector: Detector}) {
-  if (!detector.dataSources?.length) {
-    return null;
-  }
-
+function UptimeDetectorDetails({detector}: {detector: UptimeDetector}) {
   return (
     <Fragment>
-      {detector.dataSources.map(dataSource => (
-        <Fragment key={dataSource.id}>
-          <DataSourceDetails dataSource={dataSource} />
-        </Fragment>
-      ))}
-      <ConfigDetails detector={detector} />
+      {detector.dataSources.map(dataSource => {
+        return (
+          <Fragment key={dataSource.id}>
+            <DetailItem>{middleEllipsis(dataSource.queryObj.url, 40)}</DetailItem>
+            <DetailItem>{getDuration(dataSource.queryObj.intervalSeconds)}</DetailItem>
+          </Fragment>
+        );
+      })}
     </Fragment>
   );
 }
 
+function Details({detector}: {detector: Detector}) {
+  const detectorType = detector.type;
+  switch (detectorType) {
+    case 'metric_issue':
+      return <MetricDetectorDetails detector={detector} />;
+    case 'uptime_domain_failure':
+      return <UptimeDetectorDetails detector={detector} />;
+    // TODO: Implement details for Cron detectors
+    case 'uptime_subscription':
+    case 'error':
+      return null;
+    default:
+      unreachable(detectorType);
+      return null;
+  }
+}
+
 export function DetectorLink({detector, className}: DetectorLinkProps) {
   const org = useOrganization();
   const project = useProjectFromId({project_id: detector.projectId});
diff --git a/static/app/views/detectors/components/forms/editDetectorActions.spec.tsx b/static/app/views/detectors/components/forms/editDetectorActions.spec.tsx
@@ -1,4 +1,4 @@
-import {DetectorFixture} from 'sentry-fixture/detectors';
+import {MetricDetectorFixture} from 'sentry-fixture/detectors';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 
 import {
@@ -13,7 +13,7 @@ import {EditDetectorActions} from './editDetectorActions';
 
 describe('EditDetectorActions', () => {
   it('calls delete mutation when deletion is confirmed', async () => {
-    const detector = DetectorFixture();
+    const detector = MetricDetectorFixture();
     const organization = OrganizationFixture();
 
     const mockDeleteDetector = MockApiClient.addMockResponse({
diff --git a/static/app/views/detectors/components/forms/metric/metricFormData.tsx b/static/app/views/detectors/components/forms/metric/metricFormData.tsx
@@ -10,7 +10,8 @@ import {
 } from 'sentry/types/workflowEngine/dataConditions';
 import type {
   Detector,
-  DetectorConfig,
+  MetricDetector,
+  MetricDetectorConfig,
   MetricDetectorUpdatePayload,
 } from 'sentry/types/workflowEngine/detectors';
 import {defined} from 'sentry/utils';
@@ -325,7 +326,7 @@ export function metricDetectorFormDataToEndpointPayload(
   const dataSource = createDataSource(data);
 
   // Create config based on detection type
-  let config: DetectorConfig;
+  let config: MetricDetectorConfig;
   switch (data.kind) {
     case 'percent':
       config = {
@@ -368,7 +369,7 @@ export function metricDetectorFormDataToEndpointPayload(
  * Convert the detector conditions array to the flattened form data
  */
 function processDetectorConditions(
-  detector: Detector
+  detector: MetricDetector
 ): PrioritizeLevelFormData &
   Pick<MetricDetectorFormData, 'conditionValue' | 'conditionType'> {
   // Get conditions from the condition group
@@ -422,6 +423,11 @@ function processDetectorConditions(
 export function metricSavedDetectorToFormData(
   detector: Detector
 ): MetricDetectorFormData {
+  if (detector.type !== 'metric_issue') {
+    // This should never happen
+    throw new Error('Detector type mismatch');
+  }
+
   // Get the first data source (assuming metric detectors have one)
   const dataSource = detector.dataSources?.[0];
 
diff --git a/static/app/views/detectors/components/forms/uptime/fields.tsx b/static/app/views/detectors/components/forms/uptime/fields.tsx
@@ -70,6 +70,11 @@ export function uptimeFormDataToEndpointPayload(
 export function uptimeSavedDetectorToFormData(
   detector: Detector
 ): UptimeDetectorFormData {
+  if (detector.type !== 'uptime_domain_failure') {
+    // This should never happen
+    throw new Error('Detector type mismatch');
+  }
+
   const dataSource = detector.dataSources?.[0];
   const environment = 'environment' in detector.config ? detector.config.environment : '';
 
diff --git a/static/app/views/detectors/detail.spec.tsx b/static/app/views/detectors/detail.spec.tsx
@@ -1,5 +1,8 @@
 import {AutomationFixture} from 'sentry-fixture/automations';
-import {DetectorFixture, SnubaQueryDataSourceFixture} from 'sentry-fixture/detectors';
+import {
+  MetricDetectorFixture,
+  SnubaQueryDataSourceFixture,
+} from 'sentry-fixture/detectors';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 import {ProjectFixture} from 'sentry-fixture/project';
 import {TeamFixture} from 'sentry-fixture/team';
@@ -26,7 +29,7 @@ describe('DetectorDetails', function () {
       },
     },
   });
-  const snubaQueryDetector = DetectorFixture({
+  const snubaQueryDetector = MetricDetectorFixture({
     projectId: project.id,
     dataSources: [dataSource],
     owner: `team:${ownerTeam.id}`,
diff --git a/static/app/views/detectors/list.spec.tsx b/static/app/views/detectors/list.spec.tsx
diff --git a/static/app/views/detectors/utils/metricDetectorSuffix.spec.tsx b/static/app/views/detectors/utils/metricDetectorSuffix.spec.tsx
diff --git a/static/app/views/detectors/utils/metricDetectorSuffix.tsx b/static/app/views/detectors/utils/metricDetectorSuffix.tsx
diff --git a/tests/js/fixtures/detectors.ts b/tests/js/fixtures/detectors.ts

Original file line number	Diff line number	Diff line change
`@@ -41,9 +41,11 @@ function SnubaQueryDetails({dataSource}: {dataSource: SnubaQueryDataSource}) {`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`function DetailsPanel({detector}: DetailsPanelProps) {`
`44`		`- const dataSource = detector.dataSources?.[0];`
`45`		`- if (dataSource?.type === 'snuba_query_subscription') {`
`46`		`- return <SnubaQueryDetails dataSource={dataSource} />;`
	`44`	`+ if (detector.type === 'metric_issue') {`
	`45`	`+ const dataSource = detector.dataSources?.[0];`
	`46`	`+ if (dataSource?.type === 'snuba_query_subscription') {`
	`47`	`+ return <SnubaQueryDetails dataSource={dataSource} />;`
	`48`	`+ }`
`47`	`49`	`}`
`48`	`50`
`49`	`51`	`return (`