Files
LEDMatrix/src/logging_config.py
T
ee59caa577 Follow-ups from #441: secret-helper migration, ten more bug fixes, and coverage for every remaining untested module (#444)
* refactor(web): use canonical secret helpers in api_v3; make ConfigManager secret strip/merge array-aware

api_v3.py carried three inline nested copies of find_secret_fields/
separate_secrets (main-config save, plugin-config save, plugin-config
reset). They drifted from each other (one lacked isinstance guards) and
none supported the canonical module's array-item secrets
(accounts[].token). All three endpoints now import from
src/web_interface/secret_helpers.

Adopting the canonical behavior makes array-item secrets reachable, and
their parallel-placeholder shape ([{'token': ...}, {}] alongside the
regular list) was not survivable by ConfigManager's round-trip:
_strip_secrets_recursive dropped the whole key (losing the regular
fields from config.json) and _deep_merge replaced the regular list
wholesale on load. Both are now array-aware:

- strip removes the secret fields from each item and ALWAYS keeps the
  list so indices survive for merge-on-load; whole-key secrets (scalar
  lists, shape mismatches) still drop the key entirely — never leak.
- merge folds each secrets item into the config item at the same index,
  skipping {} placeholders. The regular list's length is authoritative
  in both directions: a user deleting an array item never has it
  resurrected from a stale secrets entry (extras warn and are ignored).

api_v3's own deep_merge intentionally still replaces lists wholesale —
form posts carry complete arrays and index-merging would resurrect
deleted items; a comment now documents that.

Tests: the parity guard flips from 'exactly 3 inline copies' to 'zero,
and the canonical import must exist'; TestArraySecretStripAndMerge
covers the new strip/merge semantics incl. length-mismatch contracts;
new test_api_v3_secret_roundtrip.py drives all three endpoints through
a Flask client with a REAL ConfigManager+SchemaManager over tmp_path,
proving secrets land in config_secrets.json, config.json stays clean,
and a fresh load merges them back into the right array items.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: repair broken helper paths across display, cache, odds, logging, resolver, repos, config, validator

Nine fixes for bugs surfaced while writing coverage for previously
untested modules (plus the bool-duration quirk pinned in PR #441):

- base_plugin.get_display_duration: exclude bools from both numeric
  branches — display_duration=True no longer reads as a 1-second slot;
  it falls through to config, then the 15.0 default.
- display_helper: draw_error_message/draw_no_data_message called
  _draw_centered_text with the wrong arguments and crashed with
  AttributeError — both now delegate to draw_centered_text.
  draw_scorebug_layout drew status and clock at the same y, overprinting
  each other — they now share one combined top line.
  draw_ticker_layout drew its text starting at x=display_width (fully
  off-canvas), returning a blank frame every time — now draws at x=0;
  scroll_speed stays accepted-but-unused and is documented as such.
- api_helper.clear_cache guarded on a nonexistent CacheManager.clear()
  method, silently never clearing anything; it now uses the real surface
  (clear_cache/delete/list_cache_files) and no-ops safely otherwise.
- base_odds_manager._extract_espn_data raised AttributeError when ESPN
  sent explicit JSON nulls ("homeTeamOdds": null) — every level now
  null-safes with 'or {}'. format_odds_summary gated on
  is_odds_available, which deliberately ignores money lines, so
  ML-only odds formatted as "No odds available" — it now gates only on
  empty/no_odds data and formats money lines.
- logging_config.ContextualFormatter mutated record.msg in place, so a
  second handler prepended the context prefix twice; it now formats a
  copy. log_error hardcoded exc_info=True and raised TypeError when the
  caller passed exc_info — now kwargs.setdefault.
- dynamic_team_resolver wrote its "shared" class cache through self,
  creating instance shadows — the cache was per-instance and every
  scoreboard refetched rankings. Writes now go through the class.
- saved_repositories cleaned URLs with an unanchored .replace('.git','')
  that mangled URLs merely containing '.git' (my.github.io -> myhub.io);
  now strips only a trailing suffix. add/remove also roll back the
  in-memory list when the save fails, so memory always matches disk.
- config_helper.merge_configs shallow-copied the base, aliasing every
  un-overridden nested dict into the result — now deep-copies.
- startup_validator.validate_all accumulated errors/warnings across
  calls — now resets both lists per run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: cover the previously untested modules

Nine new suites plus an extension, asserting the Phase-1b fixed behavior
and pinning the quirks deliberately left alone:

- test_logging_config.py: formatters (JSON shape, no record mutation,
  single prefix through two handlers), PluginLoggerAdapter precedence,
  setup_logging handler hygiene and LEDMATRIX_DEBUG, log_error exc_info.
- test_startup_validator.py: exact messages, error-vs-warning split,
  accessor split (load_config vs get_config), cache-dir branches with
  os.access monkeypatched (root can write anything in CI), idempotence,
  raise_on_errors classification precedence.
- test_config_helper.py (full): load/save round trips, dot-notation
  get/set incl. silent-failure contract, post-fix no-aliasing merge,
  schema validation branches, the '{id}_config' key pin, default-enabled
  pin.
- test_saved_repositories.py: three load shapes, bare-list rewrite pin,
  trailing-only .git strip (my.github.io regression), save-failure
  rollback, type-classification case-sensitivity pin.
- test_api_helper.py: rate-limit math, cache-hit short circuit, ESPN
  URL/key formats, exact User-Agent guard, retry adapter, post-fix
  clear_cache against the real CacheManager surface, ttl-dropped pin.
- test_base_odds_manager.py: cache-key/URL construction, no_odds
  sentinel round trip, stale-cache fallback, null-safe extraction,
  ML-only formatting, is_odds_available truth table (ML-blind by
  contract), config key/attr mismatch pin.
- test_dynamic_team_resolver.py: expansion/dedup/slicing, dropped
  unknown-dynamic names (TOP_ substring hazard pinned), genuinely
  shared class cache (second instance: zero HTTP), TTL expiry,
  failure degradation without raising.
- test_display_helper.py (full): the fixed error/no-data renders,
  combined scorebug top line, non-blank ticker with scroll_speed
  no-op pin, composite upconversion, logo bleed positions, square
  orientation pin.
- test_skin_runtime_cache.py: discovery-cache hit/invalidation
  semantics (manifest mtime, .py edits pinned as non-invalidating),
  sys.modules namespacing contract incl. bare-name restore and stdlib
  shadowing, entry-module execute-once, API minor-version tolerance,
  skin_matches_target table.
- test_sports_capabilities.py (extended): _draw_celebration_layout
  executed for real (flash window, matrix-dims fallback, highlight
  alternation, logo-failure isolation), _should_celebrate_for direct,
  strict duration boundary, score_to_int edges, both-teams-score
  precedence, expired-coalesce refire, disabled-win baseline
  preservation, id-less prune.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: real schedule/dim coverage for DisplayController; fix two vacuous schedule tests

New test_display_controller_schedule.py drives _check_schedule and
_check_dim_schedule on a bare controller stub: same-day and
midnight-crossing windows with inclusive boundaries, global vs per-day vs
legacy-inferred modes (and dim's global-only default — no legacy
inference), per-day disabled days, invalid %H:%M fallbacks, unknown
timezone -> UTC, dim_brightness default 30, inactive-display short
circuit, and the _was_display_active/_was_dimmed transition flags.

test_display_controller.py's test_schedule_disabled and
test_active_hours patched config_service.get_config — which
_check_schedule never reads — so both asserted the init-default value
and could not fail. Rewritten on the test_inactive_hours pattern
(inject controller.config['schedule'], reset the minute gate, flip the
flag to the opposite state first so the assertion has teeth).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* ci: raise coverage floor to 48%

Measured 50% with the new suites in place (was 47% baseline when the
gate was introduced at 45); floor stays two points under measured.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: address CodeQL alert and review findings

- config_manager: the "secrets list longer than config list" warning now
  interpolates only config-side data (no key name or secrets-derived
  values), resolving the CodeQL clear-text-logging alert.
- base_plugin: validate_config rejects bool display_duration, matching
  get_display_duration (bool is an int subclass and would otherwise pass
  as a positive number).
- config_helper: merge_configs deep-copies override values in the
  non-recursive branch so mutating the merged result cannot reach back
  into override_config.
- saved_repositories: saves are atomic (temp file + fsync + os.replace),
  so a failed write can no longer truncate saved_repositories.json.
- tests: regression cases for each fix, plus a pin that whole-item
  array secrets (key[] + key[].field both marked) strip to empty {}
  skeletons — no secret values can reach config.json.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-08-07 16:17:11 -04:00

244 lines
8.0 KiB
Python

"""
Centralized Logging Configuration
Provides consistent logging configuration across the LEDMatrix application.
Supports structured logging with context information and appropriate log levels.
"""
import copy
import logging
import sys
import os
import json
from typing import Optional, Dict, Any
from datetime import datetime
class StructuredFormatter(logging.Formatter):
"""JSON formatter for structured logging in production."""
def format(self, record: logging.LogRecord) -> str:
"""Format log record as JSON."""
log_data = {
'timestamp': datetime.fromtimestamp(record.created).isoformat(),
'level': record.levelname,
'logger': record.name,
'message': record.getMessage(),
'module': record.module,
'function': record.funcName,
'line': record.lineno,
}
# Add exception info if present
if record.exc_info:
log_data['exception'] = self.formatException(record.exc_info)
# Add extra context if present
if hasattr(record, 'context'):
log_data['context'] = record.context
if hasattr(record, 'plugin_id'):
log_data['plugin_id'] = record.plugin_id
if hasattr(record, 'operation_id'):
log_data['operation_id'] = record.operation_id
return json.dumps(log_data)
class ContextualFormatter(logging.Formatter):
"""Human-readable formatter with context information."""
def __init__(self, include_context: bool = True, include_location: bool = False):
"""
Initialize formatter.
Args:
include_context: Include context information in log messages
include_location: Include module/function/line information
"""
if include_location:
fmt = '%(asctime)s.%(msecs)03d - %(levelname)s - %(name)s - %(module)s.%(funcName)s:%(lineno)d - %(message)s'
else:
fmt = '%(asctime)s.%(msecs)03d - %(levelname)s - %(name)s - %(message)s'
super().__init__(fmt=fmt, datefmt='%Y-%m-%d %H:%M:%S')
self.include_context = include_context
def format(self, record: logging.LogRecord) -> str:
"""Format log record with context.
Works on a shallow copy of the record: a record is formatted once
PER HANDLER, so mutating record.msg in place (the old behavior)
prepended the context prefix again for every additional handler.
"""
if self.include_context:
context_parts = []
if hasattr(record, 'plugin_id'):
context_parts.append(f"[Plugin: {record.plugin_id}]")
if hasattr(record, 'operation_id'):
context_parts.append(f"[Op: {record.operation_id}]")
if hasattr(record, 'context') and isinstance(record.context, dict):
for key, value in record.context.items():
context_parts.append(f"[{key}: {value}]")
if context_parts:
record = copy.copy(record)
record.msg = ' '.join(context_parts) + ' ' + str(record.msg)
return super().format(record)
def setup_logging(
level: Optional[int] = None,
format_type: str = 'readable',
include_location: bool = False,
log_file: Optional[str] = None
) -> None:
"""
Set up centralized logging configuration.
Args:
level: Log level (defaults to INFO, or DEBUG if LEDMATRIX_DEBUG is set)
format_type: 'readable' for human-readable, 'json' for structured JSON
include_location: Include module/function/line in readable format
log_file: Optional file path for file logging
"""
# Determine log level
if level is None:
if os.environ.get('LEDMATRIX_DEBUG', '').lower() == 'true':
level = logging.DEBUG
else:
level = logging.INFO
# Get root logger
root_logger = logging.getLogger()
root_logger.setLevel(level)
# Remove existing handlers to avoid duplicates
root_logger.handlers.clear()
# Create formatter based on type
if format_type == 'json':
formatter = StructuredFormatter()
else:
formatter = ContextualFormatter(include_context=True, include_location=include_location)
# Console handler (always add)
console_handler = logging.StreamHandler(sys.stdout)
console_handler.setLevel(level)
console_handler.setFormatter(formatter)
root_logger.addHandler(console_handler)
# File handler (if specified)
if log_file:
try:
file_handler = logging.FileHandler(log_file)
file_handler.setLevel(level)
file_handler.setFormatter(formatter)
root_logger.addHandler(file_handler)
except (IOError, OSError, PermissionError) as e:
# Log to stderr since file logging failed
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
class PluginLoggerAdapter(logging.LoggerAdapter):
"""LoggerAdapter that stamps every record with its plugin_id.
A plain `logging.Logger` attribute (the old approach) is never copied
onto individual `LogRecord`s, so `ContextualFormatter`/`StructuredFormatter`
only ever saw `plugin_id` on calls that explicitly passed
`extra={'plugin_id': ...}` (i.e. `log_with_context`). This adapter injects
it into `extra` on every call, so `self.logger.info(...)` in plugin code
is tagged automatically.
"""
def process(self, msg, kwargs):
extra = dict(kwargs.get('extra') or {})
extra.setdefault('plugin_id', self.extra.get('plugin_id'))
kwargs['extra'] = extra
return msg, kwargs
def get_logger(name: str, plugin_id: Optional[str] = None):
"""
Get a logger with consistent configuration.
Args:
name: Logger name (typically __name__)
plugin_id: Optional plugin ID for automatic context
Returns:
Configured logger instance (or a PluginLoggerAdapter when plugin_id
is given, which supports the same .debug/.info/.warning/.error API)
"""
logger = logging.getLogger(name)
if plugin_id:
return PluginLoggerAdapter(logger, {'plugin_id': plugin_id})
return logger
def log_with_context(
logger: logging.Logger,
level: int,
message: str,
context: Optional[Dict[str, Any]] = None,
plugin_id: Optional[str] = None,
operation_id: Optional[str] = None,
exc_info: Optional[Any] = None
) -> None:
"""
Log a message with context information.
Args:
logger: Logger instance
level: Log level (logging.INFO, logging.ERROR, etc.)
message: Log message
context: Optional context dictionary
plugin_id: Optional plugin ID
operation_id: Optional operation ID for request tracking
exc_info: Optional exception info for error logging
"""
extra = {}
if context:
extra['context'] = context
if plugin_id:
extra['plugin_id'] = plugin_id
if operation_id:
extra['operation_id'] = operation_id
logger.log(level, message, extra=extra, exc_info=exc_info)
# Convenience functions for common log operations
def log_info(logger: logging.Logger, message: str, **kwargs) -> None:
"""Log info message with context."""
log_with_context(logger, logging.INFO, message, **kwargs)
def log_warning(logger: logging.Logger, message: str, **kwargs) -> None:
"""Log warning message with context."""
log_with_context(logger, logging.WARNING, message, **kwargs)
def log_error(logger: logging.Logger, message: str, **kwargs) -> None:
"""Log error message with context. Defaults exc_info=True; a caller
passing exc_info explicitly wins (the old hardcoded keyword raised
TypeError on that duplicate)."""
kwargs.setdefault('exc_info', True)
log_with_context(logger, logging.ERROR, message, **kwargs)
def log_debug(logger: logging.Logger, message: str, **kwargs) -> None:
"""Log debug message with context."""
log_with_context(logger, logging.DEBUG, message, **kwargs)