Skip to content

Conversation

@benjaminleonard
Copy link
Contributor

Adding for parity with the Figma designs:
image


Splits the copy code modal and the button so that the modal can be called without a regular button.

It might be simpler to just remove the button part completely. Will defer to you guys.

The docs link thing is placeholder. There's two potential improvements that probably should be made.

  1. We adapt the actions menu to take links (both internal and externalthat use theDropdown.LinkItem`. This has the benefit of making the other links in these menus openable with a command click
  2. The way we link to docs is potentially brittle. Want some way of validating that these sections do in fact exist. But a hash docs link won't 404.

@vercel
Copy link

vercel bot commented Feb 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Feb 18, 2025 10:23pm

@david-crespo david-crespo changed the base branch from main to oxql_disk_metrics February 18, 2025 13:36
@david-crespo
Copy link
Collaborator

1 is #1855, which I've been meaning to get to. I don't think it's particularly hard.

oxqlDocs: 'https://docs.oxide.computer/guides/operator/system-metrics#_oxql_quickstart',
// TODO: update URL once https://github.com/oxidecomputer/docs/pull/426 is merged
oxqlSchemaDocs: (metric: string) =>
`https://docs-git-timeseries-guide-oxidecomputer.vercel.app/guides/operator/available-metric-data#_${metric.replace(':', '')}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couple things about this. I like the idea of linking to a doc, though I'm not sure the schema is that helpful in understanding the graph.

I'm not that worried about us breaking these links. We can pretty easily test them directly if we want: have playwright click the link and go to the site and see whether the right thing is in the viewport. Speaking of which, it seems like the site is kind of janky around that, possibly a timing thing due to the page being so long. If I open this link in a new tab, I see it briefly at the right part of the page before jump back up to the top, presumably when something else loads or hydrates or whatever. If I hit enter in the URL bar again, it jumps to the right place.

https://docs-git-timeseries-guide-oxidecomputer.vercel.app/guides/operator/available-metric-data#_virtual_machinevcpu_usage

Related to that, it seems to cut off the heading. Other fragment links on other pages don't seem to have this problem. It could be because they render a little differently because they have the [discrete] tag on them in the asciidoc to keep them out of the sidebar ToC.

image image

Copy link
Collaborator

@david-crespo david-crespo Feb 18, 2025

Choose a reason for hiding this comment

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

Eh, I take it back about it not being helpful — seeing that description of what this value is is pretty helpful. I do think we should consider tweaking the copy and maybe the visual hierarchy to make it easier to parse all that info. Metadata (a name I put in there) is not that great, it's actually almost the opposite! Fields are the metadata, and the table labeled metadata actually describes the data. I will work on it in the docs site PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the discrete thing and hopefully the link to the sections works better now. There was a hydration error due to asciidoc shenanigans.

@david-crespo
Copy link
Collaborator

Trying to think of more specific copy than "docs" that really says "what even is this data". I think it could use an external link icon. I can make that part of the solution in #2343 — distinguish three ways between non-links, local links, and external links. The first two would look the same I think, latter would have the icon.

image

@david-crespo david-crespo marked this pull request as ready for review February 18, 2025 23:00
@david-crespo
Copy link
Collaborator

Merging and we can do the followup work in the main PR.

@david-crespo david-crespo merged commit 8f4aa9b into oxql_disk_metrics Feb 18, 2025
7 checks passed
@david-crespo david-crespo deleted the metrics-more-actions branch February 18, 2025 23:00
@benjaminleonard
Copy link
Contributor Author

The discrete link thing I'm planning to fix. Basically it's missing the anchor tag with the offset applied. Need to dig into the way discrete should behave. We only really care that it's not in the ToC.

david-crespo added a commit that referenced this pull request Feb 27, 2025
* Add stub of Disk Metrics using OxQL

* Refactoring; getting chart working; needs better default situation

* refacotrs; remove old DiskMetrics; add writes and flushes charts

* initial stub for CPU metrics

* file reorg

* More CPU metrics, though we mgiht need to rethink the long-term plan

* working group_by

* Updates to routes to handle sub-tabs

* Dropdown working on CPU charts

* cleanup

* more work on charts; networking

* Standardize wrapper components

* Reorder charts a bit

* getting side tabs into place

* Update side tab CSS

* Consolidate SideTabs into legacy tabs, using props to control layout

* refactoring; getting rollups working for disks and network interfaces

* Pass pre-formed query string to metric component

* Move date selector up a level, using useContext

* Update routes in path-builder test

* Small refactor to align approach to useState and dropdowns for network and disk metrics tabs

* Add static values for metrics for testing and mock service worker

* Removes TS guard that was a bit onerous; relying on casting now, though, which isn't great

* Updated mock data for disks

* small refactor before integrating Ben's PR

* Refactoring chart logic

* Add tests for OxQL charts

* Better handle cumulative_u64 data with initial sum value

* Instance metrics design tweaks (#2676)

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
Co-authored-by: Charlie Park <charlie@oxidecomputer.com>

* a little code cleanup

* make getOxqlQuery args more generic and structured

* view/copy oxql modal

* inline oxql query modal, remove comment about showing query

* NonEmptyArray whaaaaaaat

* highlight oxql

* Add 'More about OxQL queries' button/link to modal

* test for rendered oxql in modal

* Better link style for OxQL docs

* slightly smaller text

* clean up my weird half-finished metrics props change

* CopyCode footer

* handle no nics case on network metrics

* small aria label fix

* Add restriction to only turn on query reloading once initial data have succcessfully loaded in

* Simplify CPU utilization tab

* Metrics more actions (#2700)

* OxQL metrics more actions

* take CopyCodeModal refactor further, fix motion import

* move oxql schema docs thing into links file

---------

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>

* tweak more actions menu copy one more time

* Dynamic chart Y axis width (#2697)

* Dynamic Y axis width

* Remove spacing and make tick size/margin explicit

* Need spacing on old cards

* Updates, and better logic on utilization chart; still accounting for cpus count

* Updates to incorporate nCPUs in utilization calculation

* small refactor

* Updated test for utilization

* Move OxqlMetric files to own component directory

* A few more tests

* update import

* tests are easier to make sense of when you can see all the data at once

* Default to single state on CPU utiization tab; offer 'total' option

* Update metrics schema URL

* Metrics error & loading states (#2698)

* Move some loaders to parent component

* Update dropdown to cap at 24 hours and handle minimum mean_within

* Use seconds when determining durations

* remove intervalPicker until OxQL is faster

* Less twitchy datepicker wrap

* clean up chart loading states

* init MetricsContext with null instead of dummy values

* Update mock numbers so CPU utilization range is normal

* use lazy imports in the routes

* blarg lint

* Clean up CPU charts

* revert CpuStateMetric component

* utils file tweaks, abstract slightly less

* replace getUnit with explicit unit prop

* use date-fns

* use delay function for sleeps

---------

Co-authored-by: Benjamin Leonard <benji@oxide.computer>
Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants