feat: improve tenant diagnostics - CPU #2633

Raubzeug · 2025-07-29T13:54:07Z

CI Results

Test Status: ⚠️ FLAKY

Total	Passed	Failed	Flaky	Skipped
354	350	0	2	2

Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 85.35 MB | Main: 85.35 MB
Diff: +3.95 KB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information

Test recordings for failed tests are available in the full report.
Bundle size is measured for the entire 'dist' directory.
📊 indicates links to detailed reports.
🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

artemmufazalov

Sorting in table is not descending

artemmufazalov · 2025-07-29T14:15:15Z

src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TopQueries.tsx

-    const columns = React.useMemo(() => {
-        return getTenantOverviewTopQueriesColumns();
-    }, []);
-
    const {currentData, isFetching, error} = topQueriesApi.useGetTopQueriesQuery(
        {database: tenantName, timeFrame: 'hour', limit: TENANT_OVERVIEW_TABLES_LIMIT},


sortOrder is needed here

fixed, stand redeployed

astandrik · 2025-07-30T11:57:57Z

bugbot run

Copilot

Pull Request Overview

This PR improves the tenant diagnostics CPU section by simplifying the UI structure and enhancing the user experience. The main changes include replacing a complex tabbed interface with a unified dashboard layout and improving scroll position management for better navigation.

Key changes:

Replaced CPU tabs (nodes/shards/queries) with a unified vertical layout using StatsWrapper components
Extracted reusable QueriesTableWithDrawer component for better code reuse
Added scroll position persistence for the diagnostics overview tab

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/utils/hooks/useScrollPosition.ts	New hook for managing scroll position persistence using sessionStorage
src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TenantCpu.tsx	Complete refactor from tabbed to unified layout with StatsWrapper components
src/containers/Tenant/Diagnostics/TopQueries/QueriesTableWithDrawer.tsx	New reusable component combining table and drawer functionality
src/containers/Tenant/Diagnostics/TenantOverview/StatsWrapper/StatsWrapper.tsx	New wrapper component for consistent section layouts
src/components/SeeAllButton/SeeAllButton.tsx	New reusable button component for navigation to detailed views
src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx	Refactored to use parameterized query text columns
src/containers/Tenant/Diagnostics/Diagnostics.tsx	Added scroll position hook integration

Copilot · 2025-07-30T11:58:28Z

src/utils/hooks/useScrollPosition.ts

+import {debounce} from 'lodash';
+import {useHistory} from 'react-router-dom';
+
+const DEBAUNCE_DELAY = 100;


The constant name 'DEBAUNCE_DELAY' contains a typo. It should be 'DEBOUNCE_DELAY'.

Suggested change

const DEBAUNCE_DELAY = 100;

const DEBOUNCE_DELAY = 100;

Copilot · 2025-07-30T11:58:29Z

src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx

 const queryHashColumn: Column<KeyValueRow> = {
    name: QUERIES_COLUMNS_IDS.QueryHash,
    header: QUERIES_COLUMNS_TITLES.QueryHash,
-    render: ({row}) => generateHash(String(row.QueryText)),
+    render: ({row}) => <div className={b('query-hash')}>{generateHash(String(row.QueryText))}</div>,


[nitpick] Adding a wrapper div with CSS class for the query hash changes the column behavior. Consider if this styling change is necessary or if it should be handled differently to maintain consistency with other hash displays.

Suggested change

render: ({row}) => <div className={b('query-hash')}>{generateHash(String(row.QueryText))}</div>,

render: ({row}) => generateHash(String(row.QueryText)),

cursor

✅ Bugbot reviewed your changes and found no bugs!

astandrik · 2025-07-30T12:45:11Z

src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx

 }
 export function getRunningQueriesColumns() {
    const columns = [
        queryHashColumn,
        userSIDColumn,
        queryStartColumn,
-        queryTextColumn,
+        getQueryTextColumn(),


maybe create getRunningQueryTextColumn and getTopQueryTextColumn

I dont see that we use functions for columns - and current solution looks inconsistent

@claude what do you think of it?

Claude finished @astandrik's task —— View job

@astandrik, I've analyzed the column patterns across the codebase, and I agree with your suggestion. Here's my analysis:

Current Codebase Patterns

After reviewing the column definitions across the codebase, I found that the established pattern is to use individual get*Column functions rather than parameterized functions. Examples:

nodesColumns (src/components/nodesColumns/columns.tsx:37-94):

getNodeIdColumn()

getHostColumn()

getNetworkHostColumn() (different variant, not parameterized)

getDataCenterColumn()

Operations (src/containers/Operations/columns.tsx:21-29):

Uses a single getColumns() function that builds all columns internally based on the kind parameter

Doesn't use parameterized individual column functions

TopicData (src/containers/Tenant/Diagnostics/TopicData/columns/columns.tsx):

Exports individual column constants: offsetColumn, timestampCreateColumn, etc.

No parameterized column functions

SchemaViewer (src/containers/Tenant/Schema/SchemaViewer/columns.tsx):

Also uses individual column constants: idColumn, nameColumn, etc.

Recommendation

The current PR approach of having separate functions would be more consistent:

getRunningQueryTextColumn()

getTopQueryTextColumn()

This follows the established pattern in the codebase where different column variants get their own functions rather than using parameters. The getNetworkHostColumn() vs getHostColumn() example from nodesColumns is a good precedent for this approach.

The parameterized approach would be a departure from the existing patterns and might make the codebase less consistent overall.

Todo:

Read the current columns.tsx implementation

Search for other column patterns in the codebase

Provide feedback on the consistency question

Raubzeug · 2025-07-30T14:51:20Z

Stand redeployed

src/utils/hooks/useScrollPosition.ts

src/containers/Tenant/Diagnostics/TopQueries/RunningQueriesData.tsx

claude · 2025-07-31T13:09:40Z

Claude finished @Raubzeug's task —— View job

Code Review Summary

I've completed my review of this PR that refactors the tenant diagnostics CPU section. Here are my findings:

✅ Strengths

Good architectural refactoring: The move from tabbed interface to unified vertical layout with StatsWrapper components improves code reusability and UX consistency.
Proper component extraction: The new QueriesTableWithDrawer component effectively consolidates table and drawer functionality, reducing code duplication.
Correct API patterns: All API calls properly use window.api through RTK Query's injectEndpoints pattern (src/store/reducers/executeTopQueries/executeTopQueries.ts:127).
Good i18n implementation: New components properly use i18n for user-facing text (e.g., SeeAllButton component).
Performance considerations: Components use React optimization hooks appropriately (useCallback, useMemo).

🔍 Issues to Address

Typo in constant name (src/utils/hooks/useScrollPosition.ts:6):
```
const DEBAUNCE_DELAY = 100; // Should be DEBOUNCE_DELAY
```

Hardcoded strings in column renderers (src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx:84,123):

{row.UserSID || '–'}  // Should use i18n for the dash
{row.ApplicationName || '–'}  // Should use i18n for the dash

Console warnings not internationalized (src/utils/hooks/useScrollPosition.ts:27,39,59):
- Console warnings are developer-facing and don't need i18n, but consider if these should be in production code at all.
Column function inconsistency: As noted by @astandrik, the parameterized column functions could be split into getRunningQueryTextColumn and getTopQueryTextColumn for better consistency with the codebase patterns.
Scroll position hook complexity: The useScrollPosition hook is quite complex for its limited use case (back/forward navigation only). Consider if anchors would be a simpler solution as suggested by @astandrik.

📋 Recommendations

Fix the typo in DEBAUNCE_DELAY → DEBOUNCE_DELAY.
Consider extracting the empty value dash ('–') to a constant or i18n entry.
Evaluate if the scroll position hook complexity is justified for the use case.
Consider splitting the column functions as suggested for consistency.

✅ All Critical Requirements Met

✅ TypeScript type safety with proper interfaces
✅ React best practices and Gravity UI components
✅ Redux Toolkit patterns with RTK Query
✅ BEM naming convention with cn() utility
✅ API calls through window.api
✅ No state mutations in RTK Query
✅ Proper loading states and error handling

The PR significantly improves the code structure and user experience. The issues found are minor and don't block the functionality.

astandrik · 2025-07-31T13:19:09Z

@claude actually some you addressed are already fixed

claude · 2025-07-31T13:42:34Z

Claude finished @astandrik's task —— View job

Thanks for the clarification @astandrik! I've checked the current state of the code and you're right - some issues have been fixed.

✅ Issues Already Fixed

DEBOUNCE_DELAY typo - Fixed! The constant is now correctly spelled in src/utils/hooks/useScrollPosition.ts:6
CPU Cores column alignment - Fixed! Changed to DataTable.LEFT in src/components/ShardsTable/columns.tsx

🔍 Remaining Issues

Hardcoded '–' strings (src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx:84,123):
```
{row.UserSID || '–'}
{row.ApplicationName || '–'}
```
These could use the existing EMPTY_DATA_PLACEHOLDER constant or be added to i18n. However, this is a minor issue that doesn't block functionality.
Console warnings (src/utils/hooks/useScrollPosition.ts:27,39,59):
- These are developer-facing console warnings and don't need i18n
- Consider if these warnings should be in production code at all

📊 Overall Assessment

The PR successfully improves the tenant diagnostics CPU section with:

Better architectural patterns (StatsWrapper, QueriesTableWithDrawer)
Proper i18n for new components
Cleaner code structure without the complex tabbed interface
Good adherence to the codebase conventions

The remaining issues are minor and don't affect the functionality. Great work on the refactoring!

Raubzeug requested review from artemmufazalov and astandrik as code owners July 29, 2025 13:54

Raubzeug linked an issue Jul 29, 2025 that may be closed by this pull request

Improve CPU section in Tenant - Diagnostics - Info #2632

Closed

Raubzeug force-pushed the cpu-redesign branch from 38131db to 61acb4f Compare July 29, 2025 13:55

artemmufazalov approved these changes Jul 29, 2025

View reviewed changes

artemmufazalov requested changes Jul 29, 2025

View reviewed changes

artemmufazalov reviewed Jul 29, 2025

View reviewed changes

Raubzeug requested a review from artemmufazalov July 30, 2025 06:49

artemmufazalov previously approved these changes Jul 30, 2025

View reviewed changes

Raubzeug dismissed artemmufazalov’s stale review via 8dd0678 July 30, 2025 11:54

astandrik requested a review from Copilot July 30, 2025 11:57

Copilot AI reviewed Jul 30, 2025

View reviewed changes

cursor bot reviewed Jul 30, 2025

View reviewed changes

astandrik requested changes Jul 30, 2025

View reviewed changes

Raubzeug added 7 commits July 30, 2025 15:59

feat: improve tenant diagnostics - CPU

e37a382

fix: review

f8567af

fix: remove description

da66f3b

fix: keyset

f02ed9d

fix: design review fixes

295a87f

fix: review

78af278

fix

fc44f2c

Raubzeug force-pushed the cpu-redesign branch from b06d664 to fc44f2c Compare July 30, 2025 14:13

fix

0cf3e4c

Raubzeug requested review from artemmufazalov and astandrik July 30, 2025 14:51

Raubzeug added 2 commits July 31, 2025 09:50

fix: styles

9421525

fix: test

efca9a8

astandrik requested changes Jul 31, 2025

View reviewed changes

src/utils/hooks/useScrollPosition.ts Show resolved Hide resolved

astandrik requested changes Jul 31, 2025

View reviewed changes

src/utils/hooks/useScrollPosition.ts Show resolved Hide resolved

astandrik reviewed Jul 31, 2025

View reviewed changes

src/containers/Tenant/Diagnostics/TopQueries/RunningQueriesData.tsx Show resolved Hide resolved

Raubzeug enabled auto-merge July 31, 2025 10:50

artemmufazalov approved these changes Jul 31, 2025

View reviewed changes

Raubzeug requested a review from astandrik July 31, 2025 12:17

Merge branch 'main' into cpu-redesign

9ba6efb

astandrik approved these changes Jul 31, 2025

View reviewed changes

Raubzeug added this pull request to the merge queue Jul 31, 2025

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 31, 2025

Raubzeug added this pull request to the merge queue Jul 31, 2025

github-merge-queue bot pushed a commit that referenced this pull request Jul 31, 2025

feat: improve tenant diagnostics - CPU (#2633)

aca98e7

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 31, 2025

astandrik added this pull request to the merge queue Jul 31, 2025

Merged via the queue into main with commit 84e75ea Jul 31, 2025
13 checks passed

astandrik deleted the cpu-redesign branch July 31, 2025 16:45

ydb-platform-bot mentioned this pull request Jul 31, 2025

chore(main): release 11.5.0 #2612

Merged

	render: ({row}) => <div className={b('query-hash')}>{generateHash(String(row.QueryText))}</div>,
	render: ({row}) => generateHash(String(row.QueryText)),

feat: improve tenant diagnostics - CPU #2633

feat: improve tenant diagnostics - CPU #2633

Uh oh!

Conversation

Raubzeug commented Jul 29, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Results

Test Status: ⚠️ FLAKY

⏭️ Skipped Tests (2)

Bundle Size: ✅

Uh oh!

artemmufazalov left a comment

Choose a reason for hiding this comment

Uh oh!

artemmufazalov Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Raubzeug Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik commented Jul 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

✅ Bugbot reviewed your changes and found no bugs!

Uh oh!

astandrik Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astandrik Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

claude bot Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Current Codebase Patterns

Recommendation

Todo:

Uh oh!

Raubzeug commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude bot commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

✅ Strengths

🔍 Issues to Address

📋 Recommendations

✅ All Critical Requirements Met

Uh oh!

astandrik commented Jul 31, 2025

Uh oh!

claude bot commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Issues Already Fixed

🔍 Remaining Issues

📊 Overall Assessment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Raubzeug commented Jul 29, 2025 •

edited by github-actions bot

Loading

astandrik Jul 30, 2025 •

edited

Loading

claude bot Jul 31, 2025 •

edited

Loading

claude bot commented Jul 31, 2025 •

edited

Loading

claude bot commented Jul 31, 2025 •

edited

Loading