Merge pull request #25 from Redislabs-Solution-Architects/GTI-686/fix-cluster-zone-region-grouping

GTI-686: normalize Redis Cluster zone values in Region

Author: ymendez-redis

memorystore.py

@@ -24,9 +24,10 @@
 import argparse
 import csv
 import os
+import re
 import sys
 import time
-from typing import Dict, List, Optional, Any
+from typing import Dict, List, Optional, Any, Tuple
 from collections import defaultdict
 
 from google.oauth2 import service_account
@@ -74,6 +75,28 @@
     "instance_type",
 )
 
+_GCP_ZONE_RE = re.compile(r"^([a-z]+(?:-[a-z]+)+\d+)-[a-z]$")
+
+
+def _normalize_location(value: Optional[str]) -> Tuple[str, str]:
+    """Split a GCP location label into its (region, zone) pair.
+
+    Encodes GCP's location hierarchy: a zone is always
+    ``<region>-<letter>`` (e.g. ``us-central1-a`` -> region
+    ``us-central1``). When the input matches that shape the parent
+    region is returned as the first element and the original value as
+    the zone. Region-shaped inputs (``us-east4``) and any non-matching
+    string (``""``, ``"global"``, multi-region aliases like ``"us"``)
+    are passed through as the region with an empty zone, so callers
+    never silently lose information.
+    """
+    if not value:
+        return ("", "")
+    match = _GCP_ZONE_RE.match(value)
+    if match:
+        return (match.group(1), value)
+    return (value, "")
+
 
 def _pick(labels: Dict[str, str], keys) -> Optional[str]:
     for k in keys:
@@ -192,8 +215,9 @@ def _accumulate_commands(results, table, product_name: str, project_id: str):
         entry["InstanceId"] = (
             rlabels.get("instance_id") or rlabels.get("cluster_id") or ""
         )
-        entry["Region"] = _pick(rlabels, REGION_LABELS) or entry["Region"]
-        entry["Zone"] = _pick(rlabels, ZONE_LABELS) or entry["Zone"]
+        region, zone_from_loc = _normalize_location(_pick(rlabels, REGION_LABELS))
+        entry["Region"] = region or entry["Region"]
+        entry["Zone"] = _pick(rlabels, ZONE_LABELS) or zone_from_loc or entry["Zone"]
         entry["NodeType"] = _pick(rlabels, NODETYPE_LABELS) or entry["NodeType"]
 
         # Node role if provided (e.g., 'primary'/'replica')

test_msstats.py

@@ -20,6 +20,8 @@
     _resolve_inst_key,
     _attach_memory_usage,
     _attach_capacity_scalar,
+    _accumulate_commands,
+    _normalize_location,
 )
 
 
@@ -556,6 +558,82 @@ def test_attach_capacity_scalar_uses_resolve_inst_key(self):
             table["my-project/memorystore-valkey"]["abc123"]["MaxMemory"], 10000000
         )
 
+    def test_normalize_location_zone_form(self):
+        """GCP zone string is split into (region, zone)"""
+        self.assertEqual(
+            _normalize_location("us-central1-a"),
+            ("us-central1", "us-central1-a"),
+        )
+
+    def test_normalize_location_region_form(self):
+        """Region-shaped string is passed through as region, zone blank"""
+        self.assertEqual(_normalize_location("us-east4"), ("us-east4", ""))
+
+    def test_normalize_location_multi_segment_zone(self):
+        """Multi-word region prefix (e.g. asia-northeast1) is handled"""
+        self.assertEqual(
+            _normalize_location("asia-northeast1-c"),
+            ("asia-northeast1", "asia-northeast1-c"),
+        )
+
+    def test_normalize_location_empty_and_none(self):
+        """Empty string and None return ('', '')"""
+        self.assertEqual(_normalize_location(""), ("", ""))
+        self.assertEqual(_normalize_location(None), ("", ""))
+
+    def _make_cmd_ts(self, resource_labels, cmd="GET"):
+        mock_ts = MagicMock()
+        mock_ts.resource.labels = resource_labels
+        mock_ts.metric.labels = {"cmd": cmd}
+        mock_point = MagicMock()
+        mock_point.interval.start_time.timestamp.return_value = 1000.0
+        mock_point.value.int64_value = 1
+        mock_point.value.double_value = 0
+        mock_ts.points = [mock_point]
+        return mock_ts
+
+    def test_accumulate_commands_cluster_location_splits_into_region_and_zone(self):
+        """Redis Cluster: location=<zone> must split into Region + Zone"""
+        table = {}
+        ts = self._make_cmd_ts(
+            {"cluster_id": "c1", "shard_id": "s0", "location": "us-central1-a"}
+        )
+        _accumulate_commands([ts], table, "Redis Cluster", "proj")
+        entry = table["proj/c1"]["s0"]
+        self.assertEqual(entry["Region"], "us-central1")
+        self.assertEqual(entry["Zone"], "us-central1-a")
+
+    def test_accumulate_commands_standalone_keeps_region_and_explicit_zone(self):
+        """Redis standalone: explicit region + zone labels survive untouched"""
+        table = {}
+        ts = self._make_cmd_ts(
+            {
+                "instance_id": "projects/proj/locations/us-central1/instances/r1",
+                "node_id": "n0",
+                "region": "us-central1",
+                "zone": "us-central1-a",
+            }
+        )
+        _accumulate_commands([ts], table, "Redis", "proj")
+        entry = table["projects/proj/locations/us-central1/instances/r1"]["n0"]
+        self.assertEqual(entry["Region"], "us-central1")
+        self.assertEqual(entry["Zone"], "us-central1-a")
+
+    def test_accumulate_commands_standalone_region_only_leaves_zone_blank(self):
+        """Redis standalone: region-only label leaves Zone blank (regex no match)"""
+        table = {}
+        ts = self._make_cmd_ts(
+            {
+                "instance_id": "projects/proj/locations/us-east4/instances/r1",
+                "node_id": "n0",
+                "region": "us-east4",
+            }
+        )
+        _accumulate_commands([ts], table, "Redis", "proj")
+        entry = table["projects/proj/locations/us-east4/instances/r1"]["n0"]
+        self.assertEqual(entry["Region"], "us-east4")
+        self.assertEqual(entry["Zone"], "")
+
 
 if __name__ == "__main__":
     unittest.main()