From b09324ac5c220d9f9ca1c5b8cdfed33ceb4ba5ae Mon Sep 17 00:00:00 2001 From: wunder957 Date: Thu, 31 Aug 2023 09:44:57 +0800 Subject: [PATCH] Improve CLI and monitor on error (#29) * Improve CLI and monitor on error * Add dev-tools for debugging * Improve debug tools and log * Fix testing * Improve config and check repo config uptodate --- .gitignore | 4 +++- dev-tools/entrypoint.py | 20 ++++++++++++++++++ duetector/cli/main.py | 13 ++++++++---- duetector/config.py | 3 ++- duetector/managers/tracer.py | 2 +- duetector/monitors/base.py | 6 +++++- duetector/monitors/bcc_monitor.py | 13 +++++++++--- duetector/monitors/sh_monitor.py | 3 --- duetector/tools/config_generator.py | 7 +++++-- tests/test_bcc_monitor.py | 2 +- tests/test_config.py | 1 - tests/test_repo.py | 32 +++++++++++++++++++++++++++++ 12 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 dev-tools/entrypoint.py create mode 100644 tests/test_repo.py diff --git a/.gitignore b/.gitignore index 712b8f6..8cbd122 100644 --- a/.gitignore +++ b/.gitignore @@ -570,4 +570,6 @@ dmypy.json cython_debug/ .idea/** -duetector-dbcollector.sqlite3 +duetector-dbcollector.sqlite3* +dev-tools/duetector-dbcollector.sqlite3* +dev-tools/config.toml diff --git a/dev-tools/entrypoint.py b/dev-tools/entrypoint.py new file mode 100644 index 0000000..e638dff --- /dev/null +++ b/dev-tools/entrypoint.py @@ -0,0 +1,20 @@ +import os + +os.chdir(os.path.dirname(os.path.abspath(__file__))) +os.environ["DUETECTOR_LOG_LEVEL"] = "DEBUG" + +import re +import sys +from pathlib import Path + +from pkg_resources import load_entry_point + +db_file = Path("./duetector-dbcollector.sqlite3") +config_file = Path("./config.toml") + +if __name__ == "__main__": + db_file.unlink(missing_ok=True) + sys.argv[0] = re.sub(r"(-script\.pyw?|\.exe)?$", "", sys.argv[0]) + sys.argv.append("start") + sys.argv.extend(["--config", config_file.resolve().as_posix()]) + sys.exit(load_entry_point("duetector", "console_scripts", "duectl")()) diff --git a/duetector/cli/main.py b/duetector/cli/main.py index 295e515..106f4ec 100644 --- a/duetector/cli/main.py +++ b/duetector/cli/main.py @@ -48,6 +48,9 @@ def generate_dynamic_config(load_current_config, path, load_env, dump_path): c = ConfigGenerator(load=load_current_config, path=path, load_env=load_env) if path.as_posix() == Path(dump_path).expanduser().absolute().as_posix(): + logger.info( + f"Dump path is same as origin path, rename {path} to {path.with_suffix('.old')}" + ) shutil.move(path, path.with_suffix(".old")) c.generate(dump_path) @@ -143,16 +146,18 @@ def start( m.start_polling() def _shutdown(sig=None, frame=None): + logger.info("Exiting...") for m in monitors: m.shutdown() + for m in monitors: + logger.info(m.summary()) + exit(0) signal.signal(signal.SIGINT, _shutdown) signal.signal(signal.SIGTERM, _shutdown) logger.info("Waiting for KeyboardInterrupt or SIGTERM...") - signal.pause() - logger.info("Exiting...Get summary...") - for m in monitors: - logger.info(m.summary()) + while True: + signal.pause() @click.group() diff --git a/duetector/config.py b/duetector/config.py index 3f9c989..1263a9c 100644 --- a/duetector/config.py +++ b/duetector/config.py @@ -1,3 +1,4 @@ +import copy import os import shutil from pathlib import Path @@ -167,7 +168,7 @@ def __init__(self, config: Optional[Union[Config, Dict[str, Any]]] = None, *args if self.config_scope: for score in self.config_scope.split("."): config = config.get(score.lower(), {}) - c = self.default_config.copy() + c = copy.deepcopy(self.default_config) def _recursive_update(c, config): for k, v in config.items(): diff --git a/duetector/managers/tracer.py b/duetector/managers/tracer.py index ae480ed..dabbf5a 100644 --- a/duetector/managers/tracer.py +++ b/duetector/managers/tracer.py @@ -40,7 +40,7 @@ def init(self, tracer_type=Tracer, ignore_disabled=True) -> List[Tracer]: objs = [] for f in self.pm.hook.init_tracer(config=self.config.config_dict): if not f or (f.disabled and ignore_disabled): - logger.info(f"Tracer {f.__class__.__name__} is not available (None or Disabled)") + logger.debug(f"Tracer {f.__class__.__name__} is not available (None or Disabled)") continue if not isinstance(f, tracer_type): logger.debug( diff --git a/duetector/monitors/base.py b/duetector/monitors/base.py index a62489c..4437290 100644 --- a/duetector/monitors/base.py +++ b/duetector/monitors/base.py @@ -55,7 +55,11 @@ def poll(self, tracer: Tracer): raise NotImplementedError def summary(self) -> Dict: - return {collector.__class__.__name__: collector.summary() for collector in self.collectors} + return { + self.__class__.__name__: { + collector.__class__.__name__: collector.summary() for collector in self.collectors + } + } def start_polling(self): logger.info(f"Start polling {self.__class__.__name__}") diff --git a/duetector/monitors/bcc_monitor.py b/duetector/monitors/bcc_monitor.py index a6d4dae..54fd315 100644 --- a/duetector/monitors/bcc_monitor.py +++ b/duetector/monitors/bcc_monitor.py @@ -52,6 +52,8 @@ def init(self): # Prevrent ImportError for CI testing without bcc from bcc import BPF # noqa + err_tracers = [] + for tracer in self.tracers: try: bpf = BPF(text=tracer.prog) @@ -59,6 +61,10 @@ def init(self): logger.error(f"Failed to compile {tracer.__class__.__name__}") logger.exception(e) if self.continue_on_exception: + logger.info( + f"Continuing on exception. {tracer.__class__.__name__} will be disabled." + ) + err_tracers.append(tracer) continue else: raise e @@ -67,6 +73,10 @@ def init(self): self.bpf_tracers[tracer] = bpf logger.info(f"Tracer {tracer.__class__.__name__} attached") + # Remove tracers that failed to compile + for tracer in err_tracers: + self.tracers.remove(tracer) + def _set_callback(self, host, tracer): def _(data): for filter in self.filters: @@ -81,9 +91,6 @@ def _(data): def poll(self, tracer: BccTracer): # type: ignore tracer.get_poller(self.bpf_tracers[tracer])(**tracer.poll_args) - def summary(self): - return {collector.__class__.__name__: collector.summary() for collector in self.collectors} - if __name__ == "__main__": m = BccMonitor() diff --git a/duetector/monitors/sh_monitor.py b/duetector/monitors/sh_monitor.py index 451192c..328c406 100644 --- a/duetector/monitors/sh_monitor.py +++ b/duetector/monitors/sh_monitor.py @@ -120,9 +120,6 @@ def poll_all(self): def poll(self, tracer: ShellTracer): # type: ignore return self.host.poll(tracer) - def summary(self): - return {collector.__class__.__name__: collector.summary() for collector in self.collectors} - if __name__ == "__main__": m = ShMonitor() diff --git a/duetector/tools/config_generator.py b/duetector/tools/config_generator.py index 2973e7f..7a0efe0 100644 --- a/duetector/tools/config_generator.py +++ b/duetector/tools/config_generator.py @@ -10,6 +10,10 @@ def _recursive_load(config_scope: str, config_dict: dict, default_config: dict): + """ + Support .(dot) separated config_scope + + """ *prefix, config_scope = config_scope.lower().split(".") last = config_dict for p in prefix: @@ -48,7 +52,6 @@ def __init__(self, load=True, path=None, load_env=True): c.default_config, ) - # Support .(dot) separated config_scope for m in self.monitors: _recursive_load(m.config_scope, self.dynamic_config, m.default_config) @@ -80,6 +83,6 @@ def generate(self, dump_path): if __name__ == "__main__": _HERE = Path(__file__).parent - c = ConfigGenerator(load=False) + c = ConfigGenerator(load=False, load_env=False) config_path = _HERE / ".." / "static/config.toml" c.generate(config_path) diff --git a/tests/test_bcc_monitor.py b/tests/test_bcc_monitor.py index 920ea8e..8c0818d 100644 --- a/tests/test_bcc_monitor.py +++ b/tests/test_bcc_monitor.py @@ -110,7 +110,7 @@ def test_bcc_monitor(bcc_monitor: MockMonitor): bcc_monitor.poll_all() bcc_monitor.shutdown() assert bcc_monitor.summary() - bcc_monitor.summary()["DBCollector"]["BccMockTracer"]["last"] == Tracking( + bcc_monitor.summary()["MockMonitor"]["DBCollector"]["BccMockTracer"]["last"] == Tracking( tracer="BccMockTracer", pid=9999, uid=9999, diff --git a/tests/test_config.py b/tests/test_config.py index 0cd57c2..edefc03 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -69,7 +69,6 @@ def test_load_env(config_loader: ConfigLoader, monkeypatch): def test_generate(config_generator: ConfigGenerator, tmpdir): - tmpdir.join("config-generated.toml") generated_file = tmpdir.join("config-generated.toml") config_generator.generate(generated_file) assert generated_file.exists() diff --git a/tests/test_repo.py b/tests/test_repo.py new file mode 100644 index 0000000..cf47522 --- /dev/null +++ b/tests/test_repo.py @@ -0,0 +1,32 @@ +from pathlib import Path + +import pytest +import tomli + +from duetector.config import ConfigLoader +from duetector.tools.config_generator import ConfigGenerator + +_HERE = Path(__file__).parent.absolute() + + +def test_repo_config_uptodate(tmpdir): + # Check default config in repo is up to date + CONFIG_IN_REPO = _HERE / ".." / "duetector/static/config.toml" + GENERATED_CONFIG = tmpdir.join("g-default-config.toml") + config_generator = ConfigGenerator(load=False, load_env=False) + config_generator.generate(GENERATED_CONFIG) + assert GENERATED_CONFIG.exists() + + generated_config = tomli.loads(GENERATED_CONFIG.read_text(encoding="utf-8")) + repo_config = ConfigLoader( + path=CONFIG_IN_REPO, + load_env=False, + dump_when_load=False, + config_dump_dir=None, + generate_config=False, + ).load_config() + assert generated_config == repo_config + + +if __name__ == "__main__": + pytest.main(["-vv", "-s", __file__])