use Cloud Monitoring for cluster autocomplete#413
Conversation
Changes the data source for GKE cluster name autocomplete from the GKE API to the Cloud Monitoring TimeSeries API. By querying the `logging.googleapis.com/log_entry_count` metric, the autocomplete can now suggest clusters that existed during the inspection time window, even if they have since been deleted.
Summary of ChangesHello @kyasbal, 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 refactors the GKE cluster name autocomplete functionality to utilize the Cloud Monitoring TimeSeries API. This strategic shift enables the system to provide more comprehensive suggestions, including clusters that may have been deleted but were active during a user-defined inspection period. The change involves integrating a new monitoring client, updating the cluster fetching logic to query metric data, and enhancing user feedback for scenarios where no clusters are identified within the specified time window. 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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the GKE cluster name autocomplete feature to use the Cloud Monitoring API instead of the GKE API. This is a great improvement as it allows autocompletion for clusters that existed within the selected time frame, even if they have been deleted. The implementation is well-structured, following existing patterns for adding new Google Cloud clients and tasks.
I've identified a potential issue with the alignment_period in the Monitoring API call which could lead to incorrect results for small time windows. I've also included a minor suggestion to improve the formatting of a user-facing hint message. Overall, this is a valuable change.
kyasbal
changed the title
kyasbal
requested review from
RyuSA,
jyane,
kkuchima and
yfuruyama
as code owners
Refactors cluster name autocomplete to utilize Cloud Monitoring, enabling historical cluster discovery. This change centralizes the autocomplete logic and removes provider-specific implementations. - feat: implement generic `AutocompleteClusterNamesTask` in `googlecloudk8scommon` using Cloud Monitoring metrics. - feat: add `QueryDistinctLabelValuesFromMetrics` utility for generic metric label querying. - refactor: remove specific autocomplete tasks for GKE, GDC, and MultiCloud in favor of the common implementation. - fix: correct task dependencies in `InputClusterNameTask` to use reference IDs. - doc: update README.md and README.ja.md with new optional permissions for autocomplete.
kyasbal
force-pushed
the
feat/historical-cluster-autocomplete
branch
from
d78fd2d to
6b53dc6
Compare
renamoo
left a comment
renamoo
left a comment
There was a problem hiding this comment.
I quickly went through the change and it looks good to me, but if this PR needs review by someone with backend expertise, feel free to wait for their review
| if err != nil { | ||
| errorString = err.Error() | ||
| } |
There was a problem hiding this comment.
How about adding a 403 error handling, like "You may miss a permission monitoring.timeSeries.list Cluster name suggestions does not work correctly."
3 participants
This PR refactors the cluster name autocomplete functionality to use Cloud Monitoring's TimeSeries API instead of the specific cluster management APIs (e.g., GKE API). This change enables KHI to suggest historical cluster names that existed within the inspection time window, even if those clusters have since been deleted.
Previously, autocomplete only supported currently running clusters via the GKE API.
NOTE: This change is not covering the autocomplete logic change for the Cloud Composer cluster name autocompletion because it uses the resource label obtained from GKE API and it's not possible to take the value from metrics API.
Key Changes
pkg/task/inspection/googlecloudk8scommon, removing duplicated implementations for GKE, GDC, and MultiCloud.monitoring.timeSeries.listto query metrics and discover cluster names.kubernetes.io/container/uptimekubernetes.io/anthos/container/uptimeQueryDistinctLabelValuesFromMetricsinpkg/api/googlecloud/monitoring_util.goto handle generic distinct value querying from partial metrics.monitoring.timeSeries.listto the recommended permissions inREADME.md.Related Issues
Closes #412
Verification