Skip to content

Commit 07b3483

Browse files
scttcperscttcper
authored
feat(aci): Display apdex option for spans (#103791)
brings back apdex option for spans stacked on #103697 <img width="1450" height="282" alt="image" src="https://github.com/user-attachments/assets/73e7146a-0a6f-4123-bf5d-9833873681fa" />
1 parent deb12c6 commit 07b3483

File tree

4 files changed

+191
-29
lines changed

4 files changed

+191
-29
lines changed

‎static/app/views/detectors/components/forms/metric/visualize.tsx‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ import {FieldValueKind} from 'sentry/views/discover/table/types';
3939
import {DEFAULT_VISUALIZATION_FIELD} from 'sentry/views/explore/contexts/pageParamsContext/visualizes';
4040
import {TraceItemDataset} from 'sentry/views/explore/types';
4141

42-
const LOCKED_SPAN_COUNT_OPTION = {
43-
value: DEFAULT_VISUALIZATION_FIELD,
44-
label: t('spans'),
45-
};
46-
4742
/**
4843
* Render a tag badge for field types, similar to dashboard widget builder
4944
*/
@@ -93,18 +88,47 @@ function renderTag(kind: FieldValueKind): React.ReactNode {
9388
return <Tag type={tagType}>{text}</Tag>;
9489
}
9590

96-
const LOGS_NOT_ALLOWED_AGGREGATES = [
91+
/**
92+
* Aggregate options excluded for the logs dataset
93+
*/
94+
const LOGS_EXCLUDED_AGGREGATES = [
9795
AggregationKey.FAILURE_RATE,
9896
AggregationKey.FAILURE_COUNT,
97+
AggregationKey.APDEX,
9998
];
10099

100+
const ADDITIONAL_EAP_AGGREGATES = [AggregationKey.APDEX];
101+
102+
/**
103+
* Locks the primary dropdown to the single option
104+
*/
105+
const LOCKED_SPAN_AGGREGATES = {
106+
[AggregationKey.APDEX]: {
107+
value: DEFAULT_VISUALIZATION_FIELD,
108+
label: 'span.duration',
109+
},
110+
[AggregationKey.COUNT]: {
111+
value: DEFAULT_VISUALIZATION_FIELD,
112+
label: 'spans',
113+
},
114+
};
115+
116+
// Type guard for locked span aggregates
117+
const isLockedSpanAggregate = (
118+
agg: string
119+
): agg is keyof typeof LOCKED_SPAN_AGGREGATES => {
120+
return agg in LOCKED_SPAN_AGGREGATES;
121+
};
122+
101123
function getEAPAllowedAggregates(dataset: DetectorDataset): Array<[string, string]> {
102-
return ALLOWED_EXPLORE_VISUALIZE_AGGREGATES.filter(aggregate => {
103-
if (dataset === DetectorDataset.LOGS) {
104-
return !LOGS_NOT_ALLOWED_AGGREGATES.includes(aggregate);
105-
}
106-
return true;
107-
}).map(aggregate => [aggregate, aggregate]);
124+
return [...ALLOWED_EXPLORE_VISUALIZE_AGGREGATES, ...ADDITIONAL_EAP_AGGREGATES]
125+
.filter(aggregate => {
126+
if (dataset === DetectorDataset.LOGS) {
127+
return !LOGS_EXCLUDED_AGGREGATES.includes(aggregate);
128+
}
129+
return true;
130+
})
131+
.map(aggregate => [aggregate, aggregate]);
108132
}
109133

110134
function getAggregateOptions(
@@ -226,10 +250,9 @@ export function Visualize() {
226250

227251
const datasetConfig = useMemo(() => getDatasetConfig(dataset), [dataset]);
228252

229-
const aggregateOptions = useMemo(
230-
() => datasetConfig.getAggregateOptions(organization, tags, customMeasurements),
231-
[organization, tags, datasetConfig, customMeasurements]
232-
);
253+
const aggregateOptions = useMemo(() => {
254+
return datasetConfig.getAggregateOptions(organization, tags, customMeasurements);
255+
}, [organization, tags, datasetConfig, customMeasurements]);
233256

234257
const fieldOptions = useMemo(() => {
235258
// For Spans dataset, use span-specific options from the provider
@@ -334,7 +357,10 @@ export function Visualize() {
334357
};
335358

336359
const lockSpanOptions =
337-
dataset === DetectorDataset.SPANS && aggregate === AggregationKey.COUNT;
360+
dataset === DetectorDataset.SPANS && isLockedSpanAggregate(aggregate);
361+
362+
// Get locked option if applicable, with proper type narrowing
363+
const lockedOption = lockSpanOptions ? LOCKED_SPAN_AGGREGATES[aggregate] : null;
338364

339365
return (
340366
<Flex direction="column" gap="md">
@@ -368,22 +394,20 @@ export function Visualize() {
368394
<StyledVisualizeSelect
369395
searchable
370396
triggerProps={{
371-
children: lockSpanOptions
372-
? LOCKED_SPAN_COUNT_OPTION.label
397+
children: lockedOption
398+
? lockedOption.label
373399
: parameters[index] || param.defaultValue || t('Select metric'),
374400
}}
375-
options={
376-
lockSpanOptions ? [LOCKED_SPAN_COUNT_OPTION] : fieldOptionsDropdown
377-
}
401+
options={lockedOption ? [lockedOption] : fieldOptionsDropdown}
378402
value={
379-
lockSpanOptions
403+
lockedOption
380404
? DEFAULT_VISUALIZATION_FIELD
381405
: parameters[index] || param.defaultValue || ''
382406
}
383407
onChange={option => {
384408
handleParameterChange(index, String(option.value));
385409
}}
386-
disabled={isTransactionsDataset || lockSpanOptions}
410+
disabled={isTransactionsDataset}
387411
/>
388412
) : param.kind === 'dropdown' && param.options ? (
389413
<StyledVisualizeSelect
@@ -404,6 +428,7 @@ export function Visualize() {
404428
/>
405429
) : (
406430
<StyledParameterInput
431+
size="md"
407432
placeholder={param.defaultValue || t('Enter value')}
408433
value={parameters[index] || ''}
409434
onChange={e => {

‎static/app/views/detectors/datasetConfig/spans.tsx‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import {t} from 'sentry/locale';
2-
import type {EventsStats} from 'sentry/types/organization';
2+
import type {SelectValue} from 'sentry/types/core';
3+
import type {TagCollection} from 'sentry/types/group';
4+
import type {EventsStats, Organization} from 'sentry/types/organization';
5+
import type {CustomMeasurementCollection} from 'sentry/utils/customMeasurements/customMeasurements';
6+
import type {AggregateParameter} from 'sentry/utils/discover/fields';
37
import {DiscoverDatasets} from 'sentry/utils/discover/types';
8+
import {AggregationKey, getFieldDefinition} from 'sentry/utils/fields';
49
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
510
import {EventTypes} from 'sentry/views/alerts/rules/metric/types';
611
import {SpansConfig} from 'sentry/views/dashboards/datasetConfig/spans';
@@ -20,19 +25,69 @@ import {
2025
translateAggregateTag,
2126
translateAggregateTagBack,
2227
} from 'sentry/views/detectors/datasetConfig/utils/translateAggregateTag';
28+
import type {FieldValue} from 'sentry/views/discover/table/types';
29+
import {FieldValueKind} from 'sentry/views/discover/table/types';
2330

2431
import type {DetectorDatasetConfig} from './base';
2532

2633
type SpansSeriesResponse = EventsStats;
2734

2835
const DEFAULT_EVENT_TYPES = [EventTypes.TRACE_ITEM_SPAN];
2936

37+
function getAggregateOptions(
38+
organization: Organization,
39+
tags?: TagCollection,
40+
customMeasurements?: CustomMeasurementCollection
41+
): Record<string, SelectValue<FieldValue>> {
42+
const base = SpansConfig.getTableFieldOptions(organization, tags, customMeasurements);
43+
44+
const apdexDefinition = getFieldDefinition(AggregationKey.APDEX, 'span');
45+
46+
if (apdexDefinition?.parameters) {
47+
// Convert field definition parameters to discover field format
48+
const convertedParameters = apdexDefinition.parameters.map<AggregateParameter>(
49+
param => {
50+
if (param.kind === 'value') {
51+
return {
52+
kind: 'value' as const,
53+
dataType: param.dataType as 'number',
54+
required: param.required,
55+
defaultValue: param.defaultValue,
56+
placeholder: param.placeholder,
57+
};
58+
}
59+
return {
60+
kind: 'column' as const,
61+
columnTypes: Array.isArray(param.columnTypes)
62+
? param.columnTypes
63+
: [param.columnTypes],
64+
required: param.required,
65+
defaultValue: param.defaultValue,
66+
};
67+
}
68+
);
69+
70+
base['function:apdex'] = {
71+
label: 'apdex',
72+
value: {
73+
kind: FieldValueKind.FUNCTION,
74+
meta: {
75+
name: 'apdex',
76+
parameters: convertedParameters,
77+
},
78+
},
79+
};
80+
}
81+
82+
return base;
83+
}
84+
3085
export const DetectorSpansConfig: DetectorDatasetConfig<SpansSeriesResponse> = {
3186
name: t('Spans'),
3287
SearchBar: TraceSearchBar,
3388
defaultEventTypes: DEFAULT_EVENT_TYPES,
3489
defaultField: SpansConfig.defaultField,
35-
getAggregateOptions: SpansConfig.getTableFieldOptions,
90+
getAggregateOptions,
3691
getSeriesQueryOptions: options => {
3792
return getDiscoverSeriesQueryOptions({
3893
...options,

‎static/app/views/detectors/edit.spec.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ describe('DetectorEdit', () => {
571571
expect(screen.queryByText('Dynamic')).not.toBeInTheDocument();
572572
});
573573

574-
it('disables column select when spans + count()', async () => {
574+
it('limited options when selecting spans + count()', async () => {
575575
const spansDetector = MetricDetectorFixture({
576576
dataSources: [
577577
SnubaQueryDataSourceFixture({
@@ -606,9 +606,14 @@ describe('DetectorEdit', () => {
606606
await screen.findByRole('link', {name: spansDetector.name})
607607
).toBeInTheDocument();
608608

609-
// Column parameter should be locked to "spans" and disabled
609+
// Column parameter should be locked to "spans" - verify only "spans" option is available
610610
const button = screen.getByRole('button', {name: 'spans'});
611-
expect(button).toBeDisabled();
611+
await userEvent.click(button);
612+
613+
// Verify only "spans" option exists in the dropdown
614+
const options = screen.getAllByRole('option');
615+
expect(options).toHaveLength(1);
616+
expect(options[0]).toHaveTextContent('spans');
612617
});
613618

614619
it('resets 1 day interval to 15 minutes when switching to dynamic detection', async () => {

‎static/app/views/detectors/new-setting.spec.tsx‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,83 @@ describe('DetectorEdit', () => {
658658
);
659659
});
660660
});
661+
662+
it('can submit a new metric detector with apdex aggregate', async () => {
663+
const mockCreateDetector = MockApiClient.addMockResponse({
664+
url: `/organizations/${organization.slug}/detectors/`,
665+
method: 'POST',
666+
body: MetricDetectorFixture({id: '789'}),
667+
});
668+
669+
render(<DetectorNewSettings />, {
670+
organization,
671+
initialRouterConfig: metricRouterConfig,
672+
});
673+
674+
const title = await screen.findByText('New Monitor');
675+
await userEvent.click(title);
676+
await userEvent.keyboard('Apdex{enter}');
677+
678+
// Change aggregate from count to apdex
679+
await userEvent.click(screen.getByRole('button', {name: 'count'}));
680+
await userEvent.click(await screen.findByRole('option', {name: 'apdex'}));
681+
682+
// Change to apdex(100)
683+
await userEvent.clear(screen.getByPlaceholderText('300'));
684+
await userEvent.type(screen.getByPlaceholderText('300'), '100');
685+
686+
// Set the high threshold for alerting
687+
await userEvent.type(
688+
screen.getByRole('spinbutton', {name: 'High threshold'}),
689+
'100'
690+
);
691+
692+
await userEvent.click(screen.getByRole('button', {name: 'Create Monitor'}));
693+
694+
await waitFor(() => {
695+
expect(mockCreateDetector).toHaveBeenCalledWith(
696+
`/organizations/${organization.slug}/detectors/`,
697+
expect.objectContaining({
698+
data: expect.objectContaining({
699+
name: 'Apdex',
700+
type: 'metric_issue',
701+
projectId: project.id,
702+
owner: null,
703+
workflowIds: [],
704+
conditionGroup: {
705+
conditions: [
706+
{
707+
comparison: 100,
708+
conditionResult: 75,
709+
type: 'gt',
710+
},
711+
{
712+
comparison: 100,
713+
conditionResult: 0,
714+
type: 'lte',
715+
},
716+
],
717+
logicType: 'any',
718+
},
719+
config: {
720+
detectionType: 'static',
721+
},
722+
dataSources: [
723+
{
724+
aggregate: 'apdex(span.duration,100)',
725+
dataset: 'events_analytics_platform',
726+
eventTypes: ['trace_item_span'],
727+
query: '',
728+
queryType: 1,
729+
timeWindow: 3600,
730+
environment: null,
731+
},
732+
],
733+
}),
734+
})
735+
);
736+
});
737+
});
661738
});
662739

663740
describe('Uptime Detector', () => {

0 commit comments

Comments
 (0)