Fixes for httpx bodies (#842)

pydantic · Feb 5, 2025 · 5303947 · 5303947
1 parent 0f01a66
commit 5303947
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 11 deletions.
diff --git a/logfire/_internal/integrations/httpx.py b/logfire/_internal/integrations/httpx.py
@@ -80,6 +80,15 @@ def instrument_httpx(
     should_capture_request_body = capture_request_body or capture_all
     should_capture_response_body = capture_response_body or capture_all
 
+    del (  # Make sure these aren't used accidentally
+        capture_all,
+        capture_headers,
+        capture_request_body,
+        capture_response_body,
+        capture_request_headers,
+        capture_response_headers,
+    )
+
     final_kwargs: dict[str, Any] = {
         'tracer_provider': logfire_instance.config.get_tracer_provider(),
         'meter_provider': logfire_instance.config.get_meter_provider(),
@@ -94,7 +103,7 @@ def instrument_httpx(
         request_hook = cast('RequestHook | None', request_hook)
         response_hook = cast('ResponseHook | None', response_hook)
         final_kwargs['request_hook'] = make_request_hook(
-            request_hook, should_capture_request_headers, capture_request_body
+            request_hook, should_capture_request_headers, should_capture_request_body
         )
         final_kwargs['response_hook'] = make_response_hook(
             response_hook,
@@ -167,11 +176,13 @@ def capture_body(self):
 
     def capture_body_if_text(self, attr_name: str = 'http.request.body.text'):
         if not self.body_is_streaming:
-            try:
-                text = self.content.decode(self.content_type_charset)
-            except (UnicodeDecodeError, LookupError):
-                return
-            self.capture_text_as_json(attr_name=attr_name, text=text)
+            content = self.content
+            if content:
+                try:
+                    text = self.content.decode(self.content_type_charset)
+                except (UnicodeDecodeError, LookupError):
+                    return
+                self.capture_text_as_json(attr_name=attr_name, text=text)
 
     def capture_body_if_form(self, attr_name: str = 'http.request.body.form') -> bool:
         if not self.content_type_header_string == 'application/x-www-form-urlencoded':
@@ -235,8 +246,6 @@ def hook(span: LogfireSpan):
                 return
             self.capture_text_as_json(span, attr_name=attr_name, text=text)
 
-            span.set_attribute(attr_name, text)
-
         self.on_response_read(hook)
 
     @cached_property

diff --git a/tests/otel_integrations/test_httpx.py b/tests/otel_integrations/test_httpx.py
@@ -400,7 +400,7 @@ def test_httpx_client_capture_full(exporter: TestExporter):
                     'logfire.msg': 'Reading response body',
                     'logfire.span_type': 'span',
                     'http.response.body.text': '{"good": "response"}',
-                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{}}}',
+                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{"type":"object"}}}',
                 },
             },
             {
@@ -489,7 +489,7 @@ async def test_async_httpx_client_capture_full(exporter: TestExporter):
                     'logfire.msg': 'Reading response body',
                     'logfire.span_type': 'span',
                     'http.response.body.text': '{"good": "response"}',
-                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{}}}',
+                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{"type":"object"}}}',
                 },
             },
             {
@@ -738,7 +738,7 @@ async def test_httpx_client_capture_all(exporter: TestExporter):
                     'logfire.msg': 'Reading response body',
                     'logfire.span_type': 'span',
                     'http.response.body.text': '{"good": "response"}',
-                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{}}}',
+                    'logfire.json_schema': '{"type":"object","properties":{"http.response.body.text":{"type":"object"}}}',
                 },
             },
             {
@@ -760,6 +760,40 @@ async def test_httpx_client_capture_all(exporter: TestExporter):
     )
 
 
+async def test_httpx_client_no_capture_empty_body(exporter: TestExporter):
+    async with httpx.AsyncClient(transport=create_transport()) as client:
+        logfire.instrument_httpx(client, capture_request_body=True)
+        await client.get('https://example.org/')
+
+    assert exporter.exported_spans_as_dict() == snapshot(
+        [
+            {
+                'name': 'GET',
+                'context': {'trace_id': 1, 'span_id': 1, 'is_remote': False},
+                'parent': None,
+                'start_time': 1000000000,
+                'end_time': 2000000000,
+                'attributes': {
+                    'http.method': 'GET',
+                    'http.request.method': 'GET',
+                    'http.url': 'https://example.org/',
+                    'url.full': 'https://example.org/',
+                    'http.host': 'example.org',
+                    'server.address': 'example.org',
+                    'network.peer.address': 'example.org',
+                    'logfire.span_type': 'span',
+                    'logfire.msg': 'GET /',
+                    'http.status_code': 200,
+                    'http.response.status_code': 200,
+                    'http.flavor': '1.1',
+                    'network.protocol.version': '1.1',
+                    'http.target': '/',
+                },
+            }
+        ]
+    )
+
+
 def test_httpx_capture_all_and_other_flags_should_warn(exporter: TestExporter):
     with httpx.Client(transport=create_transport()) as client:
         with pytest.warns(