Repository: getsentry/sentry-python

Issue:
Transport causes CPU Spikes on Larger Envelopes
### How do you use Sentry?

Sentry Saas (sentry.io)

### Version

1.31

### Steps to Reproduce

Sending this to Sentry causes the transport to create CPU spikes that are very visible in our kubernetes pods.

```python
from sentry_sdk import capture_message, push_scope

for _ in range(50):
    with push_scope() as scope:
        scope.add_attachment(bytes=b"Hello World! " * 10000, filename="attachment.txt")
        capture_message('Hello, hello.')
```

It also happens if gzip compression is disabled. It appears to be related to how `capture_envelope` works within the transport.

### Expected Result

I would not expect the CPU usage to spike.

### Actual Result

It spikes.

Discussion:
sl0thentr0py: cc @untitaker for a quick sanity check if you see problems with changing this stuff 
untitaker: I think you want `drain_conn` instead of `release_conn`. If I understood the documentation correctly, you are putting a TCP connection back into the pool while the response body still has to be read.

I would guess that if you start sending new requests into that connection, the server may choose to terminate the connection because from their POV you're misbehaving. Even if that happens, urllib3 _may_ transparently recover from that scenario by creating a new connection, but I am not sure. You'd have to test what actually happens. I suggest spinning up a http server locally and looking at wireshark (open it, look at loopback device lo0 and apply "http" filter)

I don't exactly understand what `preload_conn` does, but from what I can tell the original `True` value would've prevented this issue because it just behaves like "draining".

This assumes http1, i have no idea what happens with http2. I also just barely skimmed over the urllib3 docs for this, I have never heard of those specific parameters before, so take it with a grain of salt.
antonpirker: Looking at wireshark was a good idea @untitaker. When I set `preload_content/release_conn` for every HTTP post a new TCP connection is established. This not what we want. 

I will just revert the preload_content stuff and make the pool size configurable. Let's see if this helps.

Patch:
diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py
index 2b0bd571..5bc3e2aa 100644
--- a/sentry_sdk/consts.py
+++ b/sentry_sdk/consts.py
@@ -43,6 +43,7 @@ if TYPE_CHECKING:
             "profiler_mode": Optional[ProfilerMode],
             "otel_powered_performance": Optional[bool],
             "transport_zlib_compression_level": Optional[int],
+            "transport_num_pools": Optional[int],
             "enable_metrics": Optional[bool],
             "before_emit_metric": Optional[Callable[[str, MetricTags], bool]],
         },
diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py
index 12343fed..4b12287e 100644
--- a/sentry_sdk/transport.py
+++ b/sentry_sdk/transport.py
@@ -157,6 +157,14 @@ class HttpTransport(Transport):
         )  # type: DefaultDict[Tuple[str, str], int]
         self._last_client_report_sent = time.time()
 
+        compresslevel = options.get("_experiments", {}).get(
+            "transport_zlib_compression_level"
+        )
+        self._compresslevel = 9 if compresslevel is None else int(compresslevel)
+
+        num_pools = options.get("_experiments", {}).get("transport_num_pools")
+        self._num_pools = 2 if num_pools is None else int(num_pools)
+
         self._pool = self._make_pool(
             self.parsed_dsn,
             http_proxy=options["http_proxy"],
@@ -165,11 +173,6 @@ class HttpTransport(Transport):
             proxy_headers=options["proxy_headers"],
         )
 
-        compresslevel = options.get("_experiments", {}).get(
-            "transport_zlib_compression_level"
-        )
-        self._compresslevel = 9 if compresslevel is None else int(compresslevel)
-
         from sentry_sdk import Hub
 
         self.hub_cls = Hub
@@ -439,7 +442,7 @@ class HttpTransport(Transport):
     def _get_pool_options(self, ca_certs):
         # type: (Optional[Any]) -> Dict[str, Any]
         return {
-            "num_pools": 2,
+            "num_pools": self._num_pools,
             "cert_reqs": "CERT_REQUIRED",
             "ca_certs": ca_certs or certifi.where(),
         }


Test patch:
diff --git a/tests/test_transport.py b/tests/test_transport.py
index befba3c9..602f7843 100644
--- a/tests/test_transport.py
+++ b/tests/test_transport.py
@@ -132,6 +132,25 @@ def test_transport_works(
     assert any("Sending event" in record.msg for record in caplog.records) == debug
 
 
+@pytest.mark.parametrize(
+    "num_pools,expected_num_pools",
+    (
+        (None, 2),
+        (2, 2),
+        (10, 10),
+    ),
+)
+def test_transport_num_pools(make_client, num_pools, expected_num_pools):
+    _experiments = {}
+    if num_pools is not None:
+        _experiments["transport_num_pools"] = num_pools
+
+    client = make_client(_experiments=_experiments)
+
+    options = client.transport._get_pool_options([])
+    assert options["num_pools"] == expected_num_pools
+
+
 def test_transport_infinite_loop(capturing_server, request, make_client):
     client = make_client(
         debug=True,