]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
pybind/mgr/mgr_module: isolate logging per mgr module
authorNitzanMordhai <nmordech@ibm.com>
Thu, 12 Feb 2026 09:13:41 +0000 (09:13 +0000)
committerNitzanMordhai <nmordech@ibm.com>
Tue, 3 Mar 2026 12:24:31 +0000 (12:24 +0000)
After PR #66244, all mgr modules run inside the same Python interpreter.
That means they also share the same logging subsystem.
Previously, each module attached its handlers to the root logger. In practice,
whichever module initialized logging last effectively “owned” the root logger,
and log messages from other modules could end up attributed incorrectly.

This change scopes logging per module. Each module now registers its handlers
on a dedicated logger named after the module itself, with propagate=False to avoid
leaking messages into the root logger.

Now, the getLogger() default (no args) returns the module's named logger
rather than the root logger. This ensures self.log routes correctly.

Fixes: https://tracker.ceph.com/issues/74848
Signed-off-by: Nitzan Mordechai <nmordech@ibm.com>
src/pybind/mgr/mgr_module.py

index aca229dc1a389f676198f1cf9c20f6fc46a561b8..3a7a6a331575192944837ee2d5058f76b700bcdd 100644 (file)
@@ -678,6 +678,17 @@ class CPlusPlusHandler(logging.Handler):
         if record.levelno >= self.level:
             self._module._ceph_log(self.format(record))
 
+class MgrRootHandler(CPlusPlusHandler):
+    def __init__(self, module_inst):
+        super().__init__(module_inst)
+        self.setFormatter(logging.Formatter(
+            "[mgr %(levelname)-4s %(name)s] %(message)s"
+        ))
+
+    def emit(self, record):
+        record.name = "mgr"
+        super().emit(record)
+
 
 class ClusterLogHandler(logging.Handler):
     def __init__(self, module_inst: Any):
@@ -721,7 +732,7 @@ class MgrModuleLoggingMixin(object):
                            log_to_cluster: bool) -> None:
         self._mgr_level: Optional[str] = None
         self._module_level: Optional[str] = None
-        self._root_logger = logging.getLogger()
+        self._module_logger = logging.getLogger(self.module_name)
 
         self._unconfigure_logging()
 
@@ -733,24 +744,29 @@ class MgrModuleLoggingMixin(object):
         self.log_to_file = log_to_file
         self.log_to_cluster = log_to_cluster
 
-        self._root_logger.addHandler(self._mgr_log_handler)
+        root = logging.getLogger()
+        if not any(isinstance(h, MgrRootHandler) for h in root.handlers):
+            root.addHandler(MgrRootHandler(self))
+            root.setLevel(logging.NOTSET)
+
+        self._module_logger.addHandler(self._mgr_log_handler)
         if log_to_file:
-            self._root_logger.addHandler(self._file_log_handler)
+            self._module_logger.addHandler(self._file_log_handler)
         if log_to_cluster:
-            self._root_logger.addHandler(self._cluster_log_handler)
+            self._module_logger.addHandler(self._cluster_log_handler)
 
-        self._root_logger.setLevel(logging.NOTSET)
+        self._module_logger.propagate = False
         self._set_log_level(mgr_level, module_level, cluster_level)
 
     def _unconfigure_logging(self) -> None:
         # remove existing handlers:
         rm_handlers = [
-            h for h in self._root_logger.handlers
+            h for h in self._module_logger.handlers
             if (isinstance(h, CPlusPlusHandler)
                 or isinstance(h, FileHandler)
                 or isinstance(h, ClusterLogHandler))]
         for h in rm_handlers:
-            self._root_logger.removeHandler(h)
+            self._module_logger.removeHandler(h)
         self.log_to_file = False
         self.log_to_cluster = False
 
@@ -793,25 +809,25 @@ class MgrModuleLoggingMixin(object):
         # enable file log
         self.getLogger().warning("enabling logging to file")
         self.log_to_file = True
-        self._root_logger.addHandler(self._file_log_handler)
+        self._module_logger.addHandler(self._file_log_handler)
 
     def _disable_file_log(self) -> None:
         # disable file log
         self.getLogger().warning("disabling logging to file")
         self.log_to_file = False
-        self._root_logger.removeHandler(self._file_log_handler)
+        self._module_logger.removeHandler(self._file_log_handler)
 
     def _enable_cluster_log(self) -> None:
         # enable cluster log
         self.getLogger().warning("enabling logging to cluster")
         self.log_to_cluster = True
-        self._root_logger.addHandler(self._cluster_log_handler)
+        self._module_logger.addHandler(self._cluster_log_handler)
 
     def _disable_cluster_log(self) -> None:
         # disable cluster log
         self.getLogger().warning("disabling logging to cluster")
         self.log_to_cluster = False
-        self._root_logger.removeHandler(self._cluster_log_handler)
+        self._module_logger.removeHandler(self._cluster_log_handler)
 
     def _ceph_log_level_to_python(self, log_level: str) -> str:
         if log_level:
@@ -831,9 +847,10 @@ class MgrModuleLoggingMixin(object):
             log_level = "INFO"
         return log_level
 
-    def getLogger(self, name: Optional[str] = None) -> logging.Logger:
-        return logging.getLogger(name)
-
+    def getLogger(self, name=None):
+        if name is None:
+            return self._module_logger
+        return self._module_logger.getChild(name)
 
 class MgrStandbyModule(ceph_module.BaseMgrStandbyModule, MgrModuleLoggingMixin):
     """