ui: Sum-by state optimization (#6102)

manojVivek · pre-commit-ci-lite[bot] · web-flow · commit 5e510668a4d0 · 2025-12-12T12:25:04.000+05:30
* SumBy state optimization to persist the default state and make use of it for quesries on load when available

* [pre-commit.ci lite] apply automatic fixes

* Linter fixes

* [pre-commit.ci lite] apply automatic fixes

* Test fixes

* Initial loading fixed

---------

Co-authored-by: pre-commit-ci-lite[bot] &lt;117423508+pre-commit-ci-lite[bot]@users.noreply.github.com&gt;
diff --git a/ui/packages/shared/profile/src/ProfileSelector/MetricsGraphSection.tsx b/ui/packages/shared/profile/src/ProfileSelector/MetricsGraphSection.tsx
@@ -29,7 +29,7 @@ interface MetricsGraphSectionProps {
   querySelection: QuerySelection;
   profileSelection: ProfileSelection | null;
   comparing: boolean;
-  sumBy: string[] | null;
+  sumBy: string[] | undefined;
   defaultSumByLoading: boolean;
   queryClient: QueryServiceClient;
   queryExpressionString: string;
@@ -170,7 +170,7 @@ export function MetricsGraphSection({
                   to={querySelection.to}
                   profile={profileSelection}
                   comparing={comparing}
-                  sumBy={querySelection.sumBy ?? sumBy ?? []}
+                  sumBy={sumBy ?? []}
                   sumByLoading={defaultSumByLoading}
                   setTimeRange={handleTimeRangeChange}
                   addLabelMatcher={addLabelMatcher}
diff --git a/ui/packages/shared/profile/src/ProfileSelector/index.tsx b/ui/packages/shared/profile/src/ProfileSelector/index.tsx
@@ -209,10 +209,6 @@ const ProfileSelector = ({
         const currentTo = timeRangeSelection.getToMs(true);
         const currentRangeKey = timeRangeSelection.getRangeKey();
         // Commit with refreshed time range
-        console.log(
-          '[draftExpression] setQueryExpression: committing with refreshed time range:',
-          draftSelection.expression
-        );
         commitDraft({
           from: currentFrom,
           to: currentTo,
@@ -332,7 +328,7 @@ const ProfileSelector = ({
         querySelection={querySelection}
         profileSelection={profileSelection}
         comparing={comparing}
-        sumBy={querySelection.sumBy ?? []}
+        sumBy={querySelection.sumBy}
         defaultSumByLoading={sumByLoading}
         queryClient={queryClient}
         queryExpressionString={queryExpressionString}
diff --git a/ui/packages/shared/profile/src/ProfileView/hooks/useResetStateOnProfileTypeChange.ts b/ui/packages/shared/profile/src/ProfileView/hooks/useResetStateOnProfileTypeChange.ts
@@ -18,6 +18,8 @@ import {useProfileFilters} from '../components/ProfileFilters/useProfileFilters'
 export const useResetStateOnProfileTypeChange = (): (() => void) => {
   const [groupBy, setGroupBy] = useURLState('group_by');
   const [curPath, setCurPath] = useURLState('cur_path');
+  const [sumByA, setSumByA] = useURLState('sum_by_a');
+  const [sumByB, setSumByB] = useURLState('sum_by_b');
   const {resetFilters} = useProfileFilters();
   const [sandwichFunctionName, setSandwichFunctionName] = useURLState('sandwich_function_name');
   const batchUpdates = useURLStateBatch();
@@ -34,6 +36,12 @@ export const useResetStateOnProfileTypeChange = (): (() => void) => {
       if (sandwichFunctionName !== undefined) {
         setSandwichFunctionName(undefined);
       }
+      if (sumByA !== undefined) {
+        setSumByA(undefined);
+      }
+      if (sumByB !== undefined) {
+        setSumByB(undefined);
+      }
 
       resetFilters();
     });
diff --git a/ui/packages/shared/profile/src/SimpleMatchers/Select.tsx b/ui/packages/shared/profile/src/SimpleMatchers/Select.tsx
@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import React, {useCallback, useEffect, useRef, useState} from 'react';
+import React, {Fragment, useCallback, useEffect, useRef, useState} from 'react';
 
 import {Icon} from '@iconify/react';
 import cx from 'classnames';
@@ -350,7 +350,7 @@ const CustomSelect: React.FC<CustomSelectProps & Record<string, any>> = ({
                 </div>
               ) : (
                 groupedFilteredItems.map(group => (
-                  <>
+                  <Fragment key={group.type}>
                     {groupedFilteredItems.length > 1 &&
                     groupedFilteredItems.every(g => g.type !== '') &&
                     group.type !== '' ? (
@@ -369,7 +369,7 @@ const CustomSelect: React.FC<CustomSelectProps & Record<string, any>> = ({
                         handleSelection={handleSelection}
                       />
                     ))}
-                  </>
+                  </Fragment>
                 ))
               )}
             </div>
diff --git a/ui/packages/shared/profile/src/hooks/useLabels.ts b/ui/packages/shared/profile/src/hooks/useLabels.ts
@@ -11,6 +11,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import {useEffect} from 'react';
+
 import {LabelsRequest, LabelsResponse, QueryServiceClient, ValuesRequest} from '@parca/client';
 import {useGrpcMetadata} from '@parca/components';
 import {millisToProtoTimestamp, sanitizeLabelValue} from '@parca/utilities';
@@ -68,7 +70,9 @@ export const useLabelNames = (
     },
   });
 
-  console.log('Label names query result:', {data, error, isLoading});
+  useEffect(() => {
+    console.log('Label names query result:', {data, error, isLoading});
+  }, [data, error, isLoading]);
 
   return {
     result: {response: data, error: error as Error},
diff --git a/ui/packages/shared/profile/src/hooks/useQueryState.test.tsx b/ui/packages/shared/profile/src/hooks/useQueryState.test.tsx
@@ -69,16 +69,33 @@ vi.mock('@parca/components/src/hooks/URLState/utils', async () => {
   };
 });
 
-// Mock useSumBy to return the sumBy from URL params or undefined
+// Mock useSumBy with stateful behavior using React's useState
 vi.mock('../useSumBy', async () => {
   const actual = await vi.importActual('../useSumBy');
+  const react = await import('react');
+
   return {
     ...actual,
-    useSumBy: (_queryClient: any, _profileType: any, _timeRange: any, defaultValue: any) => ({
-      sumBy: defaultValue,
-      setSumBy: vi.fn(),
-      isLoading: false,
-    }),
+    useSumBy: (
+      _queryClient: any,
+      _profileType: any,
+      _timeRange: any,
+      _draftProfileType: any,
+      _draftTimeRange: any,
+      defaultValue: any
+    ) => {
+      const [draftSumBy, setDraftSumBy] = react.useState<string[] | undefined>(defaultValue);
+      const [sumBy, setSumBy] = react.useState<string[] | undefined>(defaultValue);
+
+      return {
+        sumBy,
+        setSumBy,
+        isLoading: false,
+        draftSumBy,
+        setDraftSumBy,
+        isDraftSumByLoading: false,
+      };
+    },
   };
 });
 
@@ -207,7 +224,9 @@ describe('useQueryState', () => {
     it('should update sumBy', async () => {
       const {result} = renderHook(() => useQueryState(), {wrapper: createWrapper()});
 
+      // sumBy only applies to delta profiles, so we need to set one first
       act(() => {
+        result.current.setDraftExpression('process_cpu:cpu:nanoseconds:cpu:nanoseconds:delta{}');
         result.current.setDraftSumBy(['namespace', 'container']);
       });
 
@@ -256,15 +275,15 @@ describe('useQueryState', () => {
       const {result} = renderHook(() => useQueryState(), {wrapper: createWrapper()});
 
       act(() => {
-        // Update multiple draft values
-        result.current.setDraftExpression('memory:inuse_space:bytes:space:bytes{}');
+        // Update multiple draft values (using delta profile since sumBy only applies to delta)
+        result.current.setDraftExpression('memory:alloc_space:bytes:space:bytes:delta{}');
         result.current.setDraftTimeRange(7000, 8000, 'relative:minute|30');
         result.current.setDraftSumBy(['pod', 'node']);
       });
 
       // All drafts should be updated
       expect(result.current.draftSelection.expression).toBe(
-        'memory:inuse_space:bytes:space:bytes{}'
+        'memory:alloc_space:bytes:space:bytes:delta{}'
       );
       expect(result.current.draftSelection.from).toBe(7000);
       expect(result.current.draftSelection.to).toBe(8000);
@@ -278,7 +297,7 @@ describe('useQueryState', () => {
         // Should only navigate once for all updates
         expect(mockNavigateTo).toHaveBeenCalledTimes(1);
         const [, params] = mockNavigateTo.mock.calls[0];
-        expect(params.expression).toBe('memory:inuse_space:bytes:space:bytes{}');
+        expect(params.expression).toBe('memory:alloc_space:bytes:space:bytes:delta{}');
         expect(params.from).toBe('7000');
         expect(params.to).toBe('8000');
         expect(params.time_selection).toBe('relative:minute|30');
@@ -430,9 +449,9 @@ describe('useQueryState', () => {
     it('should handle _b suffix correctly', async () => {
       const {result} = renderHook(() => useQueryState({suffix: '_b'}), {wrapper: createWrapper()});
 
-      // Update draft state
+      // Update draft state (using delta profile since sumBy only applies to delta)
       act(() => {
-        result.current.setDraftExpression('memory:inuse_space:bytes:space:bytes{}');
+        result.current.setDraftExpression('memory:alloc_space:bytes:space:bytes:delta{}');
         result.current.setDraftTimeRange(3333, 4444, 'relative:hour|2');
         result.current.setDraftSumBy(['label_b']);
       });
@@ -445,7 +464,7 @@ describe('useQueryState', () => {
       await waitFor(() => {
         expect(mockNavigateTo).toHaveBeenCalled();
         const [, params] = mockNavigateTo.mock.calls[mockNavigateTo.mock.calls.length - 1];
-        expect(params.expression_b).toBe('memory:inuse_space:bytes:space:bytes{}');
+        expect(params.expression_b).toBe('memory:alloc_space:bytes:space:bytes:delta{}');
         expect(params.from_b).toBe('3333');
         expect(params.to_b).toBe('4444');
         expect(params.sum_by_b).toBe('label_b');
@@ -457,9 +476,9 @@ describe('useQueryState', () => {
     it('should not update URL until commit', async () => {
       const {result} = renderHook(() => useQueryState(), {wrapper: createWrapper()});
 
-      // Make multiple draft changes
+      // Make multiple draft changes (using delta profile since sumBy only applies to delta)
       act(() => {
-        result.current.setDraftExpression('memory:inuse_space:bytes:space:bytes{}');
+        result.current.setDraftExpression('memory:alloc_space:bytes:space:bytes:delta{}');
         result.current.setDraftTimeRange(5000, 6000, 'relative:hour|3');
         result.current.setDraftSumBy(['namespace', 'pod']);
       });
@@ -476,7 +495,7 @@ describe('useQueryState', () => {
       await waitFor(() => {
         expect(mockNavigateTo).toHaveBeenCalledTimes(1);
         const [, params] = mockNavigateTo.mock.calls[0];
-        expect(params.expression).toBe('memory:inuse_space:bytes:space:bytes{}');
+        expect(params.expression).toBe('memory:alloc_space:bytes:space:bytes:delta{}');
         expect(params.from).toBe('5000');
         expect(params.to).toBe('6000');
         expect(params.sum_by).toBe('namespace,pod');
@@ -737,17 +756,17 @@ describe('useQueryState', () => {
 
   describe('State persistence after page reload', () => {
     it('should retain committed values after page reload simulation', async () => {
-      // Initial state
+      // Initial state (using delta profile since sumBy only applies to delta)
       mockLocation.search =
-        '?expression=process_cpu:cpu:nanoseconds:cpu:nanoseconds{}&from=1000&to=2000';
+        '?expression=process_cpu:cpu:nanoseconds:cpu:nanoseconds:delta{}&from=1000&to=2000';
 
       const {result: result1, unmount} = renderHook(() => useQueryState(), {
         wrapper: createWrapper(),
       });
 
-      // User makes changes to draft
+      // User makes changes to draft (using delta profile since sumBy only applies to delta)
       act(() => {
-        result1.current.setDraftExpression('memory:inuse_space:bytes:space:bytes{}');
+        result1.current.setDraftExpression('memory:alloc_space:bytes:space:bytes:delta{}');
         result1.current.setDraftTimeRange(5000, 6000, 'relative:minute|15');
         result1.current.setDraftSumBy(['namespace', 'pod']);
       });
@@ -786,7 +805,7 @@ describe('useQueryState', () => {
 
       // Verify state is loaded from URL after "reload"
       expect(result2.current.querySelection.expression).toBe(
-        'memory:inuse_space:bytes:space:bytes{}'
+        'memory:alloc_space:bytes:space:bytes:delta{}'
       );
       expect(result2.current.querySelection.from).toBe(5000);
       expect(result2.current.querySelection.to).toBe(6000);
@@ -795,7 +814,7 @@ describe('useQueryState', () => {
 
       // Draft should be synced with URL state on page load
       expect(result2.current.draftSelection.expression).toBe(
-        'memory:inuse_space:bytes:space:bytes{}'
+        'memory:alloc_space:bytes:space:bytes:delta{}'
       );
       expect(result2.current.draftSelection.from).toBe(5000);
       expect(result2.current.draftSelection.to).toBe(6000);
diff --git a/ui/packages/shared/profile/src/hooks/useQueryState.ts b/ui/packages/shared/profile/src/hooks/useQueryState.ts
diff --git a/ui/packages/shared/profile/src/useSumBy.ts b/ui/packages/shared/profile/src/useSumBy.ts
