mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
feat: component-separated logging with session context and filtering (#7991)
* feat: component-separated logging with session context and filtering Phase 1 — Gateway log isolation: - gateway.log now only receives records from gateway.* loggers (platform adapters, session management, slash commands, delivery) - agent.log remains the catch-all (all components) - errors.log remains WARNING+ catch-all - Moved gateway.log handler creation from gateway/run.py into hermes_logging.setup_logging(mode='gateway') with _ComponentFilter Phase 2 — Session ID injection: - Added set_session_context(session_id) / clear_session_context() API using threading.local() for per-thread session tracking - _SessionFilter enriches every log record with session_tag attribute - Log format: '2026-04-11 10:23:45 INFO [session_id] logger.name: msg' - Session context set at start of run_conversation() in run_agent.py - Thread-isolated: gateway conversations on different threads don't leak Phase 3 — Component filtering in hermes logs: - Added --component flag: hermes logs --component gateway|agent|tools|cli|cron - COMPONENT_PREFIXES maps component names to logger name prefixes - Works with all existing filters (--level, --session, --since, -f) - Logger name extraction handles both old and new log formats Files changed: - hermes_logging.py: _SessionFilter, _ComponentFilter, COMPONENT_PREFIXES, set/clear_session_context(), gateway.log creation in setup_logging() - gateway/run.py: removed redundant gateway.log handler (now in hermes_logging) - run_agent.py: set_session_context() at start of run_conversation() - hermes_cli/logs.py: --component filter, logger name extraction - hermes_cli/main.py: --component argument on logs subparser Addresses community request for component-separated, filterable logging. Zero changes to existing logger names — __name__ already provides hierarchy. * fix: use LogRecord factory instead of per-handler _SessionFilter The _SessionFilter approach required attaching a filter to every handler we create. Any handler created outside our _add_rotating_handler (like the gateway stderr handler, or third-party handlers) would crash with KeyError: 'session_tag' if it used our format string. Replace with logging.setLogRecordFactory() which injects session_tag into every LogRecord at creation time — process-global, zero per-handler wiring needed. The factory is installed at import time (before setup_logging) so session_tag is available from the moment hermes_logging is imported. - Idempotent: marker attribute prevents double-wrapping on module reload - Chains with existing factory: won't break third-party record factories - Removes _SessionFilter from _add_rotating_handler and setup_verbose_logging - Adds tests: record factory injection, idempotency, arbitrary handler compat
This commit is contained in:
parent
723b5bec85
commit
fd73937ec8
7 changed files with 728 additions and 230 deletions
|
|
@ -3,6 +3,7 @@
|
|||
import logging
|
||||
import os
|
||||
import stat
|
||||
import threading
|
||||
from logging.handlers import RotatingFileHandler
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
|
@ -34,6 +35,8 @@ def _reset_logging_state():
|
|||
h.close()
|
||||
else:
|
||||
pre_existing.append(h)
|
||||
# Ensure the record factory is installed (it's idempotent).
|
||||
hermes_logging._install_session_record_factory()
|
||||
yield
|
||||
# Restore — remove any handlers added during the test.
|
||||
for h in list(root.handlers):
|
||||
|
|
@ -41,6 +44,7 @@ def _reset_logging_state():
|
|||
root.removeHandler(h)
|
||||
h.close()
|
||||
hermes_logging._logging_initialized = False
|
||||
hermes_logging.clear_session_context()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
|
@ -220,6 +224,294 @@ class TestSetupLogging:
|
|||
]
|
||||
assert agent_handlers[0].level == logging.WARNING
|
||||
|
||||
def test_record_factory_installed(self, hermes_home):
|
||||
"""The custom record factory injects session_tag on all records."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
factory = logging.getLogRecordFactory()
|
||||
assert getattr(factory, "_hermes_session_injector", False), (
|
||||
"Record factory should have _hermes_session_injector marker"
|
||||
)
|
||||
# Verify session_tag exists on a fresh record
|
||||
record = factory("test", logging.INFO, "", 0, "msg", (), None)
|
||||
assert hasattr(record, "session_tag")
|
||||
|
||||
|
||||
class TestGatewayMode:
|
||||
"""setup_logging(mode='gateway') creates a filtered gateway.log."""
|
||||
|
||||
def test_gateway_log_created(self, hermes_home):
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home, mode="gateway")
|
||||
root = logging.getLogger()
|
||||
|
||||
gw_handlers = [
|
||||
h for h in root.handlers
|
||||
if isinstance(h, RotatingFileHandler)
|
||||
and "gateway.log" in getattr(h, "baseFilename", "")
|
||||
]
|
||||
assert len(gw_handlers) == 1
|
||||
|
||||
def test_gateway_log_not_created_in_cli_mode(self, hermes_home):
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home, mode="cli")
|
||||
root = logging.getLogger()
|
||||
|
||||
gw_handlers = [
|
||||
h for h in root.handlers
|
||||
if isinstance(h, RotatingFileHandler)
|
||||
and "gateway.log" in getattr(h, "baseFilename", "")
|
||||
]
|
||||
assert len(gw_handlers) == 0
|
||||
|
||||
def test_gateway_log_receives_gateway_records(self, hermes_home):
|
||||
"""gateway.log captures records from gateway.* loggers."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home, mode="gateway")
|
||||
|
||||
gw_logger = logging.getLogger("gateway.platforms.telegram")
|
||||
gw_logger.info("telegram connected")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
gw_log = hermes_home / "logs" / "gateway.log"
|
||||
assert gw_log.exists()
|
||||
assert "telegram connected" in gw_log.read_text()
|
||||
|
||||
def test_gateway_log_rejects_non_gateway_records(self, hermes_home):
|
||||
"""gateway.log does NOT capture records from tools.*, agent.*, etc."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home, mode="gateway")
|
||||
|
||||
tool_logger = logging.getLogger("tools.terminal_tool")
|
||||
tool_logger.info("running command")
|
||||
|
||||
agent_logger = logging.getLogger("agent.context_compressor")
|
||||
agent_logger.info("compressing context")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
gw_log = hermes_home / "logs" / "gateway.log"
|
||||
if gw_log.exists():
|
||||
content = gw_log.read_text()
|
||||
assert "running command" not in content
|
||||
assert "compressing context" not in content
|
||||
|
||||
def test_agent_log_still_receives_all(self, hermes_home):
|
||||
"""agent.log (catch-all) still receives gateway AND tool records."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home, mode="gateway")
|
||||
|
||||
logging.getLogger("gateway.run").info("gateway msg")
|
||||
logging.getLogger("tools.file_tools").info("file msg")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
agent_log = hermes_home / "logs" / "agent.log"
|
||||
content = agent_log.read_text()
|
||||
assert "gateway msg" in content
|
||||
assert "file msg" in content
|
||||
|
||||
|
||||
class TestSessionContext:
|
||||
"""set_session_context / clear_session_context + _SessionFilter."""
|
||||
|
||||
def test_session_tag_in_log_output(self, hermes_home):
|
||||
"""When session context is set, log lines include [session_id]."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
hermes_logging.set_session_context("abc123")
|
||||
|
||||
test_logger = logging.getLogger("test.session_tag")
|
||||
test_logger.info("tagged message")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
agent_log = hermes_home / "logs" / "agent.log"
|
||||
content = agent_log.read_text()
|
||||
assert "[abc123]" in content
|
||||
assert "tagged message" in content
|
||||
|
||||
def test_no_session_tag_without_context(self, hermes_home):
|
||||
"""Without session context, log lines have no session tag."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
hermes_logging.clear_session_context()
|
||||
|
||||
test_logger = logging.getLogger("test.no_session")
|
||||
test_logger.info("untagged message")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
agent_log = hermes_home / "logs" / "agent.log"
|
||||
content = agent_log.read_text()
|
||||
assert "untagged message" in content
|
||||
# Should not have any [xxx] session tag
|
||||
import re
|
||||
for line in content.splitlines():
|
||||
if "untagged message" in line:
|
||||
assert not re.search(r"\[.+?\]", line.split("INFO")[1].split("test.no_session")[0])
|
||||
|
||||
def test_clear_session_context(self, hermes_home):
|
||||
"""After clearing, session tag disappears."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
hermes_logging.set_session_context("xyz789")
|
||||
hermes_logging.clear_session_context()
|
||||
|
||||
test_logger = logging.getLogger("test.cleared")
|
||||
test_logger.info("after clear")
|
||||
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
agent_log = hermes_home / "logs" / "agent.log"
|
||||
content = agent_log.read_text()
|
||||
assert "[xyz789]" not in content
|
||||
|
||||
def test_session_context_thread_isolated(self, hermes_home):
|
||||
"""Session context is per-thread — one thread's context doesn't leak."""
|
||||
hermes_logging.setup_logging(hermes_home=hermes_home)
|
||||
|
||||
results = {}
|
||||
|
||||
def thread_a():
|
||||
hermes_logging.set_session_context("thread_a_session")
|
||||
logging.getLogger("test.thread_a").info("from thread A")
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
def thread_b():
|
||||
hermes_logging.set_session_context("thread_b_session")
|
||||
logging.getLogger("test.thread_b").info("from thread B")
|
||||
for h in logging.getLogger().handlers:
|
||||
h.flush()
|
||||
|
||||
ta = threading.Thread(target=thread_a)
|
||||
tb = threading.Thread(target=thread_b)
|
||||
ta.start()
|
||||
ta.join()
|
||||
tb.start()
|
||||
tb.join()
|
||||
|
||||
agent_log = hermes_home / "logs" / "agent.log"
|
||||
content = agent_log.read_text()
|
||||
|
||||
# Each thread's message should have its own session tag
|
||||
for line in content.splitlines():
|
||||
if "from thread A" in line:
|
||||
assert "[thread_a_session]" in line
|
||||
assert "[thread_b_session]" not in line
|
||||
if "from thread B" in line:
|
||||
assert "[thread_b_session]" in line
|
||||
assert "[thread_a_session]" not in line
|
||||
|
||||
|
||||
class TestRecordFactory:
|
||||
"""Unit tests for the custom LogRecord factory."""
|
||||
|
||||
def test_record_has_session_tag(self):
|
||||
"""Every record gets a session_tag attribute."""
|
||||
factory = logging.getLogRecordFactory()
|
||||
record = factory("test", logging.INFO, "", 0, "msg", (), None)
|
||||
assert hasattr(record, "session_tag")
|
||||
|
||||
def test_empty_tag_without_context(self):
|
||||
hermes_logging.clear_session_context()
|
||||
factory = logging.getLogRecordFactory()
|
||||
record = factory("test", logging.INFO, "", 0, "msg", (), None)
|
||||
assert record.session_tag == ""
|
||||
|
||||
def test_tag_with_context(self):
|
||||
hermes_logging.set_session_context("sess_42")
|
||||
factory = logging.getLogRecordFactory()
|
||||
record = factory("test", logging.INFO, "", 0, "msg", (), None)
|
||||
assert record.session_tag == " [sess_42]"
|
||||
|
||||
def test_idempotent_install(self):
|
||||
"""Calling _install_session_record_factory() twice doesn't double-wrap."""
|
||||
hermes_logging._install_session_record_factory()
|
||||
factory_a = logging.getLogRecordFactory()
|
||||
hermes_logging._install_session_record_factory()
|
||||
factory_b = logging.getLogRecordFactory()
|
||||
assert factory_a is factory_b
|
||||
|
||||
def test_works_with_any_handler(self):
|
||||
"""A handler using %(session_tag)s works even without _SessionFilter."""
|
||||
hermes_logging.set_session_context("any_handler_test")
|
||||
handler = logging.StreamHandler()
|
||||
handler.setFormatter(logging.Formatter("%(session_tag)s %(message)s"))
|
||||
|
||||
logger = logging.getLogger("_test_any_handler")
|
||||
logger.addHandler(handler)
|
||||
logger.setLevel(logging.DEBUG)
|
||||
try:
|
||||
# Should not raise KeyError
|
||||
logger.info("hello")
|
||||
finally:
|
||||
logger.removeHandler(handler)
|
||||
|
||||
|
||||
class TestComponentFilter:
|
||||
"""Unit tests for _ComponentFilter."""
|
||||
|
||||
def test_passes_matching_prefix(self):
|
||||
f = hermes_logging._ComponentFilter(("gateway",))
|
||||
record = logging.LogRecord(
|
||||
"gateway.run", logging.INFO, "", 0, "msg", (), None
|
||||
)
|
||||
assert f.filter(record) is True
|
||||
|
||||
def test_passes_nested_matching_prefix(self):
|
||||
f = hermes_logging._ComponentFilter(("gateway",))
|
||||
record = logging.LogRecord(
|
||||
"gateway.platforms.telegram", logging.INFO, "", 0, "msg", (), None
|
||||
)
|
||||
assert f.filter(record) is True
|
||||
|
||||
def test_blocks_non_matching(self):
|
||||
f = hermes_logging._ComponentFilter(("gateway",))
|
||||
record = logging.LogRecord(
|
||||
"tools.terminal_tool", logging.INFO, "", 0, "msg", (), None
|
||||
)
|
||||
assert f.filter(record) is False
|
||||
|
||||
def test_multiple_prefixes(self):
|
||||
f = hermes_logging._ComponentFilter(("agent", "run_agent", "model_tools"))
|
||||
assert f.filter(logging.LogRecord(
|
||||
"agent.compressor", logging.INFO, "", 0, "", (), None
|
||||
))
|
||||
assert f.filter(logging.LogRecord(
|
||||
"run_agent", logging.INFO, "", 0, "", (), None
|
||||
))
|
||||
assert f.filter(logging.LogRecord(
|
||||
"model_tools", logging.INFO, "", 0, "", (), None
|
||||
))
|
||||
assert not f.filter(logging.LogRecord(
|
||||
"tools.browser", logging.INFO, "", 0, "", (), None
|
||||
))
|
||||
|
||||
|
||||
class TestComponentPrefixes:
|
||||
"""COMPONENT_PREFIXES covers the expected components."""
|
||||
|
||||
def test_gateway_prefix(self):
|
||||
assert "gateway" in hermes_logging.COMPONENT_PREFIXES
|
||||
assert ("gateway",) == hermes_logging.COMPONENT_PREFIXES["gateway"]
|
||||
|
||||
def test_agent_prefix(self):
|
||||
prefixes = hermes_logging.COMPONENT_PREFIXES["agent"]
|
||||
assert "agent" in prefixes
|
||||
assert "run_agent" in prefixes
|
||||
assert "model_tools" in prefixes
|
||||
|
||||
def test_tools_prefix(self):
|
||||
assert ("tools",) == hermes_logging.COMPONENT_PREFIXES["tools"]
|
||||
|
||||
def test_cli_prefix(self):
|
||||
prefixes = hermes_logging.COMPONENT_PREFIXES["cli"]
|
||||
assert "hermes_cli" in prefixes
|
||||
assert "cli" in prefixes
|
||||
|
||||
def test_cron_prefix(self):
|
||||
assert ("cron",) == hermes_logging.COMPONENT_PREFIXES["cron"]
|
||||
|
||||
|
||||
class TestSetupVerboseLogging:
|
||||
"""setup_verbose_logging() adds a DEBUG-level console handler."""
|
||||
|
|
@ -301,6 +593,59 @@ class TestAddRotatingHandler:
|
|||
logger.removeHandler(h)
|
||||
h.close()
|
||||
|
||||
def test_log_filter_attached(self, tmp_path):
|
||||
"""Optional log_filter is attached to the handler."""
|
||||
log_path = tmp_path / "filtered.log"
|
||||
logger = logging.getLogger("_test_rotating_filter")
|
||||
formatter = logging.Formatter("%(message)s")
|
||||
component_filter = hermes_logging._ComponentFilter(("test",))
|
||||
|
||||
hermes_logging._add_rotating_handler(
|
||||
logger, log_path,
|
||||
level=logging.INFO, max_bytes=1024, backup_count=1,
|
||||
formatter=formatter,
|
||||
log_filter=component_filter,
|
||||
)
|
||||
|
||||
handlers = [h for h in logger.handlers if isinstance(h, RotatingFileHandler)]
|
||||
assert len(handlers) == 1
|
||||
assert component_filter in handlers[0].filters
|
||||
# Clean up
|
||||
for h in list(logger.handlers):
|
||||
if isinstance(h, RotatingFileHandler):
|
||||
logger.removeHandler(h)
|
||||
h.close()
|
||||
|
||||
def test_no_session_filter_on_handler(self, tmp_path):
|
||||
"""Handlers rely on record factory, not per-handler _SessionFilter."""
|
||||
log_path = tmp_path / "no_session_filter.log"
|
||||
logger = logging.getLogger("_test_no_session_filter")
|
||||
formatter = logging.Formatter("%(session_tag)s%(message)s")
|
||||
|
||||
hermes_logging._add_rotating_handler(
|
||||
logger, log_path,
|
||||
level=logging.INFO, max_bytes=1024, backup_count=1,
|
||||
formatter=formatter,
|
||||
)
|
||||
|
||||
handlers = [h for h in logger.handlers if isinstance(h, RotatingFileHandler)]
|
||||
assert len(handlers) == 1
|
||||
# No _SessionFilter on the handler — record factory handles it
|
||||
assert len(handlers[0].filters) == 0
|
||||
|
||||
# But session_tag still works (via record factory)
|
||||
hermes_logging.set_session_context("factory_test")
|
||||
logger.info("test msg")
|
||||
handlers[0].flush()
|
||||
content = log_path.read_text()
|
||||
assert "[factory_test]" in content
|
||||
|
||||
# Clean up
|
||||
for h in list(logger.handlers):
|
||||
if isinstance(h, RotatingFileHandler):
|
||||
logger.removeHandler(h)
|
||||
h.close()
|
||||
|
||||
def test_managed_mode_initial_open_sets_group_writable(self, tmp_path):
|
||||
log_path = tmp_path / "managed-open.log"
|
||||
logger = logging.getLogger("_test_rotating_managed_open")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue