[Telemetry] Add support dynamic min/max node count metrics#5961
[Telemetry] Add support dynamic min/max node count metrics#5961kadupoornima wants to merge 21 commits into
Conversation
…dot notation panics
Change-Id: I6a9d7ef958bc898930540ee532d5423b376370e7
…educe cyclomatic complexity Change-Id: I07ce053ad98d5047b25946cd8f07f41969b0bffb
…valuation null errors Change-Id: Ic5b55da2ef310fffb6d41ad596b6516db0370ffb
…N/MAX_NODE_COUNTS Change-Id: I8a99352b0ba7ff346765c0c734c6090cabf8cc42
Change-Id: I0c0b5119b8e21e8cbbbc65904115e5baae70a5b1
…caling total limits
…r default bounds zonal application logic
…lly rather than hardcoding string limits
…ultiplied alongside zones
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 enhances the telemetry collection system by introducing support for dynamic node count metrics. It enables the tracking of autoscaling and dynamic machine limits across various cloud modules while simultaneously fixing a bug that caused incorrect metric inflation for global node count settings in zonal environments. 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 introduces telemetry collection for dynamic minimum and maximum node counts in the Cluster Toolkit. It adds functions to parse and aggregate dynamic node settings from blueprints, including support for nested keys using dot notation, and includes comprehensive unit tests. Feedback on the changes highlights three key improvements: safely unmarking cty.Value during nested property traversal to prevent panics, simplifying the loop in getDynamicNodeCounts by directly iterating over the slice from config.GetAllBpModules, and adding a defensive check with Has("static_node_count") in getModuleDynamicNodeCounts to avoid potential runtime panics.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Overview
This PR introduces robust schema mappings to support capturing and analyzing bounded autoscaling and dynamic machine limits through two newly added telemetry metrics:
DYNAMIC_MIN_NODE_COUNTSandDYNAMIC_MAX_NODE_COUNTS. Alongside these metrics addition, it resolves a systemic bug affecting metric inflation regarding zonal application rules.Key Changes & Implementations:
dynamicMinNodeCountSettingsanddynamicMaxNodeCountSettingsto support properties inclusive of dynamic bounding modules across the codebase. Features evaluated include: GKE dimensions (autoscaling_max_node_count,autoscaling_total_max_nodes), Slurm nodeset ranges (node_count_dynamic_max), GKE cluster system pool dot-notated objects (system_node_pool_node_count.total_min_nodes), and HTCondor execution points (max_size).getTopLevelNodeCount&extractTargetNodeCountwhere global metrics such asautoscaling_total_max_nodeswere being inaccurately multiplied bygetZonalMultipliercounts.zonalNodeCountSettings) mapped across functions.