update tests

tests/sentry/event_manager/grouping/test_seer_grouping.py

@@ -10,7 +10,6 @@
 from sentry.testutils.helpers.eventprocessing import save_new_event
 from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.pytest.mocking import capture_results
-from sentry.utils.circuit_breaker import with_circuit_breaker
 from sentry.utils.types import NonNone
 
 
@@ -152,23 +151,6 @@ def test_obeys_seer_similarity_flags(self):
  assert get_seer_similar_issues_return_values[0][1] == existing_event.group
  assert new_event.group_id == existing_event.group_id
 
- @patch("sentry.event_manager.should_call_seer_for_grouping", return_value=True)
- @patch("sentry.event_manager.with_circuit_breaker", wraps=with_circuit_breaker)
- @patch("sentry.event_manager.get_seer_similar_issues", return_value=({}, None))
- def test_obeys_circult_breaker(
- self, mock_get_seer_similar_issues: MagicMock, mock_with_circuit_breaker: MagicMock, _
- ):
- with patch("sentry.utils.circuit_breaker._should_call_callback", return_value=True):
- save_new_event({"message": "Dogs are great!"}, self.project)
- assert mock_with_circuit_breaker.call_count == 1
- assert mock_get_seer_similar_issues.call_count == 1
-
- with patch("sentry.utils.circuit_breaker._should_call_callback", return_value=False):
- save_new_event({"message": "Adopt don't shop"}, self.project)
-
- assert mock_with_circuit_breaker.call_count == 2 # increased
- assert mock_get_seer_similar_issues.call_count == 1 # didn't increase
-
  @patch("sentry.event_manager.should_call_seer_for_grouping", return_value=True)
  @patch("sentry.event_manager.get_seer_similar_issues", return_value=({}, None))
  def test_calls_seer_if_no_group_found(self, mock_get_seer_similar_issues: MagicMock, _):

tests/sentry/grouping/ingest/test_seer.py

@@ -137,6 +137,18 @@ def test_obeys_project_ratelimit(self):
  is expected_result
  )
 
+ @with_feature("projects:similarity-embeddings-grouping")
+ def test_obeys_circuit_breaker(self):
+ for request_allowed, expected_result in [(True, True), (False, False)]:
+ with patch(
+ "sentry.grouping.ingest.seer.seer_similarity_circuit_breaker.should_allow_request",
+ return_value=request_allowed,
+ ):
+ assert (
+ should_call_seer_for_grouping(self.event, self.primary_hashes)
+ is expected_result
+ )
+
  @with_feature("projects:similarity-embeddings-grouping")
  def test_obeys_customized_fingerprint_check(self):
  default_fingerprint_event = Event(

tests/sentry/seer/similarity/test_similar_issues.py

@@ -96,9 +96,17 @@ def test_no_groups_found(self, mock_seer_request: MagicMock, mock_metrics_incr:
  tags={"response_status": 200, "outcome": "no_similar_groups"},
  )
 
+ @mock.patch(
+ "sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
+ )
  @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
  @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
- def test_bad_response_data(self, mock_seer_request: MagicMock, mock_metrics_incr: MagicMock):
+ def test_bad_response_data(
+ self,
+ mock_seer_request: MagicMock,
+ mock_metrics_incr: MagicMock,
+ mock_record_circuit_breaker_error: MagicMock,
+ ):
  cases: list[tuple[Any, str]] = [
  (None, "AttributeError"),
  ([], "AttributeError"),
@@ -139,14 +147,22 @@ def test_bad_response_data(self, mock_seer_request: MagicMock, mock_metrics_incr
  sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
  tags={"response_status": 200, "outcome": "error", "error": expected_error},
  )
+ assert mock_record_circuit_breaker_error.call_count == 0
 
  mock_metrics_incr.reset_mock()
 
+ @mock.patch(
+ "sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
+ )
  @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
  @mock.patch("sentry.seer.similarity.similar_issues.logger")
  @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
  def test_redirect(
- self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
+ self,
+ mock_seer_request: MagicMock,
+ mock_logger: MagicMock,
+ mock_metrics_incr: MagicMock,
+ mock_record_circuit_breaker_error: MagicMock,
  ):
  mock_seer_request.return_value = HTTPResponse(
  status=308, headers={"location": "/new/and/improved/endpoint/"}
@@ -161,12 +177,20 @@ def test_redirect(
  sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
  tags={"response_status": 308, "outcome": "error", "error": "Redirect"},
  )
+ assert mock_record_circuit_breaker_error.call_count == 0
 
+ @mock.patch(
+ "sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
+ )
  @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
  @mock.patch("sentry.seer.similarity.similar_issues.logger")
  @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
  def test_request_error(
- self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
+ self,
+ mock_seer_request: MagicMock,
+ mock_logger: MagicMock,
+ mock_metrics_incr: MagicMock,
+ mock_record_circuit_breaker_error: MagicMock,
  ):
  for request_error, expected_error_tag in [
  (TimeoutError, "TimeoutError"),
@@ -192,25 +216,44 @@ def test_request_error(
  sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
  tags={"outcome": "error", "error": expected_error_tag},
  )
+ assert mock_record_circuit_breaker_error.call_count == 1
+
+ mock_logger.warning.reset_mock()
+ mock_metrics_incr.reset_mock()
+ mock_record_circuit_breaker_error.reset_mock()
 
+ @mock.patch(
+ "sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
+ )
  @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
  @mock.patch("sentry.seer.similarity.similar_issues.logger")
  @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
  def test_error_status(
- self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
+ self,
+ mock_seer_request: MagicMock,
+ mock_logger: MagicMock,
+ mock_metrics_incr: MagicMock,
+ mock_record_circuit_breaker_error: MagicMock,
  ):
- mock_seer_request.return_value = HTTPResponse("No soup for you", status=403)
+ for response, status, counts_for_circuit_breaker in [
+ ("No soup for you", 403, False),
+ ("No soup, period", 500, True),
+ ]:
+ mock_seer_request.return_value = HTTPResponse(response, status=status)
 
- assert get_similarity_data_from_seer(self.request_params) == []
- mock_logger.error.assert_called_with(
- f"Received 403 when calling Seer endpoint {SEER_SIMILAR_ISSUES_URL}.",
- extra={"response_data": "No soup for you"},
- )
- mock_metrics_incr.assert_any_call(
- "seer.similar_issues_request",
- sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
- tags={"response_status": 403, "outcome": "error", "error": "RequestError"},
- )
+ assert get_similarity_data_from_seer(self.request_params) == []
+ mock_logger.error.assert_called_with(
+ f"Received {status} when calling Seer endpoint {SEER_SIMILAR_ISSUES_URL}.",
+ extra={"response_data": response},
+ )
+ mock_metrics_incr.assert_any_call(
+ "seer.similar_issues_request",
+ sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
+ tags={"response_status": status, "outcome": "error", "error": "RequestError"},
+ )
+ assert mock_record_circuit_breaker_error.call_count == (
+ 1 if counts_for_circuit_breaker else 0
+ )
 
  @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
  def test_returns_sorted_results(self, mock_seer_request: MagicMock):