Improve Agent Overview Page#963
Open
rasika2012 wants to merge 3 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors the agent overview pages to add environment observability (metrics and recent traces), evaluation monitors, new agent/build cards, hook enhancements for time-range presets and query gating, shared component extensions for bottom-content panels, and concrete environment/docker defaults. ChangesAgent Overview Redesign with Observability and Monitoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/workspaces/libs/api-client/src/hooks/traces.ts`:
- Line 106: The auto-refresh interval in the useTraces hook calls refetch()
regardless of options.enabled, so polling continues even when the query is
disabled; update the logic around the interval (the code that sets/clears the
30s timer and the function that calls refetch()) to check options?.enabled
(and/or the returned query enabled flag) before invoking refetch, and avoid
starting the interval when options.enableAutoRefresh is true but options.enabled
is false (or clear the interval when options.enabled flips to false). Reference
the useTraces hook, the refetch() call inside the auto-refresh/interval handler,
and the options?.enabled / enableAutoRefresh flags to locate where to add the
guard and where to clear the timer.
In
`@console/workspaces/libs/shared-component/src/components/EnvironmentCard/EnvironmentCard.tsx`:
- Around line 290-293: The body rendering in EnvironmentCard currently only
treats DeploymentStatus.ERROR as a failed case while the header now includes
DeploymentStatus.FAILED; update the body condition(s) that check
currentDiployment?.status (in EnvironmentCard) so they also treat
DeploymentStatus.FAILED the same as DeploymentStatus.ERROR (e.g., change any
`status === DeploymentStatus.ERROR` checks to `status === DeploymentStatus.ERROR
|| status === DeploymentStatus.FAILED` or a shared helper) so EnvStatus and the
empty-state "Deployment Failed" message render for both statuses.
In `@console/workspaces/pages/metrics/src/Metrics.Component.tsx`:
- Around line 65-68: The current isSuspended calculation assumes unresolved
deployments are not suspended, allowing metrics to enable prematurely; change
the logic in useListAgentDeployments handling so unresolved deployment state is
treated as indeterminate and only allow metrics when we know deployment exists
and is explicitly not suspended. Specifically, update the isSuspended
computation (referencing useListAgentDeployments, deployments, envId,
isSuspended) to return undefined/null when deployments or deployments[envId] is
missing, and change the metrics gating check to require isSuspended === false
(i.e., only enable metrics when the deployment status is present and explicitly
not "suspended").
In `@console/workspaces/pages/overview/src/AgentOverview/DonutIcon.tsx`:
- Line 36: Clamp and sanitize percent before computing offset in DonutIcon:
ensure percent is a finite number and bounded between 0 and 100 (e.g. compute a
safePercent using Number.isFinite(percent) ? Math.min(Math.max(percent, 0), 100)
: 0) and then calculate offset with circumference * (1 - safePercent / 100) so
negative, NaN or infinite values cannot produce invalid SVG dash offsets; update
any usage of percent in the offset calculation to use safePercent.
In `@console/workspaces/pages/overview/src/AgentOverview/EvalMonitorsCard.tsx`:
- Around line 145-155: The generated monitor URLs (allMonitorsHref and
createMonitorHref) omit the required envId param, causing invalid evaluation
routes; update the generatePath calls used for allMonitorsHref and
createMonitorHref to include envId in the params object (e.g., { orgId,
projectId, agentId, envId }) and ensure envId is sourced in this component
(prop, hook, or derived) before calling generatePath so the evaluation routes
match the contract used in EvalMonitors.Component.
In
`@console/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsx`:
- Around line 120-133: The ExternalAgentOverview currently passes a hardcoded
apiKey="ey***" into InstrumentationDrawer which causes placeholder secrets to
appear in export commands; remove the hardcoded apiKey prop (or pass
undefined/null) from ExternalAgentOverview so InstrumentationDrawer cannot
default to a fake key, and instead rely on generatedApiKey or delay rendering
the export commands until generatedApiKey is available; update usage around the
apiKey prop and the effectiveApiKey logic in InstrumentationDrawer
(effectiveApiKey = generatedApiKey || apiKey) to ensure no placeholder is
presented to users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80472095-f410-4e34-bfa6-d4067d9ca50b
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
console/env.exampleconsole/workspaces/libs/api-client/src/hooks/metrics.tsconsole/workspaces/libs/api-client/src/hooks/traces.tsconsole/workspaces/libs/shared-component/src/components/EnvironmentCard/EnvironmentCard.tsxconsole/workspaces/pages/eval/src/subComponents/MonitorTable.tsxconsole/workspaces/pages/metrics/src/Metrics.Component.tsxconsole/workspaces/pages/overview/package.jsonconsole/workspaces/pages/overview/src/AgentOverview/AgentInfoCard.tsxconsole/workspaces/pages/overview/src/AgentOverview/AgentOverview.tsxconsole/workspaces/pages/overview/src/AgentOverview/DonutIcon.tsxconsole/workspaces/pages/overview/src/AgentOverview/EnvObservabilitySection.tsxconsole/workspaces/pages/overview/src/AgentOverview/EvalMonitorsCard.tsxconsole/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsxconsole/workspaces/pages/overview/src/AgentOverview/InternalAgentOverview.tsxconsole/workspaces/pages/overview/src/AgentOverview/KindInfoCard.tsxdeployments/docker-compose.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
console/workspaces/libs/api-client/src/hooks/traces.ts (1)
383-383:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore backward-compatible auto-refresh default behavior.
At Line 383, omitting
optionsdisables polling because!options?.enableAutoRefreshbecomestrue. Existing call sites that don’t passoptionswill stop auto-refreshing.💡 Suggested minimal fix
useEffect(() => { - if (hasCustomRange || !scopeParams || options?.enabled === false || !options?.enableAutoRefresh) return; + const isQueryEnabled = options?.enabled ?? true; + const isAutoRefreshEnabled = options?.enableAutoRefresh ?? true; + if (hasCustomRange || !scopeParams || !isQueryEnabled || !isAutoRefreshEnabled) return; const timer = setInterval(() => { if (traceListRef.current?.traces?.length) { loadNewerRef.current(); } else { refetchRef.current(); } }, 30000); return () => clearInterval(timer); }, [hasCustomRange, scopeParams, options?.enabled, options?.enableAutoRefresh]);Also applies to: 392-392
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/workspaces/libs/api-client/src/hooks/traces.ts` at line 383, The early-return condition treats a missing options object as disabling polling; change the checks so only an explicit false disables auto-refresh. In the guard that currently uses "!options?.enableAutoRefresh" (and the similar check at the other occurrence), replace the falsy check with an explicit comparison to false (e.g., options?.enableAutoRefresh === false) so that undefined/missing options leave auto-refresh enabled; update both occurrences near the checks involving hasCustomRange, scopeParams, options?.enabled and options?.enableAutoRefresh.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@console/workspaces/pages/overview/src/AgentOverview/InstrumentationDrawer.tsx`:
- Line 41: effectiveApiKey can become the literal "undefined" when apiKey is
optional and omitted; change code that derives effectiveApiKey to provide a
clear placeholder (e.g. "<YOUR_API_KEY>" or empty string) when apiKey is
undefined so template strings like export AMP_AGENT_API_KEY="${effectiveApiKey}"
do not render "undefined". Update the place where effectiveApiKey is computed
(the variable derived from apiKey) to use a fallback value and ensure both
snippets that interpolate effectiveApiKey use that fallback.
---
Duplicate comments:
In `@console/workspaces/libs/api-client/src/hooks/traces.ts`:
- Line 383: The early-return condition treats a missing options object as
disabling polling; change the checks so only an explicit false disables
auto-refresh. In the guard that currently uses "!options?.enableAutoRefresh"
(and the similar check at the other occurrence), replace the falsy check with an
explicit comparison to false (e.g., options?.enableAutoRefresh === false) so
that undefined/missing options leave auto-refresh enabled; update both
occurrences near the checks involving hasCustomRange, scopeParams,
options?.enabled and options?.enableAutoRefresh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c35c5955-6c1f-4e85-a077-ce25db292016
📒 Files selected for processing (6)
console/workspaces/libs/api-client/src/hooks/traces.tsconsole/workspaces/libs/shared-component/src/components/EnvironmentCard/EnvironmentCard.tsxconsole/workspaces/pages/metrics/src/Metrics.Component.tsxconsole/workspaces/pages/overview/src/AgentOverview/DonutIcon.tsxconsole/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsxconsole/workspaces/pages/overview/src/AgentOverview/InstrumentationDrawer.tsx
💤 Files with no reviewable changes (1)
- console/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsx
rasika2012
force-pushed
the
Agent-Class
branch
2 times, most recently
from
fcb06c6 to
15521b1
Compare
…con's percent safety checks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
console/workspaces/pages/metrics/src/Metrics.Component.tsx (1)
95-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh button can still fetch metrics for suspended environments.
Even with query
enabledgating, Line 96 callsrefetch()manually. In suspended state this allows requests that the page intends to block.💡 Suggested fix
- const handleRefresh = useCallback(() => { - refetch(); - }, [refetch]); + const handleRefresh = useCallback(() => { + if (isSuspended !== false) return; + refetch(); + }, [isSuspended, refetch]); ... <IconButton size="small" - disabled={isRefetching} + disabled={isRefetching || isSuspended !== false} onClick={handleRefresh} aria-label="Refresh" >Also applies to: 126-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/workspaces/pages/metrics/src/Metrics.Component.tsx` around lines 95 - 97, The manual refetch calls in handleRefresh (calling refetch()) allow metrics to be fetched for suspended environments; update handleRefresh to check the suspended state before calling refetch (e.g., if (isSuspended || environment.suspended) return) and do the same guard for the other refresh handler around lines 126-130 so both handlers only call refetch when the environment is not suspended.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/workspaces/libs/api-client/src/hooks/traces.ts`:
- Around line 383-384: The auto-refresh guard currently treats a missing
enableAutoRefresh as false; update the conditional in the hook (look for the
check containing hasCustomRange, scopeParams and options?.enableAutoRefresh in
traces.ts) to only disable polling when options?.enabled === false OR
options?.enableAutoRefresh === false (i.e. do not negate
options?.enableAutoRefresh with !); make the same change at the other occurrence
around the second guard (the check at ~393) so that an omitted enableAutoRefresh
defaults to enabled and polling behavior is preserved.
---
Outside diff comments:
In `@console/workspaces/pages/metrics/src/Metrics.Component.tsx`:
- Around line 95-97: The manual refetch calls in handleRefresh (calling
refetch()) allow metrics to be fetched for suspended environments; update
handleRefresh to check the suspended state before calling refetch (e.g., if
(isSuspended || environment.suspended) return) and do the same guard for the
other refresh handler around lines 126-130 so both handlers only call refetch
when the environment is not suspended.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 389de06a-6a2f-4927-b5e0-c1f7a67efa4e
📒 Files selected for processing (8)
console/workspaces/libs/api-client/src/hooks/traces.tsconsole/workspaces/libs/shared-component/src/components/EnvironmentCard/EnvironmentCard.tsxconsole/workspaces/pages/deploy/src/subComponent/DeployCard.tsxconsole/workspaces/pages/metrics/src/Metrics.Component.tsxconsole/workspaces/pages/overview/src/AgentOverview/DonutIcon.tsxconsole/workspaces/pages/overview/src/AgentOverview/EvalMonitorsCard.tsxconsole/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsxconsole/workspaces/pages/overview/src/AgentOverview/InstrumentationDrawer.tsx
💤 Files with no reviewable changes (1)
- console/workspaces/pages/overview/src/AgentOverview/ExternalAgentOverview.tsx
Comment on lines
+383
to
+384
| if (hasCustomRange || !scopeParams || options?.enabled === false | ||
| || !options?.enableAutoRefresh) return; |
Contributor
There was a problem hiding this comment.
Default auto-refresh behavior regressed to “off”.
Line 384 treats an omitted enableAutoRefresh as disabled (!options?.enableAutoRefresh), so polling never starts unless callers explicitly pass true. This silently changes existing behavior for current hook consumers.
💡 Suggested fix
+ const isAutoRefreshEnabled = options?.enableAutoRefresh ?? true;
useEffect(() => {
- if (hasCustomRange || !scopeParams || options?.enabled === false
- || !options?.enableAutoRefresh) return;
+ if (
+ hasCustomRange ||
+ !scopeParams ||
+ options?.enabled === false ||
+ !isAutoRefreshEnabled
+ ) {
+ return;
+ }
const timer = setInterval(() => {
if (traceListRef.current?.traces?.length) {
loadNewerRef.current();
} else {
refetchRef.current();
}
}, 30000);
return () => clearInterval(timer);
- }, [hasCustomRange, scopeParams, options?.enabled, options?.enableAutoRefresh]);
+ }, [hasCustomRange, scopeParams, options?.enabled, isAutoRefreshEnabled]);Also applies to: 393-393
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/workspaces/libs/api-client/src/hooks/traces.ts` around lines 383 -
384, The auto-refresh guard currently treats a missing enableAutoRefresh as
false; update the conditional in the hook (look for the check containing
hasCustomRange, scopeParams and options?.enableAutoRefresh in traces.ts) to only
disable polling when options?.enabled === false OR options?.enableAutoRefresh
=== false (i.e. do not negate options?.enableAutoRefresh with !); make the same
change at the other occurrence around the second guard (the check at ~393) so
that an omitted enableAutoRefresh defaults to enabled and polling behavior is
preserved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Issue: #737
This pull request introduces several improvements and updates across the codebase, focusing on enhanced metrics handling, UI component flexibility, environment configuration, and dependency updates. The most significant changes are grouped below:
Metrics Handling and API Hooks:
useGetAgentMetricshook now supports time range presets and disables metrics fetching when an environment is suspended, improving query accuracy and efficiency. It also updates its query key and request logic to reflect these changes. [1] [2] [3]useTraceListhook now accepts anoptionsparameter for more granular control, including an explicitenabledflag. [1] [2]UI Component Enhancements (Environment Cards):
EnvironmentCardcomponent now accepts abottomContentprop for extensibility and uses the new palette variable access pattern for consistent theming. It also improves conditional rendering for actions and status displays, especially for deployment states. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]Configuration and Environment:
.env.examplefile is updated with realistic local development defaults and additional guardrail configuration options, making setup easier for new developers.UI/UX Improvements:
ActivitytoMonitorfor better clarity. [1] [2] [3]Dependency Updates:
@wso2/oxygen-ui-charts-reactas a dependency and upgradesreselectto version 5.2.0 in the lockfile. [1] [2]Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Improvements