♻️ chore(slo): SLOs for CommitContextIntegration #81225
Conversation
| ) | ||
|
|
||
|
|
||
| class MockCommitContextIntegration(CommitContextIntegration): |
There was a problem hiding this comment.
this was the easiest way to test this.
| call for call in mock_record.mock_calls if call.args[0] == EventLifecycleOutcome.HALTED | ||
| ) | ||
| assert event_failures.args[1] == error_msg | ||
| if isinstance(error_msg, Exception): |
There was a problem hiding this comment.
this allows us to use these helpers when we pass an exception as well
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #81225 +/- ##
==========================================
+ Coverage 80.25% 80.35% +0.09%
==========================================
Files 7447 7220 -227
Lines 329627 319587 -10040
Branches 20779 20779
==========================================
- Hits 264540 256791 -7749
+ Misses 64693 62402 -2291
Partials 394 394 |
| @@ -1223,10 +1229,18 @@ def test_gh_comment_debounces(self, get_jwt, mock_comment_workflow, mock_get_com | |||
| ) | |||
| assert mock_comment_workflow.call_count == 1 | |||
|
|
|||
| start_1, failure_1, start_2, failure_2 = mock_record.mock_calls | |||
| assert start_1.args[0] == EventLifecycleOutcome.STARTED | |||
| assert failure_1.args[0] == EventLifecycleOutcome.SUCCESS | |||
There was a problem hiding this comment.
Minor nit: variable names for the failures here don't match the outcomes.
| try: | ||
| response = client.get_blame_for_files(files, extra) | ||
| except IdentityNotValid as e: | ||
| lifecycle.record_failure(e) |
There was a problem hiding this comment.
should we ask Issues team why these specific errors are caught? i'm not sure if this is a failure on our side or the user's side. it seems like this happens when refreshing the identity fails
There was a problem hiding this comment.
let me do that. i think we can record_failure to see how often this happens then adjust?
| logger.info( | ||
| _pr_comment_log(integration_name=self.integration_name, suffix="disabled"), | ||
| extra={"organization_id": project.organization_id}, |
There was a problem hiding this comment.
i remember we added logging when halts happen. does this mean we can remove these logs?
There was a problem hiding this comment.
yep, once we are satisfied with the SLOs, @GabeVillalobos is tracking a task to remove these logs.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
take 2, added ecosystem slo event lifecycles to the mixin.