Skip to content

aksd: MetricsTab & MetricsCard: Separate presentation from logic; Tests, refactors, documentation#273

Open
tejhan wants to merge 1 commit intoAzure:mainfrom
tejhan:metricsTabComponentHookExtractionAndDocumentation3
Open

aksd: MetricsTab & MetricsCard: Separate presentation from logic; Tests, refactors, documentation#273
tejhan wants to merge 1 commit intoAzure:mainfrom
tejhan:metricsTabComponentHookExtractionAndDocumentation3

Conversation

@tejhan
Copy link
Collaborator

@tejhan tejhan commented Feb 20, 2026

Description

This PR splits the MetricsTab and MetricsCard into modular components & extracts functionality into hooks following the presentation/logic separation pattern outlined in #97 & #120 .

Changes Made

  • Created useNamespaceLabels, useDeployments, usePods, usePrometheusMetrics, useCardMetrics hooks
  • Created DeploymentSelector, EmptyStateCard, MetricsSummaryBar, MetricsChart, MetricsChartsGrid, MetricsLoadingSkeleton, PodDetailsTable, MetricStatCard components
  • Created types ChartDataPoint, ResponseTimeDataPoint, MetricSummary, MemoryUnit & functions pickMemoryUnit, convertBytesToUnit, formatMemoryBrief
  • Prometheus utils moved to shared utils/prometheus/ location
  • Implemented more explicit cleanup functions
  • Fixed rendering issues during loading state
  • Full Tsdoc documentation for relevant elements
  • Implemented caching for all metrics
  • UI fixes & alignment

Rebase Changes

In order to preserve the translation preparation that was done with useTranslation, I've moved the useTranslation invokations to match the same strings but in their new file locations across the components/hooks.

All Metrics related content is now in the shared Metrics folder.

Translations, new constants & UI changes have all been included in rebase.

Type of changes

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • [x ] Code refactoring

Related Issues

Closes #97, #120

Testing

For automated testing, you can either run npm run plugin:test for the full suite including these new ones. If you want to test these suites alone, you can run cd plugins/aks-desktop, and then npx vitest run src/components/Metrics.

To manually test the components / data are rendering properly,

  1. Create/use an AKS cluster with Prometheus monitoring enabled.
  2. Create an Aks managed project
  3. Deploy an application to that project
  4. Observe metrics tab populate.

Test Cases

Describe the test cases that were run:

  1. Full test suite for all hooks & utility functions.
  2. Manual testing via creating & utilizing existing projects & observing if Metrics Tab properly loads components initially, and updates state correctly after time.

Documentation Updates

  • [x ] Code comments updated

Copilot AI review requested due to automatic review settings February 20, 2026 04:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the AKS Desktop plugin’s MetricsTab to follow a presentation/logic separation approach (per #97), while also relocating Prometheus helper utilities into a shared utils/prometheus location for reuse across tabs.

Changes:

  • Split MetricsTab into dedicated hooks (useDeployments, usePods, useNamespaceLabels, usePrometheusMetrics) and presentational components (charts grid, summary bar, selectors, loading/empty states, pod table).
  • Introduced shared MetricsTab utility types/helpers for memory unit selection and formatting.
  • Updated other features (ScalingTab, MetricsCard) to import Prometheus helpers from the new shared utils location.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
plugins/aks-desktop/src/utils/prometheus/queryPrometheus.tsx Adjusts import path and adds TSDoc for shared Prometheus querying utility.
plugins/aks-desktop/src/utils/prometheus/getPrometheusEndpoint.tsx Adjusts import paths and adds TSDoc for shared endpoint discovery utility.
plugins/aks-desktop/src/components/ScalingTab/ScalingTab.tsx Updates Prometheus helper imports to shared utils location.
plugins/aks-desktop/src/components/MetricsTab/utils.ts Adds shared MetricsTab types and memory formatting/unit conversion helpers.
plugins/aks-desktop/src/components/MetricsTab/hooks/usePrometheusMetrics.ts New hook to fetch/poll Prometheus and transform results for charts/summary.
plugins/aks-desktop/src/components/MetricsTab/hooks/usePods.ts New hook to list pods for the selected deployment and derive status/total pod count.
plugins/aks-desktop/src/components/MetricsTab/hooks/useNamespaceLabels.ts New hook to read subscription/resource-group labels from the namespace metadata.
plugins/aks-desktop/src/components/MetricsTab/hooks/useDeployments.ts New hook to list deployments and manage initial selection/loading/error.
plugins/aks-desktop/src/components/MetricsTab/components/PodDetailsTable.tsx Extracted pod table into a dedicated presentational component.
plugins/aks-desktop/src/components/MetricsTab/components/MetricsSummaryBar.tsx Extracted metrics summary bar into a dedicated component.
plugins/aks-desktop/src/components/MetricsTab/components/MetricsLoadingSkeleton.tsx Extracted loading skeleton UI into a dedicated component.
plugins/aks-desktop/src/components/MetricsTab/components/MetricsChartsGrid.tsx New component composing the set of metric charts into a grid layout.
plugins/aks-desktop/src/components/MetricsTab/components/MetricsChart.tsx New reusable chart wrapper component for Recharts line charts.
plugins/aks-desktop/src/components/MetricsTab/components/EmptyStateCard.tsx Extracted empty-state UI into a reusable component.
plugins/aks-desktop/src/components/MetricsTab/components/DeploymentSelector.tsx Extracted deployment selector dropdown into its own component.
plugins/aks-desktop/src/components/MetricsTab/MetricsTab.tsx Simplifies MetricsTab to composition of hooks + presentational components.
plugins/aks-desktop/src/components/Metrics/MetricsCard.tsx Updates Prometheus helper imports to shared utils location.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from f8510cf to 2b00035 Compare February 20, 2026 05:43
@tejhan tejhan self-assigned this Feb 20, 2026
@tejhan tejhan added the p1 priority label Feb 20, 2026
Copy link
Collaborator

@skoeva skoeva left a comment

Choose a reason for hiding this comment

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

could you write unit tests for this? I think TDD is the move and would be very helpful to for these component refactors moving forward

@tejhan
Copy link
Collaborator Author

tejhan commented Feb 24, 2026

could you write unit tests for this? I think TDD is the move and would be very helpful to for these component refactors moving forward

Yep, currently working on these.

@illume illume marked this pull request as draft February 25, 2026 10:37
@illume illume added the quality label Feb 27, 2026
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 2b00035 to 022310f Compare February 28, 2026 15:44
@tejhan tejhan changed the title aksd: Separate presentation from logic & documentation for MetricsTab aksd:Separate presentation from logic; Create documentation & tests for MetricsTab Feb 28, 2026
@tejhan tejhan changed the title aksd:Separate presentation from logic; Create documentation & tests for MetricsTab aksd: Separate presentation from logic; Create documentation & tests for MetricsTab Feb 28, 2026
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 022310f to dcb5f40 Compare February 28, 2026 15:53
@tejhan tejhan marked this pull request as ready for review February 28, 2026 16:08
Copilot AI review requested due to automatic review settings February 28, 2026 16:08
Copy link

Copilot AI left a comment

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 25 out of 25 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

plugins/aks-desktop/src/components/MetricsTab/MetricsTab.tsx:96

  • The error state mixes error (deployments) and metricsError (Prometheus) but the UI only renders {error}. When metricsError is set and error is null, the alert body will be blank. Also the AlertTitle string is hard-coded and bypasses i18n. Prefer rendering t('Metrics Unavailable') and show error ?? metricsError (and consider surfacing metricsError even when deployments exist so Prometheus failures are visible).
  if ((error || metricsError) && deployments.length === 0) {
    return (
      <Box p={3}>
        <Alert severity="warning">
          <AlertTitle>Metrics Unavailable</AlertTitle>
          <Typography variant="body2">{error}</Typography>
        </Alert>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@illume
Copy link
Collaborator

illume commented Mar 2, 2026

@skoeva can you please check the changes in here relating to ScalingTab? I don't know if they conflict with yours?

Copy link
Collaborator

@illume illume left a comment

Choose a reason for hiding this comment

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

I read through all this and it's looking good IMHO.

  • Can you please describe how to manually test this in the PR description?
  • I think the commit message subject could have the context part added (maybe just MetricsTab) and a longer description of the changes.
  • Can you please look at the open review comments?

@illume illume requested a review from Copilot March 2, 2026 16:23
@illume illume marked this pull request as draft March 2, 2026 16:26
Copy link

Copilot AI left a comment

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 25 out of 25 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

plugins/aks-desktop/src/components/MetricsTab/MetricsTab.tsx:96

  • The error banner is gated on deployments.length === 0, and within the banner it renders {error} even when the failure came from metricsError. This can result in a warning with an empty message and also hides Prometheus fetch errors once deployments are loaded. Consider (1) rendering error ?? metricsError and (2) surfacing metricsError even when deployments exist (e.g., inline alert above charts). Also Metrics Unavailable is currently hard-coded instead of translated via t(...).
  if ((error || metricsError) && deployments.length === 0) {
    return (
      <Box p={3}>
        <Alert severity="warning">
          <AlertTitle>Metrics Unavailable</AlertTitle>
          <Typography variant="body2">{error}</Typography>
        </Alert>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@skoeva
Copy link
Collaborator

skoeva commented Mar 2, 2026

@skoeva can you please check the changes in here relating to ScalingTab? I don't know if they conflict with yours?

@illume they only involve import statements from the MetricsTab code, there shouldn't be any conflict

@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from dcb5f40 to fa0adba Compare March 7, 2026 01:53
@tejhan tejhan marked this pull request as ready for review March 7, 2026 01:55
Copilot AI review requested due to automatic review settings March 7, 2026 01:55
@tejhan tejhan changed the title aksd: Separate presentation from logic; Create documentation & tests for MetricsTab aksd: MetricsTab & MetricsCard: Separate presentation from logic; Tests, refactors, documentation Mar 12, 2026
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 4402288 to 49335d6 Compare March 12, 2026 06:19
@tejhan tejhan marked this pull request as ready for review March 12, 2026 10:15
Copilot AI review requested due to automatic review settings March 12, 2026 10:15
Copy link

Copilot AI left a comment

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 27 out of 27 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@illume
Copy link
Collaborator

illume commented Mar 12, 2026

I think it's probably enough to run "npm run i18n" to fix the CI error.

There's a bunch of open review comments.

@illume illume marked this pull request as draft March 12, 2026 11:07
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch 2 times, most recently from d0fe5c4 to c5a194f Compare March 12, 2026 23:16
@tejhan tejhan marked this pull request as ready for review March 17, 2026 13:12
Copilot AI review requested due to automatic review settings March 17, 2026 13:12
Copy link

Copilot AI left a comment

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 46 out of 46 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from c5a194f to e8af8c4 Compare March 17, 2026 22:06
Copilot AI review requested due to automatic review settings March 17, 2026 22:09
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from e8af8c4 to 219f324 Compare March 17, 2026 22:09
Copy link

Copilot AI left a comment

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 48 out of 48 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 219f324 to 1877344 Compare March 17, 2026 22:16
Copilot AI review requested due to automatic review settings March 17, 2026 22:37
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 1877344 to e02935f Compare March 17, 2026 22:37
Copy link

Copilot AI left a comment

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 30 out of 30 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from e02935f to 2898454 Compare March 17, 2026 22:55
…gic into hooks & components; Tests, refactors & documentation.
Copilot AI review requested due to automatic review settings March 18, 2026 13:57
@tejhan tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 2898454 to 3ee4636 Compare March 18, 2026 13:57
Copy link

Copilot AI left a comment

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 30 out of 30 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugins/aks-desktop: Separate presentation from logic plugins/aks-desktop/src/components/MetricsTab/MetricsTab.tsx MetricsTab

5 participants