[dklawren]

Right now, when the GitHub ETL script hits an error on any repository (for
example, no access to a repo, or a GitHub/BigQuery error after retries), it
exits with a non-zero code. The Kubernetes CronJob pod then reports the whole
run as failed, even when the other repositories were processed and loaded
without problems.

Proposed change:

• Add the sentry-sdk dependency and initialize Sentry when the SENTRY_DSN
  environment variable is set. If it isn't set, nothing changes, so local
  runs, docker-compose and tests behave as they do now.
• Send each per-repository failure to Sentry with its full traceback, tagged
  with the repository name so it can be filtered in the Sentry UI. Warnings
  (retries, backoff) are attached to those events as breadcrumbs.
• When some repositories fail, exit 0 so the cron pod no longer shows as
  failed. Sentry is where those failures get tracked.
• Fatal errors still exit non-zero: missing required environment variables
  (GITHUB_REPOS, BIGQUERY_PROJECT, BIGQUERY_DATASET) and top-level
  RuntimeErrors. These are also sent to Sentry.
• Flush pending Sentry events before the process exits so they aren't lost.

Deployment requirements:

• A Sentry project for github-etl.
• SENTRY_DSN added to the CronJob's secrets. SENTRY_ENVIRONMENT is optional.

Possible follow-up: Sentry Cron Monitors. Partial failures no longer fail the
pod, so check-ins would alert us if a scheduled run is missed or hangs.

[Copilot]

🟡 Changes recommended

Sentry reporting can expose credentials, loses worker-thread breadcrumbs, and suppresses failures when Sentry is unconfigured.

3 open findings

• Sentry events may expose credentials through stack locals · New
• Unset SENTRY_DSN incorrectly masks repository failures · New
• Worker breadcrumbs are missing from Sentry events · New

What changed in this PR

Adds Sentry reporting for repository-level ETL failures while allowing partial runs to succeed.

Changes:

• Initializes Sentry conditionally and flushes events before exit.
• Tags repository failures and changes partial-failure exit behavior.
• Adds Sentry dependencies and tests.

File	Description
main.py	Adds Sentry reporting and revised exit handling.
tests/​test_main.py	Tests Sentry setup and failure behavior.
pyproject.toml	Declares the Sentry dependency.
requirements.txt	Pins the Sentry dependency.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Exception messages can expose credentials to Sentry, and some failures generate duplicate events.

2 open findings

• Redact sensitive token endpoint responses before Sentry capture · New
• Avoid duplicate Sentry events for repository failures · New

3 resolved since last review

• Unset SENTRY_DSN incorrectly masks repository failures
• Sentry events may expose credentials through stack locals
• Worker breadcrumbs are missing from Sentry events

Previously missed (2)

In code that hasn't changed since last review

Verify repository failures are captured with tracebacks

tests/​test_main.py:636

This test verifies the scope tag and exit code, but not that the failure is actually sent with a traceback. Because sentry_sdk.init is mocked, LoggingIntegration is never installed; removing logger.exception would still leave this test passing. Exercise an in-memory Sentry transport or use explicit capture_exception and assert the captured exception and repo tag.

Verify top-level RuntimeErrors are reported to Sentry

tests/​test_main.py:658

The PR also promises to report top-level RuntimeErrors, but this test checks only the return code and flush. With the SDK mocked, the logging integration is inactive, so the test passes even if no event is emitted. Add an assertion through an in-memory transport or switch to explicit capture_exception and assert that call.

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The implementation matches the described failure-handling behavior and has focused automated coverage.

0 open findings

2 resolved since last review

• Redact sensitive token endpoint responses before Sentry capture
• Avoid duplicate Sentry events for repository failures

🧠 Review effort: Balanced

[shtrom]

lgtm; we could test the non-reporting of local variable thing a bit deeper (in fact, rather than just looking at the init options).