Skip to content

Debug: log per-device data_usage response shape#128

Merged
Yeraze merged 1 commit into
mainfrom
debug/device-usage-response-shape
Jun 25, 2026
Merged

Debug: log per-device data_usage response shape#128
Yeraze merged 1 commit into
mainfrom
debug/device-usage-response-shape

Conversation

@Yeraze

@Yeraze Yeraze commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds temporary debug logging to see the actual response shape from the eero per-device data_usage endpoint
  • Need this to diagnose why per-device HourlyBandwidth records still aren't being stored after the double-unwrap fix in v2.9.2

Authored by Merlin 🪄

Temporary instrumentation to diagnose why per-device HourlyBandwidth
records aren't being stored despite the double-unwrap fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Yeraze
Yeraze merged commit 947dae5 into main Jun 25, 2026
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown

Code Review — PR #128: Debug: log per-device data_usage response shape

This PR adds six lines of diagnostic logging to _collect_device_usage to expose the raw API response shape and help diagnose why per-device HourlyBandwidth records aren't being stored. The logic is safe and the approach is reasonable for a diagnostic branch — a few issues worth addressing before or shortly after the diagnosis is complete.


Finding 1 — Wrong log level for temporary diagnostic output (most impactful)

src/collectors/data_usage_collector.py, lines 121–125 and 133

Both new logger.info(...) calls should be logger.debug(...).

The PR description explicitly calls these temporary debug logging, yet logger.info fires at every hourly collection cycle in any deployment with the default INFO log level — and cannot be silenced without a global log-level change. The existing code already follows the correct pattern: diagnostic no-data paths use logger.debug (e.g. "No device data_usage response" on line 117), while logger.info is reserved for meaningful operational summaries (bytes transferred, device count updated). These two new lines are introspection for a one-off diagnosis and belong at DEBUG.

# Before
logger.info(
    f"Device data_usage response: type={type(result).__name__}, "
    ...
)
...
logger.info(f"Extracted devices_list: len={len(devices_list)}")

# After
logger.debug(
    f"Device data_usage response: type={type(result).__name__}, "
    ...
)
...
logger.debug(f"Extracted devices_list: len={len(devices_list)}")

Finding 2 — No removal mechanism for explicitly temporary code

src/collectors/data_usage_collector.py, line 121

The PR description says "temporary" but nothing in the code enforces removal — no # TODO: remove after diagnosing comment, no follow-up issue reference, no debug-only guard. Diagnostic logs at this granularity (raw API response shape on every run, every network) routinely outlive their purpose and end up as permanent log noise. A # TODO(debug): remove after https://github.com/Yeraze/eeroVista/issues/NNN resolves comment would make the intent durable.


Finding 3 — Minor: unnecessary list() allocation on result.keys()

src/collectors/data_usage_collector.py, line 123

# Current — allocates a new list every call just for the string representation
f"keys={list(result.keys()) if isinstance(result, dict) else 'N/A'}"

# Cheaper — dict_keys view stringifies identically in f-strings
f"keys={result.keys() if isinstance(result, dict) else 'N/A'}"

dict.keys() is already iterable and renders as dict_keys(['devices', ...]) in f-strings. Wrapping it in list() allocates a fresh list on every collection cycle for no benefit. Minor, but easy to fix.


No correctness bugs

The guarding logic is sound: if not result: return on line 116 ensures result is truthy before the new log lines run. The conditional expressions in the f-string (if isinstance(result, dict) else 'N/A') correctly guard .keys(), and isinstance(result, (list, dict)) correctly guards len().


Summary: The key action item is switching both logger.info calls to logger.debug — ideally before the diagnostic branch lives long enough that the noisy INFO output becomes an issue in a deployed environment. The removal-tracking comment is a low-cost habit that will save a future cleanup pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant