Skip to content

breaking(transport): Make HTTP2Transport the default #4492

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

Draft
wants to merge 4 commits into
base: potel-base
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion sentry_sdk/consts.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class CompressionAlgo(Enum):
"transport_compression_level": Optional[int],
"transport_compression_algo": Optional[CompressionAlgo],
"transport_num_pools": Optional[int],
"transport_http2": Optional[bool],
"enable_logs": Optional[bool],
"before_send_log": Optional[Callable[[Log, Hint], Optional[Log]]],
},
Expand Down Expand Up @@ -662,6 +661,7 @@ def __init__(
custom_repr=None, # type: Optional[Callable[..., Optional[str]]]
add_full_stack=DEFAULT_ADD_FULL_STACK, # type: bool
max_stack_frames=DEFAULT_MAX_STACK_FRAMES, # type: Optional[int]
http2=None, # type: Optional[bool]
):
# type: (...) -> None
"""Initialize the Sentry SDK with the given parameters. All parameters described here can be used in a call to `sentry_sdk.init()`.
Expand Down Expand Up @@ -1024,6 +1024,8 @@ def __init__(
This is relative to the tracing sample rate - e.g. `0.5` means 50% of sampled transactions will be
profiled.

:param http2: Defaults to `True`, enables HTTP/2 support for the SDK.

:param profiles_sampler:

:param profiler_mode:
Expand Down
7 changes: 6 additions & 1 deletion sentry_sdk/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,12 @@ def make_transport(options):
# type: (Dict[str, Any]) -> Optional[Transport]
ref_transport = options["transport"]

use_http2_transport = options.get("_experiments", {}).get("transport_http2", False)
# We default to using HTTP2 transport if the user also has the required h2
# library installed (through the subclass check). The reason is h2 not being
# available on py3.7 which we still support.
use_http2_transport = options.get("http2") is not False and not issubclass(
Http2Transport, HttpTransport
)

# By default, we use the http transport class
transport_cls = (
Expand Down
4 changes: 2 additions & 2 deletions tests/integrations/excepthook/test_excepthook.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
from textwrap import dedent


TEST_PARAMETERS = [("", "HttpTransport")]
TEST_PARAMETERS = [("http2=False", "HttpTransport")]

if sys.version_info >= (3, 8):
TEST_PARAMETERS.append(('_experiments={"transport_http2": True}', "Http2Transport"))
TEST_PARAMETERS.append(("", "Http2Transport"))


@pytest.mark.parametrize("options, transport", TEST_PARAMETERS)
Expand Down
2 changes: 1 addition & 1 deletion tests/integrations/typer/test_typer.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def capture_envelope(self, envelope):
if event is not None:
print(event)

transport.HttpTransport.capture_envelope = capture_envelope
transport.BaseHttpTransport.capture_envelope = capture_envelope

init("http://foobar@localhost/123", integrations=[TyperIntegration()])

Expand Down
4 changes: 2 additions & 2 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ def test_proxy(monkeypatch, testcase, http2):

kwargs = {}

if http2:
kwargs["_experiments"] = {"transport_http2": True}
if not http2:
kwargs["http2"] = False

if testcase["arg_http_proxy"] is not None:
kwargs["http_proxy"] = testcase["arg_http_proxy"]
Expand Down
66 changes: 49 additions & 17 deletions tests/test_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ def test_transport_works(
if compression_algo is not None:
experiments["transport_compression_algo"] = compression_algo

if http2:
experiments["transport_http2"] = True
if not http2:
experiments["http2"] = False

client = make_client(
debug=debug,
Expand Down Expand Up @@ -213,7 +213,7 @@ def test_transport_num_pools(make_client, num_pools, expected_num_pools):
if num_pools is not None:
_experiments["transport_num_pools"] = num_pools

client = make_client(_experiments=_experiments)
client = make_client(_experiments=_experiments, http2=False)

options = client.transport._get_pool_options()
assert options["num_pools"] == expected_num_pools
Expand All @@ -223,17 +223,13 @@ def test_transport_num_pools(make_client, num_pools, expected_num_pools):
"http2", [True, False] if sys.version_info >= (3, 8) else [False]
)
def test_two_way_ssl_authentication(make_client, http2):
_experiments = {}
if http2:
_experiments["transport_http2"] = True

current_dir = os.path.dirname(__file__)
cert_file = f"{current_dir}/test.pem"
key_file = f"{current_dir}/test.key"
client = make_client(
cert_file=cert_file,
key_file=key_file,
_experiments=_experiments,
http2=http2,
)
options = client.transport._get_pool_options()

Expand All @@ -258,20 +254,20 @@ def test_socket_options(make_client):


def test_keep_alive_true(make_client):
client = make_client(keep_alive=True)
client = make_client(keep_alive=True, http2=False)

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


def test_keep_alive_on_by_default(make_client):
client = make_client()
client = make_client(http2=False)
options = client.transport._get_pool_options()
assert "socket_options" not in options


def test_default_timeout(make_client):
client = make_client()
client = make_client(http2=False)

options = client.transport._get_pool_options()
assert "timeout" in options
Expand All @@ -280,7 +276,7 @@ def test_default_timeout(make_client):

@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
def test_default_timeout_http2(make_client):
client = make_client(_experiments={"transport_http2": True})
client = make_client()

with mock.patch(
"sentry_sdk.transport.httpcore.ConnectionPool.request",
Expand All @@ -303,15 +299,15 @@ def test_default_timeout_http2(make_client):

@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
def test_http2_with_https_dsn(make_client):
client = make_client(_experiments={"transport_http2": True})
client = make_client()
client.transport.parsed_dsn.scheme = "https"
options = client.transport._get_pool_options()
assert options["http2"] is True


@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
def test_no_http2_with_http_dsn(make_client):
client = make_client(_experiments={"transport_http2": True})
client = make_client()
client.transport.parsed_dsn.scheme = "http"
options = client.transport._get_pool_options()
assert options["http2"] is False
Expand All @@ -324,19 +320,44 @@ def test_socket_options_override_keep_alive(make_client):
(socket.SOL_TCP, socket.TCP_KEEPCNT, 6),
]

client = make_client(socket_options=socket_options, keep_alive=False)
client = make_client(socket_options=socket_options, keep_alive=False, http2=False)

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


@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
def test_socket_options_merge_with_keep_alive_http2(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)

options = client.transport._get_pool_options()
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_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)
client = make_client(socket_options=socket_options, keep_alive=True, http2=False)

options = client.transport._get_pool_options()
try:
Expand All @@ -354,12 +375,23 @@ def test_socket_options_merge_with_keep_alive(make_client):
]


def test_socket_options_override_defaults(make_client):
@pytest.mark.skipif(not PY38, reason="HTTP2 libraries are only available in py3.8+")
def test_socket_options_override_defaults_http2(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"] == KEEP_ALIVE_SOCKET_OPTIONS


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(http2=False, socket_options=[])

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

Expand Down
Loading