From 2bfce50d38e703a30a44f54a167492ddfef36229 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Fri, 4 Oct 2024 10:03:39 +0100 Subject: [PATCH] feat: Add httpcore based HTTP2Transport (#3588) All our ingest endpoints support HTTP/2 and some even HTTP/3 which are significantly more efficient compared to HTTP/1.1 with multiplexing and, header compression, connection reuse and 0-RTT TLS. This patch adds an experimental HTTP2Transport with the help of httpcore library. It makes minimal changes to the original HTTPTransport that said with httpcore we should be able to implement asyncio support easily and remove the worker logic (see #2824). This should also open the door for future HTTP/3 support (see encode/httpx#275). --------- Co-authored-by: Ivana Kellyer --- requirements-testing.txt | 2 + sentry_sdk/client.py | 4 +- sentry_sdk/consts.py | 1 + sentry_sdk/transport.py | 360 ++++++++++++++---- setup.py | 1 + .../excepthook/test_excepthook.py | 29 +- tests/test.key | 52 +++ tests/test.pem | 30 ++ tests/test_client.py | 83 +++- tests/test_transport.py | 50 ++- tests/test_utils.py | 2 +- 11 files changed, 490 insertions(+), 124 deletions(-) create mode 100644 tests/test.key create mode 100644 tests/test.pem diff --git a/requirements-testing.txt b/requirements-testing.txt index 95c015f806..0f42d6a7df 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -10,4 +10,6 @@ executing asttokens responses pysocks +socksio +httpcore[http2] setuptools diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 0dd216ab21..1598b0327c 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -23,7 +23,7 @@ ) from sentry_sdk.serializer import serialize from sentry_sdk.tracing import trace -from sentry_sdk.transport import HttpTransport, make_transport +from sentry_sdk.transport import BaseHttpTransport, make_transport from sentry_sdk.consts import ( DEFAULT_MAX_VALUE_LENGTH, DEFAULT_OPTIONS, @@ -427,7 +427,7 @@ def _capture_envelope(envelope): self.monitor or self.metrics_aggregator or has_profiling_enabled(self.options) - or isinstance(self.transport, HttpTransport) + or isinstance(self.transport, BaseHttpTransport) ): # If we have anything on that could spawn a background thread, we # need to check if it's safe to use them. diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index b0be144659..9a6c08d0fd 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -60,6 +60,7 @@ class EndpointType(Enum): "otel_powered_performance": Optional[bool], "transport_zlib_compression_level": Optional[int], "transport_num_pools": Optional[int], + "transport_http2": Optional[bool], "enable_metrics": Optional[bool], "before_emit_metric": Optional[ Callable[[str, MetricValue, MeasurementUnit, MetricTags], bool] diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 6685d5c159..7a6b4f07b8 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -3,6 +3,7 @@ import os import gzip import socket +import ssl import time import warnings from datetime import datetime, timedelta, timezone @@ -24,13 +25,14 @@ from typing import Any from typing import Callable from typing import Dict + from typing import DefaultDict from typing import Iterable from typing import List + from typing import Mapping from typing import Optional from typing import Tuple from typing import Type from typing import Union - from typing import DefaultDict from urllib3.poolmanager import PoolManager from urllib3.poolmanager import ProxyManager @@ -193,8 +195,8 @@ def _parse_rate_limits(header, now=None): continue -class HttpTransport(Transport): - """The default HTTP transport.""" +class BaseHttpTransport(Transport): + """The base HTTP transport.""" def __init__( self, options # type: Dict[str, Any] @@ -208,19 +210,19 @@ def __init__( self._worker = BackgroundWorker(queue_size=options["transport_queue_size"]) self._auth = self.parsed_dsn.to_auth("sentry.python/%s" % VERSION) self._disabled_until = {} # type: Dict[Optional[EventDataCategory], datetime] + # We only use this Retry() class for the `get_retry_after` method it exposes self._retry = urllib3.util.Retry() self._discarded_events = defaultdict( int ) # type: DefaultDict[Tuple[EventDataCategory, str], int] self._last_client_report_sent = time.time() - compresslevel = options.get("_experiments", {}).get( + compression_level = 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._compression_level = ( + 9 if compression_level is None else int(compression_level) + ) self._pool = self._make_pool( self.parsed_dsn, @@ -269,12 +271,16 @@ def record_lost_event( self._discarded_events[data_category, reason] += quantity + def _get_header_value(self, response, header): + # type: (Any, str) -> Optional[str] + return response.headers.get(header) + def _update_rate_limits(self, response): - # type: (urllib3.BaseHTTPResponse) -> None + # type: (Union[urllib3.BaseHTTPResponse, httpcore.Response]) -> None # new sentries with more rate limit insights. We honor this header # no matter of the status code to update our internal rate limits. - header = response.headers.get("x-sentry-rate-limits") + header = self._get_header_value(response, "x-sentry-rate-limits") if header: logger.warning("Rate-limited via x-sentry-rate-limits") self._disabled_until.update(_parse_rate_limits(header)) @@ -284,8 +290,14 @@ def _update_rate_limits(self, response): # sentries if a proxy in front wants to globally slow things down. elif response.status == 429: logger.warning("Rate-limited via 429") + retry_after_value = self._get_header_value(response, "Retry-After") + retry_after = ( + self._retry.parse_retry_after(retry_after_value) + if retry_after_value is not None + else None + ) or 60 self._disabled_until[None] = datetime.now(timezone.utc) + timedelta( - seconds=self._retry.get_retry_after(response) or 60 + seconds=retry_after ) def _send_request( @@ -312,11 +324,11 @@ def record_loss(reason): } ) try: - response = self._pool.request( + response = self._request( "POST", - str(self._auth.get_api_url(endpoint_type)), - body=body, - headers=headers, + endpoint_type, + body, + headers, ) except Exception: self.on_dropped_event("network") @@ -338,7 +350,7 @@ def record_loss(reason): logger.error( "Unexpected status code: %s (body: %s)", response.status, - response.data, + getattr(response, "data", getattr(response, "content", None)), ) self.on_dropped_event("status_{}".format(response.status)) record_loss("network_error") @@ -447,11 +459,11 @@ def _send_envelope( envelope.items.append(client_report_item) body = io.BytesIO() - if self._compresslevel == 0: + if self._compression_level == 0: envelope.serialize_into(body) else: with gzip.GzipFile( - fileobj=body, mode="w", compresslevel=self._compresslevel + fileobj=body, mode="w", compresslevel=self._compression_level ) as f: envelope.serialize_into(f) @@ -466,7 +478,7 @@ def _send_envelope( headers = { "Content-Type": "application/x-sentry-envelope", } - if self._compresslevel > 0: + if self._compression_level > 0: headers["Content-Encoding"] = "gzip" self._send_request( @@ -479,8 +491,109 @@ def _send_envelope( def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): # type: (Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any] + raise NotImplementedError() + + def _in_no_proxy(self, parsed_dsn): + # type: (Dsn) -> bool + no_proxy = getproxies().get("no") + if not no_proxy: + return False + for host in no_proxy.split(","): + host = host.strip() + if parsed_dsn.host.endswith(host) or parsed_dsn.netloc.endswith(host): + return True + return False + + def _make_pool( + self, + parsed_dsn, # type: Dsn + http_proxy, # type: Optional[str] + https_proxy, # type: Optional[str] + ca_certs, # type: Optional[Any] + cert_file, # type: Optional[Any] + key_file, # type: Optional[Any] + proxy_headers, # type: Optional[Dict[str, str]] + ): + # type: (...) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + raise NotImplementedError() + + def _request( + self, + method, + endpoint_type, + body, + headers, + ): + # type: (str, EndpointType, Any, Mapping[str, str]) -> Union[urllib3.BaseHTTPResponse, httpcore.Response] + raise NotImplementedError() + + def capture_envelope( + self, envelope # type: Envelope + ): + # type: (...) -> None + def send_envelope_wrapper(): + # type: () -> None + with capture_internal_exceptions(): + self._send_envelope(envelope) + self._flush_client_reports() + + if not self._worker.submit(send_envelope_wrapper): + self.on_dropped_event("full_queue") + for item in envelope.items: + self.record_lost_event("queue_overflow", item=item) + + def flush( + self, + timeout, # type: float + callback=None, # type: Optional[Any] + ): + # type: (...) -> None + logger.debug("Flushing HTTP transport") + + if timeout > 0: + self._worker.submit(lambda: self._flush_client_reports(force=True)) + self._worker.flush(timeout, callback) + + def kill(self): + # type: () -> None + logger.debug("Killing HTTP transport") + self._worker.kill() + + @staticmethod + def _warn_hub_cls(): + # type: () -> None + """Convenience method to warn users about the deprecation of the `hub_cls` attribute.""" + warnings.warn( + "The `hub_cls` attribute is deprecated and will be removed in a future release.", + DeprecationWarning, + stacklevel=3, + ) + + @property + def hub_cls(self): + # type: () -> type[sentry_sdk.Hub] + """DEPRECATED: This attribute is deprecated and will be removed in a future release.""" + HttpTransport._warn_hub_cls() + return self._hub_cls + + @hub_cls.setter + def hub_cls(self, value): + # type: (type[sentry_sdk.Hub]) -> None + """DEPRECATED: This attribute is deprecated and will be removed in a future release.""" + HttpTransport._warn_hub_cls() + self._hub_cls = value + + +class HttpTransport(BaseHttpTransport): + if TYPE_CHECKING: + _pool: Union[PoolManager, ProxyManager] + + def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): + # type: (Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any] + + num_pools = self.options.get("_experiments", {}).get("transport_num_pools") options = { - "num_pools": self._num_pools, + "num_pools": 2 if num_pools is None else int(num_pools), "cert_reqs": "CERT_REQUIRED", } @@ -513,17 +626,6 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): return options - def _in_no_proxy(self, parsed_dsn): - # type: (Dsn) -> bool - no_proxy = getproxies().get("no") - if not no_proxy: - return False - for host in no_proxy.split(","): - host = host.strip() - if parsed_dsn.host.endswith(host) or parsed_dsn.netloc.endswith(host): - return True - return False - def _make_pool( self, parsed_dsn, # type: Dsn @@ -555,7 +657,7 @@ def _make_pool( if proxy.startswith("socks"): use_socks_proxy = True try: - # Check if PySocks depencency is available + # Check if PySocks dependency is available from urllib3.contrib.socks import SOCKSProxyManager except ImportError: use_socks_proxy = False @@ -573,61 +675,155 @@ def _make_pool( else: return urllib3.PoolManager(**opts) - def capture_envelope( - self, envelope # type: Envelope + def _request( + self, + method, + endpoint_type, + body, + headers, ): - # type: (...) -> None - def send_envelope_wrapper(): - # type: () -> None - with capture_internal_exceptions(): - self._send_envelope(envelope) - self._flush_client_reports() + # type: (str, EndpointType, Any, Mapping[str, str]) -> urllib3.BaseHTTPResponse + return self._pool.request( + method, + self._auth.get_api_url(endpoint_type), + body=body, + headers=headers, + ) - if not self._worker.submit(send_envelope_wrapper): - self.on_dropped_event("full_queue") - for item in envelope.items: - self.record_lost_event("queue_overflow", item=item) - def flush( - self, - timeout, # type: float - callback=None, # type: Optional[Any] - ): - # type: (...) -> None - logger.debug("Flushing HTTP transport") +try: + import httpcore +except ImportError: + # Sorry, no Http2Transport for you + class Http2Transport(HttpTransport): + def __init__( + self, options # type: Dict[str, Any] + ): + # type: (...) -> None + super().__init__(options) + logger.warning( + "You tried to use HTTP2Transport but don't have httpcore[http2] installed. Falling back to HTTPTransport." + ) - if timeout > 0: - self._worker.submit(lambda: self._flush_client_reports(force=True)) - self._worker.flush(timeout, callback) +else: + + class Http2Transport(BaseHttpTransport): # type: ignore + """The HTTP2 transport based on httpcore.""" + + if TYPE_CHECKING: + _pool: Union[ + httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool + ] + + def _get_header_value(self, response, header): + # type: (httpcore.Response, str) -> Optional[str] + return next( + ( + val.decode("ascii") + for key, val in response.headers + if key.decode("ascii").lower() == header + ), + None, + ) - def kill(self): - # type: () -> None - logger.debug("Killing HTTP transport") - self._worker.kill() + def _request( + self, + method, + endpoint_type, + body, + headers, + ): + # type: (str, EndpointType, Any, Mapping[str, str]) -> httpcore.Response + response = self._pool.request( + method, + self._auth.get_api_url(endpoint_type), + content=body, + headers=headers, # type: ignore + ) + return response + + def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): + # type: (Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any] + options = { + "http2": True, + "retries": 3, + } # type: Dict[str, Any] + + socket_options = ( + self.options["socket_options"] + if self.options["socket_options"] is not None + else [] + ) - @staticmethod - def _warn_hub_cls(): - # type: () -> None - """Convenience method to warn users about the deprecation of the `hub_cls` attribute.""" - warnings.warn( - "The `hub_cls` attribute is deprecated and will be removed in a future release.", - DeprecationWarning, - stacklevel=3, - ) + 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) - @property - def hub_cls(self): - # type: () -> type[sentry_sdk.Hub] - """DEPRECATED: This attribute is deprecated and will be removed in a future release.""" - HttpTransport._warn_hub_cls() - return self._hub_cls + options["socket_options"] = socket_options - @hub_cls.setter - def hub_cls(self, value): - # type: (type[sentry_sdk.Hub]) -> None - """DEPRECATED: This attribute is deprecated and will be removed in a future release.""" - HttpTransport._warn_hub_cls() - self._hub_cls = value + ssl_context = ssl.create_default_context() + ssl_context.load_verify_locations( + ca_certs # User-provided bundle from the SDK init + or os.environ.get("SSL_CERT_FILE") + or os.environ.get("REQUESTS_CA_BUNDLE") + or certifi.where() + ) + cert_file = cert_file or os.environ.get("CLIENT_CERT_FILE") + key_file = key_file or os.environ.get("CLIENT_KEY_FILE") + if cert_file is not None: + ssl_context.load_cert_chain(cert_file, key_file) + + options["ssl_context"] = ssl_context + + return options + + def _make_pool( + self, + parsed_dsn, # type: Dsn + http_proxy, # type: Optional[str] + https_proxy, # type: Optional[str] + ca_certs, # type: Optional[Any] + cert_file, # type: Optional[Any] + key_file, # type: Optional[Any] + proxy_headers, # type: Optional[Dict[str, str]] + ): + # type: (...) -> Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + proxy = None + no_proxy = self._in_no_proxy(parsed_dsn) + + # try HTTPS first + if parsed_dsn.scheme == "https" and (https_proxy != ""): + proxy = https_proxy or (not no_proxy and getproxies().get("https")) + + # maybe fallback to HTTP proxy + if not proxy and (http_proxy != ""): + proxy = http_proxy or (not no_proxy and getproxies().get("http")) + + opts = self._get_pool_options(ca_certs, cert_file, key_file) + + if proxy: + if proxy_headers: + opts["proxy_headers"] = proxy_headers + + if proxy.startswith("socks"): + try: + if "socket_options" in opts: + socket_options = opts.pop("socket_options") + if socket_options: + logger.warning( + "You have defined socket_options but using a SOCKS proxy which doesn't support these. We'll ignore socket_options." + ) + return httpcore.SOCKSProxy(proxy_url=proxy, **opts) + except RuntimeError: + logger.warning( + "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support.", + proxy, + ) + else: + return httpcore.HTTPProxy(proxy_url=proxy, **opts) + + return httpcore.ConnectionPool(**opts) class _FunctionTransport(Transport): @@ -663,8 +859,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) + # By default, we use the http transport class - transport_cls = HttpTransport # type: Type[Transport] + transport_cls = ( + Http2Transport if use_http2_transport else HttpTransport + ) # type: Type[Transport] if isinstance(ref_transport, Transport): return ref_transport diff --git a/setup.py b/setup.py index b5be538292..0432533247 100644 --- a/setup.py +++ b/setup.py @@ -58,6 +58,7 @@ def get_file_text(file_name): "fastapi": ["fastapi>=0.79.0"], "flask": ["flask>=0.11", "blinker>=1.1", "markupsafe"], "grpcio": ["grpcio>=1.21.1", "protobuf>=3.8.0"], + "http2": ["httpcore[http2]==1.*"], "httpx": ["httpx>=0.16.0"], "huey": ["huey>=2"], "huggingface_hub": ["huggingface_hub>=0.22"], diff --git a/tests/integrations/excepthook/test_excepthook.py b/tests/integrations/excepthook/test_excepthook.py index 7cb4e8b765..82fe6c6861 100644 --- a/tests/integrations/excepthook/test_excepthook.py +++ b/tests/integrations/excepthook/test_excepthook.py @@ -5,7 +5,14 @@ from textwrap import dedent -def test_excepthook(tmpdir): +TEST_PARAMETERS = [("", "HttpTransport")] + +if sys.version_info >= (3, 8): + TEST_PARAMETERS.append(('_experiments={"transport_http2": True}', "Http2Transport")) + + +@pytest.mark.parametrize("options, transport", TEST_PARAMETERS) +def test_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( @@ -18,14 +25,16 @@ def capture_envelope(self, envelope): if event is not None: print(event) - transport.HttpTransport.capture_envelope = capture_envelope + transport.{transport}.capture_envelope = capture_envelope - init("http://foobar@localhost/123") + init("http://foobar@localhost/123", {options}) frame_value = "LOL" 1/0 - """ + """.format( + transport=transport, options=options + ) ) ) @@ -40,7 +49,8 @@ def capture_envelope(self, envelope): assert b"capture_envelope was called" in output -def test_always_value_excepthook(tmpdir): +@pytest.mark.parametrize("options, transport", TEST_PARAMETERS) +def test_always_value_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( @@ -55,17 +65,20 @@ def capture_envelope(self, envelope): if event is not None: print(event) - transport.HttpTransport.capture_envelope = capture_envelope + transport.{transport}.capture_envelope = capture_envelope sys.ps1 = "always_value_test" init("http://foobar@localhost/123", - integrations=[ExcepthookIntegration(always_run=True)] + integrations=[ExcepthookIntegration(always_run=True)], + {options} ) frame_value = "LOL" 1/0 - """ + """.format( + transport=transport, options=options + ) ) ) diff --git a/tests/test.key b/tests/test.key new file mode 100644 index 0000000000..bf066c169d --- /dev/null +++ b/tests/test.key @@ -0,0 +1,52 @@ +-----BEGIN PRIVATE KEY----- +MIIJQgIBADANBgkqhkiG9w0BAQEFAASCCSwwggkoAgEAAoICAQCNSgCTO5Pc7o21 +BfvfDv/UDwDydEhInosNG7lgumqelT4dyJcYWoiDYAZ8zf6mlPFaw3oYouq+nQo/ +Z5eRNQD6AxhXw86qANjcfs1HWoP8d7jgR+ZelrshadvBBGYUJhiDkjUWb8jU7b9M +28z5m4SA5enfSrQYZfVlrX8MFxV70ws5duLye92FYjpqFBWeeGtmsw1iWUO020Nj +bbngpcRmRiBq41KuPydD8IWWQteoOVAI3U2jwEI2foAkXTHB+kQF//NtUWz5yiZY +4ugjY20p0t8Asom1oDK9pL2Qy4EQpsCev/6SJ+o7sK6oR1gyrzodn6hcqJbqcXvp +Y6xgXIO02H8wn7e3NkAJZkfFWJAyIslYrurMcnZwDaLpzL35vyULseOtDfsWQ3yq +TflXHcA2Zlujuv7rmq6Q+GCaLJxbmj5bPUvv8DAARd97BXf57s6C9srT8kk5Ekbf +URWRiO8j5XDLPyqsaP1c/pMPee1CGdtY6gf9EDWgmivgAYvH27pqzKh0JJAsmJ8p +1Zp5xFMtEkzoTlKL2jqeyS6zBO/o+9MHJld5OHcUvlWm767vKKe++aV2IA3h9nBQ +vmbCQ9i0ufGXZYZtJUYk6T8EMLclvtQz4yLRAYx0PLFOKfi1pAfDAHBFEfwWmuCk +cYqw8erbbfoj0qpnuDEj45iUtH5gRwIDAQABAoICADqdqfFrNSPiYC3qxpy6x039 +z4HG1joydDPC/bxwek1CU1vd3TmATcRbMTXT7ELF5f+mu1+/Ly5XTmoRmyLl33rZ +j97RYErNQSrw/E8O8VTrgmqhyaQSWp45Ia9JGORhDaiAHsApLiOQYt4LDlW7vFQR +jl5RyreYjR9axCuK5CHT44M6nFrHIpb0spFRtcph4QThYbscl2dP0/xLCGN3wixA +CbDukF2z26FnBrTZFEk5Rcf3r/8wgwfCoXz0oPD91/y5PA9tSY2z3QbhVDdiR2aj +klritxj/1i0xTGfm1avH0n/J3V5bauTKnxs3RhL4+V5S33FZjArFfAfOjzQHDah6 +nqz43dAOf83QYreMivxyAnQvU3Cs+J4RKYUsIQzsLpRs/2Wb7nK3W/p+bLdRIl04 +Y+xcX+3aKBluKoVMh7CeQDtr8NslSNO+YfGNmGYfD2f05da1Wi+FWqTrXXY2Y/NB +3VJDLgMuNgT5nsimrCl6ZfNcBtyDhsCUPN9V8sGZooEnjG0eNIX/OO3mlEI5GXfY +oFoXsjPX53aYZkOPVZLdXq0IteKGCFZCBhDVOmAqgALlVl66WbO+pMlBB+L7aw/h +H1NlBmrzfOXlYZi8SbmO0DSqC0ckXZCSdbmjix9aOhpDk/NlUZF29xCfQ5Mwk4gk +FboJIKDa0kKXQB18UV4ZAoIBAQC/LX97kOa1YibZIYdkyo0BD8jgjXZGV3y0Lc5V +h5mjOUD2mQ2AE9zcKtfjxEBnFYcC5RFe88vWBuYyLpVdDuZeiAfQHP4bXT+QZRBi +p51PjMuC+5zd5XlGeU5iwnfJ6TBe0yVfSb7M2N88LEeBaVCRcP7rqyiSYnwVkaHN +9Ow1PwJ4BiX0wIn62fO6o6CDo8x9KxXK6G+ak5z83AFSV8+ZGjHMEYcLaVfOj8a2 +VFbc2eX1V0ebgJOZVx8eAgjLV6fJahJ1/lT+8y9CzHtS7b3RvU/EsD+7WLMFUxHJ +cPVL6/iHBsV8heKxFfdORSBtBgllQjzv6rzuJ2rZDqQBZF0TAoIBAQC9MhjeEtNw +J8jrnsfg5fDJMPCg5nvb6Ck3z2FyDPJInK+b/IPvcrDl/+X+1vHhmGf5ReLZuEPR +0YEeAWbdMiKJbgRyca5xWRWgP7+sIFmJ9Calvf0FfFzaKQHyLAepBuVp5JMCqqTc +9Rw+5X5MjRgQxvJRppO/EnrvJ3/ZPJEhvYaSqvFQpYR4U0ghoQSlSxoYwCNuKSga +EmpItqZ1j6bKCxy/TZbYgM2SDoSzsD6h/hlLLIU6ecIsBPrF7C+rwxasbLLomoCD +RqjCjsLsgiQU9Qmg01ReRWjXa64r0JKGU0gb+E365WJHqPQgyyhmeYhcXhhUCj+B +Anze8CYU8xp9AoIBAFOpjYh9uPjXoziSO7YYDezRA4+BWKkf0CrpgMpdNRcBDzTb +ddT+3EBdX20FjUmPWi4iIJ/1ANcA3exIBoVa5+WmkgS5K1q+S/rcv3bs8yLE8qq3 +gcZ5jcERhQQjJljt+4UD0e8JTr5GiirDFefENsXvNR/dHzwwbSzjNnPzIwuKL4Jm +7mVVfQySJN8gjDYPkIWWPUs2vOBgiOr/PHTUiLzvgatUYEzWJN74fHV+IyUzFjdv +op6iffU08yEmssKJ8ZtrF/ka/Ac2VRBee/mmoNMQjb/9gWZzQqSp3bbSAAbhlTlB +9VqxHKtyeW9/QNl1MtdlTVWQ3G08Qr4KcitJyJECggEAL3lrrgXxUnpZO26bXz6z +vfhu2SEcwWCvPxblr9W50iinFDA39xTDeONOljTfeylgJbe4pcNMGVFF4f6eDjEv +Y2bc7M7D5CNjftOgSBPSBADk1cAnxoGfVwrlNxx/S5W0aW72yLuDJQLIdKvnllPt +TwBs+7od5ts/R9WUijFdhabmJtWIOiFebUcQmYeq/8MpqD5GZbUkH+6xBs/2UxeZ +1acWLpbMnEUt0FGeUOyPutxlAm0IfVTiOWOCfbm3eJU6kkewWRez2b0YScHC/c/m +N/AI23dL+1/VYADgMpRiwBwTwxj6kFOQ5sRphfUUjSo/4lWmKyhrKPcz2ElQdP9P +jQKCAQEAqsAD7r443DklL7oPR/QV0lrjv11EtXcZ0Gff7ZF2FI1V/CxkbYolPrB+ +QPSjwcMtyzxy6tXtUnaH19gx/K/8dBO/vnBw1Go/tvloIXidvVE0wemEC+gpTVtP +fLVplwBhcyxOMMGJcqbIT62pzSUisyXeb8dGn27BOUqz69u+z+MKdHDMM/loKJbj +TRw8MB8+t51osJ/tA3SwQCzS4onUMmwqE9eVHspANQeWZVqs+qMtpwW0lvs909Wv +VZ1o9pRPv2G9m7aK4v/bZO56DOx+9/Rp+mv3S2zl2Pkd6RIuD0UR4v03bRz3ACpf +zQTVuucYfxc1ph7H0ppUOZQNZ1Fo7w== +-----END PRIVATE KEY----- diff --git a/tests/test.pem b/tests/test.pem new file mode 100644 index 0000000000..2473a09452 --- /dev/null +++ b/tests/test.pem @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFETCCAvkCFEtmfMHeEvO+RUV9Qx0bkr7VWpdSMA0GCSqGSIb3DQEBCwUAMEUx +CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl +cm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjQwOTE3MjEwNDE1WhcNMjUwOTE3MjEw +NDE1WjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UE +CgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIICIjANBgkqhkiG9w0BAQEFAAOC +Ag8AMIICCgKCAgEAjUoAkzuT3O6NtQX73w7/1A8A8nRISJ6LDRu5YLpqnpU+HciX +GFqIg2AGfM3+ppTxWsN6GKLqvp0KP2eXkTUA+gMYV8POqgDY3H7NR1qD/He44Efm +Xpa7IWnbwQRmFCYYg5I1Fm/I1O2/TNvM+ZuEgOXp30q0GGX1Za1/DBcVe9MLOXbi +8nvdhWI6ahQVnnhrZrMNYllDtNtDY2254KXEZkYgauNSrj8nQ/CFlkLXqDlQCN1N +o8BCNn6AJF0xwfpEBf/zbVFs+comWOLoI2NtKdLfALKJtaAyvaS9kMuBEKbAnr/+ +kifqO7CuqEdYMq86HZ+oXKiW6nF76WOsYFyDtNh/MJ+3tzZACWZHxViQMiLJWK7q +zHJ2cA2i6cy9+b8lC7HjrQ37FkN8qk35Vx3ANmZbo7r+65qukPhgmiycW5o+Wz1L +7/AwAEXfewV3+e7OgvbK0/JJORJG31EVkYjvI+Vwyz8qrGj9XP6TD3ntQhnbWOoH +/RA1oJor4AGLx9u6asyodCSQLJifKdWaecRTLRJM6E5Si9o6nskuswTv6PvTByZX +eTh3FL5Vpu+u7yinvvmldiAN4fZwUL5mwkPYtLnxl2WGbSVGJOk/BDC3Jb7UM+Mi +0QGMdDyxTin4taQHwwBwRRH8FprgpHGKsPHq2236I9KqZ7gxI+OYlLR+YEcCAwEA +ATANBgkqhkiG9w0BAQsFAAOCAgEAgFVmFmk7duJRYqktcc4/qpbGUQTaalcjBvMQ +SnTS0l3WNTwOeUBbCR6V72LOBhRG1hqsQJIlXFIuoFY7WbQoeHciN58abwXan3N+ +4Kzuue5oFdj2AK9UTSKE09cKHoBD5uwiuU1oMGRxvq0+nUaJMoC333TNBXlIFV6K +SZFfD+MpzoNdn02PtjSBzsu09szzC+r8ZyKUwtG6xTLRBA8vrukWgBYgn9CkniJk +gLw8z5FioOt8ISEkAqvtyfJPi0FkUBb/vFXwXaaM8Vvn++ssYiUes0K5IzF+fQ5l +Bv8PIkVXFrNKuvzUgpO9IaUuQavSHFC0w0FEmbWsku7UxgPvLFPqmirwcnrkQjVR +eyE25X2Sk6AucnfIFGUvYPcLGJ71Z8mjH0baB2a/zo8vnWR1rqiUfptNomm42WMm +PaprIC0684E0feT+cqbN+LhBT9GqXpaG3emuguxSGMkff4RtPv/3DOFNk9KAIK8i +7GWCBjW5GF7mkTdQtYqVi1d87jeuGZ1InF1FlIZaswWGeG6Emml+Gxa50Z7Kpmc7 +f2vZlg9E8kmbRttCVUx4kx5PxKOI6s/ebKTFbHO+ZXJtm8MyOTrAJLfnFo4SUA90 +zX6CzyP1qu1/qdf9+kT0o0JeEsqg+0f4yhp3x/xH5OsAlUpRHvRr2aB3ZYi/4Vwj +53fMNXk= +-----END CERTIFICATE----- diff --git a/tests/test_client.py b/tests/test_client.py index 60799abc58..450e19603f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -246,7 +246,10 @@ def test_transport_option(monkeypatch): }, ], ) -def test_proxy(monkeypatch, testcase): +@pytest.mark.parametrize( + "http2", [True, False] if sys.version_info >= (3, 8) else [False] +) +def test_proxy(monkeypatch, testcase, http2): if testcase["env_http_proxy"] is not None: monkeypatch.setenv("HTTP_PROXY", testcase["env_http_proxy"]) if testcase["env_https_proxy"] is not None: @@ -256,6 +259,9 @@ def test_proxy(monkeypatch, testcase): kwargs = {} + if http2: + kwargs["_experiments"] = {"transport_http2": True} + if testcase["arg_http_proxy"] is not None: kwargs["http_proxy"] = testcase["arg_http_proxy"] if testcase["arg_https_proxy"] is not None: @@ -265,13 +271,31 @@ def test_proxy(monkeypatch, testcase): client = Client(testcase["dsn"], **kwargs) + proxy = getattr( + client.transport._pool, + "proxy", + getattr(client.transport._pool, "_proxy_url", None), + ) if testcase["expected_proxy_scheme"] is None: - assert client.transport._pool.proxy is None + assert proxy is None else: - assert client.transport._pool.proxy.scheme == testcase["expected_proxy_scheme"] + scheme = ( + proxy.scheme.decode("ascii") + if isinstance(proxy.scheme, bytes) + else proxy.scheme + ) + assert scheme == testcase["expected_proxy_scheme"] if testcase.get("arg_proxy_headers") is not None: - assert client.transport._pool.proxy_headers == testcase["arg_proxy_headers"] + proxy_headers = ( + dict( + (k.decode("ascii"), v.decode("ascii")) + for k, v in client.transport._pool._proxy_headers + ) + if http2 + else client.transport._pool.proxy_headers + ) + assert proxy_headers == testcase["arg_proxy_headers"] @pytest.mark.parametrize( @@ -281,68 +305,79 @@ def test_proxy(monkeypatch, testcase): "dsn": "https://foo@sentry.io/123", "arg_http_proxy": "http://localhost/123", "arg_https_proxy": None, - "expected_proxy_class": "", + "should_be_socks_proxy": False, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": "socks4a://localhost/123", "arg_https_proxy": None, - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": "socks4://localhost/123", "arg_https_proxy": None, - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": "socks5h://localhost/123", "arg_https_proxy": None, - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": "socks5://localhost/123", "arg_https_proxy": None, - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": None, "arg_https_proxy": "socks4a://localhost/123", - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": None, "arg_https_proxy": "socks4://localhost/123", - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": None, "arg_https_proxy": "socks5h://localhost/123", - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, { "dsn": "https://foo@sentry.io/123", "arg_http_proxy": None, "arg_https_proxy": "socks5://localhost/123", - "expected_proxy_class": "", + "should_be_socks_proxy": True, }, ], ) -def test_socks_proxy(testcase): +@pytest.mark.parametrize( + "http2", [True, False] if sys.version_info >= (3, 8) else [False] +) +def test_socks_proxy(testcase, http2): kwargs = {} + if http2: + kwargs["_experiments"] = {"transport_http2": True} + if testcase["arg_http_proxy"] is not None: kwargs["http_proxy"] = testcase["arg_http_proxy"] if testcase["arg_https_proxy"] is not None: kwargs["https_proxy"] = testcase["arg_https_proxy"] client = Client(testcase["dsn"], **kwargs) - assert str(type(client.transport._pool)) == testcase["expected_proxy_class"] + assert ("socks" in str(type(client.transport._pool)).lower()) == testcase[ + "should_be_socks_proxy" + ], ( + f"Expected {kwargs} to result in SOCKS == {testcase['should_be_socks_proxy']}" + f"but got {str(type(client.transport._pool))}" + ) def test_simple_transport(sentry_init): @@ -533,7 +568,17 @@ def test_capture_event_works(sentry_init): @pytest.mark.parametrize("num_messages", [10, 20]) -def test_atexit(tmpdir, monkeypatch, num_messages): +@pytest.mark.parametrize( + "http2", [True, False] if sys.version_info >= (3, 8) else [False] +) +def test_atexit(tmpdir, monkeypatch, num_messages, http2): + if http2: + options = '_experiments={"transport_http2": True}' + transport = "Http2Transport" + else: + options = "" + transport = "HttpTransport" + app = tmpdir.join("app.py") app.write( dedent( @@ -547,13 +592,13 @@ def capture_envelope(self, envelope): message = event.get("message", "") print(message) - transport.HttpTransport.capture_envelope = capture_envelope - init("http://foobar@localhost/123", shutdown_timeout={num_messages}) + transport.{transport}.capture_envelope = capture_envelope + init("http://foobar@localhost/123", shutdown_timeout={num_messages}, {options}) for _ in range({num_messages}): capture_message("HI") """.format( - num_messages=num_messages + transport=transport, options=options, num_messages=num_messages ) ) ) diff --git a/tests/test_transport.py b/tests/test_transport.py index 2e2ad3c4cd..8c69a47c54 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -2,7 +2,9 @@ import pickle import gzip import io +import os import socket +import sys from collections import defaultdict, namedtuple from datetime import datetime, timedelta, timezone from unittest import mock @@ -91,7 +93,7 @@ def make_client(request, capturing_server): def inner(**kwargs): return Client( "http://foobar@{}/132".format(capturing_server.url[len("http://") :]), - **kwargs + **kwargs, ) return inner @@ -115,7 +117,10 @@ def mock_transaction_envelope(span_count): @pytest.mark.parametrize("debug", (True, False)) @pytest.mark.parametrize("client_flush_method", ["close", "flush"]) @pytest.mark.parametrize("use_pickle", (True, False)) -@pytest.mark.parametrize("compressionlevel", (0, 9)) +@pytest.mark.parametrize("compression_level", (0, 9)) +@pytest.mark.parametrize( + "http2", [True, False] if sys.version_info >= (3, 8) else [False] +) def test_transport_works( capturing_server, request, @@ -125,15 +130,22 @@ def test_transport_works( make_client, client_flush_method, use_pickle, - compressionlevel, + compression_level, + http2, maybe_monkeypatched_threading, ): caplog.set_level(logging.DEBUG) + + experiments = { + "transport_zlib_compression_level": compression_level, + } + + if http2: + experiments["transport_http2"] = True + client = make_client( debug=debug, - _experiments={ - "transport_zlib_compression_level": compressionlevel, - }, + _experiments=experiments, ) if use_pickle: @@ -152,7 +164,7 @@ def test_transport_works( out, err = capsys.readouterr() assert not err and not out assert capturing_server.captured - assert capturing_server.captured[0].compressed == (compressionlevel > 0) + assert capturing_server.captured[0].compressed == (compression_level > 0) assert any("Sending envelope" in record.msg for record in caplog.records) == debug @@ -176,16 +188,26 @@ def test_transport_num_pools(make_client, num_pools, expected_num_pools): assert options["num_pools"] == expected_num_pools -def test_two_way_ssl_authentication(make_client): +@pytest.mark.parametrize( + "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 client = make_client(_experiments=_experiments) - options = client.transport._get_pool_options( - [], "/path/to/cert.pem", "/path/to/key.pem" - ) - assert options["cert_file"] == "/path/to/cert.pem" - assert options["key_file"] == "/path/to/key.pem" + current_dir = os.path.dirname(__file__) + cert_file = f"{current_dir}/test.pem" + key_file = f"{current_dir}/test.key" + options = client.transport._get_pool_options([], cert_file, key_file) + + if http2: + assert options["ssl_context"] is not None + else: + assert options["cert_file"] == cert_file + assert options["key_file"] == key_file def test_socket_options(make_client): @@ -208,7 +230,7 @@ def test_keep_alive_true(make_client): assert options["socket_options"] == KEEP_ALIVE_SOCKET_OPTIONS -def test_keep_alive_off_by_default(make_client): +def test_keep_alive_on_by_default(make_client): client = make_client() options = client.transport._get_pool_options([]) assert "socket_options" not in options diff --git a/tests/test_utils.py b/tests/test_utils.py index c46cac7f9f..eaf382c773 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -71,7 +71,7 @@ def _normalize_distribution_name(name): ), # UTC time ( "2021-01-01T00:00:00.000000", - datetime(2021, 1, 1, tzinfo=datetime.now().astimezone().tzinfo), + datetime(2021, 1, 1, tzinfo=timezone.utc), ), # No TZ -- assume UTC ( "2021-01-01T00:00:00Z",