]> 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)
committerPatrick Donnelly <pdonnell@ibm.com>
Fri, 8 May 2026 19:38:59 +0000 (15:38 -0400)
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>
(cherry picked from commit a89e1760566138fbfc8159d5ab3c01a61f8ae415)

src/pybind/mgr/mgr_module.py

index 368639587928a1369a8eb79ac0c842a04c37cdf1..78f172636522564385a617017edd21a40ec1005a 100644 (file)
@@ -677,6 +677,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):
@@ -720,7 +731,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()
 
@@ -732,24 +743,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
 
@@ -792,25 +808,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:
@@ -830,9 +846,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):
     """