Browse Source

feat(explore): Syncing components with tags loading state. (#78214)

The explore aggregate Chart and Table components need to wait for
groupBys to finish loading. Otherwise we might pass tags that are
invalid for that selected project to the queries.

Unwanted behaviour:
[link](https://sentry.sentry.io/traces/?field=project&field=id&field=span.op&field=span.description&field=span.duration&field=timestamp&groupBy=organization_id&mode=aggregate&project=6178942&query=%21organization_id%3A%22%22&statsPeriod=1h&utc=true)

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Abdullah Khan 5 months ago
parent
commit
ef2661a600

+ 2 - 2
static/app/components/performance/spanSearchQueryBuilder.tsx

@@ -76,11 +76,11 @@ export function SpanSearchQueryBuilder({
     return placeholder ?? t('Search for spans, users, tags, and more');
   }, [placeholder]);
 
-  const customTags = useSpanFieldCustomTags({
+  const {data: customTags} = useSpanFieldCustomTags({
     projects: projects ?? selection.projects,
   });
 
-  const supportedTags = useSpanFieldSupportedTags({
+  const {data: supportedTags} = useSpanFieldSupportedTags({
     projects: projects ?? selection.projects,
   });
 

+ 2 - 2
static/app/views/explore/charts/index.tsx

@@ -51,7 +51,7 @@ export function ExploreCharts({query}: ExploreChartsProps) {
   const [dataset] = useDataset();
   const [visualizes, setVisualizes] = useVisualizes();
   const [interval, setInterval, intervalOptions] = useChartInterval();
-  const [groupBys] = useGroupBys();
+  const {groupBys, isLoadingGroupBys} = useGroupBys();
   const [resultMode] = useResultMode();
   const topEvents = useTopEvents();
 
@@ -85,7 +85,7 @@ export function ExploreCharts({query}: ExploreChartsProps) {
       search: new MutableSearch(query ?? ''),
       yAxis: yAxes,
       interval: interval ?? getInterval(pageFilters.selection.datetime, 'metrics'),
-      enabled: true,
+      enabled: !isLoadingGroupBys,
       fields,
       orderby,
       topEvents,

+ 15 - 5
static/app/views/explore/contexts/spanTagsContext.tsx

@@ -2,17 +2,27 @@ import type React from 'react';
 import {createContext, useContext} from 'react';
 
 import type {TagCollection} from 'sentry/types/group';
-import {useSpanFieldSupportedTags} from 'sentry/views/performance/utils/useSpanFieldSupportedTags';
+import type {UseApiQueryResult} from 'sentry/utils/queryClient';
+import type RequestError from 'sentry/utils/requestError/requestError';
+import {
+  type SpanFieldsResponse,
+  useSpanFieldSupportedTags,
+} from 'sentry/views/performance/utils/useSpanFieldSupportedTags';
 
-const SpanTagsContext = createContext<TagCollection | undefined>(undefined);
+const SpanTagsContext = createContext<
+  | (Omit<UseApiQueryResult<SpanFieldsResponse, RequestError>, 'data'> & {
+      data: TagCollection;
+    })
+  | undefined
+>(undefined);
 
 export function SpanTagsProvider({children}: {children: React.ReactNode}) {
-  const tags = useSpanFieldSupportedTags();
+  const results = useSpanFieldSupportedTags();
 
-  return <SpanTagsContext.Provider value={tags}>{children}</SpanTagsContext.Provider>;
+  return <SpanTagsContext.Provider value={results}>{children}</SpanTagsContext.Provider>;
 }
 
-export const useSpanTags = (): TagCollection => {
+export const useSpanTags = () => {
   const context = useContext(SpanTagsContext);
 
   if (context === undefined) {

+ 4 - 2
static/app/views/explore/hooks/useGroupBys.spec.tsx

@@ -28,10 +28,10 @@ describe('useGroupBys', function () {
       ],
     });
 
-    let groupBys, setGroupBys;
+    let groupBys, setGroupBys, isLoadingGroupBys;
 
     function TestPage() {
-      [groupBys, setGroupBys] = useGroupBys();
+      ({groupBys, setGroupBys, isLoadingGroupBys} = useGroupBys());
       return null;
     }
 
@@ -54,8 +54,10 @@ describe('useGroupBys', function () {
       </SpanTagsProvider>
     );
 
+    expect(isLoadingGroupBys).toBe(true);
     await waitFor(() => expect(mockSpanTagsApiCall).toHaveBeenCalledTimes(1));
     expect(groupBys).toEqual(['']); // default
+    expect(isLoadingGroupBys).toBe(false);
 
     act(() => setGroupBys(['foo', 'bar']));
     expect(groupBys).toEqual(['foo', 'bar']);

+ 27 - 14
static/app/views/explore/hooks/useGroupBys.tsx

@@ -2,47 +2,56 @@ import {useCallback, useMemo} from 'react';
 import type {Location} from 'history';
 
 import type {TagCollection} from 'sentry/types/group';
+import type {UseApiQueryResult} from 'sentry/utils/queryClient';
 import {decodeList} from 'sentry/utils/queryString';
+import type RequestError from 'sentry/utils/requestError/requestError';
 import {useLocation} from 'sentry/utils/useLocation';
 import {useNavigate} from 'sentry/utils/useNavigate';
 import type {Field} from 'sentry/views/explore/hooks/useSampleFields';
+import type {SpanFieldsResponse} from 'sentry/views/performance/utils/useSpanFieldSupportedTags';
 
 import {useSpanTags} from '../contexts/spanTagsContext';
 
 interface Options {
   location: Location;
   navigate: ReturnType<typeof useNavigate>;
-  tags: TagCollection;
+  tagsResults: Omit<UseApiQueryResult<SpanFieldsResponse, RequestError>, 'data'> & {
+    data: TagCollection;
+  };
 }
 
-export function useGroupBys(): [Field[], (fields: Field[]) => void] {
+type GroupBysResult = {
+  groupBys: Field[];
+  isLoadingGroupBys: boolean;
+  setGroupBys: (fields: Field[]) => void;
+};
+
+export function useGroupBys(): GroupBysResult {
   const location = useLocation();
   const navigate = useNavigate();
-  const tags = useSpanTags();
-  const options = {location, navigate, tags};
+  const tagsResults = useSpanTags();
+  const options = {location, navigate, tagsResults};
 
   return useGroupBysImpl(options);
 }
 
-function useGroupBysImpl({
-  location,
-  navigate,
-  tags,
-}: Options): [Field[], (fields: Field[]) => void] {
+function useGroupBysImpl({location, navigate, tagsResults}: Options): GroupBysResult {
+  const {data: tags, isLoading: isLoadingTags} = tagsResults;
+
   const groupBys = useMemo(() => {
     const rawGroupBys = decodeList(location.query.groupBy);
 
     // Filter out groupBys that are not in span field supported tags
-    const validGroupBys = rawGroupBys.filter(
-      groupBy => groupBy === '' || tags.hasOwnProperty(groupBy)
-    );
+    const validGroupBys = isLoadingTags
+      ? []
+      : rawGroupBys.filter(groupBy => groupBy === '' || tags?.hasOwnProperty(groupBy));
 
     if (validGroupBys.length) {
       return validGroupBys;
     }
 
     return [''];
-  }, [location.query.groupBy, tags]);
+  }, [location.query.groupBy, tags, isLoadingTags]);
 
   const setGroupBys = useCallback(
     (newGroupBys: Field[]) => {
@@ -57,5 +66,9 @@ function useGroupBysImpl({
     [location, navigate]
   );
 
-  return [groupBys, setGroupBys];
+  return {
+    groupBys,
+    setGroupBys,
+    isLoadingGroupBys: isLoadingTags,
+  };
 }

+ 1 - 2
static/app/views/explore/hooks/useResultsMode.spec.tsx

@@ -27,7 +27,7 @@ describe('useResultMode', function () {
 
     function TestPage() {
       [sampleFields] = useSampleFields();
-      [, setGroupBys] = useGroupBys();
+      ({setGroupBys} = useGroupBys());
       [resultMode, setResultMode] = useResultMode();
       return null;
     }
@@ -76,7 +76,6 @@ describe('useResultMode', function () {
       'span.description',
       'span.duration',
       'timestamp',
-      'release',
     ]);
   });
 });

+ 1 - 1
static/app/views/explore/hooks/useResultsMode.tsx

@@ -27,7 +27,7 @@ function useResultModeImpl({
   navigate,
 }: Options): [ResultMode, (newMode: ResultMode) => void] {
   const [sampleFields] = useSampleFields();
-  const [groupBys] = useGroupBys();
+  const {groupBys} = useGroupBys();
 
   const resultMode: ResultMode = useMemo(() => {
     const rawMode = decodeScalar(location.query.mode);

+ 1 - 1
static/app/views/explore/hooks/useTopEvents.tsx

@@ -8,7 +8,7 @@ export const TOP_EVENTS_LIMIT: number = 5;
 
 export function useTopEvents(): number | undefined {
   const [visualizes] = useVisualizes();
-  const [groupBys] = useGroupBys();
+  const {groupBys} = useGroupBys();
   const [resultMode] = useResultMode();
 
   const hasChartWithMultipleYaxes = useMemo(() => {

+ 2 - 2
static/app/views/explore/tables/aggregatesTable.tsx

@@ -45,9 +45,8 @@ export function AggregatesTable({}: AggregatesTableProps) {
   const organization = useOrganization();
   const {selection} = usePageFilters();
   const topEvents = useTopEvents();
-
   const [dataset] = useDataset();
-  const [groupBys] = useGroupBys();
+  const {groupBys, isLoadingGroupBys} = useGroupBys();
   const [visualizes] = useVisualizes();
   const fields = useMemo(() => {
     return [...groupBys, ...visualizes.flatMap(visualize => visualize.yAxes)].filter(
@@ -75,6 +74,7 @@ export function AggregatesTable({}: AggregatesTableProps) {
     eventView,
     initialData: [],
     referrer: 'api.explore.spans-aggregates-table',
+    enabled: !isLoadingGroupBys,
   });
 
   const {tableStyles} = useTableStyles({

+ 1 - 1
static/app/views/explore/tables/index.tsx

@@ -42,7 +42,7 @@ function ExploreAggregatesTable() {
 function ExploreSamplesTable() {
   const [tab, setTab] = useState(Tab.SPAN);
   const [fields, setFields] = useSampleFields();
-  const tags = useSpanTags();
+  const {data: tags} = useSpanTags();
 
   const openColumnEditor = useCallback(() => {
     openModal(

Some files were not shown because too many files changed in this diff