From cac0a0e54a3b043f9a44b41425dd06e702163175 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 18 Sep 2024 10:13:09 +0100 Subject: [PATCH 01/24] feat: Add httpcore based HTTP2Transport --- requirements-testing.txt | 1 + sentry_sdk/client.py | 3 +- sentry_sdk/transport.py | 465 +++++++++++++++++- setup.py | 1 + .../excepthook/test_excepthook.py | 31 +- tests/test.key | 52 ++ tests/test.pem | 30 ++ tests/test_client.py | 77 ++- tests/test_transport.py | 25 +- tests/test_utils.py | 2 +- 10 files changed, 645 insertions(+), 42 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..2cbd6cf140 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -10,4 +10,5 @@ executing asttokens responses pysocks +socksio setuptools diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 0dd216ab21..288dfd4527 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 HttpTransport, Http2Transport, make_transport from sentry_sdk.consts import ( DEFAULT_MAX_VALUE_LENGTH, DEFAULT_OPTIONS, @@ -428,6 +428,7 @@ def _capture_envelope(envelope): or self.metrics_aggregator or has_profiling_enabled(self.options) or isinstance(self.transport, HttpTransport) + or isinstance(self.transport, Http2Transport) ): # 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/transport.py b/sentry_sdk/transport.py index 6685d5c159..a9819a3970 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 @@ -555,7 +556,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 @@ -630,6 +631,462 @@ def hub_cls(self, value): self._hub_cls = value +class Http2Transport(Transport): + """The HTTP2 transport based on httpcore.""" + + def __init__( + self, options # type: Dict[str, Any] + ): + # type: (...) -> None + from sentry_sdk.consts import VERSION + + # Import lazily here as this transport is optional + import httpcore + + self._httpcore = httpcore + + Transport.__init__(self, options) + assert self.parsed_dsn is not None + self.options = options # type: Dict[str, Any] + self._worker = BackgroundWorker(queue_size=options["transport_queue_size"]) + self._auth = self.parsed_dsn.to_auth("sentry.python/%s" % VERSION) + # We only use this Retry() class for the `get_retry_after` method it exposes + self._retry = urllib3.util.Retry() + self._disabled_until = {} # type: Dict[Optional[EventDataCategory], datetime] + self._discarded_events = defaultdict( + int + ) # type: DefaultDict[Tuple[EventDataCategory, 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) + + self._pool = self._make_pool( + self.parsed_dsn, + http_proxy=options["http_proxy"], + https_proxy=options["https_proxy"], + ca_certs=options["ca_certs"], + cert_file=options["cert_file"], + key_file=options["key_file"], + proxy_headers=options["proxy_headers"], + ) + + # Backwards compatibility for deprecated `self.hub_class` attribute + self._hub_cls = sentry_sdk.Hub + + def record_lost_event( + self, + reason, # type: str + data_category=None, # type: Optional[EventDataCategory] + item=None, # type: Optional[Item] + *, + quantity=1, # type: int + ): + # type: (...) -> None + if not self.options["send_client_reports"]: + return + + if item is not None: + data_category = item.data_category + quantity = 1 # If an item is provided, we always count it as 1 (except for attachments, handled below). + + if data_category == "transaction": + # Also record the lost spans + event = item.get_transaction_event() or {} + + # +1 for the transaction itself + span_count = len(event.get("spans") or []) + 1 + self.record_lost_event(reason, "span", quantity=span_count) + + elif data_category == "attachment": + # quantity of 0 is actually 1 as we do not want to count + # empty attachments as actually empty. + quantity = len(item.get_bytes()) or 1 + + elif data_category is None: + raise TypeError("data category not provided") + + self._discarded_events[data_category, reason] += quantity + + def _get_header_value(self, response, header): + return next( + ( + val.decode("ascii") + for key, val in response.headers + if key.decode("ascii").lower() == header + ), + None, + ) + + def _update_rate_limits(self, response): + # type: (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 = 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)) + + # old sentries only communicate global rate limit hits via the + # retry-after header on 429. This header can also be emitted on new + # 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=retry_after + ) + + def _send_request( + self, + body, # type: bytes + headers, # type: Dict[str, str] + endpoint_type=EndpointType.ENVELOPE, # type: EndpointType + envelope=None, # type: Optional[Envelope] + ): + # type: (...) -> None + + def record_loss(reason): + # type: (str) -> None + if envelope is None: + self.record_lost_event(reason, data_category="error") + else: + for item in envelope.items: + self.record_lost_event(reason, item=item) + + headers.update( + { + "User-Agent": str(self._auth.client), + "X-Sentry-Auth": str(self._auth.to_header()), + } + ) + try: + response = self._pool.request( + "POST", + str(self._auth.get_api_url(endpoint_type)), + content=body, + headers=headers, + ) + response.headers = response.headers + except Exception: + self.on_dropped_event("network") + record_loss("network_error") + raise + + try: + self._update_rate_limits(response) + + if response.status == 429: + # if we hit a 429. Something was rate limited but we already + # acted on this in `self._update_rate_limits`. Note that we + # do not want to record event loss here as we will have recorded + # an outcome in relay already. + self.on_dropped_event("status_429") + pass + + elif response.status >= 300 or response.status < 200: + logger.error( + "Unexpected status code: %s (body: %s)", + response.status, + response.data, + ) + self.on_dropped_event("status_{}".format(response.status)) + record_loss("network_error") + finally: + response.close() + + def on_dropped_event(self, reason): + # type: (str) -> None + return None + + def _fetch_pending_client_report(self, force=False, interval=60): + # type: (bool, int) -> Optional[Item] + if not self.options["send_client_reports"]: + return None + + if not (force or self._last_client_report_sent < time.time() - interval): + return None + + discarded_events = self._discarded_events + self._discarded_events = defaultdict(int) + self._last_client_report_sent = time.time() + + if not discarded_events: + return None + + return Item( + PayloadRef( + json={ + "timestamp": time.time(), + "discarded_events": [ + {"reason": reason, "category": category, "quantity": quantity} + for ( + (category, reason), + quantity, + ) in discarded_events.items() + ], + } + ), + type="client_report", + ) + + def _flush_client_reports(self, force=False): + # type: (bool) -> None + client_report = self._fetch_pending_client_report(force=force, interval=60) + if client_report is not None: + self.capture_envelope(Envelope(items=[client_report])) + + def _check_disabled(self, category): + # type: (str) -> bool + def _disabled(bucket): + # type: (Any) -> bool + + # The envelope item type used for metrics is statsd + # whereas the rate limit category is metric_bucket + if bucket == "statsd": + bucket = "metric_bucket" + + ts = self._disabled_until.get(bucket) + return ts is not None and ts > datetime.now(timezone.utc) + + return _disabled(category) or _disabled(None) + + def _is_rate_limited(self): + # type: () -> bool + return any( + ts > datetime.now(timezone.utc) for ts in self._disabled_until.values() + ) + + def _is_worker_full(self): + # type: () -> bool + return self._worker.full() + + def is_healthy(self): + # type: () -> bool + return not (self._is_worker_full() or self._is_rate_limited()) + + def _send_envelope( + self, envelope # type: Envelope + ): + # type: (...) -> None + + # remove all items from the envelope which are over quota + new_items = [] + for item in envelope.items: + if self._check_disabled(item.data_category): + if item.data_category in ("transaction", "error", "default", "statsd"): + self.on_dropped_event("self_rate_limits") + self.record_lost_event("ratelimit_backoff", item=item) + else: + new_items.append(item) + + # Since we're modifying the envelope here make a copy so that others + # that hold references do not see their envelope modified. + envelope = Envelope(headers=envelope.headers, items=new_items) + + if not envelope.items: + return None + + # since we're already in the business of sending out an envelope here + # check if we have one pending for the stats session envelopes so we + # can attach it to this enveloped scheduled for sending. This will + # currently typically attach the client report to the most recent + # session update. + client_report_item = self._fetch_pending_client_report(interval=30) + if client_report_item is not None: + envelope.items.append(client_report_item) + + body = io.BytesIO() + if self._compresslevel == 0: + envelope.serialize_into(body) + else: + with gzip.GzipFile( + fileobj=body, mode="w", compresslevel=self._compresslevel + ) as f: + envelope.serialize_into(f) + + assert self.parsed_dsn is not None + logger.debug( + "Sending envelope [%s] project:%s host:%s", + envelope.description, + self.parsed_dsn.project_id, + self.parsed_dsn.host, + ) + + headers = { + "Content-Type": "application/x-sentry-envelope", + } + if self._compresslevel > 0: + headers["Content-Encoding"] = "gzip" + + self._send_request( + body.getvalue(), + headers=headers, + endpoint_type=EndpointType.ENVELOPE, + envelope=envelope, + ) + return None + + 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, + } + + socket_options = ( + self.options["socket_options"] + if self.options["socket_options"] is not None + else [] + ) # type: List[Tuple[int, int, int | bytes]] + + 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) + + options["socket_options"] = socket_options + + 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 _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] + 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 self._httpcore.SOCKSProxy(proxy_url=proxy, **opts) + except RuntimeError: + use_socks_proxy = False + logger.warning( + "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support.", + proxy, + ) + else: + return self._httpcore.HTTPProxy(proxy_url=proxy, **opts) + else: + return self._httpcore.ConnectionPool(**opts) + + 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 _FunctionTransport(Transport): """ DEPRECATED: Users wishing to provide a custom transport should subclass @@ -663,8 +1120,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: ignore[type-abstract] + ) # type: Type[Transport] if isinstance(ref_transport, Transport): return ref_transport diff --git a/setup.py b/setup.py index c11b6b771e..eeed44ba11 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..aa6411a9b7 100644 --- a/tests/integrations/excepthook/test_excepthook.py +++ b/tests/integrations/excepthook/test_excepthook.py @@ -5,11 +5,18 @@ from textwrap import dedent -def test_excepthook(tmpdir): +@pytest.mark.parametrize( + "options, transport", + [ + ("", "HttpTransport"), + ('_experiments={"transport_http2": True}', "Http2Transport"), + ], +) +def test_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( - """ + f""" from sentry_sdk import init, transport def capture_envelope(self, envelope): @@ -18,9 +25,9 @@ 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" @@ -40,11 +47,18 @@ def capture_envelope(self, envelope): assert b"capture_envelope was called" in output -def test_always_value_excepthook(tmpdir): +@pytest.mark.parametrize( + "options, transport", + [ + ("", "HttpTransport"), + ('_experiments={"transport_http2": True}', "Http2Transport"), + ], +) +def test_always_value_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( - """ + f""" import sys from sentry_sdk import init, transport from sentry_sdk.integrations.excepthook import ExcepthookIntegration @@ -55,11 +69,12 @@ 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" 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..c8c52cbdb5 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -246,7 +246,8 @@ def test_transport_option(monkeypatch): }, ], ) -def test_proxy(monkeypatch, testcase): +@pytest.mark.parametrize("http2", [True, 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 +257,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 +269,28 @@ 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 = getattr( + client.transport._pool, + "proxy_headers", + getattr(client.transport._pool, "_proxy_headers", None), + ) + proxy_headers == testcase["arg_proxy_headers"] @pytest.mark.parametrize( @@ -281,68 +300,76 @@ 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]) +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" + ]} but got {str(type(client.transport._pool))}" def test_simple_transport(sentry_init): @@ -533,11 +560,19 @@ 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]) +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( - """ + f""" import time from sentry_sdk import init, transport, capture_message @@ -547,14 +582,12 @@ 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 - ) + """ ) ) diff --git a/tests/test_transport.py b/tests/test_transport.py index 2e2ad3c4cd..9829801453 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -2,6 +2,7 @@ import pickle import gzip import io +import os import socket from collections import defaultdict, namedtuple from datetime import datetime, timedelta, timezone @@ -91,7 +92,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 @@ -176,16 +177,24 @@ 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]) +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 +217,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", From 3e3ac9500e1ef9a7c9883e905e49a1e59e09f20d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 22:53:03 +0100 Subject: [PATCH 02/24] fix logic errors --- sentry_sdk/transport.py | 1 - tests/test_client.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index a9819a3970..aba3256d95 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -1020,7 +1020,6 @@ def _make_pool( ) return self._httpcore.SOCKSProxy(proxy_url=proxy, **opts) except RuntimeError: - use_socks_proxy = False logger.warning( "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support.", proxy, diff --git a/tests/test_client.py b/tests/test_client.py index c8c52cbdb5..c81443d0f6 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -290,7 +290,7 @@ def test_proxy(monkeypatch, testcase, http2): "proxy_headers", getattr(client.transport._pool, "_proxy_headers", None), ) - proxy_headers == testcase["arg_proxy_headers"] + assert proxy_headers == testcase["arg_proxy_headers"] @pytest.mark.parametrize( From 30c8039a19bb2afbc84775f1b5b79e8cca176267 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 22:55:51 +0100 Subject: [PATCH 03/24] fix f-string error pre py3.10 --- tests/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_client.py b/tests/test_client.py index c81443d0f6..6331216f45 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -368,7 +368,7 @@ def test_socks_proxy(testcase, http2): 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" + 'should_be_socks_proxy' ]} but got {str(type(client.transport._pool))}" From 0a4f4e58485f818b1c8f34b864b45bafd030190d Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:01:02 +0100 Subject: [PATCH 04/24] fix f-string error pre py3.10 --- tests/test_client.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 6331216f45..99bb667cb8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -367,9 +367,10 @@ def test_socks_proxy(testcase, http2): client = Client(testcase["dsn"], **kwargs) 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' - ]} but got {str(type(client.transport._pool))}" + ], ( + 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): From 488dbd0a0c13be81365536303e7993af2995a485 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:03:09 +0100 Subject: [PATCH 05/24] fix type error --- sentry_sdk/transport.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index aba3256d95..0eb708c14e 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -721,7 +721,8 @@ def _get_header_value(self, response, header): ) def _update_rate_limits(self, response): - # type: (httpcore.Response) -> None + # type: (httpcore.Response) -> None # type: ignore + # Added type ignore above as `httpcore` is optional and imported lazily # new sentries with more rate limit insights. We honor this header # no matter of the status code to update our internal rate limits. From 8dfb4ec0f6f1794cacbdc2daccd9c184b2f45349 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:04:53 +0100 Subject: [PATCH 06/24] remove type def referencing httpcore for now --- sentry_sdk/transport.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 0eb708c14e..9effb637ca 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -721,9 +721,6 @@ def _get_header_value(self, response, header): ) def _update_rate_limits(self, response): - # type: (httpcore.Response) -> None # type: ignore - # Added type ignore above as `httpcore` is optional and imported lazily - # new sentries with more rate limit insights. We honor this header # no matter of the status code to update our internal rate limits. header = self._get_header_value(response, "x-sentry-rate-limits") From 82ac04812310e5aac4c8d2e96cd04518941619df Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:19:56 +0100 Subject: [PATCH 07/24] disable new transport in 3.7-, add httpcore to test reqs --- requirements-testing.txt | 1 + .../excepthook/test_excepthook.py | 22 +++++++------------ tests/test_client.py | 12 +++++++--- tests/test_transport.py | 4 +++- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/requirements-testing.txt b/requirements-testing.txt index 2cbd6cf140..0f42d6a7df 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -11,4 +11,5 @@ asttokens responses pysocks socksio +httpcore[http2] setuptools diff --git a/tests/integrations/excepthook/test_excepthook.py b/tests/integrations/excepthook/test_excepthook.py index aa6411a9b7..6deabadfc5 100644 --- a/tests/integrations/excepthook/test_excepthook.py +++ b/tests/integrations/excepthook/test_excepthook.py @@ -5,13 +5,13 @@ from textwrap import dedent -@pytest.mark.parametrize( - "options, transport", - [ - ("", "HttpTransport"), - ('_experiments={"transport_http2": True}', "Http2Transport"), - ], -) +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( @@ -47,13 +47,7 @@ def capture_envelope(self, envelope): assert b"capture_envelope was called" in output -@pytest.mark.parametrize( - "options, transport", - [ - ("", "HttpTransport"), - ('_experiments={"transport_http2": True}', "Http2Transport"), - ], -) +@pytest.mark.parametrize("options, transport", TEST_PARAMETERS) def test_always_value_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( diff --git a/tests/test_client.py b/tests/test_client.py index 99bb667cb8..5cc3ee205e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -246,7 +246,9 @@ def test_transport_option(monkeypatch): }, ], ) -@pytest.mark.parametrize("http2", [True, False]) +@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"]) @@ -352,7 +354,9 @@ def test_proxy(monkeypatch, testcase, http2): }, ], ) -@pytest.mark.parametrize("http2", [True, False]) +@pytest.mark.parametrize( + "http2", [True, False] if sys.version_info >= (3, 8) else [False] +) def test_socks_proxy(testcase, http2): kwargs = {} @@ -561,7 +565,9 @@ def test_capture_event_works(sentry_init): @pytest.mark.parametrize("num_messages", [10, 20]) -@pytest.mark.parametrize("http2", [True, False]) +@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}' diff --git a/tests/test_transport.py b/tests/test_transport.py index 9829801453..bbd00924cd 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -177,7 +177,9 @@ def test_transport_num_pools(make_client, num_pools, expected_num_pools): assert options["num_pools"] == expected_num_pools -@pytest.mark.parametrize("http2", [True, False]) +@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: From 8fa0a6a178ede15db8fbef0c2739aed3ae0ab9b7 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:23:44 +0100 Subject: [PATCH 08/24] use latest flake8 --- requirements-linting.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-linting.txt b/requirements-linting.txt index 3b88581e24..64f9945293 100644 --- a/requirements-linting.txt +++ b/requirements-linting.txt @@ -1,6 +1,6 @@ mypy black -flake8==5.0.4 # flake8 depends on pyflakes>=3.0.0 and this dropped support for Python 2 "# type:" comments +flake8 types-certifi types-protobuf types-gevent From fdf4943355bc561a42ba25e8f1f519fe4178f0c4 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:25:05 +0100 Subject: [PATCH 09/24] add missing import --- tests/test_transport.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_transport.py b/tests/test_transport.py index bbd00924cd..6b9506cba5 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -4,6 +4,7 @@ import io import os import socket +import sys from collections import defaultdict, namedtuple from datetime import datetime, timedelta, timezone from unittest import mock From bc470b3cc1f0b7d1c31ffc3679b8aa7ef3c85dde Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:26:41 +0100 Subject: [PATCH 10/24] go back to old flake8 --- requirements-linting.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-linting.txt b/requirements-linting.txt index 64f9945293..3b88581e24 100644 --- a/requirements-linting.txt +++ b/requirements-linting.txt @@ -1,6 +1,6 @@ mypy black -flake8 +flake8==5.0.4 # flake8 depends on pyflakes>=3.0.0 and this dropped support for Python 2 "# type:" comments types-certifi types-protobuf types-gevent From 4168a17fb50a975dad8624db619c96933e6c2657 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:29:06 +0100 Subject: [PATCH 11/24] don't use f-doc-strings --- tests/integrations/excepthook/test_excepthook.py | 6 ++++-- tests/test_client.py | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/integrations/excepthook/test_excepthook.py b/tests/integrations/excepthook/test_excepthook.py index 6deabadfc5..4734a3490f 100644 --- a/tests/integrations/excepthook/test_excepthook.py +++ b/tests/integrations/excepthook/test_excepthook.py @@ -16,7 +16,7 @@ def test_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( - f""" + """ from sentry_sdk import init, transport def capture_envelope(self, envelope): @@ -32,7 +32,9 @@ def capture_envelope(self, envelope): frame_value = "LOL" 1/0 - """ + """.format( + transport=transport, options=options + ) ) ) diff --git a/tests/test_client.py b/tests/test_client.py index 5cc3ee205e..a39fc7bd1d 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -579,7 +579,7 @@ def test_atexit(tmpdir, monkeypatch, num_messages, http2): app = tmpdir.join("app.py") app.write( dedent( - f""" + """ import time from sentry_sdk import init, transport, capture_message @@ -594,7 +594,9 @@ def capture_envelope(self, envelope): for _ in range({num_messages}): capture_message("HI") - """ + """.format( + transport=transport, options=options, num_messages=num_messages + ) ) ) From 727d6f1098d09ffd57759ba68298dd92e82a7895 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:38:27 +0100 Subject: [PATCH 12/24] one more lint fix --- tests/integrations/excepthook/test_excepthook.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integrations/excepthook/test_excepthook.py b/tests/integrations/excepthook/test_excepthook.py index 4734a3490f..82fe6c6861 100644 --- a/tests/integrations/excepthook/test_excepthook.py +++ b/tests/integrations/excepthook/test_excepthook.py @@ -54,7 +54,7 @@ def test_always_value_excepthook(tmpdir, options, transport): app = tmpdir.join("app.py") app.write( dedent( - f""" + """ import sys from sentry_sdk import init, transport from sentry_sdk.integrations.excepthook import ExcepthookIntegration @@ -76,7 +76,9 @@ def capture_envelope(self, envelope): frame_value = "LOL" 1/0 - """ + """.format( + transport=transport, options=options + ) ) ) From 929298a91f7f56a9e383bbee1da54d862518895c Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 30 Sep 2024 23:49:12 +0100 Subject: [PATCH 13/24] fix test --- tests/test_client.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index a39fc7bd1d..450e19603f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -287,10 +287,13 @@ def test_proxy(monkeypatch, testcase, http2): assert scheme == testcase["expected_proxy_scheme"] if testcase.get("arg_proxy_headers") is not None: - proxy_headers = getattr( - client.transport._pool, - "proxy_headers", - getattr(client.transport._pool, "_proxy_headers", None), + 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"] From 007bbbd095d863300ba8a462e9b60792b57debb8 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 00:04:55 +0100 Subject: [PATCH 14/24] fix some typing issues --- sentry_sdk/transport.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 9effb637ca..01d33e6fec 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -711,6 +711,9 @@ def record_lost_event( self._discarded_events[data_category, reason] += quantity def _get_header_value(self, response, header): + # type: (Any, str) -> Optional[str] + # Cannot use `httpcore.response` for types as `httpcore` is optional + # and is lazily imported return next( ( val.decode("ascii") @@ -721,6 +724,10 @@ def _get_header_value(self, response, header): ) def _update_rate_limits(self, response): + # type(Any) -> None + # Cannot use `httpcore.response` for types as `httpcore` is optional + # and is lazily imported + # new sentries with more rate limit insights. We honor this header # no matter of the status code to update our internal rate limits. header = self._get_header_value(response, "x-sentry-rate-limits") @@ -938,13 +945,13 @@ def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): options = { "http2": True, "retries": 3, - } + } # type: Dict[str, Any] socket_options = ( self.options["socket_options"] if self.options["socket_options"] is not None else [] - ) # type: List[Tuple[int, int, int | bytes]] + ) used_options = {(o[0], o[1]) for o in socket_options} for default_option in KEEP_ALIVE_SOCKET_OPTIONS: @@ -990,7 +997,8 @@ def _make_pool( key_file, # type: Optional[Any] proxy_headers, # type: Optional[Dict[str, str]] ): - # type: (...) -> Union[PoolManager, ProxyManager] + # type: (...) -> Any + # Cannot use `httpcore.*` for types as `httpcore` is optional and is lazily imported proxy = None no_proxy = self._in_no_proxy(parsed_dsn) @@ -1024,8 +1032,8 @@ def _make_pool( ) else: return self._httpcore.HTTPProxy(proxy_url=proxy, **opts) - else: - return self._httpcore.ConnectionPool(**opts) + + return self._httpcore.ConnectionPool(**opts) def capture_envelope( self, envelope # type: Envelope From 5df98cc8031294ccf5dd0bdae77710a295004249 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 12:35:05 +0100 Subject: [PATCH 15/24] unify duplicated code --- sentry_sdk/transport.py | 695 ++++++++++++---------------------------- tests/test_transport.py | 22 +- 2 files changed, 222 insertions(+), 495 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 01d33e6fec..90a7bd139c 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -194,8 +194,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] @@ -209,19 +209,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, @@ -270,12 +270,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)) @@ -285,8 +289,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( @@ -313,11 +323,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") @@ -339,7 +349,7 @@ def record_loss(reason): logger.error( "Unexpected status code: %s (body: %s)", response.status, - response.data, + getattr(response, "data", getattr(response, "content")), ) self.on_dropped_event("status_{}".format(response.status)) record_loss("network_error") @@ -448,11 +458,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) @@ -467,7 +477,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( @@ -480,39 +490,7 @@ 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] - options = { - "num_pools": self._num_pools, - "cert_reqs": "CERT_REQUIRED", - } - - socket_options = None # type: Optional[List[Tuple[int, int, int | bytes]]] - - if self.options["socket_options"] is not None: - socket_options = self.options["socket_options"] - - if self.options["keep_alive"]: - if socket_options is None: - 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: - socket_options.append(default_option) - - if socket_options is not None: - options["socket_options"] = socket_options - - options["ca_certs"] = ( - 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() - ) - - options["cert_file"] = cert_file or os.environ.get("CLIENT_CERT_FILE") - options["key_file"] = key_file or os.environ.get("CLIENT_KEY_FILE") - - return options + raise NotImplementedError() def _in_no_proxy(self, parsed_dsn): # type: (Dsn) -> bool @@ -535,44 +513,18 @@ def _make_pool( key_file, # type: Optional[Any] proxy_headers, # type: Optional[Dict[str, str]] ): - # type: (...) -> Union[PoolManager, ProxyManager] - 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) + # type: (...) -> Union[PoolManager, ProxyManager, httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + raise NotImplementedError() - if proxy: - if proxy_headers: - opts["proxy_headers"] = proxy_headers - - if proxy.startswith("socks"): - use_socks_proxy = True - try: - # Check if PySocks dependency is available - from urllib3.contrib.socks import SOCKSProxyManager - except ImportError: - use_socks_proxy = False - logger.warning( - "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support. Please add `PySocks` (or `urllib3` with the `[socks]` extra) to your dependencies.", - proxy, - ) - - if use_socks_proxy: - return SOCKSProxyManager(proxy, **opts) - else: - return urllib3.PoolManager(**opts) - else: - return urllib3.ProxyManager(proxy, **opts) - else: - return urllib3.PoolManager(**opts) + def _request( + self, + method, + endpoint_type, + body, + headers, + ): + # type: (str, EndpointType, Any, Dict[str, str]) -> Union[urllib3.BaseHTTPResponse, httpcore.Response] + raise NotImplementedError() def capture_envelope( self, envelope # type: Envelope @@ -631,362 +583,45 @@ def hub_cls(self, value): self._hub_cls = value -class Http2Transport(Transport): - """The HTTP2 transport based on httpcore.""" - - def __init__( - self, options # type: Dict[str, Any] - ): - # type: (...) -> None - from sentry_sdk.consts import VERSION - - # Import lazily here as this transport is optional - import httpcore - - self._httpcore = httpcore - - Transport.__init__(self, options) - assert self.parsed_dsn is not None - self.options = options # type: Dict[str, Any] - self._worker = BackgroundWorker(queue_size=options["transport_queue_size"]) - self._auth = self.parsed_dsn.to_auth("sentry.python/%s" % VERSION) - # We only use this Retry() class for the `get_retry_after` method it exposes - self._retry = urllib3.util.Retry() - self._disabled_until = {} # type: Dict[Optional[EventDataCategory], datetime] - self._discarded_events = defaultdict( - int - ) # type: DefaultDict[Tuple[EventDataCategory, 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) - - self._pool = self._make_pool( - self.parsed_dsn, - http_proxy=options["http_proxy"], - https_proxy=options["https_proxy"], - ca_certs=options["ca_certs"], - cert_file=options["cert_file"], - key_file=options["key_file"], - proxy_headers=options["proxy_headers"], - ) - - # Backwards compatibility for deprecated `self.hub_class` attribute - self._hub_cls = sentry_sdk.Hub - - def record_lost_event( - self, - reason, # type: str - data_category=None, # type: Optional[EventDataCategory] - item=None, # type: Optional[Item] - *, - quantity=1, # type: int - ): - # type: (...) -> None - if not self.options["send_client_reports"]: - return - - if item is not None: - data_category = item.data_category - quantity = 1 # If an item is provided, we always count it as 1 (except for attachments, handled below). - - if data_category == "transaction": - # Also record the lost spans - event = item.get_transaction_event() or {} - - # +1 for the transaction itself - span_count = len(event.get("spans") or []) + 1 - self.record_lost_event(reason, "span", quantity=span_count) - - elif data_category == "attachment": - # quantity of 0 is actually 1 as we do not want to count - # empty attachments as actually empty. - quantity = len(item.get_bytes()) or 1 - - elif data_category is None: - raise TypeError("data category not provided") - - self._discarded_events[data_category, reason] += quantity - - def _get_header_value(self, response, header): - # type: (Any, str) -> Optional[str] - # Cannot use `httpcore.response` for types as `httpcore` is optional - # and is lazily imported - return next( - ( - val.decode("ascii") - for key, val in response.headers - if key.decode("ascii").lower() == header - ), - None, - ) - - def _update_rate_limits(self, response): - # type(Any) -> None - # Cannot use `httpcore.response` for types as `httpcore` is optional - # and is lazily imported - - # new sentries with more rate limit insights. We honor this header - # no matter of the status code to update our internal 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)) - - # old sentries only communicate global rate limit hits via the - # retry-after header on 429. This header can also be emitted on new - # 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=retry_after - ) - - def _send_request( - self, - body, # type: bytes - headers, # type: Dict[str, str] - endpoint_type=EndpointType.ENVELOPE, # type: EndpointType - envelope=None, # type: Optional[Envelope] - ): - # type: (...) -> None - - def record_loss(reason): - # type: (str) -> None - if envelope is None: - self.record_lost_event(reason, data_category="error") - else: - for item in envelope.items: - self.record_lost_event(reason, item=item) - - headers.update( - { - "User-Agent": str(self._auth.client), - "X-Sentry-Auth": str(self._auth.to_header()), - } - ) - try: - response = self._pool.request( - "POST", - str(self._auth.get_api_url(endpoint_type)), - content=body, - headers=headers, - ) - response.headers = response.headers - except Exception: - self.on_dropped_event("network") - record_loss("network_error") - raise - - try: - self._update_rate_limits(response) - - if response.status == 429: - # if we hit a 429. Something was rate limited but we already - # acted on this in `self._update_rate_limits`. Note that we - # do not want to record event loss here as we will have recorded - # an outcome in relay already. - self.on_dropped_event("status_429") - pass - - elif response.status >= 300 or response.status < 200: - logger.error( - "Unexpected status code: %s (body: %s)", - response.status, - response.data, - ) - self.on_dropped_event("status_{}".format(response.status)) - record_loss("network_error") - finally: - response.close() - - def on_dropped_event(self, reason): - # type: (str) -> None - return None - - def _fetch_pending_client_report(self, force=False, interval=60): - # type: (bool, int) -> Optional[Item] - if not self.options["send_client_reports"]: - return None - - if not (force or self._last_client_report_sent < time.time() - interval): - return None - - discarded_events = self._discarded_events - self._discarded_events = defaultdict(int) - self._last_client_report_sent = time.time() - - if not discarded_events: - return None - - return Item( - PayloadRef( - json={ - "timestamp": time.time(), - "discarded_events": [ - {"reason": reason, "category": category, "quantity": quantity} - for ( - (category, reason), - quantity, - ) in discarded_events.items() - ], - } - ), - type="client_report", - ) - - def _flush_client_reports(self, force=False): - # type: (bool) -> None - client_report = self._fetch_pending_client_report(force=force, interval=60) - if client_report is not None: - self.capture_envelope(Envelope(items=[client_report])) - - def _check_disabled(self, category): - # type: (str) -> bool - def _disabled(bucket): - # type: (Any) -> bool - - # The envelope item type used for metrics is statsd - # whereas the rate limit category is metric_bucket - if bucket == "statsd": - bucket = "metric_bucket" - - ts = self._disabled_until.get(bucket) - return ts is not None and ts > datetime.now(timezone.utc) - - return _disabled(category) or _disabled(None) - - def _is_rate_limited(self): - # type: () -> bool - return any( - ts > datetime.now(timezone.utc) for ts in self._disabled_until.values() - ) - - def _is_worker_full(self): - # type: () -> bool - return self._worker.full() - - def is_healthy(self): - # type: () -> bool - return not (self._is_worker_full() or self._is_rate_limited()) - - def _send_envelope( - self, envelope # type: Envelope - ): - # type: (...) -> None - - # remove all items from the envelope which are over quota - new_items = [] - for item in envelope.items: - if self._check_disabled(item.data_category): - if item.data_category in ("transaction", "error", "default", "statsd"): - self.on_dropped_event("self_rate_limits") - self.record_lost_event("ratelimit_backoff", item=item) - else: - new_items.append(item) - - # Since we're modifying the envelope here make a copy so that others - # that hold references do not see their envelope modified. - envelope = Envelope(headers=envelope.headers, items=new_items) - - if not envelope.items: - return None - - # since we're already in the business of sending out an envelope here - # check if we have one pending for the stats session envelopes so we - # can attach it to this enveloped scheduled for sending. This will - # currently typically attach the client report to the most recent - # session update. - client_report_item = self._fetch_pending_client_report(interval=30) - if client_report_item is not None: - envelope.items.append(client_report_item) - - body = io.BytesIO() - if self._compresslevel == 0: - envelope.serialize_into(body) - else: - with gzip.GzipFile( - fileobj=body, mode="w", compresslevel=self._compresslevel - ) as f: - envelope.serialize_into(f) - - assert self.parsed_dsn is not None - logger.debug( - "Sending envelope [%s] project:%s host:%s", - envelope.description, - self.parsed_dsn.project_id, - self.parsed_dsn.host, - ) +class HttpTransport(BaseHttpTransport): + def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): + # type: (Optional[Any], Optional[Any], Optional[Any]) -> Dict[str, Any] - headers = { - "Content-Type": "application/x-sentry-envelope", + num_pools = self.options.get("_experiments", {}).get("transport_num_pools") + options = { + "num_pools": 2 if num_pools is None else int(num_pools), + "cert_reqs": "CERT_REQUIRED", } - if self._compresslevel > 0: - headers["Content-Encoding"] = "gzip" - self._send_request( - body.getvalue(), - headers=headers, - endpoint_type=EndpointType.ENVELOPE, - envelope=envelope, - ) - return None + socket_options = None # type: Optional[List[Tuple[int, int, int | bytes]]] - 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 [] - ) + if self.options["socket_options"] is not None: + socket_options = self.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: - socket_options.append(default_option) + if self.options["keep_alive"]: + if socket_options is None: + 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: + socket_options.append(default_option) - options["socket_options"] = socket_options + if socket_options is not None: + options["socket_options"] = socket_options - ssl_context = ssl.create_default_context() - ssl_context.load_verify_locations( + options["ca_certs"] = ( 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 + options["cert_file"] = cert_file or os.environ.get("CLIENT_CERT_FILE") + options["key_file"] = key_file or os.environ.get("CLIENT_KEY_FILE") 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 @@ -997,8 +632,7 @@ def _make_pool( key_file, # type: Optional[Any] proxy_headers, # type: Optional[Dict[str, str]] ): - # type: (...) -> Any - # Cannot use `httpcore.*` for types as `httpcore` is optional and is lazily imported + # type: (...) -> Union[PoolManager, ProxyManager] proxy = None no_proxy = self._in_no_proxy(parsed_dsn) @@ -1017,79 +651,162 @@ def _make_pool( opts["proxy_headers"] = proxy_headers if proxy.startswith("socks"): + use_socks_proxy = True 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 self._httpcore.SOCKSProxy(proxy_url=proxy, **opts) - except RuntimeError: + # Check if PySocks dependency is available + from urllib3.contrib.socks import SOCKSProxyManager + except ImportError: + use_socks_proxy = False logger.warning( - "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support.", + "You have configured a SOCKS proxy (%s) but support for SOCKS proxies is not installed. Disabling proxy support. Please add `PySocks` (or `urllib3` with the `[socks]` extra) to your dependencies.", proxy, ) - else: - return self._httpcore.HTTPProxy(proxy_url=proxy, **opts) - return self._httpcore.ConnectionPool(**opts) + if use_socks_proxy: + return SOCKSProxyManager(proxy, **opts) + else: + return urllib3.PoolManager(**opts) + else: + return urllib3.ProxyManager(proxy, **opts) + 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, Dict[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 - if timeout > 0: - self._worker.submit(lambda: self._flush_client_reports(force=True)) - self._worker.flush(timeout, callback) + class Http2Transport(BaseHttpTransport): + """The HTTP2 transport based on httpcore.""" - def kill(self): - # type: () -> None - logger.debug("Killing HTTP transport") - self._worker.kill() + 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, + ) - @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, - ) + def _request( + self, + method, + endpoint_type, + body, + headers, + ): + # type: (str, EndpointType, Any, Dict[str, str]) -> httpcore.Response + response = self._pool.request( + method, + self._auth.get_api_url(endpoint_type), + content=body, + headers=headers, + ) + response.headers = response.headers + 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 [] + ) - @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 + 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) - @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 + options["socket_options"] = socket_options + + 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) + +except ImportError: + # Sorry, no Http2Transport for you + pass class _FunctionTransport(Transport): diff --git a/tests/test_transport.py b/tests/test_transport.py index 6b9506cba5..8c69a47c54 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -117,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, @@ -127,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: @@ -154,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 From 9e7e6e90f11d947e3d1b68847259ac9a49003156 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 13:20:51 +0100 Subject: [PATCH 16/24] fix some type issues --- sentry_sdk/client.py | 5 ++--- sentry_sdk/transport.py | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 288dfd4527..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, Http2Transport, make_transport +from sentry_sdk.transport import BaseHttpTransport, make_transport from sentry_sdk.consts import ( DEFAULT_MAX_VALUE_LENGTH, DEFAULT_OPTIONS, @@ -427,8 +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, Http2Transport) + 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/transport.py b/sentry_sdk/transport.py index 90a7bd139c..7b12f53e82 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -19,7 +19,7 @@ from sentry_sdk.worker import BackgroundWorker from sentry_sdk.envelope import Envelope, Item, PayloadRef -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Mapping, cast if TYPE_CHECKING: from typing import Any @@ -349,7 +349,7 @@ def record_loss(reason): logger.error( "Unexpected status code: %s (body: %s)", response.status, - getattr(response, "data", getattr(response, "content")), + getattr(response, "data", getattr(response, "content", None)), ) self.on_dropped_event("status_{}".format(response.status)) record_loss("network_error") @@ -523,7 +523,7 @@ def _request( body, headers, ): - # type: (str, EndpointType, Any, Dict[str, str]) -> Union[urllib3.BaseHTTPResponse, httpcore.Response] + # type: (str, EndpointType, Any, Mapping[str, str]) -> Union[urllib3.BaseHTTPResponse, httpcore.Response] raise NotImplementedError() def capture_envelope( @@ -678,8 +678,9 @@ def _request( body, headers, ): - # type: (str, EndpointType, Any, Dict[str, str]) -> urllib3.BaseHTTPResponse - return self._pool.request( + # type: (str, EndpointType, Any, Mapping[str, str]) -> urllib3.BaseHTTPResponse + pool = cast(self._pool, Union[PoolManager, ProxyManager]) + return pool.request( method, self._auth.get_api_url(endpoint_type), body=body, @@ -711,8 +712,12 @@ def _request( body, headers, ): - # type: (str, EndpointType, Any, Dict[str, str]) -> httpcore.Response - response = self._pool.request( + # type: (str, EndpointType, Any, Mapping[str, str]) -> httpcore.Response + pool = cast( + self._pool, + Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool], + ) + response = pool.request( method, self._auth.get_api_url(endpoint_type), content=body, From 05546470b76514c7c354392b593ed0f6a58f5c20 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 13:40:23 +0100 Subject: [PATCH 17/24] moar typing fixes --- sentry_sdk/transport.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 7b12f53e82..03944b14d7 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -584,6 +584,8 @@ def hub_cls(self, value): class HttpTransport(BaseHttpTransport): + _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] @@ -679,8 +681,7 @@ def _request( headers, ): # type: (str, EndpointType, Any, Mapping[str, str]) -> urllib3.BaseHTTPResponse - pool = cast(self._pool, Union[PoolManager, ProxyManager]) - return pool.request( + return self._pool.request( method, self._auth.get_api_url(endpoint_type), body=body, @@ -694,6 +695,8 @@ def _request( class Http2Transport(BaseHttpTransport): """The HTTP2 transport based on httpcore.""" + _pool: Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + def _get_header_value(self, response, header): # type: (httpcore.Response, str) -> Optional[str] return next( @@ -713,11 +716,7 @@ def _request( headers, ): # type: (str, EndpointType, Any, Mapping[str, str]) -> httpcore.Response - pool = cast( - self._pool, - Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool], - ) - response = pool.request( + response = self._pool.request( method, self._auth.get_api_url(endpoint_type), content=body, @@ -851,7 +850,7 @@ def make_transport(options): # By default, we use the http transport class transport_cls = ( - Http2Transport if use_http2_transport else HttpTransport # type: ignore[type-abstract] + Http2Transport if use_http2_transport else HttpTransport ) # type: Type[Transport] if isinstance(ref_transport, Transport): From 5bd5c42ac3769dc93f4bf291d34431b80579d632 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 13:43:36 +0100 Subject: [PATCH 18/24] MOAR TYPE FIXES! --- sentry_sdk/transport.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 03944b14d7..277ea0895f 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -11,6 +11,7 @@ from urllib.request import getproxies import urllib3 +from urllib3.poolmanager import PoolManager, ProxyManager import certifi import sentry_sdk @@ -19,7 +20,7 @@ from sentry_sdk.worker import BackgroundWorker from sentry_sdk.envelope import Envelope, Item, PayloadRef -from typing import TYPE_CHECKING, Mapping, cast +from typing import TYPE_CHECKING, Mapping, Union if TYPE_CHECKING: from typing import Any @@ -33,9 +34,6 @@ from typing import Union from typing import DefaultDict - from urllib3.poolmanager import PoolManager - from urllib3.poolmanager import ProxyManager - from sentry_sdk._types import Event, EventDataCategory KEEP_ALIVE_SOCKET_OPTIONS = [] From 80f601f240c21c2bd751237a80ea5d1668d8590b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 13:48:47 +0100 Subject: [PATCH 19/24] loving python typing --- sentry_sdk/transport.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 277ea0895f..117216c675 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -31,7 +31,6 @@ from typing import Optional from typing import Tuple from typing import Type - from typing import Union from typing import DefaultDict from sentry_sdk._types import Event, EventDataCategory From 5243cbf77b33013813a69c291a09ed7edbeba5c6 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 14:00:15 +0100 Subject: [PATCH 20/24] python typing fun continues --- sentry_sdk/transport.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 117216c675..5171f6d613 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -11,7 +11,6 @@ from urllib.request import getproxies import urllib3 -from urllib3.poolmanager import PoolManager, ProxyManager import certifi import sentry_sdk @@ -20,18 +19,23 @@ from sentry_sdk.worker import BackgroundWorker from sentry_sdk.envelope import Envelope, Item, PayloadRef -from typing import TYPE_CHECKING, Mapping, Union +from typing import TYPE_CHECKING if TYPE_CHECKING: 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 DefaultDict + from typing import Union + + from urllib3.poolmanager import PoolManager + from urllib3.poolmanager import ProxyManager from sentry_sdk._types import Event, EventDataCategory @@ -581,7 +585,8 @@ def hub_cls(self, value): class HttpTransport(BaseHttpTransport): - _pool: Union[PoolManager, ProxyManager] + 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] @@ -692,7 +697,10 @@ def _request( class Http2Transport(BaseHttpTransport): """The HTTP2 transport based on httpcore.""" - _pool: Union[httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool] + if TYPE_CHECKING: + _pool: Union[ + httpcore.SOCKSProxy, httpcore.HTTPProxy, httpcore.ConnectionPool + ] def _get_header_value(self, response, header): # type: (httpcore.Response, str) -> Optional[str] From 0abc269dd7a80fff9d104a5c379154834d97a334 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 1 Oct 2024 14:02:49 +0100 Subject: [PATCH 21/24] silence mypy --- 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 5171f6d613..de2459208f 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -725,7 +725,7 @@ def _request( method, self._auth.get_api_url(endpoint_type), content=body, - headers=headers, + headers=headers, # type: ignore ) response.headers = response.headers return response From b8785d4bb39c282af3bfb54acdb907f2a894c7f0 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 3 Oct 2024 21:04:00 +0100 Subject: [PATCH 22/24] remove werid self-assignment line --- sentry_sdk/transport.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index de2459208f..710d4d9313 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -727,7 +727,6 @@ def _request( content=body, headers=headers, # type: ignore ) - response.headers = response.headers return response def _get_pool_options(self, ca_certs, cert_file=None, key_file=None): From 7795d8b11a6f7dd6edc5e93bbd29666e53004f9b Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 3 Oct 2024 21:05:42 +0100 Subject: [PATCH 23/24] add transport_http2 to experiment constants list --- sentry_sdk/consts.py | 1 + 1 file changed, 1 insertion(+) 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] From 129ab07ce8304088ed363699c4148f0dc41089c8 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 3 Oct 2024 21:12:49 +0100 Subject: [PATCH 24/24] safer fallback --- sentry_sdk/transport.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 710d4d9313..7a6b4f07b8 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -693,8 +693,21 @@ def _request( 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." + ) - class Http2Transport(BaseHttpTransport): +else: + + class Http2Transport(BaseHttpTransport): # type: ignore """The HTTP2 transport based on httpcore.""" if TYPE_CHECKING: @@ -812,10 +825,6 @@ def _make_pool( return httpcore.ConnectionPool(**opts) -except ImportError: - # Sorry, no Http2Transport for you - pass - class _FunctionTransport(Transport): """