Skip to content

Commit 7f7f6fc

Browse files
committed
Fix cost unavailable state handling
1 parent cfd0666 commit 7f7f6fc

5 files changed

Lines changed: 43 additions & 14 deletions

File tree

web/src/api/client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ const DASHBOARD_REFRESH_INTERVAL_MS = 30_000
4242
const AUDIT_REFRESH_INTERVAL_MS = 60_000
4343
const ISSUES_REFRESH_INTERVAL_MS = 30_000
4444
const COST_REFRESH_INTERVAL_MS = 60_000
45-
const COST_DISCOVERY_RETRY_INTERVAL_MS = 2_000
45+
const COST_DISCOVERY_RETRY_INTERVAL_MS = 5_000
4646
export const COST_DISCOVERY_GRACE_MS = 30_000
4747
const COST_TREND_REFRESH_INTERVAL_MS = 120_000
4848
const CHANGES_REFRESH_INTERVAL_MS = 60_000
@@ -630,7 +630,7 @@ export function useOpenCostWorkloads(namespace: string, options?: { enabled?: bo
630630
queryKey: ['opencost-workloads', namespace],
631631
queryFn: () => fetchJSON(`/opencost/workloads?namespace=${encodeURIComponent(namespace)}`),
632632
enabled: (options?.enabled ?? true) && Boolean(namespace),
633-
refetchInterval: costRefetchInterval(false, clusterInfo.data?.context),
633+
refetchInterval: costRefetchInterval(COST_REFRESH_INTERVAL_MS, clusterInfo.data?.context),
634634
staleTime: 30000,
635635
})
636636
}

web/src/components/cost/ApplicationCostTab.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,6 @@ describe('getApplicationCostState', () => {
6666
}
6767

6868
expect(getApplicationCostState(current, undefined, { currentLoading: false, trendLoading: false })).toBe('access_denied')
69+
expect(getApplicationCostState(current, undefined, { trendError: true })).toBe('access_denied')
6970
})
7071
})

web/src/components/cost/ApplicationCostTab.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ export function ApplicationCostTab({ app, workloads, onSelectWorkloadCost }: App
129129
const trend = trendData
130130
const hasCurrent = current?.available === true && (current.coverage?.included ?? 0) > 0
131131
const totals = hasCurrent ? current?.totals : undefined
132-
const coverage = current?.coverage ?? trend?.coverage
132+
const coverage = state === 'partial_missing_current'
133+
? (trend?.coverage ?? current?.coverage)
134+
: (current?.coverage ?? trend?.coverage)
133135
const included = coverage?.included ?? 0
134136
const total = coverage?.total ?? workloads.length
135137
const unavailableCount = coverage?.unavailable?.length ?? 0
@@ -366,12 +368,12 @@ export function getApplicationCostState(
366368
return 'data'
367369
}
368370
if (trendHasData) return 'partial_missing_current'
369-
if (queryError) return 'load_error'
370-
if (loading) return 'loading'
371-
372371
const reason = current?.reason ?? trend?.reason
373372
if (reason === 'no_prometheus' || reason === 'query_error' || reason === 'access_denied' || reason === 'not_found')
374373
return reason
374+
if (queryError) return 'load_error'
375+
if (loading) return 'loading'
376+
375377
return 'no_metrics'
376378
}
377379

web/src/components/cost/WorkloadCostTab.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, it } from 'vitest'
22
import { getWorkloadCostState } from './WorkloadCostTab'
33
import { buildLineChart } from './chart'
4-
import type { OpenCostWorkloadDetailResponse, OpenCostWorkloadTrendResponse } from '../../api/client'
4+
import { ApiError, type OpenCostWorkloadDetailResponse, type OpenCostWorkloadTrendResponse } from '../../api/client'
55

66
describe('getWorkloadCostState', () => {
77
it('treats scaled-to-zero as a valid zero state', () => {
@@ -124,6 +124,20 @@ describe('getWorkloadCostState', () => {
124124

125125
expect(getWorkloadCostState(current, undefined, false)).toBe('access_denied')
126126
expect(getWorkloadCostState(undefined, missing, false)).toBe('not_found')
127+
expect(getWorkloadCostState(undefined, undefined, { currentError: new ApiError('denied', 403) })).toBe('access_denied')
128+
expect(getWorkloadCostState(undefined, undefined, { trendError: new ApiError('missing', 404) })).toBe('not_found')
129+
})
130+
131+
it('shows Prometheus discovery as soon as one query reports it', () => {
132+
const current: OpenCostWorkloadDetailResponse = {
133+
available: false,
134+
reason: 'no_prometheus',
135+
namespace: 'default',
136+
kind: 'Deployment',
137+
name: 'checkout',
138+
}
139+
140+
expect(getWorkloadCostState(current, undefined, { trendLoading: true })).toBe('no_prometheus')
127141
})
128142
})
129143

web/src/components/cost/WorkloadCostTab.tsx

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useEffect, useMemo, useState } from 'react'
22
import { clsx } from 'clsx'
33
import { AlertCircle, DollarSign, HelpCircle, Loader2, TrendingUp } from 'lucide-react'
44
import {
5+
ApiError,
56
useOpenCostWorkload,
67
useOpenCostWorkloadTrend,
78
COST_DISCOVERY_GRACE_MS,
@@ -40,8 +41,8 @@ type WorkloadCostState =
4041
interface WorkloadCostQueryStatus {
4142
currentLoading?: boolean
4243
trendLoading?: boolean
43-
currentError?: boolean
44-
trendError?: boolean
44+
currentError?: unknown
45+
trendError?: unknown
4546
}
4647

4748
interface WorkloadCostTabProps {
@@ -62,8 +63,8 @@ export function WorkloadCostTab({ kind, namespace, name }: WorkloadCostTabProps)
6263
const state = getWorkloadCostState(currentQuery.data, trendData, {
6364
currentLoading: currentQuery.isLoading,
6465
trendLoading,
65-
currentError: currentQuery.isError,
66-
trendError: trendQuery.isError,
66+
currentError: currentQuery.error,
67+
trendError: trendQuery.error,
6768
})
6869

6970
useEffect(() => {
@@ -296,15 +297,26 @@ export function getWorkloadCostState(
296297
return 'data'
297298
}
298299
if (trendHasData) return 'partial_missing_current'
300+
const reason =
301+
current?.reason ??
302+
trend?.reason ??
303+
costUnavailableReasonFromError(queryStatus.currentError) ??
304+
costUnavailableReasonFromError(queryStatus.trendError)
305+
if (reason === 'no_prometheus' || reason === 'query_error' || reason === 'access_denied' || reason === 'not_found')
306+
return reason
299307
if (queryError) return 'load_error'
300308
if (loading) return 'loading'
301309

302-
const reason = current?.reason ?? trend?.reason
303-
if (reason === 'no_prometheus' || reason === 'query_error' || reason === 'access_denied' || reason === 'not_found')
304-
return reason
305310
return 'no_metrics'
306311
}
307312

313+
function costUnavailableReasonFromError(error: unknown): CostUnavailableReason | undefined {
314+
if (!(error instanceof ApiError)) return undefined
315+
if (error.status === 403) return 'access_denied'
316+
if (error.status === 404) return 'not_found'
317+
return undefined
318+
}
319+
308320
function WorkloadCostDiscovering({ isFetching, onRetry }: { isFetching: boolean; onRetry: () => void }) {
309321
return (
310322
<div className="flex h-full min-h-[320px] items-center justify-center">

0 commit comments

Comments
 (0)