aksd: Separate presentation from logic; Create documentation & tests for MetricsTab by tejhan · Pull Request #273 · Azure/aks-desktop

tejhan · 2026-02-20T04:56:56Z

Description

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

Changes Made

Created useNamespaceLabels, useDeployments, usePods, usePrometheusMetrics hooks
Created DeploymentSelector, EmptyStateCard, MetricsSummaryBar, MetricsChart, MetricsChartsGrid, MetricsLoadingSkeleton, PodDetailsTable 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

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.

I've also updated file locations for other unrelated files that utilized the queries in /utils/prometheus that previously existed in /MetricsTab/.

Type of changes

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

Related Issues

Closes #97

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

Testing

[ x] Manual testing completed

Test Cases

Describe the test cases that were run:

Creating / utilizing an existing project & observing if Metrics Tab properly loads components initially, and updates state correctly after time.

(This will be followed up by integration tests for our hooks)

Documentation Updates

[x ] Code comments updated

Copilot

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.

plugins/aks-desktop/src/components/MetricsTab/hooks/useDeployments.ts

plugins/aks-desktop/src/components/MetricsTab/hooks/usePrometheusMetrics.ts

plugins/aks-desktop/src/components/MetricsTab/hooks/useDeployments.ts

plugins/aks-desktop/src/components/MetricsTab/hooks/usePrometheusMetrics.ts

plugins/aks-desktop/src/components/MetricsTab/hooks/usePods.ts

plugins/aks-desktop/src/components/MetricsTab/components/DeploymentSelector.tsx

plugins/aks-desktop/src/components/MetricsTab/hooks/usePods.ts

sniok · 2026-02-20T15:10:30Z

plugins/aks-desktop/src/components/MetricsTab/hooks/useDeployments.ts

+ * @param namespace - Kubernetes namespace to list deployments from.
+ * @param cluster - Cluster context name.
+ */
+export function useDeployments(


there's also a useDeployment hook in the aks-desktop/plugins/aks-desktop/src/components/Scaling/hooks/useDeployments.ts. is it the same one?

They are both similar but IIRC the hook for Scaling has slight differences due to 1) fetching of replica count/ desired amounts & 2) implementation is slightly different, primarily how the data is transformed & prepared for consumption.

They both have very similar underlying logic however & only major differences are cleanup logic & data selection/transformation, so I think in upcoming PR's we can modify useDeployments to be consumable for both Scaling & Metrics cases.

My goal for this PR was to keep these hooks metric-scoped for now, but as the next changes roll out for the ScalingTab & MetricsCard, we can definitely merge them for shared utility. (We already have some cross utility so it would make sense.)

skoeva

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 · 2026-02-24T13:27:43Z

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.

…or MetricsTab

Copilot

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.

Copilot · 2026-02-28T16:12:51Z

plugins/aks-desktop/src/components/MetricsTab/components/MetricsChart.tsx

+/** General chart component with configurable title, lines, and optional empty-state message. */
+export const MetricsChart: React.FC<MetricsChartProps> = ({
+  title,
+  data,
+  lines,
+  yAxisLabel,
+  emptyMessage = 'No data available',
+}) => {


emptyMessage defaults to a hard-coded English string ('No data available') and this component doesn’t call t(), so the default empty state can’t be localized (e.g., when MetricsChartsGrid doesn’t pass emptyMessage). Consider requiring emptyMessage as a prop, or inject i18n here (e.g., accept a translated string or use useTranslation inside MetricsChart).

Copilot · 2026-02-28T16:12:51Z

plugins/aks-desktop/src/components/MetricsTab/hooks/useDeployments.ts

+    try {
+      K8s.ResourceClasses.Deployment.apiList(
+        (deploymentList: Deployment[]) => {
+          const deploymentData = deploymentList.map((d: Deployment) => ({
+            name: d.getName(),
+            namespace: d.getNamespace(),
+          }));
+
+          setDeployments(deploymentData);
+
+          // Auto-select first deployment (If no deployment is currently selected)
+          if (deploymentData.length > 0 && !selectedDeployment) {
+            setSelectedDeployment(deploymentData[0].name);
+          }
+
+          setLoading(false);
+        },
+        (error: any) => {
+          console.error('MetricsTab: Error fetching deployments:', error);
+          setError(t('Failed to fetch deployments'));
+          setLoading(false);
+        },
+        {
+          namespace: namespace,
+          cluster: cluster,
+        }
+      )().then(cancel => {
+        cancelFn = cancel;
+      });


This uses K8s.ResourceClasses.Deployment.apiList(... )()().then(...) to obtain a cancel function, but elsewhere in the repo apiList is treated as returning a callable runner where invoking it returns an unsubscribe function synchronously (e.g. components/Scaling/hooks/useDeployments.ts uses const runWatcher = ...; const unsubscribe = runWatcher();). Using .then here will throw if the runtime return value is a function, and it makes cleanup timing unreliable. Align this hook with the established runner/unsubscribe pattern (capture the runner, call it, and if the return is a function, return it from the effect cleanup).

Copilot · 2026-02-28T16:12:52Z

plugins/aks-desktop/src/components/MetricsTab/hooks/usePods.ts

+    try {
+      // First, fetch the Deployment to get its selector
+      K8s.ResourceClasses.Deployment.apiGet(
+        async (deployment: Deployment) => {
+          // Get the selector from the deployment spec
+          const selector = deployment.spec?.selector?.matchLabels;
+          if (!selector) {
+            console.error('MetricsTab: No selector found in deployment spec');
+            setPods([]);
+            return;
+          }
+
+          // Convert selector object to label selector string
+          const labelSelector = Object.entries(selector)
+            .map(([key, value]) => `${key}=${value}`)
+            .join(',');
+
+          // Now use the deployment's selector to find pods
+          K8s.ResourceClasses.Pod.apiList(
+            (podList: Pod[]) => {
+              const podData: PodInfo[] = podList.map((p: any) => {
+                const status = p.status?.phase || 'Unknown';
+                const restarts =
+                  p.status?.containerStatuses?.reduce(
+                    (sum: number, cs: any) => sum + (cs.restartCount || 0),
+                    0
+                  ) || 0;
+
+                return {
+                  name: p.metadata.name,
+                  status: status,
+                  cpuUsage: 'N/A', // Will be updated from metrics
+                  memoryUsage: 'N/A', // Will be updated from metrics
+                  restarts: restarts,
+                };
+              });
+
+              setPods(podData);
+              setTotalPods(podData.length);
+              setProjectStatus(podData.every(p => p.status === 'Running') ? 'Healthy' : 'Degraded');
+            },
+            (error: any) => {
+              console.error('MetricsTab: Error fetching pods:', error);
+            },
+            {
+              namespace: namespace,
+              cluster: cluster,
+              queryParams: {
+                labelSelector: labelSelector,
+              },
+            }
+          )().then(cancel => {
+            cancelPodList = cancel;
+          });
+        },
+        selectedDeployment,
+        namespace,
+        (err: any) => {
+          console.error('MetricsTab: Error fetching deployment for pod selector:', err);
+        },
+        {
+          cluster: cluster,
+        }
+      )().then(cancel => {
+        cancelDeploymentGet = cancel;
+      });


Same issue as useDeployments: the Headlamp apiGet/apiList runner is handled via )().then(cancel => ...). The repo’s established pattern treats the runner invocation as returning an unsubscribe function synchronously (see components/Scaling/hooks/useHPAInfo.ts / useDeployments.ts). If the runtime return is a function, .then will fail and the watchers won’t be cleaned up correctly when switching deployments/unmounting. Capture the runner return value and unsubscribe in the effect cleanup using the same pattern used in Scaling hooks.

Copilot · 2026-02-28T16:12:52Z

plugins/aks-desktop/src/components/MetricsTab/hooks/usePrometheusMetrics.ts

+      // Process Request & Error data (combined)
+      const combinedData: RequestErrorDataPoint[] = [];
+      if (requestResults.length > 0 && requestResults[0].values) {
+        requestResults[0].values.forEach((v: [number, string], idx: number) => {
+          const timestamp = new Date(v[0] * 1000).toLocaleTimeString();
+          const requestRate = parseFloat(v[1]);
+          const errorRate =
+            errorResults.length > 0 && errorResults[0].values[idx]
+              ? parseFloat(errorResults[0].values[idx][1])
+              : 0;
+
+          combinedData.push({
+            timestamp,
+            requestRate: parseFloat(requestRate.toFixed(2)),
+            errorRate: parseFloat(errorRate.toFixed(2)),
+          });
+        });
+        setRequestErrorData(combinedData);
+
+        // Get latest values for summary
+        if (combinedData.length > 0) {
+          const latest = combinedData[combinedData.length - 1];
+          setSummary(prev => ({
+            ...prev,
+            requestRate: `${latest.requestRate}/sec`,
+            errorRate: `${latest.errorRate}%`,
+          }));
+        }


The request/error and other derived series assume samples are finite numbers. Prometheus queries can legitimately return NaN/Inf (e.g., division by zero in the error-rate or response-time queries), which will propagate into state and can render as NaN%, Infinity/sec, or break chart rendering. Guard with Number.isFinite(...) when parsing samples and fall back to 0 or omit invalid points before calling toFixed/updating summary values.

Copilot · 2026-02-28T16:12:52Z

plugins/aks-desktop/src/components/MetricsTab/hooks/usePrometheusMetrics.ts

+  const fetchMetrics = useCallback(async () => {
+    if (!namespace || !selectedDeployment || !subscription || !cluster) return;
+
+    setError(null);
+    setMetricsLoading(true);
+
+    try {
+      // Extract resource group from label if available, otherwise fetch
+      let resourceGroup = resourceGroupLabel;
+
+      if (!resourceGroup) {
+        const result = await getClusterResourceIdAndGroup(cluster, subscription);
+        resourceGroup = result.resourceGroup;
+
+        if (!resourceGroup) {
+          throw new Error('Could not find resource group for cluster');
+        }
+      }
+
+      const promEndpoint = await getPrometheusEndpoint(resourceGroup, cluster, subscription);
+
+      const end = Math.floor(Date.now() / 1000);
+      const start = end - 7200; // Last 2 hours
+      const step = 60;
+
+      // Query CPU usage
+      const cpuQuery = `sum by (namespace) (rate(container_cpu_usage_seconds_total{namespace="${namespace}", container!=""}[5m]))`;


fetchMetrics sets metricsLoading but doesn’t clear prior chart/summary state when starting a new fetch. When switching deployments (or when a fetch returns empty arrays), the UI can continue showing stale data from the previous selection while metricsLoading is true (and MetricsTab’s skeleton only shows when cpuData.length===0). Consider resetting the relevant series/summary at the start of a new fetch (especially when selectedDeployment changes) so loading/empty states reflect the current selection.

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

Copilot started reviewing on behalf of tejhan February 20, 2026 04:57 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from f8510cf to 2b00035 Compare February 20, 2026 05:43

tejhan self-assigned this Feb 20, 2026

tejhan added the p1 priority label Feb 20, 2026

sniok reviewed Feb 20, 2026

View reviewed changes

skoeva requested changes Feb 23, 2026

View reviewed changes

illume marked this pull request as draft February 25, 2026 10:37

illume added the quality label Feb 27, 2026

tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 2b00035 to 022310f Compare February 28, 2026 15:44

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 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

aksd:Separate presentation from logic; Create documentation & tests f…

dcb5f40

…or MetricsTab

tejhan force-pushed the metricsTabComponentHookExtractionAndDocumentation3 branch from 022310f to dcb5f40 Compare February 28, 2026 15:53

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

Copilot started reviewing on behalf of tejhan February 28, 2026 16:09 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

Conversation

tejhan commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made

Rebase Changes

Type of changes

Related Issues

Testing

Test Cases

Documentation Updates

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sniok Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

tejhan Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

skoeva left a comment

Choose a reason for hiding this comment

Uh oh!

tejhan commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tejhan commented Feb 20, 2026 •

edited

Loading