Skip to content

Commit

Permalink
ref(metrics): Convert more shared code for metric issue details to ho…
Browse files Browse the repository at this point in the history
…oks (#85385)

Requires #85377

Adds a util file to a new directory to pull out some shared logic for
the correlated section and the metrics graph. Will also make it a bit
easier to read through both files.

For the new `useMetricAlertId` hook, it mimics a pattern for cron/uptime
([example](https://github.com/getsentry/sentry/blob/cdd59af574fedd32005429827aefc06cdceab811/static/app/views/issueDetails/streamline/issueCronCheckTimeline.tsx#L36))
where by we fetch any event in 90d to have the best shot of finding a
`alert_rule_id` ctx key since they are identical across all events. That
way, even if a user picks small date range, or bad query, we don't
unload the graph when the event isn't found.

All this refactoring is to make it more reviewable as I take a look at
changing the graph design
  • Loading branch information
leeandher authored Feb 19, 2025
1 parent 4779f43 commit 4fb28df
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,13 @@ import {getReplayIdFromEvent} from 'sentry/utils/replays/getReplayIdFromEvent';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import {MetricIssuesSection} from 'sentry/views/issueDetails/metricIssues/metricIssuesSection';
import {SectionKey} from 'sentry/views/issueDetails/streamline/context';
import {EventDetails} from 'sentry/views/issueDetails/streamline/eventDetails';
import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection';
import {TraceDataSection} from 'sentry/views/issueDetails/traceDataSection';
import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';

import MetricIssuesSection from '../metricIssuesSection';

const LLMMonitoringSection = lazy(
() => import('sentry/components/events/interfaces/llm-monitoring/llmMonitoringSection')
);
Expand Down Expand Up @@ -230,7 +229,6 @@ export function EventDetailsContent({
<MetricIssuesSection
organization={organization}
group={group}
event={event}
project={project}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,36 @@
import {Fragment, lazy, useMemo} from 'react';
import {lazy} from 'react';
import {useTheme} from '@emotion/react';
import styled from '@emotion/styled';
import moment from 'moment-timezone';

import {DateTime} from 'sentry/components/dateTime';
import LazyLoad from 'sentry/components/lazyLoad';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Event} from 'sentry/types/event';
import type {Group} from 'sentry/types/group';
import type {Project} from 'sentry/types/project';
import useApi from 'sentry/utils/useApi';
import useOrganization from 'sentry/utils/useOrganization';
import type {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
import {
getFilter,
getPeriodInterval,
} from 'sentry/views/alerts/rules/metric/details/utils';
import {Dataset, TimePeriod} from 'sentry/views/alerts/rules/metric/types';
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
import {extractEventTypeFilterFromRule} from 'sentry/views/alerts/rules/metric/utils/getEventTypeFilter';
import {isCrashFreeAlert} from 'sentry/views/alerts/rules/metric/utils/isCrashFreeAlert';
import {useMetricRule} from 'sentry/views/alerts/rules/metric/utils/useMetricRule';
import {
useMetricIssueAlertId,
useMetricTimePeriod,
} from 'sentry/views/issueDetails/metricIssues/utils';

const MetricChart = lazy(
() => import('sentry/views/alerts/rules/metric/details/metricChart')
);

export function MetricIssueChart({
event,
group,
project,
}: {
event: Event | undefined;
group: Group;
project: Project;
}) {
export function MetricIssuesChart({group, project}: {group: Group; project: Project}) {
const theme = useTheme();
const api = useApi();
const organization = useOrganization();

const ruleId = event?.contexts?.metric_alert?.alert_rule_id;

const ruleId = useMetricIssueAlertId({groupId: group.id});
const {data: rule} = useMetricRule(
{
orgSlug: organization.slug,
Expand All @@ -55,33 +45,7 @@ export function MetricIssueChart({
enabled: !!ruleId,
}
);

const openPeriod = group.openPeriods?.[0];
const timePeriod = useMemo((): TimePeriodType | null => {
if (!openPeriod) {
return null;
}
const start = openPeriod.start;
let end = openPeriod.end;
if (!end) {
end = new Date().toISOString();
}
return {
start,
end,
period: TimePeriod.SEVEN_DAYS,
usingPeriod: false,
label: t('Custom time'),
display: (
<Fragment>
<DateTime date={moment.utc(start)} />
{' — '}
<DateTime date={moment.utc(end)} />
</Fragment>
),
custom: true,
};
}, [openPeriod]);
const timePeriod = useMetricTimePeriod({openPeriod: group.openPeriods?.[0]});

if (!rule || !timePeriod) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,39 +1,36 @@
import {Fragment, useMemo} from 'react';
import moment from 'moment-timezone';

import {LinkButton} from 'sentry/components/button';
import {DateTime} from 'sentry/components/dateTime';
import {t} from 'sentry/locale';
import type {Event} from 'sentry/types/event';
import type {Group} from 'sentry/types/group';
import type {Organization} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import {useLocation} from 'sentry/utils/useLocation';
import type {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
import RelatedIssues from 'sentry/views/alerts/rules/metric/details/relatedIssues';
import RelatedTransactions from 'sentry/views/alerts/rules/metric/details/relatedTransactions';
import {Dataset, TimePeriod} from 'sentry/views/alerts/rules/metric/types';
import {Dataset} from 'sentry/views/alerts/rules/metric/types';
import {extractEventTypeFilterFromRule} from 'sentry/views/alerts/rules/metric/utils/getEventTypeFilter';
import {isCrashFreeAlert} from 'sentry/views/alerts/rules/metric/utils/isCrashFreeAlert';
import {useMetricRule} from 'sentry/views/alerts/rules/metric/utils/useMetricRule';
import {
useMetricIssueAlertId,
useMetricTimePeriod,
} from 'sentry/views/issueDetails/metricIssues/utils';
import {SectionKey} from 'sentry/views/issueDetails/streamline/context';
import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection';

interface MetricIssuesSectionProps {
event: Event;
group: Group;
organization: Organization;
project: Project;
}

export default function MetricIssuesSection({
export function MetricIssuesSection({
organization,
event,
group,
project,
}: MetricIssuesSectionProps) {
const location = useLocation();
const ruleId = event.contexts?.metric_alert?.alert_rule_id;

const ruleId = useMetricIssueAlertId({groupId: group.id});
const {data: rule} = useMetricRule(
{
orgSlug: organization.slug,
Expand All @@ -48,33 +45,7 @@ export default function MetricIssuesSection({
enabled: !!ruleId,
}
);

const openPeriod = group.openPeriods?.[0];
const timePeriod = useMemo((): TimePeriodType | null => {
if (!openPeriod) {
return null;
}
const start = openPeriod.start;
let end = openPeriod.end;
if (!end) {
end = new Date().toISOString();
}
return {
start,
end,
period: TimePeriod.SEVEN_DAYS,
usingPeriod: false,
label: t('Custom time'),
display: (
<Fragment>
<DateTime date={moment.utc(start)} />
{' — '}
<DateTime date={moment.utc(end)} />
</Fragment>
),
custom: true,
};
}, [openPeriod]);
const timePeriod = useMetricTimePeriod({openPeriod: group.openPeriods?.[0]});

if (!rule || !timePeriod) {
return null;
Expand Down
77 changes: 77 additions & 0 deletions static/app/views/issueDetails/metricIssues/utils.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import {Fragment, useMemo} from 'react';
import moment from 'moment-timezone';

import {DateTime} from 'sentry/components/dateTime';
import {t} from 'sentry/locale';
import type {Event} from 'sentry/types/event';
import type {GroupOpenPeriod} from 'sentry/types/group';
import {useApiQuery} from 'sentry/utils/queryClient';
import useOrganization from 'sentry/utils/useOrganization';
import {useUser} from 'sentry/utils/useUser';
import type {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
import {TimePeriod} from 'sentry/views/alerts/rules/metric/types';
import {useIssueDetails} from 'sentry/views/issueDetails/streamline/context';
import {getGroupEventQueryKey} from 'sentry/views/issueDetails/utils';

export function useMetricIssueAlertId({groupId}: {groupId: string}): string | undefined {
/**
* This should be removed once the metric alert rule ID is set on the issue.
* This will fetch an event from the max range if the detector details
* are not available (e.g. time range has changed and page refreshed)
*/
const user = useUser();
const organization = useOrganization();
const {detectorDetails} = useIssueDetails();
const {detectorId, detectorType} = detectorDetails;

const hasMetricDetector = detectorId && detectorType === 'metric_alert';

const {data: event} = useApiQuery<Event>(
getGroupEventQueryKey({
orgSlug: organization.slug,
groupId,
eventId: user.options.defaultIssueEvent,
environments: [],
}),
{
staleTime: Infinity,
enabled: !hasMetricDetector,
retry: false,
}
);

// Fall back to the fetched event in case the provider doesn't have the detector details
return hasMetricDetector ? detectorId : event?.contexts?.metric_alert?.alert_rule_id;
}

export function useMetricTimePeriod({
openPeriod,
}: {
openPeriod?: GroupOpenPeriod;
}): TimePeriodType | null {
return useMemo((): TimePeriodType | null => {
if (!openPeriod) {
return null;
}
const start = openPeriod.start;
let end = openPeriod.end;
if (!end) {
end = new Date().toISOString();
}
return {
start,
end,
period: TimePeriod.SEVEN_DAYS,
usingPeriod: false,
label: t('Custom time'),
display: (
<Fragment>
<DateTime date={moment.utc(start)} />
{' — '}
<DateTime date={moment.utc(end)} />
</Fragment>
),
custom: true,
};
}, [openPeriod]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {getConfigForIssueType} from 'sentry/utils/issueTypeConfig';
import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import useOrganization from 'sentry/utils/useOrganization';
import {MetricIssuesChart} from 'sentry/views/issueDetails/metricIssues/metricIssueChart';
import {useIssueDetails} from 'sentry/views/issueDetails/streamline/context';
import {EventGraph} from 'sentry/views/issueDetails/streamline/eventGraph';
import {
Expand All @@ -25,7 +26,6 @@ import {
import {IssueCronCheckTimeline} from 'sentry/views/issueDetails/streamline/issueCronCheckTimeline';
import IssueTagsPreview from 'sentry/views/issueDetails/streamline/issueTagsPreview';
import {IssueUptimeCheckTimeline} from 'sentry/views/issueDetails/streamline/issueUptimeCheckTimeline';
import {MetricIssueChart} from 'sentry/views/issueDetails/streamline/metricIssueChart';
import {OccurrenceSummary} from 'sentry/views/issueDetails/streamline/occurrenceSummary';
import {getDetectorDetails} from 'sentry/views/issueDetails/streamline/sidebar/detectorSection';
import {ToggleSidebar} from 'sentry/views/issueDetails/streamline/sidebar/toggleSidebar';
Expand Down Expand Up @@ -121,7 +121,7 @@ export function EventDetailsHeader({group, event, project}: EventDetailsHeaderPr
<EventGraph event={event} group={group} style={{flex: 1}} />
)}
{issueTypeConfig.header.graph.type === 'detector-history' && (
<MetricIssueChart group={group} project={project} event={event} />
<MetricIssuesChart group={group} project={project} />
)}
{issueTypeConfig.header.graph.type === 'uptime-checks' && (
<IssueUptimeCheckTimeline group={group} />
Expand Down

0 comments on commit 4fb28df

Please sign in to comment.