Skip to content

Make OxQL instance metrics work for non-fleet viewer users #2773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions app/components/oxql-metrics/OxqlMetric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
* https://github.com/oxidecomputer/omicron/tree/main/oximeter/oximeter/schema
*/

import { useQuery } from '@tanstack/react-query'
import { Children, useEffect, useMemo, useState, type ReactNode } from 'react'
import type { LoaderFunctionArgs } from 'react-router'

import { apiQueryClient, useApiQuery } from '@oxide/api'
import { apiq, queryClient } from '@oxide/api'

import { CopyCodeModal } from '~/components/CopyCode'
import { MoreActionsMenu } from '~/components/MoreActionsMenu'
import { getInstanceSelector } from '~/hooks/use-params'
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'
Expand All @@ -37,10 +38,9 @@ import {

export async function loader({ params }: LoaderFunctionArgs) {
const { project, instance } = getInstanceSelector(params)
await apiQueryClient.prefetchQuery('instanceView', {
path: { instance },
query: { project },
})
await queryClient.prefetchQuery(
apiq('instanceView', { path: { instance }, query: { project } })
)
return null
}

Expand All @@ -53,10 +53,10 @@ 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()
Copy link
Collaborator Author

@david-crespo david-crespo Apr 1, 2025

Choose a reason for hiding this comment

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

This will blow up when we try to use this outside of a project, but that's fine — we will figure it out at that time.

const query = toOxqlStr(queryObj)
const { data: metrics, error } = useApiQuery(
'systemTimeseriesQuery',
{ body: { query } }
const { data: metrics, error } = useQuery(
apiq('timeseriesQuery', { body: { query }, query: { project } })
// avoid graphs flashing blank while loading when you change the time
// { placeholderData: (x) => x }
)
Expand Down
2 changes: 1 addition & 1 deletion app/pages/project/instances/MetricsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default function MetricsTab() {

const { intervalPicker } = useIntervalPicker({
enabled: isIntervalPickerEnabled && preset !== 'custom',
isLoading: useIsFetching({ queryKey: ['systemTimeseriesQuery'] }) > 0,
isLoading: useIsFetching({ queryKey: ['timeseriesQuery'] }) > 0,
// sliding the range forward is sufficient to trigger a refetch
fn: () => onRangeChange(preset),
})
Expand Down
22 changes: 17 additions & 5 deletions mock-api/msw/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1602,12 +1602,25 @@ export const handlers = makeHandlers({
requireFleetViewer(params.cookies)
return handleMetrics(params)
},
async systemTimeseriesQuery(params) {
if (Math.random() > 0.95) throw 500 // random failure
requireFleetViewer(params.cookies)
async timeseriesQuery({ body, query }) {
lookup.project(query) // 404 if project doesn't exist

// We could try to do something analogous to what the API does, namely
// adding on silo and project to the oxql query to make sure only allowed
// data turns up, but since this endpoint is always called from within a
// project and constructed with project IDs, this would be unlikely to catch
// console bugs.
// https://github.com/oxidecomputer/omicron/blob/cf38148d/nexus/src/app/metrics.rs#L154-L179

// timeseries queries are slower than most other queries
await delay(1000)
return handleOxqlMetrics(body)
},
async systemTimeseriesQuery({ cookies, body }) {
requireFleetViewer(cookies)
// timeseries queries are slower than most other queries
await delay(1000)
return handleOxqlMetrics(params.body)
return handleOxqlMetrics(body)
},
siloMetric: handleMetrics,
affinityGroupList: ({ query }) => {
Expand Down Expand Up @@ -1787,7 +1800,6 @@ export const handlers = makeHandlers({
systemTimeseriesSchemaList: NotImplemented,
targetReleaseView: NotImplemented,
targetReleaseUpdate: NotImplemented,
timeseriesQuery: NotImplemented,
userBuiltinList: NotImplemented,
userBuiltinView: NotImplemented,
})
43 changes: 43 additions & 0 deletions test/e2e/instance-metrics.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, you can obtain one at https://mozilla.org/MPL/2.0/.
*
* Copyright Oxide Computer Company
*/

import { expect, test } from '@playwright/test'

import { getPageAsUser } from './utils'

test('Click through instance metrics', async ({ page }) => {
await page.goto('/projects/mock-project/instances/db1/metrics/cpu')

await expect(
page.getByRole('heading', { name: 'CPU Utilization: Running' })
).toBeVisible()
await expect(page.getByText('Something went wrong')).toBeHidden()

await page.getByRole('tab', { name: 'Disk' }).click()
await expect(page).toHaveURL('/projects/mock-project/instances/db1/metrics/disk')
await expect(page.getByRole('heading', { name: 'Disk Reads' })).toBeVisible()
await expect(page.getByText('Something went wrong')).toBeHidden()

// exact distinguishes from top level "networking" tab
await page.getByRole('tab', { name: 'Network', exact: true }).click()
await expect(page).toHaveURL('/projects/mock-project/instances/db1/metrics/network')
await expect(page.getByRole('heading', { name: 'Bytes Sent' })).toBeVisible()
await expect(page.getByText('Something went wrong')).toBeHidden()
})

// TODO: more detailed tests using the dropdowns to change CPU state and disk

test('Instance metrics work for non-fleet viewer', async ({ browser }) => {
const page = await getPageAsUser(browser, 'Hans Jonas')
await page.goto('/projects/mock-project/instances/db1/metrics/cpu')
await expect(
page.getByRole('heading', { name: 'CPU Utilization: Running' })
).toBeVisible()
// we don't want an error, we want the data!
await expect(page.getByText('Something went wrong')).toBeHidden()
})
Loading