Skip recording exceptions on NonRecordingSpan (#1497)

Author: alexmojaki

logfire/_internal/main.py

@@ -758,6 +758,9 @@ def log(
                 start_time=start_time,
             )
 
+            if not span.is_recording():
+                return
+
             if exc_info:
                 if exc_info is True:
                     exc_info = sys.exc_info()
@@ -2479,10 +2482,6 @@ def record_exception(
         if self._span is None:
             raise RuntimeError('Span has not been started')
 
-        # Check if the span has been sampled out first, since _record_exception is somewhat expensive.
-        if not self._span.is_recording():
-            return
-
         self._span.record_exception(
             exception,
             attributes=attributes,

logfire/_internal/tracer.py

@@ -422,6 +422,9 @@ def record_exception(
     """Similar to the OTEL SDK Span.record_exception method, with our own additions."""
     from ..types import ExceptionCallbackHelper
 
+    if not span.is_recording():
+        return
+
     if is_starlette_http_exception(exception):
         if 400 <= exception.status_code < 500:
             # Don't mark 4xx HTTP exceptions as errors, they are expected to happen in normal operation.

tests/test_exceptions.py

@@ -349,3 +349,19 @@ def exception_callback(helper: ExceptionCallbackHelper) -> None:
             }
         ]
     )
+
+
+def test_nonrecording_span(exporter: TestExporter, config_kwargs: dict[str, Any]):
+    config_kwargs['sampling'] = logfire.SamplingOptions(head=0)
+    logfire.configure(**config_kwargs)
+
+    with pytest.raises(ValueError):
+        with logfire.span('span') as span:
+            try:
+                raise ValueError('test')
+            except ValueError as e:
+                span.record_exception(e)
+                logfire.exception('span exception')
+                raise
+
+    assert exporter.exported_spans_as_dict() == snapshot([])