diff --git a/static/app/views/detectors/components/forms/metric/visualize.tsx b/static/app/views/detectors/components/forms/metric/visualize.tsx index 57f3f245e31e52..125b39cb48a8f1 100644 --- a/static/app/views/detectors/components/forms/metric/visualize.tsx +++ b/static/app/views/detectors/components/forms/metric/visualize.tsx @@ -39,11 +39,6 @@ import {FieldValueKind} from 'sentry/views/discover/table/types'; import {DEFAULT_VISUALIZATION_FIELD} from 'sentry/views/explore/contexts/pageParamsContext/visualizes'; import {TraceItemDataset} from 'sentry/views/explore/types'; -const LOCKED_SPAN_COUNT_OPTION = { - value: DEFAULT_VISUALIZATION_FIELD, - label: t('spans'), -}; - /** * Render a tag badge for field types, similar to dashboard widget builder */ @@ -93,18 +88,47 @@ function renderTag(kind: FieldValueKind): React.ReactNode { return {text}; } -const LOGS_NOT_ALLOWED_AGGREGATES = [ +/** + * Aggregate options excluded for the logs dataset + */ +const LOGS_EXCLUDED_AGGREGATES = [ AggregationKey.FAILURE_RATE, AggregationKey.FAILURE_COUNT, + AggregationKey.APDEX, ]; +const ADDITIONAL_EAP_AGGREGATES = [AggregationKey.APDEX]; + +/** + * Locks the primary dropdown to the single option + */ +const LOCKED_SPAN_AGGREGATES = { + [AggregationKey.APDEX]: { + value: DEFAULT_VISUALIZATION_FIELD, + label: 'span.duration', + }, + [AggregationKey.COUNT]: { + value: DEFAULT_VISUALIZATION_FIELD, + label: 'spans', + }, +}; + +// Type guard for locked span aggregates +const isLockedSpanAggregate = ( + agg: string +): agg is keyof typeof LOCKED_SPAN_AGGREGATES => { + return agg in LOCKED_SPAN_AGGREGATES; +}; + function getEAPAllowedAggregates(dataset: DetectorDataset): Array<[string, string]> { - return ALLOWED_EXPLORE_VISUALIZE_AGGREGATES.filter(aggregate => { - if (dataset === DetectorDataset.LOGS) { - return !LOGS_NOT_ALLOWED_AGGREGATES.includes(aggregate); - } - return true; - }).map(aggregate => [aggregate, aggregate]); + return [...ALLOWED_EXPLORE_VISUALIZE_AGGREGATES, ...ADDITIONAL_EAP_AGGREGATES] + .filter(aggregate => { + if (dataset === DetectorDataset.LOGS) { + return !LOGS_EXCLUDED_AGGREGATES.includes(aggregate); + } + return true; + }) + .map(aggregate => [aggregate, aggregate]); } function getAggregateOptions( @@ -226,10 +250,9 @@ export function Visualize() { const datasetConfig = useMemo(() => getDatasetConfig(dataset), [dataset]); - const aggregateOptions = useMemo( - () => datasetConfig.getAggregateOptions(organization, tags, customMeasurements), - [organization, tags, datasetConfig, customMeasurements] - ); + const aggregateOptions = useMemo(() => { + return datasetConfig.getAggregateOptions(organization, tags, customMeasurements); + }, [organization, tags, datasetConfig, customMeasurements]); const fieldOptions = useMemo(() => { // For Spans dataset, use span-specific options from the provider @@ -334,7 +357,10 @@ export function Visualize() { }; const lockSpanOptions = - dataset === DetectorDataset.SPANS && aggregate === AggregationKey.COUNT; + dataset === DetectorDataset.SPANS && isLockedSpanAggregate(aggregate); + + // Get locked option if applicable, with proper type narrowing + const lockedOption = lockSpanOptions ? LOCKED_SPAN_AGGREGATES[aggregate] : null; return ( @@ -368,22 +394,20 @@ export function Visualize() { { handleParameterChange(index, String(option.value)); }} - disabled={isTransactionsDataset || lockSpanOptions} + disabled={isTransactionsDataset} /> ) : param.kind === 'dropdown' && param.options ? ( ) : ( { diff --git a/static/app/views/detectors/datasetConfig/spans.tsx b/static/app/views/detectors/datasetConfig/spans.tsx index c1e53d71b45322..3fdc9c8723c33e 100644 --- a/static/app/views/detectors/datasetConfig/spans.tsx +++ b/static/app/views/detectors/datasetConfig/spans.tsx @@ -1,6 +1,11 @@ import {t} from 'sentry/locale'; -import type {EventsStats} from 'sentry/types/organization'; +import type {SelectValue} from 'sentry/types/core'; +import type {TagCollection} from 'sentry/types/group'; +import type {EventsStats, Organization} from 'sentry/types/organization'; +import type {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/customMeasurements'; +import type {AggregateParameter} from 'sentry/utils/discover/fields'; import {DiscoverDatasets} from 'sentry/utils/discover/types'; +import {AggregationKey, getFieldDefinition} from 'sentry/utils/fields'; import {MutableSearch} from 'sentry/utils/tokenizeSearch'; import {EventTypes} from 'sentry/views/alerts/rules/metric/types'; import {SpansConfig} from 'sentry/views/dashboards/datasetConfig/spans'; @@ -20,6 +25,8 @@ import { translateAggregateTag, translateAggregateTagBack, } from 'sentry/views/detectors/datasetConfig/utils/translateAggregateTag'; +import type {FieldValue} from 'sentry/views/discover/table/types'; +import {FieldValueKind} from 'sentry/views/discover/table/types'; import type {DetectorDatasetConfig} from './base'; @@ -27,12 +34,60 @@ type SpansSeriesResponse = EventsStats; const DEFAULT_EVENT_TYPES = [EventTypes.TRACE_ITEM_SPAN]; +function getAggregateOptions( + organization: Organization, + tags?: TagCollection, + customMeasurements?: CustomMeasurementCollection +): Record> { + const base = SpansConfig.getTableFieldOptions(organization, tags, customMeasurements); + + const apdexDefinition = getFieldDefinition(AggregationKey.APDEX, 'span'); + + if (apdexDefinition?.parameters) { + // Convert field definition parameters to discover field format + const convertedParameters = apdexDefinition.parameters.map( + param => { + if (param.kind === 'value') { + return { + kind: 'value' as const, + dataType: param.dataType as 'number', + required: param.required, + defaultValue: param.defaultValue, + placeholder: param.placeholder, + }; + } + return { + kind: 'column' as const, + columnTypes: Array.isArray(param.columnTypes) + ? param.columnTypes + : [param.columnTypes], + required: param.required, + defaultValue: param.defaultValue, + }; + } + ); + + base['function:apdex'] = { + label: 'apdex', + value: { + kind: FieldValueKind.FUNCTION, + meta: { + name: 'apdex', + parameters: convertedParameters, + }, + }, + }; + } + + return base; +} + export const DetectorSpansConfig: DetectorDatasetConfig = { name: t('Spans'), SearchBar: TraceSearchBar, defaultEventTypes: DEFAULT_EVENT_TYPES, defaultField: SpansConfig.defaultField, - getAggregateOptions: SpansConfig.getTableFieldOptions, + getAggregateOptions, getSeriesQueryOptions: options => { return getDiscoverSeriesQueryOptions({ ...options, diff --git a/static/app/views/detectors/edit.spec.tsx b/static/app/views/detectors/edit.spec.tsx index c9866b519b4d4a..363ad4938ab2c8 100644 --- a/static/app/views/detectors/edit.spec.tsx +++ b/static/app/views/detectors/edit.spec.tsx @@ -571,7 +571,7 @@ describe('DetectorEdit', () => { expect(screen.queryByText('Dynamic')).not.toBeInTheDocument(); }); - it('disables column select when spans + count()', async () => { + it('limited options when selecting spans + count()', async () => { const spansDetector = MetricDetectorFixture({ dataSources: [ SnubaQueryDataSourceFixture({ @@ -606,9 +606,14 @@ describe('DetectorEdit', () => { await screen.findByRole('link', {name: spansDetector.name}) ).toBeInTheDocument(); - // Column parameter should be locked to "spans" and disabled + // Column parameter should be locked to "spans" - verify only "spans" option is available const button = screen.getByRole('button', {name: 'spans'}); - expect(button).toBeDisabled(); + await userEvent.click(button); + + // Verify only "spans" option exists in the dropdown + const options = screen.getAllByRole('option'); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('spans'); }); it('resets 1 day interval to 15 minutes when switching to dynamic detection', async () => { diff --git a/static/app/views/detectors/new-setting.spec.tsx b/static/app/views/detectors/new-setting.spec.tsx index 69a4b52da0f91c..11b25926a17470 100644 --- a/static/app/views/detectors/new-setting.spec.tsx +++ b/static/app/views/detectors/new-setting.spec.tsx @@ -658,6 +658,83 @@ describe('DetectorEdit', () => { ); }); }); + + it('can submit a new metric detector with apdex aggregate', async () => { + const mockCreateDetector = MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/detectors/`, + method: 'POST', + body: MetricDetectorFixture({id: '789'}), + }); + + render(, { + organization, + initialRouterConfig: metricRouterConfig, + }); + + const title = await screen.findByText('New Monitor'); + await userEvent.click(title); + await userEvent.keyboard('Apdex{enter}'); + + // Change aggregate from count to apdex + await userEvent.click(screen.getByRole('button', {name: 'count'})); + await userEvent.click(await screen.findByRole('option', {name: 'apdex'})); + + // Change to apdex(100) + await userEvent.clear(screen.getByPlaceholderText('300')); + await userEvent.type(screen.getByPlaceholderText('300'), '100'); + + // Set the high threshold for alerting + await userEvent.type( + screen.getByRole('spinbutton', {name: 'High threshold'}), + '100' + ); + + await userEvent.click(screen.getByRole('button', {name: 'Create Monitor'})); + + await waitFor(() => { + expect(mockCreateDetector).toHaveBeenCalledWith( + `/organizations/${organization.slug}/detectors/`, + expect.objectContaining({ + data: expect.objectContaining({ + name: 'Apdex', + type: 'metric_issue', + projectId: project.id, + owner: null, + workflowIds: [], + conditionGroup: { + conditions: [ + { + comparison: 100, + conditionResult: 75, + type: 'gt', + }, + { + comparison: 100, + conditionResult: 0, + type: 'lte', + }, + ], + logicType: 'any', + }, + config: { + detectionType: 'static', + }, + dataSources: [ + { + aggregate: 'apdex(span.duration,100)', + dataset: 'events_analytics_platform', + eventTypes: ['trace_item_span'], + query: '', + queryType: 1, + timeWindow: 3600, + environment: null, + }, + ], + }), + }) + ); + }); + }); }); describe('Uptime Detector', () => {