feat: renderMonitoring to diagnostics #3032

astandrik · 2025-10-30T14:25:06Z

greptile-review

CI Results

Test Status: ⚠️ FLAKY

Total	Passed	Failed	Flaky	Skipped
378	375	0	1	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: 47.10 MB | Main: 47.09 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.

greptile-apps

_{6 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

astandrik · 2025-10-30T14:44:00Z

lets use renderMonitoring pattern

greptile-apps

_{6 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Copilot

Pull Request Overview

This PR adds a new "Monitoring" tab to the tenant diagnostics interface, integrating an external monitoring system through the UIFactory extension mechanism. The monitoring tab is conditionally rendered based on whether the renderMonitoring function is provided in the UIFactory configuration.

Key changes:

Extended UIFactory with a renderMonitoring function type that accepts tenant/schema context
Added "monitoring" tab to the diagnostics tabs configuration
Implemented conditional insertion of monitoring tab as the second tab when the render function is available

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/uiFactory/types.ts	Defines `RenderMonitoring` type with props for rendering monitoring content and adds it to UIFactory interface
src/store/reducers/tenant/constants.ts	Adds 'monitoring' constant to diagnostics tab IDs
src/containers/Tenant/Diagnostics/DiagnosticsPages.ts	Imports uiFactory and conditionally inserts monitoring tab as second tab when render function is available
src/containers/Tenant/Diagnostics/Diagnostics.tsx	Adds case handler for monitoring tab that invokes `uiFactory.renderMonitoring` with tenant context
.rooignore	Adds gitignore-style file for Roo tool configuration (unrelated to monitoring feature)

src/containers/Tenant/Diagnostics/DiagnosticsPages.ts

Raubzeug · 2025-10-30T15:33:10Z

src/containers/Tenant/Diagnostics/DiagnosticsPages.ts

+    const result = applyFilters(seeded, type, options);
+
+    // Add monitoring tab as second tab if renderMonitoring is available
+    if (uiFactory.renderMonitoring) {


It seems this check should be in getDatabasePages.

Copilot

Pull Request Overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Copilot · 2025-11-01T07:36:34Z

src/routes.ts

    clusters: `/${CLUSTERS}`,
    cluster: `/:environment?/${CLUSTER}/:activeTab?`,
-    tenant: `/:environment?/${TENANT}`,
+    tenant: `/${TENANT}`,


Removing the optional environment parameter from the tenant route appears to be an unrelated change to the monitoring feature. This could be a breaking change that affects routing throughout the application. If this is intentional, ensure it's documented in the PR description or moved to a separate commit/PR.

Suggested change

tenant: `/${TENANT}`,

tenant: `/:environment?/${TENANT}`,

src/containers/Tenant/Diagnostics/DiagnosticsPages.ts

.rooignore

astandrik · 2025-11-01T07:38:46Z

greptile-review

Raubzeug · 2025-11-01T07:37:12Z

src/routes.ts

    clusters: `/${CLUSTERS}`,
    cluster: `/:environment?/${CLUSTER}/:activeTab?`,
-    tenant: `/:environment?/${TENANT}`,
+    tenant: `/${TENANT}`,


please return environment param!

Raubzeug · 2025-11-01T07:38:26Z

src/utils/additionalProps.ts

+    name?: string,
+    type?: ETenantType,
+): string | null {
+    if (!additionalProps?.getMonitoringLink) {


what do u think about get rid of additionalProps here and use uiFactory?

resolved in PM

Raubzeug · 2025-11-01T07:38:48Z

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

                <div className={b('info')}>
-                    <div className={b('top-label')}>{tenantType}</div>
+                    <Flex alignItems="center" gap="2" className={b('top-label')}>
+                        <div>{tenantType}</div>


do we really need this wrapper?

@greptile what do you think?

Looking at line 203, you have:

<div>{tenantType}</div>

This wrapper div doesn't seem necessary. The tenantType is just a string being rendered, and it's already inside a Flex container that handles the layout. You could simplify this to just:

Suggested change

<div>{tenantType}</div>

{tenantType}

The div wrapper doesn't add any styling, semantic meaning, or layout benefits here - the parent Flex component is already handling the spacing and alignment. Removing it would clean up the DOM and make the code more concise.

@greptile but will flex work with "just a string being rendered"? I thought flex requires block elements?

You're absolutely right to question this! Actually, Flex containers can work with text nodes directly, but there are some important considerations:

Flex does work with text nodes: When you have a string directly inside a Flex container, the browser automatically wraps it in an anonymous inline box, which becomes a flex item.

However, there might be styling implications: Looking at the code, the parent Flex has alignItems="center" and gap="2". The text will participate in the flex layout, but you might lose some control over styling compared to having it in a div.

Consistency with the codebase: Looking at the surrounding code, I see other text elements are often wrapped in divs or other components for styling purposes.

So while it would technically work, keeping the div wrapper might actually be the better choice here for:

Consistent styling control

Better semantic structure

Easier future modifications (adding classes, etc.)

You're right to keep the wrapper div - it's not unnecessary bloat but rather good practice for maintainable styling and layout control. My initial suggestion was too hasty!

Copilot

Pull Request Overview

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

src/utils/additionalProps.ts

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

src/containers/Tenant/Diagnostics/DiagnosticsPages.ts

greptile-apps

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

feat: option to insert additional diagnostics tab

7068373

astandrik requested review from Raubzeug, adameat and artemmufazalov as code owners October 30, 2025 14:25

greptile-apps bot reviewed Oct 30, 2025

View reviewed changes

astandrik closed this Oct 30, 2025

fix: use render pattern

7151974

astandrik reopened this Oct 30, 2025

greptile-apps bot reviewed Oct 30, 2025

View reviewed changes

astandrik requested a review from Copilot October 30, 2025 14:52

astandrik changed the title ~~feat: option to insert additional diagnostics tab~~ feat: renderMonitoring to diagnostics Oct 30, 2025

Copilot AI reviewed Oct 30, 2025

View reviewed changes

src/containers/Tenant/Diagnostics/DiagnosticsPages.ts Outdated Show resolved Hide resolved

Raubzeug requested changes Oct 31, 2025

View reviewed changes

fix: better code and buttons

f6ff486

astandrik requested review from Raubzeug and Copilot November 1, 2025 07:34

Copilot AI reviewed Nov 1, 2025

View reviewed changes

astandrik added 2 commits November 1, 2025 10:37

fix: remove rooignore

ed3b31c

fix: review fix

5dcff7a

Raubzeug requested changes Nov 1, 2025

View reviewed changes

fix: review fixes

463aa07

astandrik requested review from Raubzeug and Copilot November 1, 2025 08:17

Copilot AI reviewed Nov 1, 2025

View reviewed changes

greptile-apps bot reviewed Nov 1, 2025

View reviewed changes

fix: review fix

0be3f53

Raubzeug approved these changes Nov 1, 2025

View reviewed changes

astandrik added this pull request to the merge queue Nov 1, 2025

Merged via the queue into main with commit 66e8cf8 Nov 1, 2025
7 checks passed

astandrik deleted the astandrik.additional-tabs branch November 1, 2025 09:23

ydb-platform-bot mentioned this pull request Nov 1, 2025

chore(main): release 12.13.0 #3031

Merged

feat: renderMonitoring to diagnostics #3032

feat: renderMonitoring to diagnostics #3032

Uh oh!

Conversation

astandrik commented Oct 30, 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!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

astandrik commented Oct 30, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Raubzeug Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

astandrik commented Nov 1, 2025

Uh oh!

Raubzeug Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

Raubzeug Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

Raubzeug Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

astandrik commented Oct 30, 2025 •

edited by github-actions bot

Loading