fix errorhandling of LoggingHandler.emit

CHANGELOG.asciidoc

@@ -47,7 +47,7 @@ endif::[]
 * Fix return for `opentelemetry.Span.is_recording()` {pull}1530[#1530]
 * Fix error logging for bad SERVICE_NAME config {pull}1546[#1546]
 * Do not instrument old versions of Tornado > 6.0 due to incompatibility {pull}1566[#1566]
-
+* Fix a problem with our logging handler failing to report internal errors in its emitter {pull}1568[#1568]
 
 [[release-notes-6.x]]
 === Python Agent version 6.x

elasticapm/handlers/logbook.py

@@ -75,15 +75,15 @@ def emit(self, record):
 
         # Avoid typical config issues by overriding loggers behavior
         if record.channel.startswith("elasticapm.errors"):
-            sys.stderr.write(to_unicode(record.message + "\n"))
+            sys.stderr.write(to_unicode(record.message) + "\n")
             return
 
         try:
             return self._emit(record)
         except Exception:
             sys.stderr.write("Top level ElasticAPM exception caught - failed creating log record.\n")
-            sys.stderr.write(to_unicode(record.msg + "\n"))
-            sys.stderr.write(to_unicode(traceback.format_exc() + "\n"))
+            sys.stderr.write(to_unicode(record.message) + "\n")
+            sys.stderr.write(traceback.format_exc() + "\n")
 
             try:
                 self.client.capture("Exception")

elasticapm/handlers/logging.py

@@ -40,7 +40,6 @@
 from elasticapm.base import Client
 from elasticapm.traces import execution_context
 from elasticapm.utils import wrapt
-from elasticapm.utils.encoding import to_unicode
 from elasticapm.utils.stacks import iter_stack_frames
 
 
@@ -74,15 +73,15 @@ def emit(self, record):
 
         # Avoid typical config issues by overriding loggers behavior
         if record.name.startswith(("elasticapm.errors",)):
-            sys.stderr.write(to_unicode(record.message) + "\n")
+            sys.stderr.write(record.getMessage() + "\n")
             return
 
         try:
             return self._emit(record)
         except Exception:
             sys.stderr.write("Top level ElasticAPM exception caught - failed creating log record.\n")
-            sys.stderr.write(to_unicode(record.msg + "\n"))
-            sys.stderr.write(to_unicode(traceback.format_exc() + "\n"))
+            sys.stderr.write(record.getMessage() + "\n")
+            sys.stderr.write(traceback.format_exc() + "\n")
 
             try:
                 self.client.capture("Exception")

tests/handlers/logbook/logbook_tests.py

@@ -169,6 +169,15 @@ def test_logbook_handler_emit_error(capsys, elasticapm_client):
     assert "Oops" in err
 
 
+def test_logbook_handler_emit_error_non_str_message(capsys, elasticapm_client):
+    handler = LogbookHandler(elasticapm_client)
+    handler._emit = lambda record: 1 / 0
+    handler.emit(LogRecord("x", 1, ValueError("oh no")))
+    out, err = capsys.readouterr()
+    assert "Top level ElasticAPM exception caught" in err
+    assert "oh no" in err
+
+
 def test_logbook_handler_dont_emit_elasticapm(capsys, elasticapm_client):
     handler = LogbookHandler(elasticapm_client)
     handler.emit(LogRecord("elasticapm.errors", 1, "Oops"))

tests/handlers/logging/logging_tests.py

@@ -229,19 +229,28 @@ def test_logger_setup():
 def test_logging_handler_emit_error(capsys, elasticapm_client):
     handler = LoggingHandler(elasticapm_client)
     handler._emit = lambda: 1 / 0
-    handler.emit(LogRecord("x", 1, "/ab/c/", 10, "Oops", [], None))
+    handler.emit(LogRecord("x", 1, "/ab/c/", 10, "Oops", (), None))
     out, err = capsys.readouterr()
     assert "Top level ElasticAPM exception caught" in err
     assert "Oops" in err
 
 
 def test_logging_handler_dont_emit_elasticapm(capsys, elasticapm_client):
     handler = LoggingHandler(elasticapm_client)
-    handler.emit(LogRecord("elasticapm.errors", 1, "/ab/c/", 10, "Oops", [], None))
+    handler.emit(LogRecord("elasticapm.errors", 1, "/ab/c/", 10, "Oops", (), None))
     out, err = capsys.readouterr()
     assert "Oops" in err
 
 
+def test_logging_handler_emit_error_non_str_message(capsys, elasticapm_client):
+    handler = LoggingHandler(elasticapm_client)
+    handler._emit = lambda: 1 / 0
+    handler.emit(LogRecord("x", 1, "/ab/c/", 10, ValueError("oh no"), (), None))
+    out, err = capsys.readouterr()
+    assert "Top level ElasticAPM exception caught" in err
+    assert "oh no" in err
+
+
 def test_arbitrary_object(logger):
     logger.error(["a", "list", "of", "strings"])
     assert len(logger.client.events) == 1