Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add optional keep_alive #2842

Merged
merged 16 commits into from
Mar 20, 2024
1 change: 1 addition & 0 deletions sentry_sdk/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
27 changes: 26 additions & 1 deletion sentry_sdk/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io
import gzip
import socket
import time
from datetime import timedelta
from collections import defaultdict
Expand Down Expand Up @@ -40,6 +41,21 @@
from urllib import getproxies # type: ignore


KEEP_ALIVE_SOCKET_OPTIONS = []
for option in [
(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]))
except AttributeError:

Check warning on line 53 in sentry_sdk/transport.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/transport.py#L53

Added line #L53 was not covered by tests
# a specific option might not be available on specific systems,
# e.g. TCP_KEEPIDLE doesn't exist on macOS
pass

Check warning on line 56 in sentry_sdk/transport.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/transport.py#L56

Added line #L56 was not covered by tests


class Transport(object):
"""Baseclass for all transports.

Expand Down Expand Up @@ -446,9 +462,18 @@
"ca_certs": ca_certs or certifi.where(),
}

if self.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"] = []

Check warning on line 470 in sentry_sdk/transport.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/transport.py#L470

Added line #L470 was not covered by tests

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)

Check warning on line 475 in sentry_sdk/transport.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/transport.py#L475

Added line #L475 was not covered by tests

return options

def _in_no_proxy(self, parsed_dsn):
Expand Down
54 changes: 53 additions & 1 deletion tests/test_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -167,6 +167,58 @@ def test_socket_options(make_client):
assert options["socket_options"] == socket_options


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),
(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_socket_options_merge_with_keep_alive(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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing TCP_KEEPIDLE probably breaks CI (because is linux)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably also in other tests missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, fixed now

]


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.
client = make_client(socket_options=[])

options = client.transport._get_pool_options([])
assert options["socket_options"] == []


def test_transport_infinite_loop(capturing_server, request, make_client):
client = make_client(
debug=True,
Expand Down
Loading