Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions tests/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,77 @@ def mock_query(su_info, su_id):
assert exporter.storage_capacity.labels(storage_unit_id="SU_002")._value.get() == 500
assert call_count == 2

def test_storage_metrics_skips_capacity_when_none(self):
"""When capacity is None (unlimited storage), capacity/utilization
gauges must not be populated, but other metrics still are.

Regression test guarding against re-introducing ``Gauge.set(None)``,
which raises ``TypeError`` inside ``float()`` and floods the logs
with warnings once per collection interval.
"""
exporter = TQMetricsExporter()

fake_su_info = MagicMock()
fake_su_info.id = "SU_UNLIMITED"
exporter._storage_unit_infos = {"SU_UNLIMITED": fake_su_info}

exporter._query_storage_unit = MagicMock(
return_value={
"storage_unit_id": "SU_UNLIMITED",
"capacity": None,
"active_keys": 42,
"process_rss_bytes": 128 * 1024 * 1024,
}
)

exporter.collect_storage_metrics()

# capacity / utilization must have no series for this storage unit
assert ("SU_UNLIMITED",) not in exporter.storage_capacity._metrics
assert ("SU_UNLIMITED",) not in exporter.storage_utilization._metrics
# active_keys and memory_rss are still reported
assert exporter.storage_active_keys.labels(storage_unit_id="SU_UNLIMITED")._value.get() == 42
assert exporter.storage_memory_rss.labels(storage_unit_id="SU_UNLIMITED")._value.get() == 128 * 1024 * 1024

def test_storage_metrics_prunes_stale_capacity_on_switch_to_unlimited(self):
"""If a storage unit transitions from a numeric capacity to unlimited,
the previously-set ``storage_capacity`` / ``storage_utilization``
series must be removed so dashboards do not keep serving stale values.
"""
exporter = TQMetricsExporter()

fake_su_info = MagicMock()
fake_su_info.id = "SU_SWITCH"
exporter._storage_unit_infos = {"SU_SWITCH": fake_su_info}

# First collection: numeric capacity → both gauges get populated.
exporter._query_storage_unit = MagicMock(
return_value={
"storage_unit_id": "SU_SWITCH",
"capacity": 1000,
"active_keys": 250,
"process_rss_bytes": 64 * 1024 * 1024,
}
)
exporter.collect_storage_metrics()
assert exporter.storage_capacity.labels(storage_unit_id="SU_SWITCH")._value.get() == 1000
assert exporter.storage_utilization.labels(storage_unit_id="SU_SWITCH")._value.get() == 0.25

# Second collection: unlimited capacity → stale series must be pruned.
exporter._query_storage_unit = MagicMock(
return_value={
"storage_unit_id": "SU_SWITCH",
"capacity": None,
"active_keys": 300,
"process_rss_bytes": 64 * 1024 * 1024,
}
)
exporter.collect_storage_metrics()
assert ("SU_SWITCH",) not in exporter.storage_capacity._metrics
assert ("SU_SWITCH",) not in exporter.storage_utilization._metrics
# active_keys keeps updating with the latest value.
assert exporter.storage_active_keys.labels(storage_unit_id="SU_SWITCH")._value.get() == 300


# ---------------------------------------------------------------------------
# Test: ZMQ request type registration
Expand Down
21 changes: 19 additions & 2 deletions transfer_queue/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,26 @@ def collect_storage_metrics(self) -> None:
label = metrics.get("storage_unit_id", su_id)
capacity = metrics.get("capacity", 0)
active = metrics.get("active_keys", 0)
self.storage_capacity.labels(storage_unit_id=label).set(capacity)
# ``capacity`` is ``None`` when the storage unit is configured with
# unlimited capacity (``total_storage_size=None``).
if capacity is not None:
self.storage_capacity.labels(storage_unit_id=label).set(capacity)
self.storage_utilization.labels(storage_unit_id=label).set(
Comment on lines +335 to +339
active / capacity if capacity > 0 else 0.0
)

@0oshowero0 0oshowero0 Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have an else here to deal with the remove logic. For example

     else:
         for gauge in (self.storage_capacity, self.storage_utilization):
             try:
                  gauge.remove(label)
             except (KeyError, ValueError):
                  pass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added an else branch to prune the stale storage_capacity / storage_utilization series when a unit transitions to unlimited, mirroring the existing partition-level pruning. Also covered Copilot's suggestion with two unit tests in test_metrics.py.

else:
# If the storage unit was previously reporting a numeric
# capacity and later transitions to unlimited (e.g. after
# re-registration), remove the stale series so dashboards
# do not keep serving outdated capacity / utilization
# values. This mirrors the stale-label pruning applied to
# partition-level gauges above.
for gauge in (self.storage_capacity, self.storage_utilization):
try:
gauge.remove(label)
except (KeyError, ValueError):
pass
self.storage_active_keys.labels(storage_unit_id=label).set(active)
self.storage_utilization.labels(storage_unit_id=label).set(active / capacity if capacity > 0 else 0.0)
self.storage_memory_rss.labels(storage_unit_id=label).set(metrics.get("process_rss_bytes", 0))

# Per-operation request stats
Expand Down
Loading