Skip to content

Commit 8a4fa6a

Browse files
authored
feat(webhook): Support skipping sending GitHub webhooks to Codecov (#107946)
We're seeing intermittent slow downs doing code reviews for the getsentry PRs. The forwarding [from control to region](https://redash.getsentry.net/queries/10522#13744) tends to be slow. I've noticed a lot of Codecov API calls failing on [this issue](https://sentry.sentry.io/issues/6696659077). My theory is that we skip the calls to Codecov we will stop having this problem.
1 parent 2afac17 commit 8a4fa6a

3 files changed

Lines changed: 99 additions & 10 deletions

File tree

src/sentry/hybridcloud/tasks/deliver_webhooks.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,36 @@ def perform_region_request(region: Region, payload: WebhookPayload) -> None:
555555
raise DeliveryFailed() from err
556556

557557

558+
def _should_skip_codecov_forward_for_github_owner(payload: WebhookPayload) -> bool:
559+
"""
560+
Return True if this payload should be skipped (not forwarded to Codecov).
561+
The payload is still deleted by the caller when skipped.
562+
"""
563+
skip_github_owners = options.get("codecov.forward-webhooks.skip-github-owners") or ()
564+
skip_set = (
565+
frozenset(str(x) for x in skip_github_owners if x) if skip_github_owners else frozenset()
566+
)
567+
if not skip_set:
568+
return False
569+
try:
570+
body = orjson.loads(payload.request_body)
571+
except orjson.JSONDecodeError:
572+
return False
573+
if not isinstance(body, dict):
574+
return False
575+
repository = body.get("repository") if isinstance(body.get("repository"), dict) else None
576+
owner = (
577+
repository.get("owner")
578+
if repository and isinstance(repository.get("owner"), dict)
579+
else None
580+
)
581+
login = owner.get("login") if owner else None
582+
if isinstance(login, str) and login in skip_set:
583+
metrics.incr("hybridcloud.deliver_webhooks.send_request_to_codecov.filtered")
584+
return True
585+
return False
586+
587+
558588
def perform_codecov_request(payload: WebhookPayload) -> None:
559589
"""
560590
We don't retry forwarding Codecov requests for now. We want to prove out that it would work.
@@ -581,6 +611,9 @@ def perform_codecov_request(payload: WebhookPayload) -> None:
581611
)
582612
return
583613

614+
if _should_skip_codecov_forward_for_github_owner(payload):
615+
return
616+
584617
# hard coding this because the endpoint path is different from the original request
585618
endpoint = "/webhooks/sentry"
586619

src/sentry/options/defaults.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,13 @@
700700
register("codecov.forward-webhooks.rollout", default=0.0, flags=FLAG_AUTOMATOR_MODIFIABLE)
701701
# if a region is in this list, it's safe to forward to codecov
702702
register("codecov.forward-webhooks.regions", default=[], flags=FLAG_AUTOMATOR_MODIFIABLE)
703+
# GitHub owners whose webhooks we skip forwarding to Codecov (payload is still deleted)
704+
register(
705+
"codecov.forward-webhooks.skip-github-owners",
706+
type=Sequence,
707+
default=["getsentry"],
708+
flags=FLAG_AUTOMATOR_MODIFIABLE,
709+
)
703710

704711

705712
# GitHub Integration

tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,7 @@ def test_drain_success(self) -> None:
348348

349349
@responses.activate
350350
@override_settings(CODECOV_API_BASE_URL="https://api.codecov.io")
351-
@override_options(
352-
{
353-
"codecov.api-bridge-signing-secret": "test",
354-
}
355-
)
351+
@override_options({"codecov.api-bridge-signing-secret": "test"})
356352
@override_regions(region_config)
357353
def test_drain_success_codecov(self) -> None:
358354
responses.add(
@@ -393,11 +389,7 @@ def test_drain_codecov_configuration_error(self) -> None:
393389

394390
@responses.activate
395391
@override_settings(CODECOV_API_BASE_URL="https://api.codecov.io")
396-
@override_options(
397-
{
398-
"codecov.api-bridge-signing-secret": "test",
399-
}
400-
)
392+
@override_options({"codecov.api-bridge-signing-secret": "test"})
401393
@override_regions(region_config)
402394
def test_drain_codecov_request_error(self) -> None:
403395
responses.add(
@@ -417,6 +409,63 @@ def test_drain_codecov_request_error(self) -> None:
417409
assert hook is None
418410
assert len(responses.calls) == 3
419411

412+
@responses.activate
413+
def test_drain_codecov_filtered_getsentry_owner(self) -> None:
414+
"""Skip list uses default option (getsentry); no client or request needed."""
415+
records = create_payloads_with_destination_type(
416+
1, "github:codecov:123", DestinationType.CODECOV
417+
)
418+
records[0].request_body = '{"repository":{"owner":{"login":"getsentry"}}}'
419+
records[0].save(update_fields=["request_body"])
420+
drain_mailbox(records[0].id)
421+
422+
assert len(responses.calls) == 0
423+
assert not WebhookPayload.objects.filter().exists()
424+
425+
@responses.activate
426+
@override_settings(CODECOV_API_BASE_URL="https://api.codecov.io")
427+
@override_options({"codecov.api-bridge-signing-secret": "test"})
428+
@override_regions(region_config)
429+
def test_drain_codecov_not_filtered_other_owner(self) -> None:
430+
responses.add(
431+
responses.POST,
432+
"https://api.codecov.io/webhooks/sentry",
433+
status=200,
434+
body="",
435+
)
436+
records = create_payloads_with_destination_type(
437+
3, "github:codecov:123", DestinationType.CODECOV
438+
)
439+
for record in records:
440+
record.request_body = '{"repository":{"owner":{"login":"other-org"}}}'
441+
record.save(update_fields=["request_body"])
442+
drain_mailbox(records[0].id)
443+
444+
assert len(responses.calls) == 3
445+
assert not WebhookPayload.objects.filter().exists()
446+
447+
@responses.activate
448+
@override_settings(CODECOV_API_BASE_URL="https://api.codecov.io")
449+
@override_options({"codecov.api-bridge-signing-secret": "test"})
450+
@override_regions(region_config)
451+
def test_drain_codecov_not_filtered_malformed_body(self) -> None:
452+
responses.add(
453+
responses.POST,
454+
"https://api.codecov.io/webhooks/sentry",
455+
status=200,
456+
body="",
457+
)
458+
records = create_payloads_with_destination_type(
459+
3, "github:codecov:123", DestinationType.CODECOV
460+
)
461+
for record in records:
462+
record.request_body = "{}"
463+
record.save(update_fields=["request_body"])
464+
drain_mailbox(records[0].id)
465+
466+
assert len(responses.calls) == 3
467+
assert not WebhookPayload.objects.filter().exists()
468+
420469
@responses.activate
421470
@override_regions(region_config)
422471
def test_drain_time_limit(self) -> None:

0 commit comments

Comments
 (0)