From 1219279718f56ebdce4b6e27d8bbc25a2b755a99 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 18 Mar 2024 16:40:22 +0100 Subject: [PATCH 01/13] feat: Add optional keep_alive --- sentry_sdk/consts.py | 1 + sentry_sdk/transport.py | 15 +++++++++++++++ tests/test_transport.py | 22 +++++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 83076c762f..6af08b4a40 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -264,6 +264,7 @@ def __init__( ignore_errors=[], # type: Sequence[Union[type, str]] # noqa: B006 max_request_body_size="medium", # type: str socket_options=None, # type: Optional[List[Tuple[int, int, int | bytes]]] + keep_alive=False, # type: bool before_send=None, # type: Optional[EventProcessor] before_breadcrumb=None, # type: Optional[BreadcrumbProcessor] debug=None, # type: Optional[bool] diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index b924ae502a..0bce0c8763 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -2,6 +2,7 @@ import io import gzip +import socket import time from datetime import timedelta from collections import defaultdict @@ -40,6 +41,18 @@ from urllib import getproxies # type: ignore +KEEP_ALIVE_SOCKET_OPTIONS = [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 10), + (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), +] +try: + KEEP_ALIVE_SOCKET_OPTIONS.append((socket.SOL_TCP, socket.TCP_KEEPIDLE, 45)) +except AttributeError: + # socket.TCP_KEEPIDLE doesn't exist on all systems (e.g. MacOS) + pass + + class Transport(object): """Baseclass for all transports. @@ -448,6 +461,8 @@ def _get_pool_options(self, ca_certs): if self.options["socket_options"]: options["socket_options"] = self.options["socket_options"] + elif self.options["keep_alive"]: + options["socket_options"] = KEEP_ALIVE_SOCKET_OPTIONS return options diff --git a/tests/test_transport.py b/tests/test_transport.py index aa471b9081..29704c45cb 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -13,7 +13,7 @@ from sentry_sdk import Hub, Client, add_breadcrumb, capture_message, Scope from sentry_sdk._compat import datetime_utcnow -from sentry_sdk.transport import _parse_rate_limits +from sentry_sdk.transport import KEEP_ALIVE_SOCKET_OPTIONS, _parse_rate_limits from sentry_sdk.envelope import Envelope, parse_json from sentry_sdk.integrations.logging import LoggingIntegration @@ -167,6 +167,26 @@ def test_socket_options(make_client): assert options["socket_options"] == socket_options +def test_keep_alive(make_client): + client = make_client(keep_alive=True) + + options = client.transport._get_pool_options([]) + assert options["socket_options"] == KEEP_ALIVE_SOCKET_OPTIONS + + +def test_socket_options_override_keep_alive(make_client): + socket_options = [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 10), + (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), + ] + + client = make_client(socket_options=socket_options, keep_alive=False) + + options = client.transport._get_pool_options([]) + assert options["socket_options"] == socket_options + + def test_transport_infinite_loop(capturing_server, request, make_client): client = make_client( debug=True, From 4a91a220ecacf951c865404766220f9b5cd13e92 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 12:04:23 +0100 Subject: [PATCH 02/13] guard against any unknown option --- sentry_sdk/transport.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 0bce0c8763..d83e9338dd 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -41,16 +41,19 @@ from urllib import getproxies # type: ignore -KEEP_ALIVE_SOCKET_OPTIONS = [ +KEEP_ALIVE_SOCKET_OPTIONS = [] +for option in [ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), + (socket.SOL_TCP, socket.TCP_KEEPIDLE, 45), (socket.SOL_TCP, socket.TCP_KEEPINTVL, 10), (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), -] -try: - KEEP_ALIVE_SOCKET_OPTIONS.append((socket.SOL_TCP, socket.TCP_KEEPIDLE, 45)) -except AttributeError: - # socket.TCP_KEEPIDLE doesn't exist on all systems (e.g. MacOS) - pass +]: + try: + KEEP_ALIVE_SOCKET_OPTIONS.append(option) + except AttributeError: + # a specific option might not be available on specific systems, + # e.g. TCP_KEEPIDLE doesn't exist on macOS + pass class Transport(object): From d71232406bc55228338fbdf52a7739e9cab346a8 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 12:16:53 +0100 Subject: [PATCH 03/13] fix --- sentry_sdk/transport.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index d83e9338dd..51278213cb 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -43,13 +43,13 @@ KEEP_ALIVE_SOCKET_OPTIONS = [] for option in [ - (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), - (socket.SOL_TCP, socket.TCP_KEEPIDLE, 45), - (socket.SOL_TCP, socket.TCP_KEEPINTVL, 10), - (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), + (socket.SOL_SOCKET, lambda: getattr(socket, "SO_KEEPALIVE"), 1), + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPIDLE"), 45), + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPINTVL"), 10), + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPCNT"), 6), ]: try: - KEEP_ALIVE_SOCKET_OPTIONS.append(option) + KEEP_ALIVE_SOCKET_OPTIONS.append((option[0], option[1](), option[2])) except AttributeError: # a specific option might not be available on specific systems, # e.g. TCP_KEEPIDLE doesn't exist on macOS From becdef6009d7e350bf3c98b61b08ad82e2a3dd0a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 12:34:52 +0100 Subject: [PATCH 04/13] ignore flake warnings --- sentry_sdk/transport.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 51278213cb..bc0d82cbda 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -43,10 +43,10 @@ KEEP_ALIVE_SOCKET_OPTIONS = [] for option in [ - (socket.SOL_SOCKET, lambda: getattr(socket, "SO_KEEPALIVE"), 1), - (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPIDLE"), 45), - (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPINTVL"), 10), - (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPCNT"), 6), + (socket.SOL_SOCKET, lambda: getattr(socket, "SO_KEEPALIVE"), 1), # noqa: B009 + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPIDLE"), 45), # noqa: B009 + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPINTVL"), 10), # noqa: B009 + (socket.SOL_TCP, lambda: getattr(socket, "TCP_KEEPCNT"), 6), # noqa: B009 ]: try: KEEP_ALIVE_SOCKET_OPTIONS.append((option[0], option[1](), option[2])) From b2c689b8654b6ddf896f47198fc5e8a8c276f142 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 12:47:46 +0100 Subject: [PATCH 05/13] add another small test --- tests/test_transport.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_transport.py b/tests/test_transport.py index 29704c45cb..d6a453d4a0 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -187,6 +187,13 @@ def test_socket_options_override_keep_alive(make_client): assert options["socket_options"] == socket_options +def test_keep_alive_off_by_default(make_client): + client = make_client() + + options = client.transport._get_pool_options([]) + assert options["socket_options"] != KEEP_ALIVE_SOCKET_OPTIONS + + def test_transport_infinite_loop(capturing_server, request, make_client): client = make_client( debug=True, From 4449fa6e452a3257277cc94cd97e35b7b637a215 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 12:57:49 +0100 Subject: [PATCH 06/13] fix --- tests/test_transport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_transport.py b/tests/test_transport.py index d6a453d4a0..cbaf857cc3 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -191,7 +191,7 @@ def test_keep_alive_off_by_default(make_client): client = make_client() options = client.transport._get_pool_options([]) - assert options["socket_options"] != KEEP_ALIVE_SOCKET_OPTIONS + assert "socket_options" not in options def test_transport_infinite_loop(capturing_server, request, make_client): From 19534decf4512c7db132d6fd07c42c408a847930 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:19:38 +0100 Subject: [PATCH 07/13] allow to override keep_alive with socket_options --- sentry_sdk/transport.py | 15 ++++++++++++--- tests/test_transport.py | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index bc0d82cbda..6df3c310e1 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -462,10 +462,19 @@ def _get_pool_options(self, ca_certs): "ca_certs": ca_certs or certifi.where(), } + socket_options = [] + if self.options["socket_options"]: - options["socket_options"] = self.options["socket_options"] - elif self.options["keep_alive"]: - options["socket_options"] = KEEP_ALIVE_SOCKET_OPTIONS + socket_options = self.options["socket_options"] + + if self.options["keep_alive"]: + used_options = {(o[0], o[1]) for o in socket_options} + for default_option in KEEP_ALIVE_SOCKET_OPTIONS: + if (default_option[0], default_option[1]) not in used_options: + socket_options.append(default_option) + + if socket_options: + options["socket_options"] = socket_options return options diff --git a/tests/test_transport.py b/tests/test_transport.py index cbaf857cc3..0070d4adb9 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -187,6 +187,22 @@ def test_socket_options_override_keep_alive(make_client): assert options["socket_options"] == socket_options +def test_socket_options_merge_keep_alive_with_socket_options(make_client): + socket_options = [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), + ] + + client = make_client(socket_options=socket_options, keep_alive=True) + + options = client.transport._get_pool_options([]) + assert options["socket_options"] == [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), + (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), + ] + + def test_keep_alive_off_by_default(make_client): client = make_client() From db3dcbac928ea865bc5c330f1151412833816ac5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:25:16 +0100 Subject: [PATCH 08/13] fix: account for empty socket_options --- sentry_sdk/transport.py | 10 +++------- tests/test_transport.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 6df3c310e1..f872a5a99d 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -462,19 +462,15 @@ def _get_pool_options(self, ca_certs): "ca_certs": ca_certs or certifi.where(), } - socket_options = [] - if self.options["socket_options"]: socket_options = self.options["socket_options"] + options["socket_options"] = socket_options if self.options["keep_alive"]: - used_options = {(o[0], o[1]) for o in socket_options} + used_options = {(o[0], o[1]) for o in options["socket_options"] or []} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: if (default_option[0], default_option[1]) not in used_options: - socket_options.append(default_option) - - if socket_options: - options["socket_options"] = socket_options + options["socket_options"].append(default_option) return options diff --git a/tests/test_transport.py b/tests/test_transport.py index 0070d4adb9..4b7ad9b81d 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -187,7 +187,7 @@ def test_socket_options_override_keep_alive(make_client): assert options["socket_options"] == socket_options -def test_socket_options_merge_keep_alive_with_socket_options(make_client): +def test_socket_options_merge_with_keep_alive(make_client): socket_options = [ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), @@ -203,6 +203,18 @@ def test_socket_options_merge_keep_alive_with_socket_options(make_client): ] +def test_socket_options_override_defaults(make_client): + # If socket_options are set to [], this doesn't mean the user doesn't want + # any custom socket_options, but rather that they want to disable the urllib3 + # socket option defaults, so we need to set this and not ignore it. + socket_options = [] + + client = make_client(socket_options=socket_options, keep_alive=True) + + options = client.transport._get_pool_options([]) + assert options["socket_options"] == [] + + def test_keep_alive_off_by_default(make_client): client = make_client() From 15129354e805d67f42ee19a861bea4a3dfc7f16a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:26:00 +0100 Subject: [PATCH 09/13] another fix --- sentry_sdk/transport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index f872a5a99d..1f9d0d797b 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -467,7 +467,7 @@ def _get_pool_options(self, ca_certs): options["socket_options"] = socket_options if self.options["keep_alive"]: - used_options = {(o[0], o[1]) for o in options["socket_options"] or []} + used_options = {(o[0], o[1]) for o in options.get("socket_options") or []} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: if (default_option[0], default_option[1]) not in used_options: options["socket_options"].append(default_option) From da7c3a721587ef93d158cad316093baaca8bfcbd Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:32:59 +0100 Subject: [PATCH 10/13] another fix --- sentry_sdk/transport.py | 8 +++++--- tests/test_transport.py | 19 ++++++++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 1f9d0d797b..20a9b2d285 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -462,11 +462,13 @@ def _get_pool_options(self, ca_certs): "ca_certs": ca_certs or certifi.where(), } - if self.options["socket_options"]: - socket_options = self.options["socket_options"] - options["socket_options"] = socket_options + if self.options["socket_options"] is not None: + options["socket_options"] = self.options["socket_options"] if self.options["keep_alive"]: + if not options.get("socket_options"): + options["socket_options"] = [] + used_options = {(o[0], o[1]) for o in options.get("socket_options") or []} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: if (default_option[0], default_option[1]) not in used_options: diff --git a/tests/test_transport.py b/tests/test_transport.py index 4b7ad9b81d..a566f8342c 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -167,13 +167,19 @@ def test_socket_options(make_client): assert options["socket_options"] == socket_options -def test_keep_alive(make_client): +def test_keep_alive_true(make_client): client = make_client(keep_alive=True) options = client.transport._get_pool_options([]) assert options["socket_options"] == KEEP_ALIVE_SOCKET_OPTIONS +def test_keep_alive_off_by_default(make_client): + client = make_client() + options = client.transport._get_pool_options([]) + assert "socket_options" not in options + + def test_socket_options_override_keep_alive(make_client): socket_options = [ (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1), @@ -207,21 +213,12 @@ def test_socket_options_override_defaults(make_client): # If socket_options are set to [], this doesn't mean the user doesn't want # any custom socket_options, but rather that they want to disable the urllib3 # socket option defaults, so we need to set this and not ignore it. - socket_options = [] - - client = make_client(socket_options=socket_options, keep_alive=True) + client = make_client(socket_options=[]) options = client.transport._get_pool_options([]) assert options["socket_options"] == [] -def test_keep_alive_off_by_default(make_client): - client = make_client() - - options = client.transport._get_pool_options([]) - assert "socket_options" not in options - - def test_transport_infinite_loop(capturing_server, request, make_client): client = make_client( debug=True, From d9a72426706d48cd0ae073c7936d38136c9c0830 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:35:22 +0100 Subject: [PATCH 11/13] simplify --- sentry_sdk/transport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 20a9b2d285..18ac04a45b 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -469,7 +469,7 @@ def _get_pool_options(self, ca_certs): if not options.get("socket_options"): options["socket_options"] = [] - used_options = {(o[0], o[1]) for o in options.get("socket_options") or []} + used_options = {(o[0], o[1]) for o in options["socket_options"]} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: if (default_option[0], default_option[1]) not in used_options: options["socket_options"].append(default_option) From feedc2c73cf5cb41056ce52e8290a3a51c952326 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 14:50:13 +0100 Subject: [PATCH 12/13] make mypy happy --- sentry_sdk/transport.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 18ac04a45b..9ea9cd0c98 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -22,6 +22,7 @@ from typing import Callable from typing import Dict from typing import Iterable + from typing import List from typing import Optional from typing import Tuple from typing import Type @@ -462,17 +463,22 @@ def _get_pool_options(self, ca_certs): "ca_certs": ca_certs or certifi.where(), } + socket_options = None # type: Optional[List[Tuple[int, int, int | bytes]]] + if self.options["socket_options"] is not None: - options["socket_options"] = self.options["socket_options"] + socket_options = self.options["socket_options"] if self.options["keep_alive"]: - if not options.get("socket_options"): - options["socket_options"] = [] + if socket_options is None: + socket_options = [] - used_options = {(o[0], o[1]) for o in options["socket_options"]} + used_options = {(o[0], o[1]) for o in socket_options} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: if (default_option[0], default_option[1]) not in used_options: - options["socket_options"].append(default_option) + socket_options.append(default_option) + + if socket_options is not None: + options["socket_options"] = socket_options return options From edc8b92e22fc4a2a013a4c19ca4f66971dd5e71f Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 19 Mar 2024 15:01:57 +0100 Subject: [PATCH 13/13] fix tests --- tests/test_transport.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/test_transport.py b/tests/test_transport.py index a566f8342c..c1f70b0108 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -202,11 +202,19 @@ def test_socket_options_merge_with_keep_alive(make_client): client = make_client(socket_options=socket_options, keep_alive=True) options = client.transport._get_pool_options([]) - assert options["socket_options"] == [ - (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), - (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), - (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), - ] + try: + assert options["socket_options"] == [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), + (socket.SOL_TCP, socket.TCP_KEEPIDLE, 45), + (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), + ] + except AttributeError: + assert options["socket_options"] == [ + (socket.SOL_SOCKET, socket.SO_KEEPALIVE, 42), + (socket.SOL_TCP, socket.TCP_KEEPINTVL, 42), + (socket.SOL_TCP, socket.TCP_KEEPCNT, 6), + ] def test_socket_options_override_defaults(make_client):