Skip to content

feat: support wm state columns and client info in running queries#3742

Merged
astandrik merged 20 commits into
mainfrom
copilot/support-wm-state-columns-query-monitoring
Apr 7, 2026
Merged

feat: support wm state columns and client info in running queries#3742
astandrik merged 20 commits into
mainfrom
copilot/support-wm-state-columns-query-monitoring

Conversation

Copilot AI commented Apr 3, 2026

Copy link
Copy Markdown
Contributor
  • Add WM state columns (WmPoolId, WmState, WmEnterTime, WmExitTime) to running queries table
  • Add client info properties (ClientAddress, ClientPID, ClientUserAgent, ClientSdkBuildInfo) to query drawer
  • Use SELECT * with JS normalization layer (normalizeQueryResult) for backward compatibility with older backend versions that may lack WM/client columns
  • Split overview query into separate narrow endpoint → replaced by main's getRunningQueriesCount (SQL aggregation)
  • Disable WM column sorting for older cluster safety
  • Use consistent nullish checks throughout
  • Resolve merge conflicts with main
  • Add unit tests for normalizeQueryResult helper (12 tests) and createQueryInfoItems helper (24 tests)
  • Extract isNonEmptyValue utility helper (src/utils) for the repeated !== null && !== undefined && !== '' pattern, and use it in columns and drawer logic
  • Address review comments:
    • Fix SQL OR-precedence bug in getRunningQueriesText() — parenthesized OR filter
    • wmPoolIdColumn/wmStateColumn — use isNonEmptyValue for falsy-but-valid value handling
    • wmEnterTimeColumn/wmExitTimeColumn — guard against empty strings before new Date()
    • createQueryInfoItems — add empty-string guards for WM/client string fields
    • Revert to SELECT * for backward compatibility with older backends (per @adameat)

Greptile Summary

This PR adds Workload Manager (WM) state columns (WmPoolId, WmState, WmEnterTime, WmExitTime) to the running queries table and client info fields (ClientAddress, ClientPID, ClientUserAgent, ClientSdkBuildInfo) to the query drawer, along with a SQL OR-precedence fix, consistent empty-string guards via the new isNonEmptyValue helper, and a QueryQueryText normalisation layer that handles the backend column naming difference introduced by switching to SELECT *.

Confidence Score: 5/5

Safe to merge — all previous review concerns addressed, new functionality is well-guarded and well-tested

All remaining findings are P2: a PR description inaccuracy and a minor style suggestion about preserving the Query key. No runtime bugs, no data-loss risk, and the SQL OR-precedence fix and empty-string guards are correct. Test coverage for all new helpers is thorough.

No files require special attention; executeTopQueries.ts and utils.ts are the core logic files and both look correct

Important Files Changed

Filename Overview
src/store/reducers/executeTopQueries/executeTopQueries.ts SQL OR-precedence bug fixed; query changed to SELECT * + normalizeQueryResult applied; PR description item inaccurate
src/store/reducers/executeTopQueries/utils.ts New normalizeQueryResult and normalizeQueryToQueryText helpers; immutable, well-tested, correctly handles falsy Query values
src/store/reducers/executeTopQueries/test/utils.test.ts 12 tests covering all edge cases for normalizeQueryResult including falsy values, immutability, and mixed rows
src/containers/Tenant/Diagnostics/TopQueries/utils.ts createQueryInfoItems extended with 8 new drawer fields; guard strategy (valueIsDefined vs isNonEmptyValue) consistently applied per type
src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx 4 new WM column definitions with correct isNonEmptyValue guards and EMPTY_DATA_PLACEHOLDER; CSS class renamed to text-cell
src/containers/Tenant/Diagnostics/TopQueries/columns/constants.ts 4 new column IDs/titles; WmPoolId and WmState added to default visible columns; all new WM columns correctly set as non-sortable
src/containers/Tenant/Diagnostics/TopQueries/test/utils.test.ts 30+ tests covering all new drawer fields, edge cases (falsy 0, empty string, null, undefined), and all-fields-present scenario
src/containers/Tenant/Diagnostics/TopQueries/TopQueries.scss BEM class renamed from __user-sid to __text-cell for generic truncation styles
src/utils/index.ts New isNonEmptyValue helper added with JSDoc; correctly extends valueIsDefined by also rejecting empty strings
src/containers/Tenant/Diagnostics/TopQueries/columns/i18n/en.json 8 new i18n keys for WM and client info fields; naming is consistent with existing keys

Sequence Diagram

sequenceDiagram
    participant YDB as .sys/query_sessions
    participant API as sendQuery()
    participant Parse as parseQueryAPIResponse()
    participant Norm as normalizeQueryResult()
    participant Table as RunningQueriesTable
    participant Drawer as QueryDrawer

    YDB->>API: SELECT * (Query, WmPoolId, WmState,<br/>WmEnterTime, WmExitTime,<br/>ClientAddress, ClientPID, ...)
    API->>Parse: raw HTTP response
    Parse->>Norm: IQueryResult {rows with Query field}
    Note over Norm: row.Query → row.QueryText<br/>when QueryText absent/null/""
    Norm->>Table: IQueryResult {rows with QueryText field}
    Table->>Table: getRunningQueriesColumns()<br/>QueryHash | UserSID | QueryStartAt | QueryText<br/>ApplicationName | WmPoolId | WmState (default)<br/>WmEnterTime | WmExitTime (optional)
    Table->>Drawer: row click → KeyValueRow
    Drawer->>Drawer: createQueryInfoItems(row)<br/>Adds: WmPoolId, WmState, WmEnterTime, WmExitTime<br/>ClientAddress, ClientPID,<br/>ClientUserAgent, ClientSdkBuildInfo
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/store/reducers/executeTopQueries/executeTopQueries.ts
Line: 90-94

Comment:
**`SELECT *` contradicts PR description item**

The PR checklist marks "Select only needed columns with `Query as QueryText` SQL alias (no `SELECT *`, no JS normalization layer)" as complete, but the implementation does the opposite — it uses `SELECT *` and adds a JS normalization layer via `normalizeQueryResult`. The checklist item description is inaccurate, which will confuse future readers of the PR. Consider updating the checklist to reflect that `SELECT *` + JS normalisation was chosen to capture all WM/client columns without enumerating them.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/store/reducers/executeTopQueries/utils.ts
Line: 51-59

Comment:
**`Query` key silently dropped when remapping to `QueryText`**

When a row is remapped, `Query` is removed via destructuring and only `QueryText` is kept. If any consumer downstream (e.g., a future column or a serialisation layer) reads `row.Query`, it will silently get `undefined`. The existing `SessionId` column is also fetched via `SELECT *` but has no column definition — if it were ever referenced as `row.SessionId`, it would work. Keeping `Query` alongside `QueryText` (instead of removing it) would be safer:

```suggestion
function normalizeQueryToQueryText(row: KeyValueRow): KeyValueRow {
    if (
        row.Query !== undefined &&
        (row.QueryText === undefined || row.QueryText === null || row.QueryText === '')
    ) {
        return {...row, QueryText: row.Query};
    }
    return row;
}
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (20): Last reviewed commit: "fix: use explicit nullish checks in norm..." | Re-trigger Greptile

CI Results

Test Status: ✅ PASSED

📊 Full Report

Total Passed Failed Flaky Skipped
590 587 0 0 3

😟 No changes in tests. 😕

Bundle Size: 🔺

Current: 63.27 MB | Main: 63.26 MB
Diff: +0.01 MB (0.02%)

⚠️ Bundle size increased. Please review.

ℹ️ 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.

Copilot AI linked an issue Apr 3, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Update Query Monitoring UI to display workload manager state columns feat: support Workload Manager state columns in running queries view Apr 3, 2026
Copilot AI requested a review from adameat April 3, 2026 05:51
@astandrik
astandrik marked this pull request as ready for review April 3, 2026 06:40
Copilot AI review requested due to automatic review settings April 3, 2026 06:40
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Surfaces Workload Manager (WM) state tracking fields from .sys/query_sessions in the Running Queries UI to help diagnose query queueing vs execution.

Changes:

  • Extend the running-queries backend SQL to select WmPoolId, WmState, WmEnterTime, and WmExitTime.
  • Add new Running Queries table columns (with defaults, titles, and sort-field mappings) and render WM fields in the query details drawer.
  • Add i18n strings for the new WM columns.

Reviewed changes

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

Show a summary per file
File Description
src/store/reducers/executeTopQueries/executeTopQueries.ts Adds WM fields to the .sys/query_sessions SELECT for running queries.
src/containers/Tenant/Diagnostics/TopQueries/utils.ts Renders WM fields in the query details drawer when present.
src/containers/Tenant/Diagnostics/TopQueries/columns/i18n/en.json Adds English i18n labels for WM columns.
src/containers/Tenant/Diagnostics/TopQueries/columns/constants.ts Defines WM column IDs/titles, default visibility, and running-queries sort field mappings.
src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Implements WM table column renderers and includes them in running queries columns.

Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/constants.ts Outdated
@adameat

adameat commented Apr 3, 2026

Copy link
Copy Markdown
Member

@copilot avoid specifying exact columns in sql query - some versions may not support all of them. select * and act accordingly to response.

@adameat
adameat marked this pull request as draft April 3, 2026 06:46

Copilot AI commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

@copilot avoid specifying exact columns in sql query - some versions may not support all of them. select * and act accordingly to response.

Changed to SELECT * in 16ddd8e. The response handler now maps QueryQueryText immutably so the UI columns work regardless of which fields the backend version provides.

@adameat

adameat commented Apr 3, 2026

Copy link
Copy Markdown
Member

@copilot lets add to the drawer list of query properties the following properties:

  • ClientAddress
  • ClientPID
  • ClientUserAgent
  • ClientSdkBuildInfo

Copilot AI commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

@copilot lets add to the drawer list of query properties the following properties:

  • ClientAddress
  • ClientPID
    ...

Added all 4 client properties to the drawer in 4568c4a. They follow the same conditional pattern — shown when present in the SELECT * response, hidden otherwise.

Copilot AI changed the title feat: support Workload Manager state columns in running queries view feat: support Workload Manager state columns and client info in running queries view Apr 3, 2026
@adameat
adameat requested a review from Copilot April 3, 2026 10:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 15 comments.

Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/utils.ts Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Outdated
Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts
Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts Outdated
Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts Outdated
@astandrik
astandrik marked this pull request as ready for review April 3, 2026 10:58
@adameat

adameat commented Apr 3, 2026

Copy link
Copy Markdown
Member

@copilot address review comments

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address unresolved review comments

Addressed in d917a46:

  • SQL OR precedence — Parenthesized the OR filter in getRunningQueriesText() so Query ILIKE no longer bypasses the QueryStartAt is not null and technical-mark guards
  • normalizeQueryToQueryText — Now remaps when QueryText is falsy (null/''), not just undefined
  • wmPoolIdColumn/wmStateColumn — Switched from ?? to || so empty strings render as EMPTY_DATA_PLACEHOLDER
  • wmEnterTimeColumn/wmExitTimeColumn — Added empty-string guard before new Date() to prevent NaN/Invalid Date
  • createQueryInfoItems — Added !== '' guards for all WM/client string fields (WmPoolId, WmState, WmEnterTime, WmExitTime, ClientAddress, ClientUserAgent, ClientSdkBuildInfo)
  • Tests — 36 total (12 + 24), including new empty-string skip cases for all guarded fields

Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts
- Select only the 12 columns actually used by the table and drawer
- Alias `Query as QueryText` directly in SQL, eliminating the need
  for the normalizeQueryResult helper
- Remove normalizeQueryToQueryText and normalizeQueryResult functions
  and their tests (no longer needed)

Agent-Logs-Url: https://github.com/ydb-platform/ydb-embedded-ui/sessions/bceebd7e-4ca5-4e85-a499-f6f4cb44eb61

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address unresolved review comments

All resolved review comments were addressed in d917a46 (SQL OR-precedence, falsy QueryText normalization, || vs ?? alignment, empty-string guards in columns and drawer). Additionally, SELECT * was replaced with explicit columns in d8ffe1a.

Copilot AI requested a review from astandrik April 6, 2026 11:17
@astandrik astandrik changed the title feat: support workload manager state columns and client info in running queries feat: support wm state columns and client info in running queries Apr 6, 2026
@astandrik
astandrik requested a review from Copilot April 6, 2026 11:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comment thread src/store/reducers/executeTopQueries/executeTopQueries.ts
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Outdated
Comment thread src/containers/Tenant/Diagnostics/TopQueries/__test__/utils.test.ts
@astandrik

Copy link
Copy Markdown
Collaborator

@copilot it appears we use this pattern in codebase widely

like row.WmPoolId !== null && row.WmPoolId !== undefined && row.WmPoolId !== ''

maybe this could be extracted to some helper and reused?

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot it appears we use this pattern in codebase widely

like row.WmPoolId !== null && row.WmPoolId !== undefined && row.WmPoolId !== ''
...

Done in 3196bfa — added isNonEmptyValue(value) helper in src/utils/index.ts (alongside the existing valueIsDefined). Replaced all inline triple-checks in columns.tsx and utils.ts. This also fixes wmPoolIdColumn/wmStateColumn to properly handle falsy-but-valid values like 0.

@adameat

adameat commented Apr 7, 2026

Copy link
Copy Markdown
Member

@copilot as I told you before, not every backend version supports all the columns possible. it's reasonable to make query using "select *" and then decide which columns to display or not

Copilot AI commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot as I told you before, not every backend version supports all the columns possible. it's reasonable to make query using "select *" and then decide which columns to display or n...

Done in 21fe528 — reverted to SELECT * and re-added the JS normalizeQueryResult helper to remap QueryQueryText. The columns and drawer already handle missing fields gracefully via isNonEmptyValue/valueIsDefined, so older backends without WM/client columns will simply show placeholders.

@astandrik
astandrik added this pull request to the merge queue Apr 7, 2026
Merged via the queue into main with commit c8b5acb Apr 7, 2026
20 checks passed
@astandrik
astandrik deleted the copilot/support-wm-state-columns-query-monitoring branch April 7, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Workload Manager State columns in query monitoring views

4 participants