From f0efef4cb376c72736968fd4d4a4f5787d3f5174 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Mon, 3 Feb 2025 10:24:58 +0000 Subject: [PATCH] more resilient console logging (#831) Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com> Co-authored-by: Alex Hall --- logfire/_internal/exporters/console.py | 18 ++++++------ tests/test_console_exporter.py | 40 ++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/logfire/_internal/exporters/console.py b/logfire/_internal/exporters/console.py index 21d431549..60c9a4630 100644 --- a/logfire/_internal/exporters/console.py +++ b/logfire/_internal/exporters/console.py @@ -182,23 +182,23 @@ def _details_parts(self, span: ReadableSpan, indent_str: str) -> TextParts: if not self._verbose or not span.attributes: return [] - file_location: str = span.attributes.get('code.filepath') # type: ignore + file_location_raw = span.attributes.get('code.filepath') + file_location = None if file_location_raw is None else str(file_location_raw) if file_location: lineno = span.attributes.get('code.lineno') if lineno: # pragma: no branch file_location += f':{lineno}' log_level_num: int = span.attributes.get(ATTRIBUTES_LOG_LEVEL_NUM_KEY) # type: ignore - log_level = NUMBER_TO_LEVEL.get(log_level_num, '') + log_level = NUMBER_TO_LEVEL.get(log_level_num) if file_location or log_level: - return [ - (indent_str, ''), - ('│', 'blue'), - (' ', ''), - (file_location, 'cyan'), - (f' {log_level}', ''), - ] + parts: TextParts = [(indent_str, ''), ('│', 'blue')] + if file_location: + parts.append((f' {file_location}', 'cyan')) + if log_level: + parts.append((f' {log_level}', '')) + return parts else: return [] diff --git a/tests/test_console_exporter.py b/tests/test_console_exporter.py index 638ac1261..663b00758 100644 --- a/tests/test_console_exporter.py +++ b/tests/test_console_exporter.py @@ -106,7 +106,7 @@ def test_simple_console_exporter_no_colors_verbose(simple_spans: list[ReadableSp [ '00:00:01.000 rootSpan', '00:00:02.000 childSpan 1', - ' │ testing.py:42 ', + ' │ testing.py:42', ] ) @@ -419,7 +419,7 @@ def test_verbose_attributes(exporter: TestExporter) -> None: assert out.getvalue().splitlines() == snapshot( [ '\x1b[32m00:00:01.000\x1b[0m Hello world!', - ' \x1b[34m│\x1b[0m \x1b[36mtest_console_exporter.py:123\x1b[0m info', + ' \x1b[34m│\x1b[0m\x1b[36m test_console_exporter.py:123\x1b[0m info', " \x1b[34m│ \x1b[0m\x1b[34mname=\x1b[0m\x1b[93;49m'\x1b[0m\x1b[93;49mworld\x1b[0m\x1b[93;49m'\x1b[0m", ' \x1b[34m│ \x1b[0m\x1b[34md=\x1b[0m\x1b[97;49m{\x1b[0m ', " \x1b[34m│ \x1b[0m \x1b[97;49m \x1b[0m\x1b[93;49m'\x1b[0m\x1b[93;49ma\x1b[0m\x1b[93;49m'\x1b[0m\x1b[97;49m:\x1b[0m\x1b[97;49m \x1b[0m\x1b[37;49m1\x1b[0m\x1b[97;49m,\x1b[0m", @@ -796,3 +796,39 @@ def test_exception(exporter: TestExporter) -> None: '\x1b[0m\x1b[97;49mby\x1b[0m\x1b[97;49m \x1b[0m\x1b[97;49mzero\x1b[0m', '', ] + + +def test_console_exporter_invalid_text(capsys: pytest.CaptureFixture[str]) -> None: + logfire.configure( + send_to_logfire=False, + console=ConsoleOptions(colors='always', include_timestamps=False, verbose=True), + ) + + logfire.info('hi', **{'code.filepath': 3, 'code.lineno': None}) # type: ignore + logfire.info('hi', **{'code.filepath': None, 'code.lineno': 'foo'}) # type: ignore + assert capsys.readouterr().out.splitlines() == snapshot( + [ + 'hi', + '\x1b[34m│\x1b[0m\x1b[36m 3\x1b[0m info', + 'hi', + '\x1b[34m│\x1b[0m info', + ] + ) + + +def test_console_exporter_invalid_text_no_color(capsys: pytest.CaptureFixture[str]) -> None: + logfire.configure( + send_to_logfire=False, + console=ConsoleOptions(colors='never', include_timestamps=False, verbose=True), + ) + + logfire.info('hi', **{'code.filepath': 3, 'code.lineno': None}) # type: ignore + logfire.info('hi', **{'code.filepath': None, 'code.lineno': 'foo'}) # type: ignore + assert capsys.readouterr().out.splitlines() == snapshot( + [ + 'hi', + '│ 3 info', + 'hi', + '│ info', + ] + )