Skip to content

fix: trigger metrics fetch on initial observability page load#553

Merged
rashadism merged 1 commit into
openchoreo:mainfrom
rashadism:fix/metrics-initial-fetch
May 15, 2026
Merged

fix: trigger metrics fetch on initial observability page load#553
rashadism merged 1 commit into
openchoreo:mainfrom
rashadism:fix/metrics-initial-fetch

Conversation

@rashadism
Copy link
Copy Markdown
Contributor

@rashadism rashadism commented May 15, 2026

Summary

Metrics page sometimes loads empty until the user clicks Refresh or changes a filter. The useEffect used a previousFiltersRef initialized from live first-render filter values, so when the page mounted with env already resolved (URL carries ?env=..., or environments resolve from cache) the first diff was a no-op and the fetch was skipped.

useMetrics already guards on env + timeRange internally, so the manual diff was redundant. Drop the ref; let the effect's dep array (env, timeRange, canViewMetricsForEnv, fetchMetrics) trigger the fetch.

Side effect: changing the custom date range now refetches automatically instead of needing Refresh.

HTTPMetricsSection is untouched — separate component, separate ref, cilium gate intact.

Test plan

  • CPU / memory cards populate on initial mount with ?env=... in the URL.
  • Initial mount with no URL env (auto-select path).
  • Env / time-range / custom date changes all refetch.
  • HTTP metrics cards still hide when cilium disabled and fetch independently when enabled.
  • Per-env permission denial still hides cards and skips fetch.

The metrics page used a ref-tracked JSON.stringify diff to decide
whether filter values had changed before firing fetchMetrics. The ref
was initialized to the live filter values at first render, so when the
page mounted with env already resolved (URL has env, or environments
resolve synchronously from cache) the first effect run compared current
to itself, marked it unchanged, and skipped the fetch. The user then
saw "no metrics data" until refreshing or changing a filter.

useMetrics already guards its fetch on env + timeRange, so the manual
diff was duplicating that guard. Drop the ref and let the effect's
dep array do the work: fire when env, timeRange, canViewMetricsForEnv,
or fetchMetrics identity changes, and rely on the hook's internal
guard for the no-op case.

HTTPMetricsSection keeps its own ref-pattern fetch; it's separately
gated by useCiliumEnabled() and unaffected by this change.

Signed-off-by: Rashad Sirajudeen <rashad@wso2.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

📝 Walkthrough

Walkthrough

ObservabilityMetricsPage.tsx removes useRef-based filter-change tracking and replaces it with a direct useEffect that fetches metrics when environment, time range, and view permissions are available. The React import is updated to remove useRef.

Changes

Metrics Fetch Effect Refactor

Layer / File(s) Summary
Metrics fetch effect refactor
plugins/openchoreo-observability/src/components/Metrics/ObservabilityMetricsPage.tsx
React import drops useRef. The metrics-fetch useEffect is rewritten to trigger directly on environment, time range, and permission conditions instead of tracking previous filters with a ref and comparing JSON stringified values; dependency list is simplified.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A ref once tracked filters with care,
But now the effect takes the air!
Simpler deps, cleaner flow,
Watch those metrics brightly glow. 📊

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is missing several required sections from the template, including Goals, Approach, User stories, Release note, Documentation, Training, Certification, Marketing, Automation tests, Security checks, Samples, Related PRs, Migrations, Test environment, and Learning. Complete the PR description by adding all missing sections from the template. At minimum, provide Goals, Approach, User stories, Release note, Documentation, and Automation tests sections with relevant information.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: trigger metrics fetch on initial observability page load' clearly and concisely describes the main bug fix: ensuring metrics are fetched when the observability page initially loads.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@rashadism rashadism merged commit 9b241bf into openchoreo:main May 15, 2026
8 checks passed
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.

2 participants