Skip to content
Draft
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
1 change: 1 addition & 0 deletions app/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from './client'
export * from './roles'
export * from './util'
export * from './__generated__/Api'
export { camelToSnake } from './__generated__/util'
// export * as ZVal from './__generated__/validate'

export type { ApiTypes }
Expand Down
16 changes: 13 additions & 3 deletions app/components/SystemMetric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import { useMemo, useRef } from 'react'

import { api, q, synthesizeData, type ChartDatum, type SystemMetricName } from '@oxide/api'

import { ChartContainer, ChartHeader, TimeSeriesChart } from './TimeSeriesChart'
import {
ChartContainer,
ChartHeader,
TimeSeriesChart,
toChartSeries,
} from './TimeSeriesChart'

// The difference between system metric and silo metric is
// 1. different endpoints
Expand Down Expand Up @@ -84,11 +89,14 @@ export function SiloMetric({
// TODO: indicate time zone somewhere. doesn't have to be in the detail view
// in the tooltip. could be just once on the end of the x-axis like GCP

const { values, timestamps } = toChartSeries(data)

return (
<ChartContainer>
<ChartHeader title={title} label={`(${unit})`} />
<TimeSeriesChart
data={data}
timestamps={timestamps}
data={values}
title={title}
interpolation="stepAfter"
startTime={startTime}
Expand Down Expand Up @@ -153,11 +161,13 @@ export function SystemMetric({
// TODO: indicate time zone somewhere. doesn't have to be in the detail view
// in the tooltip. could be just once on the end of the x-axis like GCP

const { values, timestamps } = toChartSeries(data)
return (
<ChartContainer>
<ChartHeader title={title} label={`(${unit})`} />
<TimeSeriesChart
data={data}
data={values}
timestamps={timestamps}
title={title}
interpolation="stepAfter"
startTime={startTime}
Expand Down
25 changes: 17 additions & 8 deletions app/components/TimeSeriesChart.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ describe('safe redrawing', () => {
* "wrong" calls to redraw.
*/
const props = (formatter: (v: number) => string) => ({
data: [{ timestamp: 0, value: 10 }],
data: [[10]],
timestamps: [0],
title: 'CPU',
startTime: new Date(0),
endTime: new Date(3_600_000),
Expand Down Expand Up @@ -77,19 +78,27 @@ describe('safe redrawing', () => {
// uplot-react will do a deep comparison if the data reference changes to avoid rebuilding the
// chart, but it would be even better to skip that comparison by maintaining a reference
test('an unchanged data prop sends a stable reference down to uplot-react', () => {
const data = [
{ timestamp: 0, value: 10 },
{ timestamp: 1000, value: 20 },
]
const data = [[10, 20]]
const timestamps = [0, 1000]

dataPropsPassed.length = 0
const { rerender } = render(<TimeSeriesChart {...props((v) => `${v}%`)} data={data} />)
rerender(<TimeSeriesChart {...props((v) => `${v} pct`)} data={data} />)
const { rerender } = render(
<TimeSeriesChart {...props((v) => `${v}%`)} timestamps={timestamps} data={data} />
)
rerender(
<TimeSeriesChart {...props((v) => `${v} pct`)} timestamps={timestamps} data={data} />
)

expect(dataPropsPassed.length).toBeGreaterThan(1) // it re-rendered
expect(new Set(dataPropsPassed).size).toBe(1) // but every render passed the identical reference

rerender(<TimeSeriesChart {...props((v) => `${v}%`)} data={[...data]} />)
rerender(
<TimeSeriesChart
{...props((v) => `${v}%`)}
timestamps={timestamps}
data={[...data]}
/>
)
expect(new Set(dataPropsPassed).size).toBe(2) // unless the reference changes
})
})
137 changes: 120 additions & 17 deletions app/components/TimeSeriesChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type ChartTheme = {
hoverPoint: string
axisLine: string
axisText: string
lineColors: string[]
}

// Append an alpha channel to a resolved color, e.g. `oklch(l c h)` -> `oklch(l c h / 0.6)`. Assumes
Expand All @@ -88,9 +89,20 @@ function getChartTheme(): ChartTheme {
hoverPoint: v('--content-accent'),
axisLine: v('--stroke-secondary'),
axisText: v('--content-quaternary'),
lineColors: [
'--color-green-800',
'--color-blue-800',
'--color-purple-800',
'--color-yellow-800',
'--color-red-800',
].map(v),
}
}

const seriesColor = (i: number, theme: ChartTheme): string =>
theme.lineColors[i] ||
`oklch(0.77 0.175 ${((163.7 + (i - theme.lineColors.length) * 137.508) % 360).toFixed(1)})`

function useChartTheme(): ChartTheme {
const [colors, setColors] = useState(getChartTheme)
useEffect(() => subscribeToTheme(() => setColors(getChartTheme())), [])
Expand Down Expand Up @@ -143,7 +155,8 @@ function ChartTooltip({
}

type TimeSeriesChartProps = {
data: ChartDatum[] | undefined
timestamps: number[] | undefined
data: (number | null)[][] | undefined
title: string
interpolation?: 'linear' | 'stepAfter'
startTime: Date
Expand All @@ -152,6 +165,7 @@ type TimeSeriesChartProps = {
yAxisTickFormatter?: (val: number) => string
hasError?: boolean
loading: boolean
seriesLabels?: readonly string[]
}

// this top margin is also in the chart, probably want a way of unifying the sizing between the two
Expand Down Expand Up @@ -191,7 +205,23 @@ const SkeletonMetric = ({

const defaultYAxisTickFormatter = (val: number) => val.toLocaleString()

/**
* Split a single `ChartDatum[]` into the parallel `timestamps`/`data` arrays the chart consumes.
* Returns `undefined` props when there's no data so the chart goes into the loading/empty state.
*/
export function toChartSeries(data: ChartDatum[] | undefined): {
timestamps: number[] | undefined
values: (number | null)[][] | undefined
} {
if (!data) return { timestamps: undefined, values: undefined }
return {
timestamps: data.map((d) => d.timestamp),
values: [data.map((d) => d.value)],
}
}

export function TimeSeriesChart({
timestamps,
data,
title,
interpolation = 'linear',
Expand All @@ -201,6 +231,7 @@ export function TimeSeriesChart({
yAxisTickFormatter = defaultYAxisTickFormatter,
hasError = false,
loading,
seriesLabels,
}: TimeSeriesChartProps) {
const theme = useChartTheme()
const fontPx = remToPx(AXIS_FONT_REM_XS)
Expand All @@ -210,8 +241,13 @@ export function TimeSeriesChart({

const formatTime = isSameDay(startTime, endTime) ? shortTime : shortDateTime

const dataLength = data?.length ?? 0

const [tooltip, setTooltip] = useState<{
// the x position
hoveredDataIndex: number
// which series is hovered
hoveredSeriesIndex: number
left: number
top: number
// which side of the point the box sits on
Expand All @@ -229,13 +265,20 @@ export function TimeSeriesChart({
return
}

const x = self.data[0][idx]
const y = self.data[1][idx]
if (y == null) {
// We hunt down the series whose Y is closest to the cursor position at the given X index.
// Reminder that the first series is the X values, so we start at series index 1 here.
const nearestSeriesIndex = R.firstBy(
R.range(1, self.series.length).filter((s) => self.data[s][idx] != null),
// non-null: the filter above dropped series that are null at this idx
(s) => Math.abs(self.valToPos(self.data[s][idx]!, 'y') - top)
)
if (nearestSeriesIndex === undefined) {
setTooltip(null)
return
}

const x = self.data[0][idx]

const plotRect = self.over.getBoundingClientRect()
const chartRect = self.root.getBoundingClientRect()

Expand All @@ -244,6 +287,7 @@ export function TimeSeriesChart({

setTooltip({
hoveredDataIndex: idx,
hoveredSeriesIndex: nearestSeriesIndex - 1,
// cursor coords are relative to the plot area, so we add in the diff between the plot
// and the whole container
left: plotRect.left - chartRect.left + left,
Expand Down Expand Up @@ -288,16 +332,16 @@ export function TimeSeriesChart({
},
series: [
{},
{
...R.times(dataLength, (i) => ({
show: true,
stroke: theme.stroke,
fill: theme.fill,
stroke: seriesColor(i, theme),
fill: dataLength === 1 ? theme.fill : undefined,
points: { show: false },
paths: match(interpolation)
.with('linear', () => uPlot.paths.linear?.())
.with('stepAfter', () => uPlot.paths.stepped?.({ align: 1 }))
.exhaustive(),
},
})),
],
axes: [
{
Expand Down Expand Up @@ -341,20 +385,25 @@ export function TimeSeriesChart({
},
],
padding: [null, null, null, CHART_LEFT_PAD],
focus: { alpha: 0.5 },
cursor: {
// setting this property causes non-focused series to dim on hover.
// 1e9 just means "any proximity will do"
focus: { prox: 1e9 },
x: false,
y: false,
// TODO: i like the drag and we should put it back in
drag: { x: false },
points: {
size: 6,
// TODO: with multiline, pinning the focused point color doesn't make much sense anymore
fill: theme.hoverPoint,
},
},
legend: { show: false },
plugins: [tooltipPlugin],
}) satisfies Omit<uPlot.Options, 'width' | 'height'>,
[formatTime, tooltipPlugin, interpolation, theme, axisFont, fontPx]
[dataLength, formatTime, tooltipPlugin, interpolation, theme, axisFont, fontPx]
)

// Width/height changes cause a cheaper "update" path for uplot, instead of "create", so it gets
Expand All @@ -371,11 +420,10 @@ export function TimeSeriesChart({

const aligned = useMemo<uPlot.AlignedData>(() => {
const points = data ?? []
return [
points.map(({ timestamp }) => timestamp / 1000),
points.map(({ value }) => value),
]
}, [data])
const times = timestamps ?? []

return [times.map((t) => t / 1000), ...points]
}, [data, timestamps])

if (hasError) {
return (
Expand All @@ -393,15 +441,26 @@ export function TimeSeriesChart({
)
}

if (!data || data.length === 0) {
if (!data || data.length === 0 || !timestamps || timestamps.length === 0) {
return (
<SkeletonMetric>
<MetricsEmpty />
</SkeletonMetric>
)
}

const hovered = tooltip ? data[tooltip.hoveredDataIndex] : undefined
const hovered: ChartDatum | undefined =
tooltip &&
// in case the data changed out from under us, let's at least check that we can find something
// to render
tooltip.hoveredSeriesIndex < data.length &&
tooltip.hoveredDataIndex < timestamps.length
? {
timestamp: timestamps[tooltip.hoveredDataIndex],
value: data[tooltip.hoveredSeriesIndex][tooltip.hoveredDataIndex],
}
: undefined

return (
<figure aria-label={title} className="m-0 pt-8 pr-5 pb-5 pl-0">
{/* The chart is absolutely positioned so its fixed pixel width doesn't feed back into the
Expand Down Expand Up @@ -433,12 +492,24 @@ export function TimeSeriesChart({
<ChartTooltip
timestamp={hovered.timestamp}
value={hovered.value}
seriesName={title}
seriesName={
seriesLabels
? seriesLabel(title, tooltip.hoveredSeriesIndex, seriesLabels)
: title
}
unit={unit}
/>
</div>
)}
</div>
{seriesLabels && (
<ChartLegend
title={title}
count={data.length}
seriesLabels={seriesLabels}
theme={theme}
/>
)}
</figure>
)
}
Expand Down Expand Up @@ -521,3 +592,35 @@ export function ChartHeader({ title, label, description, children }: ChartHeader
</div>
)
}

// We generally expect a list of labels to be the same length as the data list (or not provided), so
// the fallback here is just for bad behavior.
function seriesLabel(title: string, i: number, labels: readonly string[]): string {
return labels[i] ?? `${title} #${i + 1}`
}

function ChartLegend({
title,
count,
seriesLabels,
theme,
}: {
title: string
count: number
seriesLabels: readonly string[]
theme: ChartTheme
}) {
return (
<ul className="mt-2 flex max-h-24 flex-wrap gap-x-4 gap-y-1.5 overflow-y-auto pl-5">
{Array.from({ length: count }, (_, i) => (
<li key={i} className="text-mono-xs text-secondary flex items-center gap-2">
<span
className="h-0.5 w-3 shrink-0 rounded-full"
style={{ backgroundColor: seriesColor(i, theme) }}
/>
{seriesLabel(title, i, seriesLabels)}
</li>
))}
</ul>
)
}
Loading
Loading