diff --git a/app/api/util.ts b/app/api/util.ts index 4018e4041b..79a06b87dd 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -288,3 +288,5 @@ export function parseIpUtilization({ ipv4, ipv6 }: IpPoolUtilization) { }, } } + +export const OXQL_GROUP_BY_ERROR = 'Input tables to a `group_by` must be aligned' diff --git a/app/components/SystemMetric.tsx b/app/components/SystemMetric.tsx index 0cfaa28dd6..2bb31bc063 100644 --- a/app/components/SystemMetric.tsx +++ b/app/components/SystemMetric.tsx @@ -100,6 +100,9 @@ export function SiloMetric({ startTime={startTime} endTime={endTime} unit={unit !== 'Count' ? unit : undefined} + // note use of loading, not fetching, which is only true on first fetch. + // otherwise we get loading states on refetches + loading={inRange.isLoading || beforeStart.isLoading} /> ) @@ -168,6 +171,9 @@ export function SystemMetric({ startTime={startTime} endTime={endTime} unit={unit !== 'Count' ? unit : undefined} + // note use of loading, not fetching, which is only true on first fetch. + // otherwise we get loading states on refetches + loading={inRange.isLoading || beforeStart.isLoading} /> ) diff --git a/app/components/TimeSeriesChart.tsx b/app/components/TimeSeriesChart.tsx index 85486a29bf..7f77c5cf7d 100644 --- a/app/components/TimeSeriesChart.tsx +++ b/app/components/TimeSeriesChart.tsx @@ -118,6 +118,7 @@ type TimeSeriesChartProps = { unit?: string yAxisTickFormatter?: (val: number) => string hasError?: boolean + loading: boolean } const TICK_COUNT = 6 @@ -173,6 +174,7 @@ export function TimeSeriesChart({ unit, yAxisTickFormatter = (val) => val.toLocaleString(), hasError = false, + loading, }: TimeSeriesChartProps) { // We use the largest data point +20% for the graph scale. !rawData doesn't // mean it's empty (it will never be empty because we fill in artificial 0s at @@ -213,13 +215,20 @@ export function TimeSeriesChart({ ) } - if (!data || data.length === 0) { + if (loading) { return ( ) } + if (!data || data.length === 0) { + return ( + + + + ) + } // ResponsiveContainer has default height and width of 100% // https://recharts.org/en-US/api/ResponsiveContainer @@ -281,23 +290,28 @@ export function TimeSeriesChart({ } const MetricsLoadingIndicator = () => ( -
+
) -const MetricsError = () => ( +const MetricsMessage = ({ + icon, + title, + description, +}: { + icon?: React.ReactNode + title: React.ReactNode + description: React.ReactNode +}) => ( <>
-
-
- -
-
Something went wrong
-
- Please try again. If the problem persists, contact your administrator. + {icon} +
{title}
+
+ {description}
( ) +const MetricsError = () => ( + +
+ +
+ } + title="Something went wrong" + description="Please try again. If the problem persists, contact your administrator." + /> +) + +const MetricsEmpty = () => ( + No data
} + description="There is no data for this time period." + /> +) export const ChartContainer = classed.div`flex w-full grow flex-col rounded-lg border border-default` type ChartHeaderProps = { diff --git a/app/components/oxql-metrics/OxqlMetric.tsx b/app/components/oxql-metrics/OxqlMetric.tsx index c9b824670a..db8c097998 100644 --- a/app/components/oxql-metrics/OxqlMetric.tsx +++ b/app/components/oxql-metrics/OxqlMetric.tsx @@ -12,15 +12,14 @@ */ import { useQuery } from '@tanstack/react-query' -import { Children, useEffect, useMemo, useState, type ReactNode } from 'react' +import { Children, useMemo, useState, type ReactNode } from 'react' import type { LoaderFunctionArgs } from 'react-router' -import { apiq, queryClient } from '@oxide/api' +import { apiq, OXQL_GROUP_BY_ERROR, queryClient } from '@oxide/api' import { CopyCodeModal } from '~/components/CopyCode' import { MoreActionsMenu } from '~/components/MoreActionsMenu' import { getInstanceSelector, useProjectSelector } from '~/hooks/use-params' -import { useMetricsContext } from '~/pages/project/instances/common' import { LearnMore } from '~/ui/lib/CardBlock' import * as Dropdown from '~/ui/lib/DropdownMenu' import { classed } from '~/util/classed' @@ -51,23 +50,34 @@ export type OxqlMetricProps = OxqlQuery & { } export function OxqlMetric({ title, description, unit, ...queryObj }: OxqlMetricProps) { - // only start reloading data once an intial dataset has been loaded - const { setIsIntervalPickerEnabled } = useMetricsContext() - const { project } = useProjectSelector() const query = toOxqlStr(queryObj) - const { data: metrics, error } = useQuery( + const { project } = useProjectSelector() + const { + data: metrics, + error, + isLoading, + } = useQuery( apiq('timeseriesQuery', { body: { query }, query: { project } }) // avoid graphs flashing blank while loading when you change the time // { placeholderData: (x) => x } ) - useEffect(() => { - if (metrics) { - // this is too slow right now; disabling until we can make it faster - // setIsIntervalPickerEnabled(true) - } - }, [metrics, setIsIntervalPickerEnabled]) + + // HACK: omicron has a bug where it blows up on an attempt to group by on + // empty result set because it can't determine whether the data is aligned. + // Most likely it should consider empty data sets trivially aligned and just + // flow the emptiness on through, but in the meantime we have to detect + // this error and pretend it is not an error. + // See https://github.com/oxidecomputer/omicron/issues/7715 + const errorMeansEmpty = error?.message === OXQL_GROUP_BY_ERROR + const hasError = !!error && !errorMeansEmpty + const { startTime, endTime } = queryObj - const { chartData, timeseriesCount } = useMemo(() => composeOxqlData(metrics), [metrics]) + const { chartData, timeseriesCount } = useMemo( + () => + errorMeansEmpty ? { chartData: [], timeseriesCount: 0 } : composeOxqlData(metrics), + [metrics, errorMeansEmpty] + ) + const { data, label, unitForSet, yAxisTickFormatter } = useMemo(() => { if (unit === 'Bytes') return getBytesChartProps(chartData) if (unit === 'Count') return getCountChartProps(chartData) @@ -103,7 +113,9 @@ export function OxqlMetric({ title, description, unit, ...queryObj }: OxqlMetric unit={unitForSet} data={data} yAxisTickFormatter={yAxisTickFormatter} - hasError={!!error} + hasError={hasError} + // isLoading only covers first load --- future-proof against the reintroduction of interval refresh + loading={isLoading} /> ) diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 6f26058b23..c49e581d3a 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -26,7 +26,7 @@ import { } from '@oxide/api' import { json, makeHandlers, type Json } from '~/api/__generated__/msw-handlers' -import { instanceCan } from '~/api/util' +import { instanceCan, OXQL_GROUP_BY_ERROR } from '~/api/util' import { parseIp } from '~/util/ip' import { commaSeries } from '~/util/str' import { GiB } from '~/util/units' @@ -1614,7 +1614,24 @@ export const handlers = makeHandlers({ // timeseries queries are slower than most other queries await delay(1000) - return handleOxqlMetrics(body) + const data = handleOxqlMetrics(body) + + // we use other-project to test certain response cases + if (query.project === 'other-project') { + // 1. return only one data point + const points = Object.values(data.tables[0].timeseries)[0].points + if (body.query.includes('state == "run"')) { + points.timestamps = points.timestamps.slice(0, 2) + points.values = points.values.slice(0, 2) + } else if (body.query.includes('state == "emulation"')) { + points.timestamps = points.timestamps.slice(0, 1) + points.values = points.values.slice(0, 1) + } else if (body.query.includes('state == "idle"')) { + throw OXQL_GROUP_BY_ERROR + } + } + + return data }, async systemTimeseriesQuery({ cookies, body }) { requireFleetViewer(cookies) diff --git a/mock-api/oxql-metrics.ts b/mock-api/oxql-metrics.ts index a65c5a19e7..b63e165f21 100644 --- a/mock-api/oxql-metrics.ts +++ b/mock-api/oxql-metrics.ts @@ -280,7 +280,9 @@ export const getMockOxqlInstanceData = ( state?: OxqlVcpuState ): Json => { const values = state ? mockOxqlVcpuStateValues[state] : mockOxqlValues[name] - return { + // structuredClone lets us mutate data in the calling code without messing up + // the source data + return structuredClone({ tables: [ { name: name, @@ -310,5 +312,5 @@ export const getMockOxqlInstanceData = ( }, }, ], - } + }) } diff --git a/test/e2e/instance-metrics.e2e.ts b/test/e2e/instance-metrics.e2e.ts index c5f7bfdefd..55020c801a 100644 --- a/test/e2e/instance-metrics.e2e.ts +++ b/test/e2e/instance-metrics.e2e.ts @@ -8,6 +8,8 @@ import { expect, test } from '@playwright/test' +import { OXQL_GROUP_BY_ERROR } from '~/api' + import { getPageAsUser } from './utils' test('Click through instance metrics', async ({ page }) => { @@ -41,3 +43,47 @@ test('Instance metrics work for non-fleet viewer', async ({ browser }) => { // we don't want an error, we want the data! await expect(page.getByText('Something went wrong')).toBeHidden() }) + +test('empty and loading states', async ({ page }) => { + const messages: string[] = [] + page.on('console', (e) => messages.push(e.text())) + + // we have special handling in the API to return special data for this project + await page.goto('/projects/other-project/instances/failed-restarting-soon/metrics/cpu') + + const loading = page.getByLabel('Chart loading') // aria-label on loading indicator + const noData = page.getByText('No data', { exact: true }) + + // default running state returns two data points, which get turned into one by + // the data munging + await expect(loading).toBeVisible() + await expect(loading).toBeHidden() + await expect(noData).toBeHidden() + + const statePicker = page.getByRole('button', { name: 'Choose state' }) + + // emulation state returns one data point + await statePicker.click() + await page.getByRole('option', { name: 'State: Emulation' }).click() + await expect(loading).toBeVisible() + await expect(loading).toBeHidden() + await expect(noData).toBeVisible() + + // idle state returns group_by must be aligned error, treated as empty + const hasGroupByError = () => messages.some((m) => m.includes(OXQL_GROUP_BY_ERROR)) + + expect(hasGroupByError()).toBe(false) // error not in console + await statePicker.click() + await page.getByRole('option', { name: 'State: Idle' }).click() + await expect(loading).toBeVisible() + await expect(loading).toBeHidden() + await expect(page.getByText('Something went wrong')).toBeHidden() + await expect(noData).toBeVisible() + expect(hasGroupByError()).toBe(true) // error present in console + + // make sure empty state goes away again for the first one + await statePicker.click() + await page.getByRole('option', { name: 'State: Running' }).click() + await expect(noData).toBeHidden() + await expect(loading).toBeHidden() +})