Skip to content

Commit c248ca7

Browse files
committed
♻️ ref: metrics for task and create_or_update_comment
1 parent 4d0e90a commit c248ca7

File tree

3 files changed

+207
-150
lines changed

3 files changed

+207
-150
lines changed

src/sentry/integrations/source_code_management/commit_context.py

Lines changed: 193 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from sentry.auth.exceptions import IdentityNotValid
1515
from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
1616
from sentry.integrations.source_code_management.metrics import (
17+
CommitContextHaltReason,
1718
SCMIntegrationInteractionEvent,
1819
SCMIntegrationInteractionType,
1920
)
@@ -140,114 +141,140 @@ def queue_comment_task_if_needed(
140141
group_owner: GroupOwner,
141142
group_id: int,
142143
) -> None:
143-
if not OrganizationOption.objects.get_value(
144-
organization=project.organization,
145-
key="sentry:github_pr_bot",
146-
default=True,
147-
):
148-
logger.info(
149-
_pr_comment_log(integration_name=self.integration_name, suffix="disabled"),
150-
extra={"organization_id": project.organization_id},
151-
)
152-
return
153-
154-
repo_query = Repository.objects.filter(id=commit.repository_id).order_by("-date_added")
155-
group = Group.objects.get_from_cache(id=group_id)
156-
if not (
157-
group.level is not logging.INFO and repo_query.exists()
158-
): # Don't comment on info level issues
159-
logger.info(
160-
_pr_comment_log(
161-
integration_name=self.integration_name, suffix="incorrect_repo_config"
162-
),
163-
extra={"organization_id": project.organization_id},
164-
)
165-
return
166-
167-
repo: Repository = repo_query.get()
168-
169-
logger.info(
170-
_pr_comment_log(integration_name=self.integration_name, suffix="queue_comment_check"),
171-
extra={"organization_id": commit.organization_id, "merge_commit_sha": commit.key},
172-
)
173-
from sentry.integrations.github.tasks.pr_comment import github_comment_workflow
174-
175-
# client will raise an Exception if the request is not successful
176-
try:
177-
client = self.get_client()
178-
merge_commit_sha = client.get_merge_commit_sha_from_commit(
179-
repo=repo.name, sha=commit.key
144+
with self.record_event(
145+
SCMIntegrationInteractionType.QUEUE_COMMENT_TASK
146+
).capture() as lifecycle:
147+
lifecycle.add_extras(
148+
{
149+
"project_id": project.id,
150+
"organization_id": project.organization_id,
151+
"commit_id": commit.id,
152+
}
180153
)
181-
except Exception as e:
182-
sentry_sdk.capture_exception(e)
183-
return
154+
if not OrganizationOption.objects.get_value(
155+
organization=project.organization,
156+
key="sentry:github_pr_bot",
157+
default=True,
158+
):
159+
# TODO: remove logger in favor of the log recorded in lifecycle.record_halt
160+
logger.info(
161+
_pr_comment_log(integration_name=self.integration_name, suffix="disabled"),
162+
extra={"organization_id": project.organization_id},
163+
)
164+
lifecycle.record_halt(CommitContextHaltReason.PR_BOT_DISABLED)
165+
return
166+
167+
repo_query = Repository.objects.filter(id=commit.repository_id).order_by("-date_added")
168+
group = Group.objects.get_from_cache(id=group_id)
169+
if not (
170+
group.level is not logging.INFO and repo_query.exists()
171+
): # Don't comment on info level issues
172+
logger.info(
173+
_pr_comment_log(
174+
integration_name=self.integration_name, suffix="incorrect_repo_config"
175+
),
176+
extra={"organization_id": project.organization_id},
177+
)
178+
lifecycle.record_halt(CommitContextHaltReason.INCORRECT_REPO_CONFIG)
179+
return
184180

185-
if merge_commit_sha is None:
186-
logger.info(
187-
_pr_comment_log(
188-
integration_name=self.integration_name,
189-
suffix="queue_comment_workflow.commit_not_in_default_branch",
190-
),
191-
extra={
192-
"organization_id": commit.organization_id,
193-
"repository_id": repo.id,
194-
"commit_sha": commit.key,
195-
},
196-
)
197-
return
181+
repo: Repository = repo_query.get()
182+
lifecycle.add_extra("repository_id", repo.id)
198183

199-
pr_query = PullRequest.objects.filter(
200-
organization_id=commit.organization_id,
201-
repository_id=commit.repository_id,
202-
merge_commit_sha=merge_commit_sha,
203-
)
204-
if not pr_query.exists():
205184
logger.info(
206185
_pr_comment_log(
207-
integration_name=self.integration_name,
208-
suffix="queue_comment_workflow.missing_pr",
186+
integration_name=self.integration_name, suffix="queue_comment_check"
209187
),
210-
extra={
211-
"organization_id": commit.organization_id,
212-
"repository_id": repo.id,
213-
"commit_sha": commit.key,
214-
},
188+
extra={"organization_id": commit.organization_id, "merge_commit_sha": commit.key},
215189
)
216-
return
190+
scope = sentry_sdk.Scope.get_isolation_scope()
191+
scope.set_tag("queue_comment_check.merge_commit_sha", commit.key)
192+
scope.set_tag("queue_comment_check.organization_id", commit.organization_id)
193+
from sentry.integrations.github.tasks.pr_comment import github_comment_workflow
217194

218-
pr = pr_query.first()
219-
assert pr is not None
220-
# need to query explicitly for merged PR comments since we can have multiple comments per PR
221-
merged_pr_comment_query = PullRequestComment.objects.filter(
222-
pull_request_id=pr.id, comment_type=CommentType.MERGED_PR
223-
)
224-
if pr.date_added >= datetime.now(tz=timezone.utc) - timedelta(days=PR_COMMENT_WINDOW) and (
225-
not merged_pr_comment_query.exists()
226-
or group_owner.group_id not in merged_pr_comment_query[0].group_ids
227-
):
228-
lock = locks.get(
229-
_debounce_pr_comment_lock_key(pr.id), duration=10, name="queue_comment_task"
230-
)
231-
with lock.acquire():
232-
cache_key = _debounce_pr_comment_cache_key(pullrequest_id=pr.id)
233-
if cache.get(cache_key) is not None:
234-
return
195+
# client will raise an Exception if the request is not successful
196+
try:
197+
client = self.get_client()
198+
merge_commit_sha = client.get_merge_commit_sha_from_commit(
199+
repo=repo.name, sha=commit.key
200+
)
201+
except Exception as e:
202+
sentry_sdk.capture_exception(e)
203+
lifecycle.record_halt(e)
204+
return
235205

236-
# create PR commit row for suspect commit and PR
237-
PullRequestCommit.objects.get_or_create(commit=commit, pull_request=pr)
206+
if merge_commit_sha is None:
207+
logger.info(
208+
_pr_comment_log(
209+
integration_name=self.integration_name,
210+
suffix="queue_comment_workflow.commit_not_in_default_branch",
211+
),
212+
extra={
213+
"organization_id": commit.organization_id,
214+
"repository_id": repo.id,
215+
"commit_sha": commit.key,
216+
},
217+
)
218+
lifecycle.record_halt(CommitContextHaltReason.COMMIT_NOT_IN_DEFAULT_BRANCH)
219+
return
238220

221+
pr_query = PullRequest.objects.filter(
222+
organization_id=commit.organization_id,
223+
repository_id=commit.repository_id,
224+
merge_commit_sha=merge_commit_sha,
225+
)
226+
if not pr_query.exists():
239227
logger.info(
240228
_pr_comment_log(
241-
integration_name=self.integration_name, suffix="queue_comment_workflow"
229+
integration_name=self.integration_name,
230+
suffix="queue_comment_workflow.missing_pr",
242231
),
243-
extra={"pullrequest_id": pr.id, "project_id": group_owner.project_id},
232+
extra={
233+
"organization_id": commit.organization_id,
234+
"repository_id": repo.id,
235+
"commit_sha": commit.key,
236+
},
237+
)
238+
lifecycle.record_halt(CommitContextHaltReason.MISSING_PR)
239+
return
240+
241+
pr = pr_query.first()
242+
lifecycle.add_extra("pull_request_id", pr.id)
243+
assert pr is not None
244+
# need to query explicitly for merged PR comments since we can have multiple comments per PR
245+
merged_pr_comment_query = PullRequestComment.objects.filter(
246+
pull_request_id=pr.id, comment_type=CommentType.MERGED_PR
247+
)
248+
if pr.date_added >= datetime.now(tz=timezone.utc) - timedelta(
249+
days=PR_COMMENT_WINDOW
250+
) and (
251+
not merged_pr_comment_query.exists()
252+
or group_owner.group_id not in merged_pr_comment_query[0].group_ids
253+
):
254+
lock = locks.get(
255+
_debounce_pr_comment_lock_key(pr.id), duration=10, name="queue_comment_task"
244256
)
257+
with lock.acquire():
258+
cache_key = _debounce_pr_comment_cache_key(pullrequest_id=pr.id)
259+
if cache.get(cache_key) is not None:
260+
lifecycle.record_halt(CommitContextHaltReason.ALREADY_QUEUED)
261+
return
245262

246-
cache.set(cache_key, True, PR_COMMENT_TASK_TTL)
263+
# create PR commit row for suspect commit and PR
264+
PullRequestCommit.objects.get_or_create(commit=commit, pull_request=pr)
247265

248-
github_comment_workflow.delay(
249-
pullrequest_id=pr.id, project_id=group_owner.project_id
250-
)
266+
logger.info(
267+
_pr_comment_log(
268+
integration_name=self.integration_name, suffix="queue_comment_workflow"
269+
),
270+
extra={"pullrequest_id": pr.id, "project_id": group_owner.project_id},
271+
)
272+
273+
cache.set(cache_key, True, PR_COMMENT_TASK_TTL)
274+
275+
github_comment_workflow.delay(
276+
pullrequest_id=pr.id, project_id=group_owner.project_id
277+
)
251278

252279
def create_or_update_comment(
253280
self,
@@ -268,70 +295,86 @@ def create_or_update_comment(
268295
)
269296
pr_comment = pr_comment_query[0] if pr_comment_query.exists() else None
270297

271-
# client will raise ApiError if the request is not successful
272-
if pr_comment is None:
273-
resp = client.create_comment(
274-
repo=repo.name,
275-
issue_id=str(pr_key),
276-
data=(
277-
{
278-
"body": comment_body,
279-
"actions": github_copilot_actions,
280-
}
281-
if github_copilot_actions
282-
else {"body": comment_body}
283-
),
284-
)
298+
interaction_type = (
299+
SCMIntegrationInteractionType.CREATE_COMMENT
300+
if not pr_comment
301+
else SCMIntegrationInteractionType.UPDATE_COMMENT
302+
)
285303

286-
current_time = django_timezone.now()
287-
comment = PullRequestComment.objects.create(
288-
external_id=resp.body["id"],
289-
pull_request_id=pullrequest_id,
290-
created_at=current_time,
291-
updated_at=current_time,
292-
group_ids=issue_list,
293-
comment_type=comment_type,
294-
)
295-
metrics.incr(
296-
metrics_base.format(integration=self.integration_name, key="comment_created")
304+
with self.record_event(interaction_type).capture() as lifecycle:
305+
lifecycle.add_extras(
306+
{
307+
"repository_id": repo.id,
308+
"organization_id": repo.organization_id,
309+
"pull_request_id": pullrequest_id,
310+
"comment_type": comment_type,
311+
"language": language,
312+
}
297313
)
298314

299-
if comment_type == CommentType.OPEN_PR:
300-
analytics.record(
301-
"open_pr_comment.created",
302-
comment_id=comment.id,
303-
org_id=repo.organization_id,
304-
pr_id=pullrequest_id,
305-
language=(language or "not found"),
315+
if pr_comment is None:
316+
resp = client.create_comment(
317+
repo=repo.name,
318+
issue_id=str(pr_key),
319+
data=(
320+
{
321+
"body": comment_body,
322+
"actions": github_copilot_actions,
323+
}
324+
if github_copilot_actions
325+
else {"body": comment_body}
326+
),
306327
)
307-
else:
308-
resp = client.update_comment(
309-
repo=repo.name,
310-
issue_id=str(pr_key),
311-
comment_id=pr_comment.external_id,
312-
data=(
313-
{
314-
"body": comment_body,
315-
"actions": github_copilot_actions,
316-
}
317-
if github_copilot_actions
318-
else {"body": comment_body}
319-
),
328+
329+
current_time = django_timezone.now()
330+
comment = PullRequestComment.objects.create(
331+
external_id=resp.body["id"],
332+
pull_request_id=pullrequest_id,
333+
created_at=current_time,
334+
updated_at=current_time,
335+
group_ids=issue_list,
336+
comment_type=comment_type,
337+
)
338+
metrics.incr(
339+
metrics_base.format(integration=self.integration_name, key="comment_created")
340+
)
341+
342+
if comment_type == CommentType.OPEN_PR:
343+
analytics.record(
344+
"open_pr_comment.created",
345+
comment_id=comment.id,
346+
org_id=repo.organization_id,
347+
pr_id=pullrequest_id,
348+
language=(language or "not found"),
349+
)
350+
else:
351+
resp = client.update_comment(
352+
repo=repo.name,
353+
issue_id=str(pr_key),
354+
comment_id=pr_comment.external_id,
355+
data=(
356+
{
357+
"body": comment_body,
358+
"actions": github_copilot_actions,
359+
}
360+
if github_copilot_actions
361+
else {"body": comment_body}
362+
),
363+
)
364+
metrics.incr(
365+
metrics_base.format(integration=self.integration_name, key="comment_updated")
366+
)
367+
pr_comment.updated_at = django_timezone.now()
368+
pr_comment.group_ids = issue_list
369+
pr_comment.save()
370+
371+
logger_event = metrics_base.format(
372+
integration=self.integration_name, key="create_or_update_comment"
320373
)
321-
metrics.incr(
322-
metrics_base.format(integration=self.integration_name, key="comment_updated")
374+
logger.info(
375+
logger_event,
376+
extra={"new_comment": pr_comment is None, "pr_key": pr_key, "repo": repo.name},
323377
)
324-
pr_comment.updated_at = django_timezone.now()
325-
pr_comment.group_ids = issue_list
326-
pr_comment.save()
327-
328-
logger_event = metrics_base.format(
329-
integration=self.integration_name, key="create_or_update_comment"
330-
)
331-
logger.info(
332-
logger_event,
333-
extra={"new_comment": pr_comment is None, "pr_key": pr_key, "repo": repo.name},
334-
)
335378

336379

337380
class CommitContextClient(ABC):

src/sentry/integrations/source_code_management/metrics.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,13 @@ class LinkAllReposHaltReason(StrEnum):
7474
MISSING_ORGANIZATION = "missing_organization"
7575
RATE_LIMITED = "rate_limited"
7676
REPOSITORY_NOT_CREATED = "repository_not_created"
77+
78+
79+
class CommitContextHaltReason(StrEnum):
80+
"""Common reasons why a commit context integration may halt without success/failure."""
81+
82+
PR_BOT_DISABLED = "pr_bot_disabled"
83+
INCORRECT_REPO_CONFIG = "incorrect_repo_config"
84+
COMMIT_NOT_IN_DEFAULT_BRANCH = "commit_not_in_default_branch"
85+
MISSING_PR = "missing_pr"
86+
ALREADY_QUEUED = "already_queued"

0 commit comments

Comments
 (0)