Skip to content

Commit 7efb0f9

Browse files
committed
[wip] feat: forward webhooks to codecov
TODO: make this work for monolith mode, i didn't consider monolith mode when I made these changes. I think the path forward for that is to define another middleware that takes care of that and only operates in monolith mode. depends on: #91830 We want to forward webhooks Sentry receives to Codecov since Codecov depends on receiving webhooks from the code forges it supports. We're starting with only GitHub webhooks for now, but plan to support more in the future. We're using the existing WebhookPayload system to forward webhooks to Codecov by creating an additional WebhookPayload object in the GithubRequestParser. Codecov will do additional filtering on its end to filter out irrelevant webhooks so we don't have to do any filtering on Sentry's end, this is to avoid keeping track of what's relevant to Codecov in Sentry. I'm adding a destination_type column to the WebhookPayload so we can distinguish between payloads meant for Codecov and other Sentry regions because when we make the request in the drain_mailbox we want to run different logic, using the CodecovApiClient. I also made the choice to use a different mailbox_name so that the forwarding doesn't interfere with the webhooks being sent to the region silos. Finally, because we want to forward ALL webhooks to Codecov, that means we're forwarding installation events, a webhook that in Sentry gets processed in the control silo, so that gets its own mailbox identifier "installation".
1 parent af50bee commit 7efb0f9

File tree

15 files changed

+378
-22
lines changed

15 files changed

+378
-22
lines changed

migrations_lockfile.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ explore: 0004_add_explore_last_visited_table
99

1010
feedback: 0004_index_together
1111

12-
hybridcloud: 0021_django_arrayfield_scope_list
12+
hybridcloud: 0022_update_webhook_payload
1313

1414
insights: 0001_add_starred_transactions_model
1515

src/sentry/codecov/client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def _create_jwt(self):
5858

5959
def __init__(
6060
self,
61-
git_provider_user: GitProviderId,
61+
git_provider_user: GitProviderId | None,
6262
git_provider: GitProvider = GitProvider.GitHub,
6363
):
6464
"""

src/sentry/conf/server.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3383,6 +3383,8 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]:
33833383

33843384
SENTRY_REPLAYS_SERVICE_URL = "http://localhost:8090"
33853385

3386+
# Codecov is disabled by default
3387+
SENTRY_CODECOV_URL = None
33863388

33873389
SENTRY_ISSUE_ALERT_HISTORY = "sentry.rules.history.backends.postgres.PostgresRuleHistoryBackend"
33883390
SENTRY_ISSUE_ALERT_HISTORY_OPTIONS: dict[str, Any] = {}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Generated by Django 5.2.1 on 2025-05-21 20:33
2+
3+
from django.db import migrations, models
4+
5+
from sentry.new_migrations.migrations import CheckedMigration
6+
7+
8+
class Migration(CheckedMigration):
9+
# This flag is used to mark that a migration shouldn't be automatically run in production.
10+
# This should only be used for operations where it's safe to run the migration after your
11+
# code has deployed. So this should not be used for most operations that alter the schema
12+
# of a table.
13+
# Here are some things that make sense to mark as post deployment:
14+
# - Large data migrations. Typically we want these to be run manually so that they can be
15+
# monitored and not block the deploy for a long period of time while they run.
16+
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
17+
# run this outside deployments so that we don't block them. Note that while adding an index
18+
# is a schema change, it's completely safe to run the operation after the code has deployed.
19+
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
20+
21+
is_post_deployment = False
22+
23+
dependencies = [
24+
("hybridcloud", "0021_django_arrayfield_scope_list"),
25+
]
26+
27+
operations = [
28+
migrations.AddField(
29+
model_name="webhookpayload",
30+
name="destination_type",
31+
field=models.CharField(db_default="sentry_region"),
32+
),
33+
migrations.AlterField(
34+
model_name="webhookpayload",
35+
name="region_name",
36+
field=models.CharField(null=True),
37+
),
38+
]

src/sentry/hybridcloud/models/webhookpayload.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from typing import Any, Self
55

66
from django.db import models
7-
from django.db.models import Case, ExpressionWrapper, F, IntegerField, Value, When
7+
from django.db.models import Case, ExpressionWrapper, F, IntegerField, TextChoices, Value, When
88
from django.http import HttpRequest
99
from django.utils import timezone
1010

@@ -19,13 +19,25 @@
1919
BACKOFF_RATE = 1.4
2020

2121

22+
class DestinationType(TextChoices):
23+
SENTRY_REGION = "sentry_region"
24+
CODECOV = "codecov"
25+
26+
2227
@control_silo_model
2328
class WebhookPayload(Model):
2429
__relocation_scope__ = RelocationScope.Excluded
2530

2631
mailbox_name = models.CharField(null=False, blank=False)
2732
provider = models.CharField(null=True, blank=True)
28-
region_name = models.CharField(null=False)
33+
34+
# Destination attributes
35+
# Table is constantly being deleted from so let's make this non-nullable with a default value, since the table should be small at any given point in time.
36+
destination_type = models.CharField(
37+
choices=DestinationType.choices, null=False, db_default=DestinationType.SENTRY_REGION
38+
)
39+
region_name = models.CharField(null=True)
40+
2941
# May need to add organization_id in the future for debugging.
3042
integration_id = models.BigIntegerField(null=True)
3143

@@ -69,6 +81,7 @@ class Meta:
6981

7082
__repr__ = sane_repr(
7183
"mailbox_name",
84+
"destination_type",
7285
"region_name",
7386
"schedule_for",
7487
"attempts",
@@ -93,7 +106,8 @@ def get_attributes_from_request(
93106
def create_from_request(
94107
cls,
95108
*,
96-
region: str,
109+
destination_type: DestinationType,
110+
region: str | None,
97111
provider: str,
98112
identifier: int | str,
99113
request: HttpRequest,
@@ -103,6 +117,7 @@ def create_from_request(
103117
return cls.objects.create(
104118
mailbox_name=f"{provider}:{identifier}",
105119
provider=provider,
120+
destination_type=destination_type,
106121
region_name=region,
107122
integration_id=integration_id,
108123
**cls.get_attributes_from_request(request),

src/sentry/hybridcloud/tasks/deliver_webhooks.py

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,14 @@
1111
from rest_framework import status
1212

1313
from sentry import options
14+
from sentry.codecov.client import CodecovApiClient, ConfigurationError
1415
from sentry.exceptions import RestrictedIPAddress
15-
from sentry.hybridcloud.models.webhookpayload import BACKOFF_INTERVAL, MAX_ATTEMPTS, WebhookPayload
16+
from sentry.hybridcloud.models.webhookpayload import (
17+
BACKOFF_INTERVAL,
18+
MAX_ATTEMPTS,
19+
DestinationType,
20+
WebhookPayload,
21+
)
1622
from sentry.shared_integrations.exceptions import (
1723
ApiConflictError,
1824
ApiConnectionResetError,
@@ -25,7 +31,7 @@
2531
from sentry.tasks.base import instrumented_task
2632
from sentry.taskworker.config import TaskworkerConfig
2733
from sentry.taskworker.namespaces import hybridcloud_control_tasks
28-
from sentry.types.region import get_region_by_name
34+
from sentry.types.region import Region, get_region_by_name
2935
from sentry.utils import metrics
3036

3137
logger = logging.getLogger(__name__)
@@ -403,12 +409,24 @@ def deliver_message(payload: WebhookPayload) -> None:
403409

404410

405411
def perform_request(payload: WebhookPayload) -> None:
412+
destination_type = payload.destination_type
413+
414+
match destination_type:
415+
case DestinationType.SENTRY_REGION:
416+
region = get_region_by_name(name=payload.region_name)
417+
perform_region_request(region, payload)
418+
case DestinationType.CODECOV:
419+
perform_codecov_request(payload)
420+
case _:
421+
raise ValueError(f"Unknown destination type: {destination_type!r}")
422+
423+
424+
def perform_region_request(region: Region, payload: WebhookPayload) -> None:
406425
logging_context: dict[str, str | int] = {
407426
"payload_id": payload.id,
408427
"mailbox_name": payload.mailbox_name,
409428
"attempt": payload.attempts,
410429
}
411-
region = get_region_by_name(name=payload.region_name)
412430

413431
try:
414432
client = RegionSiloClient(region=region)
@@ -521,3 +539,47 @@ def perform_request(payload: WebhookPayload) -> None:
521539
extra={"error": str(err), "response_code": response_code, **logging_context},
522540
)
523541
raise DeliveryFailed() from err
542+
543+
544+
def perform_codecov_request(payload: WebhookPayload) -> None:
545+
logging_context: dict[str, str | int] = {
546+
"payload_id": payload.id,
547+
"mailbox_name": payload.mailbox_name,
548+
"attempt": payload.attempts,
549+
"request_method": payload.request_method,
550+
"request_path": payload.request_path,
551+
}
552+
553+
with metrics.timer(
554+
"hybridcloud.deliver_webhooks.send_request_to_codecov",
555+
):
556+
try:
557+
client = CodecovApiClient(None)
558+
response = client.post(
559+
endpoint=payload.request_path,
560+
data=payload.request_body.encode("utf-8"),
561+
headers=orjson.loads(payload.request_headers),
562+
)
563+
except ConfigurationError as err:
564+
metrics.incr(
565+
"hybridcloud.deliver_webhooks.send_request_to_codecov.codecov_configuration_error",
566+
)
567+
logger.warning(
568+
"deliver_webhooks.send_request_to_codecov.codecov_configuration_error",
569+
extra={"error": str(err), **logging_context},
570+
)
571+
return
572+
573+
if response is None:
574+
metrics.incr(
575+
"hybridcloud.deliver_webhooks.send_request_to_codecov.failure",
576+
)
577+
return
578+
579+
logger.debug(
580+
"deliver_webhooks.send_request_to_codecov.success",
581+
extra={
582+
"status": response.status_code,
583+
**logging_context,
584+
},
585+
)

src/sentry/integrations/middleware/hybrid_cloud/parser.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
from sentry.api.base import ONE_DAY
1616
from sentry.constants import ObjectStatus
17-
from sentry.hybridcloud.models.webhookpayload import WebhookPayload
17+
from sentry.hybridcloud.models.webhookpayload import DestinationType, WebhookPayload
1818
from sentry.hybridcloud.outbox.category import WebhookProviderIdentifier
1919
from sentry.hybridcloud.services.organization_mapping import organization_mapping_service
2020
from sentry.integrations.middleware.metrics import (
@@ -178,6 +178,7 @@ def get_response_from_webhookpayload(
178178
shard_identifier = identifier or self.webhook_identifier.value
179179
for region in regions:
180180
WebhookPayload.create_from_request(
181+
destination_type=DestinationType.SENTRY_REGION,
181182
region=region.name,
182183
provider=self.provider,
183184
identifier=shard_identifier,
@@ -337,3 +338,24 @@ def get_regions_from_organizations(
337338

338339
def get_default_missing_integration_response(self) -> HttpResponse:
339340
return HttpResponse(status=status.HTTP_400_BAD_REQUEST)
341+
342+
# Forwarding Helpers
343+
344+
def forward_to_codecov(
345+
self,
346+
identifier: int | str | None = None,
347+
integration_id: int | None = None,
348+
):
349+
shard_identifier = f"codecov:{identifier or self.webhook_identifier.value}"
350+
351+
# create webhookpayloads for each service
352+
WebhookPayload.create_from_request(
353+
destination_type=DestinationType.CODECOV,
354+
region=None,
355+
provider=self.provider,
356+
identifier=shard_identifier,
357+
integration_id=integration_id,
358+
request=self.request,
359+
)
360+
361+
return HttpResponse(status=status.HTTP_202_ACCEPTED)

src/sentry/middleware/integrations/parsers/github.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from typing import Any
66

77
import orjson
8+
from django.conf import settings
89
from django.http import HttpResponse
910

1011
from sentry.hybridcloud.outbox.category import WebhookProviderIdentifier
@@ -53,6 +54,11 @@ def get_response(self):
5354
return HttpResponse(status=400)
5455

5556
if event.get("installation") and event.get("action") in {"created", "deleted"}:
57+
if settings.SENTRY_CODECOV_URL: # check if codecov is enabled
58+
self.forward_to_codecov(
59+
identifier="installation",
60+
integration_id=None,
61+
)
5662
return self.get_response_from_control_silo()
5763

5864
try:
@@ -67,6 +73,12 @@ def get_response(self):
6773
if len(regions) == 0:
6874
return self.get_default_missing_integration_response()
6975

76+
if settings.SENTRY_CODECOV_URL: # check if codecov is enabled
77+
self.forward_to_codecov(
78+
identifier=integration.id,
79+
integration_id=integration.id,
80+
)
81+
7082
return self.get_response_from_webhookpayload(
7183
regions=regions, identifier=integration.id, integration_id=integration.id
7284
)

src/sentry/testutils/factories.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1983,7 +1983,9 @@ def create_request_access(
19831983

19841984
@staticmethod
19851985
@assume_test_silo_mode(SiloMode.CONTROL)
1986-
def create_webhook_payload(mailbox_name: str, region_name: str, **kwargs) -> WebhookPayload:
1986+
def create_webhook_payload(
1987+
mailbox_name: str, region_name: str | None, **kwargs
1988+
) -> WebhookPayload:
19871989
payload_kwargs = {
19881990
"request_method": "POST",
19891991
"request_path": "/extensions/github/webhook/",

src/sentry/testutils/outbox.py

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from django.core.handlers.wsgi import WSGIRequest
99

1010
from sentry.hybridcloud.models.outbox import OutboxBase
11-
from sentry.hybridcloud.models.webhookpayload import THE_PAST, WebhookPayload
11+
from sentry.hybridcloud.models.webhookpayload import THE_PAST, DestinationType, WebhookPayload
1212
from sentry.hybridcloud.tasks.deliver_from_outbox import (
1313
enqueue_outbox_jobs,
1414
enqueue_outbox_jobs_control,
@@ -65,19 +65,21 @@ def assert_webhook_payloads_for_mailbox(
6565
request: WSGIRequest,
6666
mailbox_name: str,
6767
region_names: list[str],
68+
destination_types: dict[DestinationType, int] | None = None,
6869
):
6970
"""
7071
A test method for asserting that a webhook payload is properly queued for
7172
the given request
7273
7374
:param request:
7475
:param mailbox_name: The mailbox name that messages should be found in.
75-
:param region_names: The regions each messages should be queued for
76+
:param region_names: Optional list of regions each messages should be queued for
77+
:param destination_type: The destination type of the messages
7678
"""
7779
expected_payload = WebhookPayload.get_attributes_from_request(request=request)
7880
region_names_set = set(region_names)
7981
messages = WebhookPayload.objects.filter(mailbox_name=mailbox_name)
80-
message_count = messages.count()
82+
message_count = messages.filter(region_name__isnull=False).count()
8183
if message_count != len(region_names_set):
8284
raise Exception(
8385
f"Mismatch: Found {message_count} WebhookPayload but {len(region_names_set)} region_names"
@@ -89,11 +91,24 @@ def assert_webhook_payloads_for_mailbox(
8991
assert message.request_body == expected_payload["request_body"]
9092
assert message.schedule_for == THE_PAST
9193
assert message.attempts == 0
92-
try:
93-
region_names_set.remove(message.region_name)
94-
except KeyError:
95-
raise Exception(
96-
f"Found ControlOutbox for '{message.region_name}', which was not in region_names: {str(region_names_set)}"
97-
)
94+
95+
if destination_types:
96+
assert message.destination_type in destination_types
97+
destination_types[message.destination_type] -= 1
98+
if destination_types[message.destination_type] == 0:
99+
del destination_types[message.destination_type]
100+
101+
if message.region_name:
102+
try:
103+
region_names_set.remove(message.region_name)
104+
except KeyError:
105+
raise Exception(
106+
f"Found ControlOutbox for '{message.region_name}', which was not in region_names: {str(region_names_set)}"
107+
)
98108
if len(region_names_set) != 0:
99109
raise Exception(f"WebhookPayload not found for some region_names: {str(region_names_set)}")
110+
111+
if destination_types and len(destination_types) != 0:
112+
raise Exception(
113+
f"WebhookPayload not found for some destination_types: {str(destination_types)}"
114+
)

0 commit comments

Comments
 (0)