mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 18:39:06 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
780fca6365 |
+54
-1
@@ -130,7 +130,12 @@ def setup_logging(
|
|||||||
# Console handler (always add)
|
# Console handler (always add)
|
||||||
console_handler = logging.StreamHandler(sys.stdout)
|
console_handler = logging.StreamHandler(sys.stdout)
|
||||||
console_handler.setLevel(level)
|
console_handler.setLevel(level)
|
||||||
console_handler.setFormatter(formatter)
|
# Under systemd, tag each line so the journal records the real severity
|
||||||
|
# rather than filing everything as informational. The file handler below
|
||||||
|
# keeps the plain formatter: the prefix is meaningful to journald and noise
|
||||||
|
# anywhere else.
|
||||||
|
console_handler.setFormatter(
|
||||||
|
JournalPriorityFormatter(formatter) if _under_systemd() else formatter)
|
||||||
root_logger.addHandler(console_handler)
|
root_logger.addHandler(console_handler)
|
||||||
|
|
||||||
# File handler (if specified)
|
# File handler (if specified)
|
||||||
@@ -145,6 +150,54 @@ def setup_logging(
|
|||||||
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
|
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
|
||||||
|
|
||||||
|
|
||||||
|
#: syslog priorities, which is what systemd parses from a "<N>" prefix on
|
||||||
|
#: stdout. Mapped from Python's levels.
|
||||||
|
_SYSLOG_PRIORITY = {
|
||||||
|
logging.CRITICAL: 2, # LOG_CRIT
|
||||||
|
logging.ERROR: 3, # LOG_ERR
|
||||||
|
logging.WARNING: 4, # LOG_WARNING
|
||||||
|
logging.INFO: 6, # LOG_INFO
|
||||||
|
logging.DEBUG: 7, # LOG_DEBUG
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class JournalPriorityFormatter(logging.Formatter):
|
||||||
|
"""Wraps a formatter, prefixing each line with its syslog priority.
|
||||||
|
|
||||||
|
Under systemd everything this process writes to stdout lands in the journal
|
||||||
|
as PRIORITY=6, whatever the Python level was. Measured on a live rig: 55
|
||||||
|
ERROR lines and 13 WARNING lines in a day, every one of them recorded as
|
||||||
|
informational, so `journalctl -p err -u ledmatrix` returned nothing at all
|
||||||
|
while errors were being logged. Anyone triaging has to grep the message
|
||||||
|
text instead, which is both slower and wrong -- a search for "oom" matches
|
||||||
|
the radar logging "zoom=9".
|
||||||
|
|
||||||
|
systemd reads a leading "<N>" on each line and uses it as the priority
|
||||||
|
(sd-daemon(3)), so this needs no extra dependency. Multi-line records get
|
||||||
|
the prefix on every line, since the journal splits them and an unprefixed
|
||||||
|
continuation would fall back to the default.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, inner: logging.Formatter):
|
||||||
|
super().__init__()
|
||||||
|
self._inner = inner
|
||||||
|
|
||||||
|
def format(self, record: logging.LogRecord) -> str:
|
||||||
|
text = self._inner.format(record)
|
||||||
|
prefix = f"<{_SYSLOG_PRIORITY.get(record.levelno, 6)}>"
|
||||||
|
return "\n".join(prefix + line for line in text.split("\n"))
|
||||||
|
|
||||||
|
|
||||||
|
def _under_systemd() -> bool:
|
||||||
|
"""True when stdout is the journal.
|
||||||
|
|
||||||
|
systemd sets JOURNAL_STREAM for services whose output it captures. Without
|
||||||
|
this check the "<N>" prefixes would show up as literal noise when the
|
||||||
|
program is run from a terminal, in the emulator, or in tests.
|
||||||
|
"""
|
||||||
|
return bool(os.environ.get("JOURNAL_STREAM"))
|
||||||
|
|
||||||
|
|
||||||
class PluginLoggerAdapter(logging.LoggerAdapter):
|
class PluginLoggerAdapter(logging.LoggerAdapter):
|
||||||
"""LoggerAdapter that stamps every record with its plugin_id.
|
"""LoggerAdapter that stamps every record with its plugin_id.
|
||||||
|
|
||||||
|
|||||||
@@ -76,9 +76,6 @@ class PluginManager:
|
|||||||
# Lock protecting plugin_manifests and plugin_directories from
|
# Lock protecting plugin_manifests and plugin_directories from
|
||||||
# concurrent mutation (background reconciliation) and reads (requests).
|
# concurrent mutation (background reconciliation) and reads (requests).
|
||||||
self._discovery_lock = threading.RLock()
|
self._discovery_lock = threading.RLock()
|
||||||
#: Directories already reported as unloadable, so the warning is
|
|
||||||
#: emitted once rather than on every discovery scan.
|
|
||||||
self._skip_reported: set = set()
|
|
||||||
|
|
||||||
# Lock protecting plugin_last_update from concurrent mutation/iteration.
|
# Lock protecting plugin_last_update from concurrent mutation/iteration.
|
||||||
# It's written from run_scheduled_updates()/update_all_plugins() (main
|
# It's written from run_scheduled_updates()/update_all_plugins() (main
|
||||||
@@ -198,45 +195,18 @@ class PluginManager:
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
manifest_path = item / "manifest.json"
|
manifest_path = item / "manifest.json"
|
||||||
if not manifest_path.exists():
|
if manifest_path.exists():
|
||||||
# Once per directory per process. Discovery runs on every
|
try:
|
||||||
# web UI page load and every config reconcile, so warning
|
with open(manifest_path, 'r', encoding='utf-8') as f:
|
||||||
# unconditionally would put a line in the journal each
|
manifest = json.load(f)
|
||||||
# time someone opened a page -- the same log-volume
|
plugin_id = manifest.get('id')
|
||||||
# problem this is meant to help diagnose.
|
if plugin_id:
|
||||||
# A directory here that carries no manifest is not a
|
plugin_ids.append(plugin_id)
|
||||||
# plugin. Said once, because the alternative is a plugin
|
new_manifests[plugin_id] = manifest
|
||||||
# that is enabled in config, enabled in plugin state,
|
new_directories[plugin_id] = item
|
||||||
# present on disk, and simply absent from the running
|
except (json.JSONDecodeError, PermissionError, OSError) as e:
|
||||||
# process with nothing anywhere to say why. Working that
|
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
|
||||||
# out afterwards means reading cache-file mtimes.
|
continue
|
||||||
if item.name not in self._skip_reported:
|
|
||||||
self._skip_reported.add(item.name)
|
|
||||||
self.logger.warning(
|
|
||||||
"Skipping %s: no manifest.json, so it cannot be "
|
|
||||||
"loaded as a plugin", item.name)
|
|
||||||
continue
|
|
||||||
try:
|
|
||||||
with open(manifest_path, 'r', encoding='utf-8') as f:
|
|
||||||
manifest = json.load(f)
|
|
||||||
except (json.JSONDecodeError, PermissionError, OSError) as e:
|
|
||||||
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
|
|
||||||
continue
|
|
||||||
|
|
||||||
plugin_id = manifest.get('id')
|
|
||||||
if not plugin_id:
|
|
||||||
# Parsed but unusable. This was the quietest path of all:
|
|
||||||
# the manifest is read successfully and then dropped.
|
|
||||||
if item.name not in self._skip_reported:
|
|
||||||
self._skip_reported.add(item.name)
|
|
||||||
self.logger.warning(
|
|
||||||
"Skipping %s: its manifest.json has no \"id\", so "
|
|
||||||
"there is nothing to register it under", item.name)
|
|
||||||
continue
|
|
||||||
|
|
||||||
plugin_ids.append(plugin_id)
|
|
||||||
new_manifests[plugin_id] = manifest
|
|
||||||
new_directories[plugin_id] = item
|
|
||||||
except (OSError, PermissionError) as e:
|
except (OSError, PermissionError) as e:
|
||||||
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)
|
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,101 @@
|
|||||||
|
"""Log lines must reach the journal with their real severity.
|
||||||
|
|
||||||
|
Everything this process writes to stdout lands in the journal as PRIORITY=6,
|
||||||
|
whatever the Python level was, because journald has no other signal. Measured
|
||||||
|
on a live rig over 24 hours: 55 lines containing " - ERROR - " and 13
|
||||||
|
containing " - WARNING - ", every one of them recorded as informational. So
|
||||||
|
|
||||||
|
journalctl -p err -u ledmatrix
|
||||||
|
|
||||||
|
returned nothing while errors were being logged, and anyone triaging has to
|
||||||
|
grep the message text instead. That is slower and it is wrong: a search for
|
||||||
|
"oom" also matches the radar logging "zoom=9", which is exactly the false
|
||||||
|
positive it produced during this audit.
|
||||||
|
|
||||||
|
systemd reads a leading "<N>" on each stdout line and uses it as the priority
|
||||||
|
(sd-daemon(3)), so this needs no extra dependency -- and it must only be
|
||||||
|
applied when systemd is actually reading, or the prefixes become literal noise
|
||||||
|
in a terminal, the emulator, and test output.
|
||||||
|
"""
|
||||||
|
import logging
|
||||||
|
import os
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from src.logging_config import JournalPriorityFormatter, _SYSLOG_PRIORITY, _under_systemd
|
||||||
|
|
||||||
|
|
||||||
|
class _Plain(logging.Formatter):
|
||||||
|
def format(self, record):
|
||||||
|
return record.getMessage()
|
||||||
|
|
||||||
|
|
||||||
|
def _record(level, msg="hello"):
|
||||||
|
return logging.LogRecord("t", level, "f.py", 1, msg, None, None)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("level,expected", [
|
||||||
|
(logging.CRITICAL, 2),
|
||||||
|
(logging.ERROR, 3),
|
||||||
|
(logging.WARNING, 4),
|
||||||
|
(logging.INFO, 6),
|
||||||
|
(logging.DEBUG, 7),
|
||||||
|
])
|
||||||
|
def test_each_level_maps_to_its_syslog_priority(level, expected):
|
||||||
|
out = JournalPriorityFormatter(_Plain()).format(_record(level))
|
||||||
|
assert out.startswith(f"<{expected}>"), out
|
||||||
|
assert _SYSLOG_PRIORITY[level] == expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_and_info_are_distinguishable():
|
||||||
|
"""The whole point: journalctl -p err must be able to tell them apart."""
|
||||||
|
fmt = JournalPriorityFormatter(_Plain())
|
||||||
|
assert fmt.format(_record(logging.ERROR))[:3] != fmt.format(_record(logging.INFO))[:3]
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_line_of_a_multiline_record_is_tagged():
|
||||||
|
"""The journal splits them, and an untagged continuation loses its level.
|
||||||
|
|
||||||
|
A traceback is the case that matters -- it is the most important thing in
|
||||||
|
the log and the longest.
|
||||||
|
"""
|
||||||
|
out = JournalPriorityFormatter(_Plain()).format(
|
||||||
|
_record(logging.ERROR, "Traceback:\nline one\nline two"))
|
||||||
|
lines = out.split("\n")
|
||||||
|
assert len(lines) == 3
|
||||||
|
assert all(line.startswith("<3>") for line in lines), lines
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_message_survives_intact():
|
||||||
|
out = JournalPriorityFormatter(_Plain()).format(_record(logging.WARNING, "disk full"))
|
||||||
|
assert out == "<4>disk full"
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_unknown_level_falls_back_to_info():
|
||||||
|
out = JournalPriorityFormatter(_Plain()).format(_record(25))
|
||||||
|
assert out.startswith("<6>")
|
||||||
|
|
||||||
|
|
||||||
|
def test_prefixing_is_off_outside_systemd():
|
||||||
|
"""Otherwise a terminal run, the emulator and pytest all show `<6>`."""
|
||||||
|
with patch.dict(os.environ, {}, clear=True):
|
||||||
|
assert not _under_systemd()
|
||||||
|
with patch.dict(os.environ, {"JOURNAL_STREAM": "8:12345"}):
|
||||||
|
assert _under_systemd()
|
||||||
|
|
||||||
|
|
||||||
|
def test_setup_uses_the_wrapper_only_under_systemd():
|
||||||
|
from src.logging_config import setup_logging
|
||||||
|
|
||||||
|
for env, expect_wrapped in (({}, False), ({"JOURNAL_STREAM": "8:1"}, True)):
|
||||||
|
with patch.dict(os.environ, env, clear=True):
|
||||||
|
setup_logging()
|
||||||
|
handlers = [h for h in logging.getLogger().handlers
|
||||||
|
if isinstance(h, logging.StreamHandler)]
|
||||||
|
assert handlers, "no stream handler installed"
|
||||||
|
wrapped = any(isinstance(h.formatter, JournalPriorityFormatter)
|
||||||
|
for h in handlers)
|
||||||
|
assert wrapped is expect_wrapped, (
|
||||||
|
f"JOURNAL_STREAM={env}: wrapped={wrapped}, expected {expect_wrapped}")
|
||||||
|
logging.getLogger().handlers.clear()
|
||||||
@@ -1,81 +0,0 @@
|
|||||||
#!/usr/bin/env python3
|
|
||||||
"""Discovery must say when it skips a directory.
|
|
||||||
|
|
||||||
A plugin can be enabled in config, enabled in plugin state, present on disk
|
|
||||||
with a valid entry point -- and simply absent from the running process, with
|
|
||||||
nothing in the journal to say why. Working that out afterwards meant comparing
|
|
||||||
cache-file mtimes to find when it had last run.
|
|
||||||
|
|
||||||
Two paths were silent. A directory with no manifest.json was ignored, and --
|
|
||||||
quieter still -- a manifest that parsed but carried no "id" was read
|
|
||||||
successfully and then dropped on the floor.
|
|
||||||
"""
|
|
||||||
import json
|
|
||||||
import logging
|
|
||||||
import sys
|
|
||||||
from pathlib import Path
|
|
||||||
from unittest.mock import MagicMock
|
|
||||||
|
|
||||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
||||||
|
|
||||||
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
|
|
||||||
|
|
||||||
|
|
||||||
def _manager(tmp_path):
|
|
||||||
pm = PluginManager.__new__(PluginManager)
|
|
||||||
pm.plugins_dir = tmp_path
|
|
||||||
pm.logger = logging.getLogger("test.discovery")
|
|
||||||
pm.plugin_manifests = {}
|
|
||||||
pm.plugin_directories = {}
|
|
||||||
pm._discovery_lock = __import__("threading").RLock()
|
|
||||||
pm._skip_reported = set()
|
|
||||||
pm.schema_manager = MagicMock()
|
|
||||||
return pm
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_directory_without_a_manifest_is_reported(tmp_path, caplog):
|
|
||||||
(tmp_path / "not-a-plugin").mkdir()
|
|
||||||
pm = _manager(tmp_path)
|
|
||||||
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
||||||
pm._scan_directory_for_plugins(tmp_path)
|
|
||||||
joined = " ".join(r.message for r in caplog.records)
|
|
||||||
assert "not-a-plugin" in joined and "manifest" in joined, (
|
|
||||||
f"skip was silent; log said: {joined!r}")
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_manifest_without_an_id_is_reported(tmp_path, caplog):
|
|
||||||
d = tmp_path / "idless"
|
|
||||||
d.mkdir()
|
|
||||||
(d / "manifest.json").write_text(json.dumps({"name": "No Id", "version": "1.0.0"}))
|
|
||||||
pm = _manager(tmp_path)
|
|
||||||
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
||||||
pm._scan_directory_for_plugins(tmp_path)
|
|
||||||
joined = " ".join(r.message for r in caplog.records)
|
|
||||||
assert "idless" in joined and "id" in joined, (
|
|
||||||
f"a parsed-but-unusable manifest vanished silently; log said: {joined!r}")
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_good_plugin_still_registers(tmp_path, caplog):
|
|
||||||
d = tmp_path / "real-plugin"
|
|
||||||
d.mkdir()
|
|
||||||
(d / "manifest.json").write_text(json.dumps(
|
|
||||||
{"id": "real-plugin", "name": "Real", "version": "1.0.0"}))
|
|
||||||
pm = _manager(tmp_path)
|
|
||||||
pm._scan_directory_for_plugins(tmp_path)
|
|
||||||
assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"
|
|
||||||
|
|
||||||
|
|
||||||
def test_the_warning_does_not_repeat_on_every_scan(tmp_path, caplog):
|
|
||||||
"""Discovery runs on every web UI page load and every config reconcile.
|
|
||||||
|
|
||||||
Warning unconditionally would put a line in the journal each time someone
|
|
||||||
opened a page -- the same log-volume problem this is meant to help
|
|
||||||
diagnose.
|
|
||||||
"""
|
|
||||||
(tmp_path / "not-a-plugin").mkdir()
|
|
||||||
pm = _manager(tmp_path)
|
|
||||||
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
||||||
for _ in range(5):
|
|
||||||
pm._scan_directory_for_plugins(tmp_path)
|
|
||||||
hits = [r for r in caplog.records if "not-a-plugin" in r.message]
|
|
||||||
assert len(hits) == 1, f"warned {len(hits)} times across 5 scans"
|
|
||||||
Reference in New Issue
Block a user