-
Notifications
You must be signed in to change notification settings - Fork 31
Add Cache metrics page #1788
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
Add Cache metrics page #1788
Conversation
b41d05d to
056cb0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new Cache Metrics page that centralizes shared components from the Origin metrics page, reusing existing graph components and updating the navigation. Key changes include:
- Adding a new layout and page for cache metrics under the cache section.
- Refactoring import paths to use centralized shared components.
- Updating the navigation configuration to include a Metrics entry under cache.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web_ui/frontend/app/cache/metrics/layout.tsx | New layout using dynamic imports for GraphContext and GraphOverlay. |
| web_ui/frontend/app/cache/metrics/page.tsx | New Cache Metrics page with multiple metric components. |
| web_ui/frontend/components/metrics/index.ts | Re-export of shared metric components. |
| web_ui/frontend/components/metrics/ProjectsTable.tsx | Refactored export style for the ProjectTable component. |
| web_ui/frontend/app/navigation.tsx | Added the Metrics navigation link for the cache section. |
| web_ui/frontend/app/origin/metrics/page.tsx | Updated import paths to reference the shared metrics components. |
| web_ui/frontend/app/director/metrics/page.tsx | Updated import paths to reference the shared metrics components. |
| web_ui/frontend/app/origin/metrics/components/index.ts | Removed redundant exports due to centralization. |
h2zh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good. I’m curious why Origin metrics page has StorageGraph but Cache’s doesn’t? Is it possible to share the layout file across Origin and Cache’s metrics pages?
- Same as Origin as they have the same metrics - Centralize shared comps
Co-authored-by: Copilot <[email protected]>
2f35477 to
1a68150
Compare
Good call I have shared the layout. I dropped the storage because this is not a metric that populated for a cache. As it is the amount of free space I would expect it to always be 0 when the cache is hot? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after the creation of associated Github issue! One nitpicky thing: the commit messages and commit content are mixed up in the last two commits. You may want to squash them.
|
@h2zh Not nitpicky at all, perfection is the goal. I will fixup. |
1a68150 to
8b2e3f2
Compare
( Draft for now until I can get my hands on a running cache to view real data )