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
3 changes: 3 additions & 0 deletions static/app/actionCreators/events.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type Options = {
end?: DateString;
environment?: readonly string[];
excludeOther?: boolean;
extrapolationMode?: string;
field?: string[];
generatePathname?: (org: OrganizationSummary) => string;
includePrevious?: boolean;
Expand Down Expand Up @@ -108,6 +109,7 @@ export const doEventsRequest = <IncludeAllArgsType extends boolean>(
includeAllArgs,
dataset,
sampling,
extrapolationMode,
}: EventsStatsOptions<IncludeAllArgsType>
): IncludeAllArgsType extends true
? Promise<ApiResult<EventsStats | MultiSeriesEventsStats>>
Expand Down Expand Up @@ -135,6 +137,7 @@ export const doEventsRequest = <IncludeAllArgsType extends boolean>(
excludeOther: excludeOther ? '1' : undefined,
dataset,
sampling,
extrapolationMode,
}).filter(([, value]) => typeof value !== 'undefined')
);

Expand Down
4 changes: 4 additions & 0 deletions static/app/components/charts/eventsRequest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ type EventsRequestPartialProps = {
* Is query out of retention
*/
expired?: boolean;
/**
* The extrapolation mode to apply to the EAP
*/
extrapolationMode?: string;
/**
* List of fields to group with when doing a topEvents request.
*/
Expand Down
3 changes: 3 additions & 0 deletions static/app/types/workflowEngine/detectors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
AlertRuleThresholdType,
Dataset,
EventTypes,
ExtrapolationMode,
} from 'sentry/views/alerts/rules/metric/types';
import type {UptimeMonitorMode} from 'sentry/views/alerts/rules/uptime/types';
import type {Monitor, MonitorConfig} from 'sentry/views/insights/crons/types';
Expand All @@ -27,6 +28,7 @@ export interface SnubaQuery {
*/
timeWindow: number;
environment?: string;
extrapolationMode?: ExtrapolationMode;
}

/**
Expand Down Expand Up @@ -179,6 +181,7 @@ interface UpdateSnubaDataSourcePayload {
query: string;
queryType: number;
timeWindow: number;
extrapolationMode?: string;
}

interface UpdateUptimeDataSourcePayload {
Expand Down
109 changes: 108 additions & 1 deletion static/app/views/alerts/rules/metric/details/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import ProjectsStore from 'sentry/stores/projectsStore';
import {trackAnalytics} from 'sentry/utils/analytics';
import MetricAlertDetails from 'sentry/views/alerts/rules/metric/details';
import {Dataset, EventTypes} from 'sentry/views/alerts/rules/metric/types';
import {
Dataset,
EventTypes,
ExtrapolationMode,
} from 'sentry/views/alerts/rules/metric/types';
import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery';

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

Expand Down Expand Up @@ -290,4 +295,106 @@ describe('MetricAlertDetails', () => {
'true'
);
});

it('uses SERVER_WEIGHTED extrapolation mode when alert has it configured', async () => {
const {organization, routerProps} = initializeOrg();
const ruleWithExtrapolation = MetricRuleFixture({
projects: [project.slug],
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
aggregate: 'count()',
query: '',
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
extrapolationMode: ExtrapolationMode.SERVER_WEIGHTED,
});

MockApiClient.addMockResponse({
url: `/organizations/org-slug/alert-rules/${ruleWithExtrapolation.id}/`,
body: ruleWithExtrapolation,
});

MockApiClient.addMockResponse({
url: `/organizations/org-slug/incidents/`,
body: [],
});

const eventsStatsRequest = MockApiClient.addMockResponse({
url: '/organizations/org-slug/events-stats/',
body: EventsStatsFixture(),
});

render(
<MetricAlertDetails
organization={organization}
{...routerProps}
params={{ruleId: ruleWithExtrapolation.id}}
/>,
{
organization,
}
);

expect(await screen.findByText(ruleWithExtrapolation.name)).toBeInTheDocument();

// Verify events-stats is called with 'serverOnly' extrapolation mode
expect(eventsStatsRequest).toHaveBeenCalledWith(
'/organizations/org-slug/events-stats/',
expect.objectContaining({
query: expect.objectContaining({
extrapolationMode: 'serverOnly',
sampling: SAMPLING_MODE.NORMAL,
}),
})
);
});

it('uses NONE extrapolation mode when alert has it configured', async () => {
const {organization, routerProps} = initializeOrg();
const ruleWithNoExtrapolation = MetricRuleFixture({
projects: [project.slug],
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
aggregate: 'count()',
query: '',
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
extrapolationMode: ExtrapolationMode.NONE,
});

MockApiClient.addMockResponse({
url: `/organizations/org-slug/alert-rules/${ruleWithNoExtrapolation.id}/`,
body: ruleWithNoExtrapolation,
});

MockApiClient.addMockResponse({
url: `/organizations/org-slug/incidents/`,
body: [],
});

const eventsStatsRequest = MockApiClient.addMockResponse({
url: '/organizations/org-slug/events-stats/',
body: EventsStatsFixture(),
});

render(
<MetricAlertDetails
organization={organization}
{...routerProps}
params={{ruleId: ruleWithNoExtrapolation.id}}
/>,
{
organization,
}
);

expect(await screen.findByText(ruleWithNoExtrapolation.name)).toBeInTheDocument();

// Verify events-stats is called with 'none' extrapolation mode
expect(eventsStatsRequest).toHaveBeenCalledWith(
'/organizations/org-slug/events-stats/',
expect.objectContaining({
query: expect.objectContaining({
extrapolationMode: 'none',
sampling: SAMPLING_MODE.HIGH_ACCURACY,
}),
})
);
});
});
10 changes: 8 additions & 2 deletions static/app/views/alerts/rules/metric/details/metricChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ import useOrganization from 'sentry/utils/useOrganization';
import {COMPARISON_DELTA_OPTIONS} from 'sentry/views/alerts/rules/metric/constants';
import {makeDefaultCta} from 'sentry/views/alerts/rules/metric/metricRulePresets';
import type {MetricRule} from 'sentry/views/alerts/rules/metric/types';
import {AlertRuleTriggerType, Dataset} from 'sentry/views/alerts/rules/metric/types';
import {
AlertRuleTriggerType,
Dataset,
ExtrapolationMode,
} from 'sentry/views/alerts/rules/metric/types';
import {isCrashFreeAlert} from 'sentry/views/alerts/rules/metric/utils/isCrashFreeAlert';
import {
isEapAlertType,
Expand Down Expand Up @@ -475,7 +479,9 @@ export default function MetricChart({
referrer: 'api.alerts.alert-rule-chart',
samplingMode:
rule.dataset === Dataset.EVENTS_ANALYTICS_PLATFORM
? SAMPLING_MODE.NORMAL
? rule.extrapolationMode === ExtrapolationMode.NONE
? SAMPLING_MODE.HIGH_ACCURACY
: SAMPLING_MODE.NORMAL
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Inconsistent sampling mode logic between chart components

The sampling mode is being applied based only on dataset === Dataset.EVENTS_ANALYTICS_PLATFORM, but in TriggersChart the same logic checks both dataset === Dataset.EVENTS_ANALYTICS_PLATFORM AND traceItemType === TraceItemDataset.SPANS. This inconsistency means sampling mode is applied to non-span trace items in the details view but not in the form view, leading to different API calls for the same alert configuration across different pages.

Fix in Cursor Fix in Web

: undefined,
},
{enabled: !shouldUseSessionsStats}
Expand Down
74 changes: 73 additions & 1 deletion static/app/views/alerts/rules/metric/duplicate.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import GlobalModal from 'sentry/components/globalModal';

import MetricRuleDuplicate from './duplicate';
import type {Action} from './types';
import {AlertRuleTriggerType} from './types';
import {AlertRuleTriggerType, Dataset, EventTypes, ExtrapolationMode} from './types';

describe('MetricRuleDuplicate', () => {
beforeEach(() => {
Expand Down Expand Up @@ -55,6 +55,14 @@ describe('MetricRuleDuplicate', () => {
url: '/organizations/org-slug/members/',
body: [MemberFixture()],
});
MockApiClient.addMockResponse({
url: '/organizations/org-slug/recent-searches/',
body: [],
});
MockApiClient.addMockResponse({
url: '/organizations/org-slug/trace-items/attributes/',
body: [],
});
});

it('renders new alert form with values copied over', async () => {
Expand Down Expand Up @@ -153,4 +161,68 @@ describe('MetricRuleDuplicate', () => {
// Duplicated alert has been called
expect(req).toHaveBeenCalled();
});

it('duplicates rule with SERVER_WEIGHTED extrapolation mode but creates new rule without it', async () => {
const ruleWithExtrapolation = MetricRuleFixture({
id: '7',
name: 'Alert Rule with Extrapolation',
dataset: Dataset.EVENTS_ANALYTICS_PLATFORM,
aggregate: 'count()',
query: '',
eventTypes: [EventTypes.TRACE_ITEM_SPAN],
extrapolationMode: ExtrapolationMode.SERVER_WEIGHTED,
});

const {organization, project, routerProps} = initializeOrg({
organization: {
access: ['alerts:write'],
},
router: {
params: {},
location: {
query: {
createFromDuplicate: 'true',
duplicateRuleId: `${ruleWithExtrapolation.id}`,
},
},
},
});

const req = MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/alert-rules/${ruleWithExtrapolation.id}/`,
body: ruleWithExtrapolation,
});

const eventsStatsRequest = MockApiClient.addMockResponse({
url: '/organizations/org-slug/events-stats/',
body: null,
});

render(
<Fragment>
<GlobalModal />
<MetricRuleDuplicate project={project} userTeamIds={[]} {...routerProps} />
</Fragment>
);

// Wait for the form to load with duplicated values
expect(await screen.findByTestId('critical-threshold')).toHaveValue('70');
expect(screen.getByTestId('alert-name')).toHaveValue(
`${ruleWithExtrapolation.name} copy`
);

// Verify the original rule was fetched
expect(req).toHaveBeenCalled();

// Verify events-stats chart requests do NOT include extrapolation mode
// (duplicated rules are treated as new rules)
expect(eventsStatsRequest).toHaveBeenCalledWith(
'/organizations/org-slug/events-stats/',
expect.objectContaining({
query: expect.not.objectContaining({
extrapolationMode: expect.anything(),
}),
})
);
});
});
Loading
Loading