chore(issues): Add LoggedRetry to log retry information

pyproject.toml

@@ -326,6 +326,7 @@ module = [
     "sentry.models.options.*",
     "sentry.monitors.consumers.monitor_consumer",
     "sentry.monkey.*",
+    "sentry.net.retry",
     "sentry.net.socket",
     "sentry.nodestore.*",
     "sentry.nodestore.base",
@@ -528,6 +529,7 @@ module = [
     "tests.sentry.issues.test_status_change_consumer",
     "tests.sentry.issues.test_update_inbox",
     "tests.sentry.middleware.test_reporting_endpoint",
+    "tests.sentry.net.test_retry",
     "tests.sentry.nodestore.bigtable.test_backend",
     "tests.sentry.organizations.*",
     "tests.sentry.post_process_forwarder.*",

src/sentry/net/retry.py

@@ -0,0 +1,50 @@
+import logging
+from types import TracebackType
+from typing import Any, Self
+
+from urllib3 import BaseHTTPResponse
+from urllib3.connectionpool import ConnectionPool
+from urllib3.util.retry import Retry
+
+default_logger = logging.getLogger(__name__)
+
+
+class LoggedRetry(Retry):
+    def __init__(self, logger: logging.Logger | None = None, **kwargs: Any) -> None:
+        super().__init__(**kwargs)
+        self.logger = logger or default_logger
+
+    def increment(
+        self,
+        method: str | None = None,
+        url: str | None = None,
+        response: BaseHTTPResponse | None = None,
+        error: Exception | None = None,
+        _pool: ConnectionPool | None = None,
+        _stacktrace: TracebackType | None = None,
+    ) -> Self:
+        # Increment uses Retry.new to instantiate a new instance so we need to
+        # manually propagate the logger as it can't be passed through increment.
+        retry = super().increment(
+            method=method,
+            url=url,
+            response=response,
+            error=error,
+            _pool=_pool,
+            _stacktrace=_stacktrace,
+        )
+        retry.logger = self.logger
+
+        extra: dict[str, str | int | None] = {
+            "request_method": method,
+            "request_url": url,
+            "retry_total_remaining": retry.total,
+        }
+        if response is not None:
+            extra["response_status"] = response.status
+        if error is not None:
+            extra["error"] = error.__class__.__name__
+
+        self.logger.info("Request retried", extra=extra)
+
+        return retry

src/sentry/seer/signed_seer_api.py

@@ -8,6 +8,7 @@
 import sentry_sdk
 from django.conf import settings
 from urllib3 import BaseHTTPResponse, HTTPConnectionPool
+from urllib3.util.retry import Retry
 
 from sentry import options
 from sentry.utils import metrics
@@ -21,7 +22,7 @@ def make_signed_seer_api_request(
     path: str,
     body: bytes,
     timeout: int | float | None = None,
-    retries: int | None = None,
+    retries: Retry | int | None = None,
     metric_tags: dict[str, Any] | None = None,
 ) -> BaseHTTPResponse:
     host = connection_pool.host

src/sentry/seer/similarity/similar_issues.py

@@ -14,6 +14,7 @@
 )
 from sentry.models.grouphashmetadata import GroupHashMetadata
 from sentry.net.http import connection_from_url
+from sentry.net.retry import LoggedRetry
 from sentry.seer.signed_seer_api import make_signed_seer_api_request
 from sentry.seer.similarity.types import (
     IncompleteSeerDataError,
@@ -65,14 +66,18 @@ def get_similarity_data_from_seer(
         options.get("seer.similarity.circuit-breaker-config"),
     )
 
+    retries: LoggedRetry | None = None
+    if total := options.get("seer.similarity.grouping-ingest-retries"):
+        retries = LoggedRetry(logger=logger, total=total)
+
     try:
         response = make_signed_seer_api_request(
             seer_grouping_connection_pool,
             SEER_SIMILAR_ISSUES_URL,
             json.dumps({"threshold": SEER_MAX_GROUPING_DISTANCE, **similar_issues_request}).encode(
                 "utf8"
             ),
-            retries=options.get("seer.similarity.grouping-ingest-retries"),
+            retries=retries,
             timeout=options.get("seer.similarity.grouping-ingest-timeout"),
             metric_tags={"referrer": referrer} if referrer else {},
         )

tests/sentry/issues/endpoints/test_group_similar_issues_embeddings.py

@@ -183,7 +183,6 @@ def test_simple(
         mock_seer_request.assert_called_with(
             "POST",
             SEER_SIMILAR_ISSUES_URL,
-            retries=options.get("seer.similarity.grouping-ingest-retries"),
             timeout=options.get("seer.similarity.grouping-ingest-timeout"),
             body=orjson.dumps(expected_seer_request_params),
             headers={"content-type": "application/json;charset=utf-8"},
@@ -613,7 +612,6 @@ def test_no_optional_params(self, mock_seer_request: mock.MagicMock) -> None:
         mock_seer_request.assert_called_with(
             "POST",
             SEER_SIMILAR_ISSUES_URL,
-            retries=options.get("seer.similarity.grouping-ingest-retries"),
             timeout=options.get("seer.similarity.grouping-ingest-timeout"),
             body=orjson.dumps(
                 {
@@ -641,7 +639,6 @@ def test_no_optional_params(self, mock_seer_request: mock.MagicMock) -> None:
         mock_seer_request.assert_called_with(
             "POST",
             SEER_SIMILAR_ISSUES_URL,
-            retries=options.get("seer.similarity.grouping-ingest-retries"),
             timeout=options.get("seer.similarity.grouping-ingest-timeout"),
             body=orjson.dumps(
                 {
@@ -672,7 +669,6 @@ def test_no_optional_params(self, mock_seer_request: mock.MagicMock) -> None:
         mock_seer_request.assert_called_with(
             "POST",
             SEER_SIMILAR_ISSUES_URL,
-            retries=options.get("seer.similarity.grouping-ingest-retries"),
             timeout=options.get("seer.similarity.grouping-ingest-timeout"),
             body=orjson.dumps(
                 {

tests/sentry/net/test_retry.py

@@ -0,0 +1,52 @@
+import logging
+from unittest.mock import Mock, call, patch
+
+from urllib3 import PoolManager
+from urllib3.exceptions import ConnectTimeoutError
+from urllib3.response import HTTPResponse
+
+from sentry.net.retry import LoggedRetry
+
+
+def test_logged_retry() -> None:
+    mock_logger = Mock(spec=logging.Logger)
+
+    pool = PoolManager(
+        retries=LoggedRetry(mock_logger, total=2, status_forcelist=[500]),
+    )
+
+    with patch(
+        "urllib3.connectionpool.HTTPConnectionPool._make_request",
+        side_effect=[
+            ConnectTimeoutError(),
+            HTTPResponse(status=500, body=b"not ok"),
+            HTTPResponse(status=200, body=b"ok"),
+        ],
+    ):
+        response = pool.request("GET", "http://example.com")
+
+    assert mock_logger.info.call_count == 2
+    mock_logger.assert_has_calls(
+        [
+            call.info(
+                "Request retried",
+                extra={
+                    "request_method": "GET",
+                    "request_url": "/",
+                    "retry_total_remaining": 1,
+                    "error": "ConnectTimeoutError",
+                },
+            ),
+            call.info(
+                "Request retried",
+                extra={
+                    "request_method": "GET",
+                    "request_url": "/",
+                    "retry_total_remaining": 0,
+                    "response_status": 500,
+                },
+            ),
+        ]
+    )
+    assert response.status == 200
+    assert response.data == b"ok"