From 19d567a6b6916cf37f5d9df27d6aaf33830c836d Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Fri, 18 Apr 2025 16:09:13 -0700 Subject: [PATCH 1/2] ref(distributions): Refactor fetching flags and their suspect scores --- .../groupFeatureFlags/flagDrawerContent.tsx | 132 +++++------------- .../useGroupFlagDrawerData.tsx | 126 +++++++++++++++++ .../useGroupSuspectFlagScores.tsx | 2 +- 3 files changed, 164 insertions(+), 96 deletions(-) create mode 100644 static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx diff --git a/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx b/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx index 7c5bf93486ce59..e931ce2e46b726 100644 --- a/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx +++ b/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx @@ -1,7 +1,7 @@ -import {useEffect, useMemo} from 'react'; +import {Fragment, useEffect} from 'react'; import {Flex} from 'sentry/components/container/flex'; -import {OrderBy, SortBy} from 'sentry/components/events/featureFlags/utils'; +import type {OrderBy, SortBy} from 'sentry/components/events/featureFlags/utils'; import LoadingError from 'sentry/components/loadingError'; import LoadingIndicator from 'sentry/components/loadingIndicator'; import {featureFlagOnboardingPlatforms} from 'sentry/data/platformCategories'; @@ -12,11 +12,7 @@ import useOrganization from 'sentry/utils/useOrganization'; import useProjects from 'sentry/utils/useProjects'; import FlagDetailsLink from 'sentry/views/issueDetails/groupFeatureFlags/flagDetailsLink'; import FlagDrawerCTA from 'sentry/views/issueDetails/groupFeatureFlags/flagDrawerCTA'; -import useGroupFeatureFlags from 'sentry/views/issueDetails/groupFeatureFlags/useGroupFeatureFlags'; -import { - type SuspectFlagScore, - useGroupSuspectFlagScores, -} from 'sentry/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores'; +import useFlagDrawerData from 'sentry/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData'; import {TagDistribution} from 'sentry/views/issueDetails/groupTags/tagDistribution'; import { Container, @@ -33,87 +29,32 @@ interface Props { } export default function FlagDrawerContent({ + debugSuspectScores, environments, group, orderBy, search, sortBy, - debugSuspectScores, }: Props) { const organization = useOrganization(); // If we're showing the suspect section at all - const enableSuspectFlags = organization.features.includes('feature-flag-suspect-flags'); - - const { - data = [], - isPending, - isError, - refetch, - } = useGroupFeatureFlags({ - groupId: group.id, - environment: environments, - }); - - // Flatten all the tag values together into a big string. - // Maybe for perf later, here we iterate over all tags&values once, (N*M) then - // later only iterate through each tag (N) as the search term changes. - const tagValues = useMemo( - () => - data.reduce>((valueMap, tag) => { - valueMap[tag.key] = tag.topValues - .map(tv => tv.value) - .join(' ') - .toLowerCase(); - return valueMap; - }, {}), - [data] - ); - - const filteredFlags = useMemo(() => { - const searchLower = search.toLowerCase(); - return data.filter(flag => { - return ( - flag.name.includes(searchLower) || - flag.key.includes(searchLower) || - tagValues[flag.key]?.includes(searchLower) - ); - }); - }, [data, search, tagValues]); + // const enableSuspectFlags = organization.features.includes('feature-flag-suspect-flags'); - const {data: suspectScores} = useGroupSuspectFlagScores({ - groupId: group.id, - environment: environments.length ? environments : undefined, - enabled: enableSuspectFlags || debugSuspectScores, + const {displayFlags, allFlagCount, isPending, isError, refetch} = useFlagDrawerData({ + environments, + group, + orderBy, + search, + sortBy, }); - const suspectScoresMap = useMemo( - () => - suspectScores - ? Object.fromEntries(suspectScores.data.map(score => [score.flag, score])) - : {}, - [suspectScores] - ); - - const sortedFlags = useMemo(() => { - if (sortBy === SortBy.ALPHABETICAL) { - const sorted = filteredFlags.toSorted((a, b) => a.key.localeCompare(b.key)); - return orderBy === OrderBy.A_TO_Z ? sorted : sorted.reverse(); - } - if (sortBy === SortBy.SUSPICION) { - return filteredFlags.toSorted( - (a, b) => - (suspectScoresMap[b.key]?.score ?? 0) - (suspectScoresMap[a.key]?.score ?? 0) - ); - } - return filteredFlags; - }, [filteredFlags, orderBy, sortBy, suspectScoresMap]); // CTA logic const {projects} = useProjects(); const project = projects.find(p => p.slug === group.project.slug)!; const showCTA = - data.length === 0 && + allFlagCount === 0 && project && !project.hasFlags && featureFlagOnboardingPlatforms.includes(project.platform ?? 'other'); @@ -122,10 +63,10 @@ export default function FlagDrawerContent({ if (!isPending && !isError && !showCTA) { trackAnalytics('flags.drawer_rendered', { organization, - numFlags: data.length, + numFlags: allFlagCount, }); } - }, [organization, data.length, isPending, isError, showCTA]); + }, [organization, allFlagCount, isPending, isError, showCTA]); return isPending ? ( @@ -136,42 +77,43 @@ export default function FlagDrawerContent({ /> ) : showCTA ? ( - ) : data.length === 0 ? ( + ) : allFlagCount === 0 ? ( {t('No feature flags were found for this issue')} - ) : sortedFlags.length === 0 ? ( + ) : displayFlags.length === 0 ? ( {t('No feature flags were found for this search')} ) : ( - - {sortedFlags.map(tag => ( -
- - - - {debugSuspectScores && ( - - )} -
- ))} - + + + {displayFlags.map(flag => ( +
+ + + + {debugSuspectScores && } +
+ ))} + + ); } -function DebugSuspectScore({scoreObj}: {scoreObj: undefined | SuspectFlagScore}) { - if (!scoreObj) { - return null; - } +function DebugSuspectScore({ + baselinePercent, + score, +}: { + baselinePercent: undefined | number; + score: undefined | number; +}) { return ( - Sus: {scoreObj.score.toFixed(5) ?? '_'} + Sus: {score?.toFixed(5) ?? '_'} Baseline:{' '} - {scoreObj.baseline_percent === undefined - ? '_' - : `${(scoreObj.baseline_percent * 100).toFixed(5)}%`} + {baselinePercent === undefined ? '_' : `${(baselinePercent * 100).toFixed(5)}%`} ); diff --git a/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx b/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx new file mode 100644 index 00000000000000..e54e58bc0c3611 --- /dev/null +++ b/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx @@ -0,0 +1,126 @@ +import {useMemo} from 'react'; + +import {OrderBy, SortBy} from 'sentry/components/events/featureFlags/utils'; +import type {Group} from 'sentry/types/group'; +import useGroupFeatureFlags from 'sentry/views/issueDetails/groupFeatureFlags/useGroupFeatureFlags'; +import {useGroupSuspectFlagScores} from 'sentry/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores'; +import type {GroupTag} from 'sentry/views/issueDetails/groupTags/useGroupTags'; + +interface SuspectGroupTag extends GroupTag { + suspect: { + baselinePercent: undefined | number; + score: undefined | number; + }; +} + +interface Props { + environments: string[]; + group: Group; + orderBy: OrderBy; + search: string; + sortBy: SortBy; +} + +interface Response { + allFlagCount: number; + displayFlags: SuspectGroupTag[]; + isError: boolean; + isPending: boolean; + refetch: () => void; +} + +export default function useFlagDrawerData({ + environments, + group, + orderBy, + search, + sortBy, +}: Props): Response { + const isSuspectEnabled = sortBy === SortBy.SUSPICION; + + // Fetch the base flag data + const { + data: groupFlags = [], + isError: isFlagsError, + isPending: isFlagsPending, + refetch: refetchFlags, + } = useGroupFeatureFlags({ + groupId: group.id, + environment: environments, + }); + + // Fetch the suspect data, if we need it for this render + const { + data: suspectScores, + isError: isSuspectError, + isPending: isSuspectPending, + } = useGroupSuspectFlagScores({ + groupId: group.id, + environment: environments.length ? environments : undefined, + enabled: isSuspectEnabled, + }); + + // Combine the flag and suspect data into SuspectGroupTag objects + const suspectFlags = useMemo(() => { + const suspectScoresMap = suspectScores + ? Object.fromEntries(suspectScores.data.map(score => [score.flag, score])) + : {}; + + return groupFlags.map(flag => ({ + ...flag, + suspect: { + baselinePercent: suspectScoresMap[flag.key]?.baseline_percent, + score: suspectScoresMap[flag.key]?.score, + }, + })); + }, [groupFlags, suspectScores]); + + // Flatten all the tag values together into a big string. + // A perf improvement: here we iterate over all tags&values once, (N*M) then + // later only iterate through each tag (N) as the search term changes. + const tagValues = useMemo( + () => + groupFlags.reduce>((valueMap, flag) => { + valueMap[flag.key] = flag.topValues + .map(tv => tv.value) + .join(' ') + .toLowerCase(); + return valueMap; + }, {}), + [groupFlags] + ); + + const filteredFlags = useMemo(() => { + const searchLower = search.toLowerCase(); + return suspectFlags.filter(flag => { + return ( + flag.name.includes(searchLower) || + flag.key.includes(searchLower) || + tagValues[flag.key]?.includes(searchLower) + ); + }); + }, [suspectFlags, search, tagValues]); + + const displayFlags = useMemo(() => { + if (sortBy === SortBy.ALPHABETICAL) { + const sorted = filteredFlags.toSorted((a, b) => a.key.localeCompare(b.key)); + return orderBy === OrderBy.A_TO_Z ? sorted : sorted.reverse(); + } + if (sortBy === SortBy.SUSPICION) { + return filteredFlags.toSorted( + (a, b) => (b.suspect.score ?? 0) - (a.suspect.score ?? 0) + ); + } + return filteredFlags; + }, [filteredFlags, orderBy, sortBy]); + + return { + allFlagCount: suspectFlags.length, + displayFlags, + isError: isSuspectEnabled ? isFlagsError || isSuspectError : isFlagsError, + isPending: isSuspectEnabled ? isFlagsPending || isSuspectPending : isFlagsPending, + refetch: () => { + refetchFlags(); + }, + }; +} diff --git a/static/app/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores.tsx b/static/app/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores.tsx index f9cb681930eb5d..ee7b86b5462ae8 100644 --- a/static/app/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores.tsx +++ b/static/app/views/issueDetails/groupFeatureFlags/useGroupSuspectFlagScores.tsx @@ -2,7 +2,7 @@ import {useApiQuery} from 'sentry/utils/queryClient'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; -export type SuspectFlagScore = { +type SuspectFlagScore = { baseline_percent: number; flag: string; score: number; From b0c78c1886f1b3a4fde51728b10ba025695d3caf Mon Sep 17 00:00:00 2001 From: Ryan Albrecht Date: Mon, 21 Apr 2025 08:11:42 -0700 Subject: [PATCH 2/2] address feedback --- .../groupFeatureFlags/flagDrawerContent.tsx | 28 +++++++++---------- .../useGroupFlagDrawerData.tsx | 14 ++++++---- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx b/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx index e931ce2e46b726..717e062a18fa9f 100644 --- a/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx +++ b/static/app/views/issueDetails/groupFeatureFlags/flagDrawerContent.tsx @@ -12,7 +12,7 @@ import useOrganization from 'sentry/utils/useOrganization'; import useProjects from 'sentry/utils/useProjects'; import FlagDetailsLink from 'sentry/views/issueDetails/groupFeatureFlags/flagDetailsLink'; import FlagDrawerCTA from 'sentry/views/issueDetails/groupFeatureFlags/flagDrawerCTA'; -import useFlagDrawerData from 'sentry/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData'; +import useGroupFlagDrawerData from 'sentry/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData'; import {TagDistribution} from 'sentry/views/issueDetails/groupTags/tagDistribution'; import { Container, @@ -38,23 +38,21 @@ export default function FlagDrawerContent({ }: Props) { const organization = useOrganization(); - // If we're showing the suspect section at all - // const enableSuspectFlags = organization.features.includes('feature-flag-suspect-flags'); - - const {displayFlags, allFlagCount, isPending, isError, refetch} = useFlagDrawerData({ - environments, - group, - orderBy, - search, - sortBy, - }); + const {displayFlags, allGroupFlagCount, isPending, isError, refetch} = + useGroupFlagDrawerData({ + environments, + group, + orderBy, + search, + sortBy, + }); // CTA logic const {projects} = useProjects(); const project = projects.find(p => p.slug === group.project.slug)!; const showCTA = - allFlagCount === 0 && + allGroupFlagCount === 0 && project && !project.hasFlags && featureFlagOnboardingPlatforms.includes(project.platform ?? 'other'); @@ -63,10 +61,10 @@ export default function FlagDrawerContent({ if (!isPending && !isError && !showCTA) { trackAnalytics('flags.drawer_rendered', { organization, - numFlags: allFlagCount, + numFlags: allGroupFlagCount, }); } - }, [organization, allFlagCount, isPending, isError, showCTA]); + }, [organization, allGroupFlagCount, isPending, isError, showCTA]); return isPending ? ( @@ -77,7 +75,7 @@ export default function FlagDrawerContent({ /> ) : showCTA ? ( - ) : allFlagCount === 0 ? ( + ) : allGroupFlagCount === 0 ? ( {t('No feature flags were found for this issue')} diff --git a/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx b/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx index e54e58bc0c3611..c9ec5537ac0bdc 100644 --- a/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx +++ b/static/app/views/issueDetails/groupFeatureFlags/useGroupFlagDrawerData.tsx @@ -22,14 +22,14 @@ interface Props { } interface Response { - allFlagCount: number; + allGroupFlagCount: number; displayFlags: SuspectGroupTag[]; isError: boolean; isPending: boolean; refetch: () => void; } -export default function useFlagDrawerData({ +export default function useGroupFlagDrawerData({ environments, group, orderBy, @@ -54,6 +54,7 @@ export default function useFlagDrawerData({ data: suspectScores, isError: isSuspectError, isPending: isSuspectPending, + refetch: refetchScores, } = useGroupSuspectFlagScores({ groupId: group.id, environment: environments.length ? environments : undefined, @@ -61,7 +62,7 @@ export default function useFlagDrawerData({ }); // Combine the flag and suspect data into SuspectGroupTag objects - const suspectFlags = useMemo(() => { + const allFlagsWithScores = useMemo(() => { const suspectScoresMap = suspectScores ? Object.fromEntries(suspectScores.data.map(score => [score.flag, score])) : {}; @@ -92,14 +93,14 @@ export default function useFlagDrawerData({ const filteredFlags = useMemo(() => { const searchLower = search.toLowerCase(); - return suspectFlags.filter(flag => { + return allFlagsWithScores.filter(flag => { return ( flag.name.includes(searchLower) || flag.key.includes(searchLower) || tagValues[flag.key]?.includes(searchLower) ); }); - }, [suspectFlags, search, tagValues]); + }, [allFlagsWithScores, search, tagValues]); const displayFlags = useMemo(() => { if (sortBy === SortBy.ALPHABETICAL) { @@ -115,12 +116,13 @@ export default function useFlagDrawerData({ }, [filteredFlags, orderBy, sortBy]); return { - allFlagCount: suspectFlags.length, + allGroupFlagCount: allFlagsWithScores.length, displayFlags, isError: isSuspectEnabled ? isFlagsError || isSuspectError : isFlagsError, isPending: isSuspectEnabled ? isFlagsPending || isSuspectPending : isFlagsPending, refetch: () => { refetchFlags(); + refetchScores(); }, }; }