-
Notifications
You must be signed in to change notification settings - Fork 8
metrics: collect mount items for detail metrics #16
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
Conversation
Summary of ChangesHello @imeoer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the observability of mounted volumes by introducing a new Prometheus metric that provides detailed information about each mounted item. It refines the cache management logic to actively collect and export these granular metrics, distinguishing between static, dynamic, and inline volume types. The changes also include new tests to ensure the accuracy of the metric collection and updates the service initialization to accommodate the enhanced cache manager. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
31248fe to
1502b07
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.
Code Review
This pull request introduces a new Prometheus metric mount_item to provide detailed information about mounted items, which is a great enhancement for observability. The changes include a new custom collector, updates to the cache manager to gather this data, and new tests.
My review focuses on the correctness and maintainability of the new logic. I've found a critical issue in the detection of 'inline' models which would prevent them from being reported. I've also suggested an improvement to the new test case to make it more robust and maintainable. Overall, these are valuable additions with a few areas for refinement.
5770725 to
125ec14
Compare
7346dab to
e2906bb
Compare
Introduce a new Prometheus metric for tracking detailed information about mounted items and refines the cache manager to export richer metrics. It also adds corresponding tests to ensure correct metric collection. The changes enhance observability of mounted volumes, distinguishing between pvc, inline and dynamic models. Additionally, it improves the cache size calculation method to avoid double-counting file sizes from hardlinks. Signed-off-by: imeoer <yansong.ys@antgroup.com>
e2906bb to
e8a85f7
Compare
gaius-qi
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.
LGTM
chlins
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.
lgtm
This pull request introduces significant improvements to the metrics collection for mounted model volumes, refactors metric naming for clarity, and adds robust test coverage for the new metrics. The main changes are grouped below:
Metrics Collection Enhancements:
MountItemCollectorinpkg/metrics/mount_collector.goto export detailed mount item metrics, including reference, type (pvc, inline, dynamic), volume name, and mount ID. This enables fine-grained tracking of mounted items.pkg/service/cache.go) now collects and sets mount item metrics, distinguishing between PVC, inline, and dynamic models, and updates aggregate metrics accordingly. [1] [2]Metric Naming and Registry Refactor:
NodeMountedStaticImages→NodeMountedPVCModels,NodeMountedDynamicImages→NodeMountedInlineModels, and addedNodeMountedDynamicModelsfor dynamic mounts. Registry initialization updated to use new metric names and register the new mount item collector. [1] [2] [3]Testing Improvements:
pkg/service/cache_test.goto verify that the cache manager correctly updates both aggregate and detailed mount item metrics, with checks for label accuracy and snapshot consistency.Code Quality and Bug Fixes:
CacheSacnInterval→CacheScanIntervalin both declaration and usage. [1] [2]StatusManager, ensuring correct status handling for mount metrics. [1] [2]Dependency Updates:
github.com/prometheus/client_modelandgithub.com/kylelemons/godebugtogo.modfor metrics and testing support. [1] [2] [3]