Skip to content

Commit b2fff1d

Browse files
authored
feat: Use extrapolation mode in alerts (#103731)
The backend returns extrapolation mode, a field used for transaction -> span migration. Use this value to call events-stats/ endpoint with the correct extrapolation mode and sampling mode. Alerts with SERVER_WEIGHTED call events-stats/ with 'serverOnly' extrapolation mode and SAMPLING_MODE.NORMAL Alerts with NONE call events-stats/ with 'none' extrapolation mode and SAMPLING_MODE.HIGH_ACCURACY Extrapolation mode will be set in the backend and not a field we'd expose to the user. We're going to have an extrapolation mode called "serverOnly" which extrapolates based on server sampling (which will match generic_metric dataset numbers) and disable extrapolation (ie extrapolation mode none) to match transaction dataset numbers This PR takes care of both legacy alerts and monitors
1 parent 1fac221 commit b2fff1d

File tree

21 files changed

+634
-21
lines changed

21 files changed

+634
-21
lines changed

static/app/actionCreators/events.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ type Options = {
4141
end?: DateString;
4242
environment?: readonly string[];
4343
excludeOther?: boolean;
44+
extrapolationMode?: string;
4445
field?: string[];
4546
generatePathname?: (org: OrganizationSummary) => string;
4647
includePrevious?: boolean;
@@ -108,6 +109,7 @@ export const doEventsRequest = <IncludeAllArgsType extends boolean>(
108109
includeAllArgs,
109110
dataset,
110111
sampling,
112+
extrapolationMode,
111113
}: EventsStatsOptions<IncludeAllArgsType>
112114
): IncludeAllArgsType extends true
113115
? Promise<ApiResult<EventsStats | MultiSeriesEventsStats>>
@@ -135,6 +137,7 @@ export const doEventsRequest = <IncludeAllArgsType extends boolean>(
135137
excludeOther: excludeOther ? '1' : undefined,
136138
dataset,
137139
sampling,
140+
extrapolationMode,
138141
}).filter(([, value]) => typeof value !== 'undefined')
139142
);
140143

static/app/components/charts/eventsRequest.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,10 @@ type EventsRequestPartialProps = {
150150
* Is query out of retention
151151
*/
152152
expired?: boolean;
153+
/**
154+
* The extrapolation mode to apply to the EAP
155+
*/
156+
extrapolationMode?: string;
153157
/**
154158
* List of fields to group with when doing a topEvents request.
155159
*/

static/app/types/workflowEngine/detectors.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type {
99
AlertRuleThresholdType,
1010
Dataset,
1111
EventTypes,
12+
ExtrapolationMode,
1213
} from 'sentry/views/alerts/rules/metric/types';
1314
import type {UptimeMonitorMode} from 'sentry/views/alerts/rules/uptime/types';
1415
import type {Monitor, MonitorConfig} from 'sentry/views/insights/crons/types';
@@ -27,6 +28,7 @@ export interface SnubaQuery {
2728
*/
2829
timeWindow: number;
2930
environment?: string;
31+
extrapolationMode?: ExtrapolationMode;
3032
}
3133

3234
/**
@@ -179,6 +181,7 @@ interface UpdateSnubaDataSourcePayload {
179181
query: string;
180182
queryType: number;
181183
timeWindow: number;
184+
extrapolationMode?: string;
182185
}
183186

184187
interface UpdateUptimeDataSourcePayload {

static/app/views/alerts/rules/metric/details/index.spec.tsx

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
1010
import ProjectsStore from 'sentry/stores/projectsStore';
1111
import {trackAnalytics} from 'sentry/utils/analytics';
1212
import MetricAlertDetails from 'sentry/views/alerts/rules/metric/details';
13-
import {Dataset, EventTypes} from 'sentry/views/alerts/rules/metric/types';
13+
import {
14+
Dataset,
15+
EventTypes,
16+
ExtrapolationMode,
17+
} from 'sentry/views/alerts/rules/metric/types';
18+
import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery';
1419

1520
jest.mock('sentry/utils/analytics');
1621

@@ -290,4 +295,106 @@ describe('MetricAlertDetails', () => {
290295
'true'
291296
);
292297
});
298+
299+
it('uses SERVER_WEIGHTED extrapolation mode when alert has it configured', async () => {
300+
const {organization, routerProps} = initializeOrg();
301+
const ruleWithExtrapolation = MetricRuleFixture({
302+
projects: [project.slug],
303+
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
304+
aggregate: 'count()',
305+
query: '',
306+
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
307+
extrapolationMode: ExtrapolationMode.SERVER_WEIGHTED,
308+
});
309+
310+
MockApiClient.addMockResponse({
311+
url: `/organizations/org-slug/alert-rules/${ruleWithExtrapolation.id}/`,
312+
body: ruleWithExtrapolation,
313+
});
314+
315+
MockApiClient.addMockResponse({
316+
url: `/organizations/org-slug/incidents/`,
317+
body: [],
318+
});
319+
320+
const eventsStatsRequest = MockApiClient.addMockResponse({
321+
url: '/organizations/org-slug/events-stats/',
322+
body: EventsStatsFixture(),
323+
});
324+
325+
render(
326+
<MetricAlertDetails
327+
organization={organization}
328+
{...routerProps}
329+
params={{ruleId: ruleWithExtrapolation.id}}
330+
/>,
331+
{
332+
organization,
333+
}
334+
);
335+
336+
expect(await screen.findByText(ruleWithExtrapolation.name)).toBeInTheDocument();
337+
338+
// Verify events-stats is called with 'serverOnly' extrapolation mode
339+
expect(eventsStatsRequest).toHaveBeenCalledWith(
340+
'/organizations/org-slug/events-stats/',
341+
expect.objectContaining({
342+
query: expect.objectContaining({
343+
extrapolationMode: 'serverOnly',
344+
sampling: SAMPLING_MODE.NORMAL,
345+
}),
346+
})
347+
);
348+
});
349+
350+
it('uses NONE extrapolation mode when alert has it configured', async () => {
351+
const {organization, routerProps} = initializeOrg();
352+
const ruleWithNoExtrapolation = MetricRuleFixture({
353+
projects: [project.slug],
354+
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
355+
aggregate: 'count()',
356+
query: '',
357+
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
358+
extrapolationMode: ExtrapolationMode.NONE,
359+
});
360+
361+
MockApiClient.addMockResponse({
362+
url: `/organizations/org-slug/alert-rules/${ruleWithNoExtrapolation.id}/`,
363+
body: ruleWithNoExtrapolation,
364+
});
365+
366+
MockApiClient.addMockResponse({
367+
url: `/organizations/org-slug/incidents/`,
368+
body: [],
369+
});
370+
371+
const eventsStatsRequest = MockApiClient.addMockResponse({
372+
url: '/organizations/org-slug/events-stats/',
373+
body: EventsStatsFixture(),
374+
});
375+
376+
render(
377+
<MetricAlertDetails
378+
organization={organization}
379+
{...routerProps}
380+
params={{ruleId: ruleWithNoExtrapolation.id}}
381+
/>,
382+
{
383+
organization,
384+
}
385+
);
386+
387+
expect(await screen.findByText(ruleWithNoExtrapolation.name)).toBeInTheDocument();
388+
389+
// Verify events-stats is called with 'none' extrapolation mode
390+
expect(eventsStatsRequest).toHaveBeenCalledWith(
391+
'/organizations/org-slug/events-stats/',
392+
expect.objectContaining({
393+
query: expect.objectContaining({
394+
extrapolationMode: 'none',
395+
sampling: SAMPLING_MODE.HIGH_ACCURACY,
396+
}),
397+
})
398+
);
399+
});
293400
});

static/app/views/alerts/rules/metric/details/metricChart.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ import useOrganization from 'sentry/utils/useOrganization';
5252
import {COMPARISON_DELTA_OPTIONS} from 'sentry/views/alerts/rules/metric/constants';
5353
import {makeDefaultCta} from 'sentry/views/alerts/rules/metric/metricRulePresets';
5454
import type {MetricRule} from 'sentry/views/alerts/rules/metric/types';
55-
import {AlertRuleTriggerType, Dataset} from 'sentry/views/alerts/rules/metric/types';
55+
import {
56+
AlertRuleTriggerType,
57+
Dataset,
58+
ExtrapolationMode,
59+
} from 'sentry/views/alerts/rules/metric/types';
5660
import {isCrashFreeAlert} from 'sentry/views/alerts/rules/metric/utils/isCrashFreeAlert';
5761
import {
5862
isEapAlertType,
@@ -475,7 +479,9 @@ export default function MetricChart({
475479
referrer: 'api.alerts.alert-rule-chart',
476480
samplingMode:
477481
rule.dataset === Dataset.EVENTS_ANALYTICS_PLATFORM
478-
? SAMPLING_MODE.NORMAL
482+
? rule.extrapolationMode === ExtrapolationMode.NONE
483+
? SAMPLING_MODE.HIGH_ACCURACY
484+
: SAMPLING_MODE.NORMAL
479485
: undefined,
480486
},
481487
{enabled: !shouldUseSessionsStats}

static/app/views/alerts/rules/metric/duplicate.spec.tsx

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import GlobalModal from 'sentry/components/globalModal';
99

1010
import MetricRuleDuplicate from './duplicate';
1111
import type {Action} from './types';
12-
import {AlertRuleTriggerType} from './types';
12+
import {AlertRuleTriggerType, Dataset, EventTypes, ExtrapolationMode} from './types';
1313

1414
describe('MetricRuleDuplicate', () => {
1515
beforeEach(() => {
@@ -55,6 +55,14 @@ describe('MetricRuleDuplicate', () => {
5555
url: '/organizations/org-slug/members/',
5656
body: [MemberFixture()],
5757
});
58+
MockApiClient.addMockResponse({
59+
url: '/organizations/org-slug/recent-searches/',
60+
body: [],
61+
});
62+
MockApiClient.addMockResponse({
63+
url: '/organizations/org-slug/trace-items/attributes/',
64+
body: [],
65+
});
5866
});
5967

6068
it('renders new alert form with values copied over', async () => {
@@ -153,4 +161,68 @@ describe('MetricRuleDuplicate', () => {
153161
// Duplicated alert has been called
154162
expect(req).toHaveBeenCalled();
155163
});
164+
165+
it('duplicates rule with SERVER_WEIGHTED extrapolation mode but creates new rule without it', async () => {
166+
const ruleWithExtrapolation = MetricRuleFixture({
167+
id: '7',
168+
name: 'Alert Rule with Extrapolation',
169+
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
170+
aggregate: 'count()',
171+
query: '',
172+
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
173+
extrapolationMode: ExtrapolationMode.SERVER_WEIGHTED,
174+
});
175+
176+
const {organization, project, routerProps} = initializeOrg({
177+
organization: {
178+
access: ['alerts:write'],
179+
},
180+
router: {
181+
params: {},
182+
location: {
183+
query: {
184+
createFromDuplicate: 'true',
185+
duplicateRuleId: `${ruleWithExtrapolation.id}`,
186+
},
187+
},
188+
},
189+
});
190+
191+
const req = MockApiClient.addMockResponse({
192+
url: `/organizations/${organization.slug}/alert-rules/${ruleWithExtrapolation.id}/`,
193+
body: ruleWithExtrapolation,
194+
});
195+
196+
const eventsStatsRequest = MockApiClient.addMockResponse({
197+
url: '/organizations/org-slug/events-stats/',
198+
body: null,
199+
});
200+
201+
render(
202+
<Fragment>
203+
<GlobalModal />
204+
<MetricRuleDuplicate project={project} userTeamIds={[]} {...routerProps} />
205+
</Fragment>
206+
);
207+
208+
// Wait for the form to load with duplicated values
209+
expect(await screen.findByTestId('critical-threshold')).toHaveValue('70');
210+
expect(screen.getByTestId('alert-name')).toHaveValue(
211+
`${ruleWithExtrapolation.name} copy`
212+
);
213+
214+
// Verify the original rule was fetched
215+
expect(req).toHaveBeenCalled();
216+
217+
// Verify events-stats chart requests do NOT include extrapolation mode
218+
// (duplicated rules are treated as new rules)
219+
expect(eventsStatsRequest).toHaveBeenCalledWith(
220+
'/organizations/org-slug/events-stats/',
221+
expect.objectContaining({
222+
query: expect.not.objectContaining({
223+
extrapolationMode: expect.anything(),
224+
}),
225+
})
226+
);
227+
});
156228
});

0 commit comments

Comments
 (0)