feat(gke): expose monitoring components as a parameter#5722
Conversation
Summary of ChangesHello, 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 improves the flexibility of the GKE cluster module by exposing the GKE monitoring components as a configurable parameter. By moving away from a hardcoded local variable, users can now selectively enable or disable specific monitoring components to better manage costs and resource utilization, while maintaining full backward compatibility with previous configurations. Highlights
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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request exposes GKE monitoring components as a customizable input variable (monitoring_components) in the gke-cluster module, replacing a previously hardcoded list. This change is accompanied by a design document and updated module documentation. The review feedback suggests setting nullable = false on the new variable to prevent potential Terraform runtime errors if null is passed.
057664e to
0f9ec04
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new monitoring_components variable to the GKE cluster module, allowing users to customize the enabled GKE monitoring components instead of relying on a hardcoded local list. The feedback suggests wrapping the concat logic with distinct() to prevent duplicate DCGM entries if it is already present in the user-provided list, and adding a validation block to the new variable to ensure only valid GKE monitoring components are specified.
Exposes the monitoring_components list as an input variable in the gke-cluster module, allowing customers to customize which components expose metrics to Cloud Monitoring. The default value is set to the current hardcoded list of components to ensure backward compatibility. TAG=agy CONV=ec98d4fc-5ce0-464b-9137-7a785b546753
0f9ec04 to
2a658a9
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request externalizes the GKE monitoring components configuration by introducing a new monitoring_components input variable, replacing the hardcoded local default. A review comment points out an issue where DCGM monitoring would still be enabled even if a user explicitly disables all other monitoring components by setting monitoring_components to an empty list. It is recommended to only enable DCGM monitoring when monitoring_components is not empty.
Exposes GKE monitoring components as a parameter. Default is the current list.