From 02de9c8b852f885faf68a66f00d58c99bbbc64f9 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Fri, 30 May 2025 11:13:32 -0400 Subject: [PATCH] feat(explore): Remove group by timestamp from explore Grouping by timestamp produces a high cardinality result that is not very useful so disallow it in the UI. --- .../views/explore/hooks/useGroupByFields.tsx | 31 +++++++++++++------ .../queryConstructors/groupBy.tsx | 21 ++++--------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/static/app/views/explore/hooks/useGroupByFields.tsx b/static/app/views/explore/hooks/useGroupByFields.tsx index e7fb2d2b5730c3..b77f609027de9b 100644 --- a/static/app/views/explore/hooks/useGroupByFields.tsx +++ b/static/app/views/explore/hooks/useGroupByFields.tsx @@ -16,14 +16,17 @@ interface UseGroupByFieldsProps { */ groupBys: string[]; tags: TagCollection; - prefix?: string; + hideEmptyOption?: boolean; } -export function useGroupByFields({tags, groupBys}: UseGroupByFieldsProps) { +export function useGroupByFields({ + tags, + groupBys, + hideEmptyOption, +}: UseGroupByFieldsProps) { const options: Array> = useMemo(() => { const potentialOptions = [ - // We do not support grouping by span id, we have a dedicated sample mode for that - ...Object.keys(tags).filter(key => key !== 'id'), + ...Object.keys(tags).filter(key => !DISALLOWED_GROUP_BY_FIELDS.has(key)), // These options aren't known to exist on this project but it was inserted into // the group bys somehow so it should be a valid options in the group bys. @@ -36,11 +39,15 @@ export function useGroupByFields({tags, groupBys}: UseGroupByFieldsProps) { return [ // hard code in an empty option - { - label: {t('\u2014')}, - value: UNGROUPED, - textValue: t('\u2014'), - }, + ...(hideEmptyOption + ? [] + : [ + { + label: {t('\u2014')}, + value: UNGROUPED, + textValue: t('\u2014'), + }, + ]), ...potentialOptions.map(key => { const kind = FieldKind.TAG; return { @@ -53,11 +60,15 @@ export function useGroupByFields({tags, groupBys}: UseGroupByFieldsProps) { }; }), ]; - }, [tags, groupBys]); + }, [tags, groupBys, hideEmptyOption]); return options; } +// Some fields don't make sense to allow users to group by as they create +// very high cardinality groupings and is not useful. +const DISALLOWED_GROUP_BY_FIELDS = new Set(['id', 'timestamp']); + const Disabled = styled('span')` color: ${p => p.theme.subText}; `; diff --git a/static/app/views/explore/multiQueryMode/queryConstructors/groupBy.tsx b/static/app/views/explore/multiQueryMode/queryConstructors/groupBy.tsx index 0743d172d25914..c08a45e41ec398 100644 --- a/static/app/views/explore/multiQueryMode/queryConstructors/groupBy.tsx +++ b/static/app/views/explore/multiQueryMode/queryConstructors/groupBy.tsx @@ -1,10 +1,10 @@ -import {useMemo} from 'react'; import styled from '@emotion/styled'; import {CompactSelect, type SelectOption} from 'sentry/components/core/compactSelect'; import {Tooltip} from 'sentry/components/core/tooltip'; import {t} from 'sentry/locale'; import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext'; +import {useGroupByFields} from 'sentry/views/explore/hooks/useGroupByFields'; import { type ReadableExploreQueryParts, useUpdateQueryAtIndex, @@ -22,20 +22,11 @@ export function GroupBySection({query, index}: Props) { const updateGroupBys = useUpdateQueryAtIndex(index); - const enabledOptions: Array> = useMemo(() => { - const potentialOptions = Object.keys(tags).filter(key => key !== 'id'); - potentialOptions.sort((a, b) => { - if (query.groupBys.includes(a) === query.groupBys.includes(b)) { - return a.localeCompare(b); - } - if (query.groupBys.includes(a)) { - return -1; - } - return 1; - }); - - return potentialOptions.map(key => ({label: key, value: key, textValue: key})); - }, [tags, query.groupBys]); + const enabledOptions: Array> = useGroupByFields({ + groupBys: [], + tags, + hideEmptyOption: true, + }); return (