fix(logging): do not unlink log record factories chained after ours

.changelog/4905.fixed

@@ -0,0 +1 @@
+`opentelemetry-instrumentation-logging`: Keep log record factories chained after ours on uninstrument

instrumentation/opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py

@@ -134,6 +134,7 @@ def log_hook(span: Span, record: LogRecord):
     """
 
     _old_factory = None
+    _our_factory = None
     _log_hook = None
     _logging_handler = None
 
@@ -217,6 +218,7 @@ def record_factory(*args, **kwargs):
 
             return record
 
+        LoggingInstrumentor._our_factory = record_factory
         logging.setLogRecordFactory(record_factory)
 
         # Here we need to handle 3 scenarios:
@@ -265,9 +267,28 @@ def record_factory(*args, **kwargs):
             LoggingInstrumentor._logging_handler = handler
 
     def _uninstrument(self, **kwargs):
-        if LoggingInstrumentor._old_factory:
-            logging.setLogRecordFactory(LoggingInstrumentor._old_factory)
-            LoggingInstrumentor._old_factory = None
+        # `logging.setLogRecordFactory` is a single global slot that callers
+        # chain by closing over whatever factory preceded them. Restoring
+        # `_old_factory` unconditionally would therefore unlink every factory
+        # installed after ours, so only restore while we are still the head of
+        # the chain. Otherwise we leave the chain alone: a node cannot be
+        # removed from the middle without the cooperation of the factory that
+        # wrapped it.
+        if LoggingInstrumentor._our_factory is not None:
+            if (
+                logging.getLogRecordFactory()
+                is LoggingInstrumentor._our_factory
+            ):
+                logging.setLogRecordFactory(LoggingInstrumentor._old_factory)
+            else:
+                _logger.warning(
+                    "Another log record factory was installed after "
+                    "LoggingInstrumentor. Leaving the log record factory chain "
+                    "untouched to avoid unlinking it; log records may continue "
+                    "to carry OpenTelemetry attributes."
+                )
+        LoggingInstrumentor._old_factory = None
+        LoggingInstrumentor._our_factory = None
 
         if LoggingInstrumentor._logging_handler:
             logging.getLogger().removeHandler(

instrumentation/opentelemetry-instrumentation-logging/tests/test_logging.py

@@ -282,6 +282,41 @@ def test_uninstrumented(self):
         ]
         self.assertEqual(logging_handler_instances, [])
 
+    def test_uninstrument_keeps_factories_chained_after_ours(self):
+        # A factory installed after ours must survive uninstrument: the
+        # logging module offers no way to unlink a factory from the middle of
+        # the chain, so restoring the old factory here would silently drop it.
+        chained_onto = logging.getLogRecordFactory()
+
+        def app_factory(*args, **kwargs):
+            record = chained_onto(*args, **kwargs)
+            record.custom_app_attribute = "some-value"
+            return record
+
+        logging.setLogRecordFactory(app_factory)
+        try:
+            LoggingInstrumentor().uninstrument()
+
+            self.assertIs(logging.getLogRecordFactory(), app_factory)
+            with self.caplog.at_level(level=logging.INFO):
+                logging.getLogger("test logger").info("hello")
+                records = [
+                    record
+                    for record in self.caplog.records
+                    if record.name == "test logger"
+                ]
+                self.assertEqual(len(records), 1)
+                self.assertEqual(records[0].custom_app_attribute, "some-value")
+        finally:
+            logging.setLogRecordFactory(chained_onto)
+
+    def test_uninstrument_restores_factory_when_nothing_chained(self):
+        original_factory = LoggingInstrumentor._old_factory
+
+        LoggingInstrumentor().uninstrument()
+
+        self.assertIs(logging.getLogRecordFactory(), original_factory)
+
     @mock.patch("logging.basicConfig")
     def test_no_op_tracer_provider(self, basic_config_mock):
         LoggingInstrumentor().uninstrument()