Skip to content

ref(seer): Remove verbose debug_log instrumentation from GitLab webhooks#118192

Draft
billyvg wants to merge 4 commits into
masterfrom
bvong/ref/gitlab-remove-debug-log-instrumentation
Draft

ref(seer): Remove verbose debug_log instrumentation from GitLab webhooks#118192
billyvg wants to merge 4 commits into
masterfrom
bvong/ref/gitlab-remove-debug-log-instrumentation

Conversation

@billyvg

@billyvg billyvg commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

Removes the per-step debug_log breadcrumbs that were added for early GitLab Seer code-review rollout debugging and are now just noise: handler_started, processor_started, dispatching_processors, scheduling_seer_task, preflight_passed, and similar (47 deletions across the merge_request, note, and seat-tracking paths).

Kept:

  • The record_webhook_filtered / record_webhook_handler_error metrics (the queryable funnel signal).
  • The debug_log calls that carry real diagnostic context — missing author data, preflight denials, dedup skips, missing-org.

No behavior change — pure logging cleanup.

Testing

prek (ruff/flake8/mypy) clean; GitLab webhook + seer code-review webhook suites pass (113 tests; the one opened_at failure under --reuse-db is a stale-DB artifact and passes on a fresh DB).

Drop the per-step debug_log breadcrumbs (handler_started, processor_started,
dispatching_processors, scheduling_seer_task, and similar) from the GitLab
merge_request and note webhook paths. These were added for early rollout
debugging and are now noise; the meaningful filter/error signals remain via
record_webhook_filtered/record_webhook_handler_error metrics and the
debug_log calls that carry diagnostic context (missing data, preflight
denials, dedup skips) are kept. No behavior change.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 22, 2026
@getsantry

getsantry Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry Bot added the Stale label Jul 14, 2026
…move-debug-log-instrumentation

# Conflicts:
#	src/sentry/seer/code_review/webhooks/merge_request.py
#	src/sentry/seer/code_review/webhooks/seat_tracking.py
billyvg added 2 commits July 20, 2026 15:42
…smatch

Follow-up to the debug_log cleanup, informed by 30d prod log volumes:

- Remove preflight_denied / note.preflight_denied INFO logs (2.5M/30d, 88%
  a single org_not_eligible_for_code_review reason) — fully redundant with the
  adjacent record_webhook_filtered(denial_reason) metric.
- Remove tracking_contributor_seat (644K/30d) — a pre-call breadcrumb bracketing
  the same track_contributor_seat() as the already-removed contributor_seat_tracked
  log, redundant with the record_contributor_action metric.
- actor_author_mismatch (659K/30d): drop WARNING -> INFO and enrich the extra with
  event_kind, merge_request_state, and actor_username. The mismatch is benign by
  design (reviewers approve, maintainers merge, bots push updates: actor != author),
  so it is not warning-worthy, and the added context tells a benign mismatch apart
  from a real anomaly without a second lookup. GitLab payloads expose the author only
  as a numeric id, so the author side stays an id.
Drop the actor_author_mismatch debug_log (and the _actor_author_mismatch_extra
helper) from both seat-tracking processors. Prod showed 659K/30d, and the
mismatch is expected traffic by design -- a reviewer approving, a maintainer
merging, or a bot pushing an update all legitimately arrive with actor != author
(action breakdown: update 319K, approved/approval 138K, merge 49K). Only the
seat-stage processor is gated to action=="open"; the action-stage processor
(the 515K bulk) fires on every whitelisted action, which is where the volume
comes from.

The 'actor != author -> skip seeding' guard stays -- it's a real correctness
check (seeding would store the actor's username against the author-keyed row).
Only the log call is removed; the skip is now silent.
@getsantry getsantry Bot removed the Stale label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant