lekss361/gendesign

Fork 0

feat(backend): sentry-sdk init для FastAPI + Celery (#204 backend) #207

Merged

lekss361 merged 2 commits from feat/204-glitchtip-backend into main

2026-05-16 15:09:17 +00:00

lekss361 commented

2026-05-16 10:40:47 +00:00

Owner

Closes #204 (backend часть)

Summary

Подключает sentry-sdk[fastapi,celery,sqlalchemy,httpx]>=2.18.0 к backend
SDK инициализируется до создания FastAPI app (module-level в main.py) и отдельно в celery_app.py для Celery-процесса
Без GLITCHTIP_DSN — SDK не инициализируется, контейнер стартует без ошибок

Интеграции

StarletteIntegration + FastApiIntegration — трейсинг HTTP-запросов
CeleryIntegration(monitor_beat_tasks=True) — ошибки и beat-мониторинг
SqlalchemyIntegration — slow/error queries
HttpxIntegration — исходящие httpx запросы
LoggingIntegration(level=INFO, event_level=ERROR) — ERROR-логи → события GlitchTip
profiles_sample_rate=0.0 — GlitchTip профили не поддерживает

Env-переменные (новые)

Var	Default	Описание
`GLITCHTIP_DSN`	(пусто)	DSN формата `https://<key>@errors.gendsgn.ru/<project_id>`
`GLITCHTIP_TRACES_SAMPLE_RATE`	`0.05`	Доля трейсируемых запросов

Acceptance checklist

uv sync проходит без ошибок
ruff check app/ — чисто
Контейнер стартует с пустым GLITCHTIP_DSN (no-op)
После деплоя с реальным DSN — тестовое событие видно в GlitchTip dashboard
Beat-монитор задач появляется в GlitchTip Crons после нескольких beat-тиков

Closes #204 (backend часть) ## Summary - Подключает `sentry-sdk[fastapi,celery,sqlalchemy,httpx]>=2.18.0` к backend - SDK инициализируется до создания FastAPI app (module-level в `main.py`) и отдельно в `celery_app.py` для Celery-процесса - Без `GLITCHTIP_DSN` — SDK не инициализируется, контейнер стартует без ошибок ## Интеграции - `StarletteIntegration` + `FastApiIntegration` — трейсинг HTTP-запросов - `CeleryIntegration(monitor_beat_tasks=True)` — ошибки и beat-мониторинг - `SqlalchemyIntegration` — slow/error queries - `HttpxIntegration` — исходящие httpx запросы - `LoggingIntegration(level=INFO, event_level=ERROR)` — ERROR-логи → события GlitchTip - `profiles_sample_rate=0.0` — GlitchTip профили не поддерживает ## Env-переменные (новые) | Var | Default | Описание | |---|---|---| | `GLITCHTIP_DSN` | (пусто) | DSN формата `https://<key>@errors.gendsgn.ru/<project_id>` | | `GLITCHTIP_TRACES_SAMPLE_RATE` | `0.05` | Доля трейсируемых запросов | ## Acceptance checklist - [ ] `uv sync` проходит без ошибок - [ ] `ruff check app/` — чисто - [ ] Контейнер стартует с пустым `GLITCHTIP_DSN` (no-op) - [ ] После деплоя с реальным DSN — тестовое событие видно в GlitchTip dashboard - [ ] Beat-монитор задач появляется в GlitchTip Crons после нескольких beat-тиков

lekss361 added 1 commit 2026-05-16 10:40:47 +00:00

feat(backend): sentry-sdk init для FastAPI + Celery через GlitchTip (#204 backend) 2f79f7c40a

Подключает sentry-sdk ^2.18 к FastAPI и Celery-воркеру:
- Интеграции: StarletteIntegration, FastApiIntegration, CeleryIntegration,
  SqlalchemyIntegration, HttpxIntegration, LoggingIntegration
- SDK инициализируется до создания FastAPI app (module-level), а также
  отдельно в celery_app.py для Celery-процесса
- Без GLITCHTIP_DSN — SDK no-op, контейнер стартует без ошибок
- profiles_sample_rate=0.0 — GlitchTip профили не поддерживает
- GIT_SHA из ENV как release-тег; fallback "unknown"

lekss361 referenced this pull request

2026-05-16 10:46:08 +00:00

feat(infra): GlitchTip self-hosted error tracking (#204 devops) #205

lekss361 commented

2026-05-16 10:47:18 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: 🟠 HIGH — два регрессии deploy-pipeline, остальное OK
Files reviewed: 7 (P1: main.py, celery_app.py, config.py, pyproject.toml · P2: .env.example · P3: test_sentry_init.py · ⚪ uv.lock)
Lines: +146 / -18
Head SHA: 2f79f7c · Mergeable: true · Conflicts: none
Time: ~12 min

🟠 High (должен быть пофиксен до merge)

1. Release tracking сломан в production — GIT_SHA нигде не выставляется

Новый код в main.py:37 и celery_app.py:25:

release=os.getenv("GIT_SHA", "unknown"),

Но GIT_SHA не задан нигде:

.forgejo/workflows/deploy.yml пишет SENTRY_RELEASE=$IMAGE_TAG в backend/.env.runtime (lines 157, 164, 180-184) — НЕ GIT_SHA
docker-compose.prod.yml env_file подгружает .env.runtime, в котором SENTRY_RELEASE=... — это значение становится os.environ["SENTRY_RELEASE"], а не GIT_SHA
backend/.env.runtime.example:6 всё ещё содержит SENTRY_RELEASE= (не обновлён)
.claude/rules/deploy.md явно фиксирует контракт SENTRY_RELEASE=$IMAGE_TAG в .env.runtime

Результат после merge: каждый event в GlitchTip будет помечен release=unknown — теряется возможность фильтра по версии, regression detection, deploy markers. Это тихая поломка: SDK инициализируется, события идут, но release-tag всегда "unknown".

Fix (выбрать один):

Вариант A — минимальное вмешательство (рекомендуется): прочитать обе env-vars в main.py и celery_app.py:
```
release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown",
```
Вариант B — обновить deploy.yml: переименовать SENTRY_RELEASE_VAL → GIT_SHA_VAL, в sed писать GIT_SHA=... в .env.runtime. Также обновить backend/.env.runtime.example (GIT_SHA=) и .claude/rules/deploy.md. Туда же README.md:109 и docker-compose.prod.yml:51 (комментарии).

Без любого из них release tracking remains broken — это не fail-deploy, но обесценивает половину ценности GlitchTip (deploy markers / version filtering).

2. Init-блок не отрабатывает в beat процессе

docker-compose.prod.yml:122:

command: ["celery", "-A", "app.workers.celery_app", "beat", ...]

celery beat -A app.workers.celery_app импортирует celery_app.py — module-level if settings.glitchtip_dsn: sentry_sdk.init(...) отработает, ✅ beat покрыт.

Но worker командуется аналогично (docker-compose.prod.yml:109) → SDK init происходит в родительском процессе worker'а до fork'а. С prefork-pool по умолчанию child-процессы наследуют SDK state через fork — это работает для error capture, но CeleryIntegration(monitor_beat_tasks=True) heartbeat-events отправляются из beat-процесса, не из worker fork'а — то есть pattern корректен.

⚠️ Однако: если в app.workers.lifecycle есть @worker_process_init.connect hook (vault упоминает Bug_Worker_Ready_EarlyReturn) — нужно убедиться, что после fork SDK переинициализируется или transport reopen'ится. Стандартный sentry-sdk это делает автоматически через multiprocessing.util.register_after_fork (с 2.x). На версии >=2.18.0 — OK.

🤔 Question: проверить руками после deploy, что test-event из worker task (celery_app.send_task(...)) реально доходит до GlitchTip — fork-safety багов в sentry-sdk[celery] 2.x исторически было несколько.

🟡 Medium (желательно fix, но не блокирующее)

3. PII в логах через LoggingIntegration(level=INFO, event_level=ERROR)

send_default_pii=False — корректно, IP / cookies / headers фильтруются. НО:

33 call site logger.error/exception в 18 файлах — каждый теперь шлёт event
В backend/app/workers/tasks/nspd_geo.py:140-148, nspd_geo.py:225-238 логируются cad_num через f-string в WARNING/INFO. Per CLAUDE.md «cad_num as PII» в задаче — это spec-level concern. Reality check: cad_num по 152-ФЗ скорее всего НЕ персональные данные (это публичный кадастровый номер, не привязан к физлицу до запроса в ЕГРН), но если у нас лиды связаны с конкретным cad_num — это уже бизнес-PII.
domrf_kn.py:620-627 — уже специально подавляет логирование в Sentry/Docker для одного класса ошибок (comment подтверждает учёт). Хороший знак — codebase осведомлён о volume.

Fix-suggestion (опционально): в LoggingIntegration поднять event_level=logging.WARNING или оставить ERROR — но добавить before_send фильтр, scrubbing'ующий поля inn, phone, email, password, token из event extra. Канонически:

def _scrub(event, hint):
    SENSITIVE = ("inn", "phone", "email", "password", "token", "api_key", "lead_phone")
    for section in ("extra", "contexts"):
        data = event.get(section, {})
        if isinstance(data, dict):
            for k in list(data):
                if any(s in k.lower() for s in SENSITIVE):
                    data[k] = "[Filtered]"
    return event

sentry_sdk.init(..., before_send=_scrub)

4. traces_sample_rate=0.05 × HttpxIntegration → утечка outbound URLs

HttpxIntegration пишет URL outbound запросов в spans. У нас scrapers ходят на:

pkk.rosreestr.ru/api/features/1?... — содержит cad_num в querystring
api.objctv.ru/... — содержит apiKey=<KEY> в querystring (config.py:48-50)
domrf-kn — токены в URL не передаются, но search params могут содержать ИНН/cad_num

При traces_sample_rate=0.05 5% всех outbound httpx-запросов отправят их URL целиком в GlitchTip. apiKey= в URL → token leakage в трейсы. Это уже security-relevant.

Fix: добавить before_send_transaction (или before_breadcrumb) которая redact'ит query params для известных доменов. Минимально:

def _redact_tx(event, hint):
    for span in event.get("spans", []):
        url = span.get("data", {}).get("url") or span.get("description", "")
        if "apiKey=" in url or "api_key=" in url:
            import re
            redacted = re.sub(r"(api[_-]?[Kk]ey=)[^&]+", r"\1[REDACTED]", url)
            if "data" in span:
                span["data"]["url"] = redacted
            span["description"] = redacted
    return event

Альтернатива (proще): передать HttpxIntegration(...) с whitelist tracing_status_codes или вообще traces_sample_rate=0 для httpx через отдельный фильтр. Самое чистое — обернуть httpx-вызовы Objective в urlsplit и держать apiKey в header'е вместо querystring (но это уже scope-creep для отдельного PR).

5. SENTRY_DSN= в .env.runtime.example/backend/.env.example orphaned

После переименования sentry_dsn → glitchtip_dsn, любой существующий .env или .env.runtime на VPS с заданным SENTRY_DSN=https://... молча игнорируется (pydantic extra="ignore"). Пользователь думает SDK работает, по факту no-op.

Fix: в config.py добавить migration warning:

@field_validator("glitchtip_dsn", mode="before")
@classmethod
def _warn_legacy_sentry(cls, v, info):
    import os, warnings
    if not v and os.getenv("SENTRY_DSN"):
        warnings.warn("SENTRY_DSN deprecated, use GLITCHTIP_DSN", DeprecationWarning, 2)
        return os.environ["SENTRY_DSN"]
    return v

Либо — короче — в backend/.env.example оставить обе переменные с пометкой "SENTRY_DSN deprecated, prefer GLITCHTIP_DSN" на 1-2 deploy-цикла.

🟢 Low / nits

celery_app.py:13 — logger = logging.getLogger(__name__) определён ПОСЛЕ import-блока, но logger.info(...) в init-блоке (line 38) вызывается ДО создания celery_app. Это работает (logger создан line 13, init line 16-39), но порядок странный — комментарий "SDK инициализируется в обоих процессах" между import-блоком и celery_app = Celery(...). Ничего критичного.
Тесты test_sentry_init.py:30-37 — тестируют логику внутри with patch (locally-defined glitchtip_dsn = None), а не реальный модуль. То есть unit-тест не покрывает фактический guard в main.py:30 — если кто-то удалит if settings.glitchtip_dsn: в main.py, тесты не упадут. Рекомендую: importlib.reload(app.main) с monkey-patched settings.glitchtip_dsn → проверить mock_init.called. Не блокирует merge.
Dependency upper bound: sentry-sdk[...]>=2.18.0 без upper bound. sentry-sdk 3.x ожидается и может содержать breaking changes (e.g., legacy SDK API removal). Реальная установлена 2.58.0 (uv.lock:1907) — OK сейчас, но при uv lock --upgrade без awareness можно подтянуть 3.x. Обычно для prod рекомендую >=2.18.0,<3.0. Минор.

Cross-file impact analysis

❌ .forgejo/workflows/deploy.yml:157-184 не обновлён — пишет SENTRY_RELEASE который теперь nobody читает (см. 🟠#1)
❌ backend/.env.runtime.example:6 не обновлён — orphaned SENTRY_RELEASE=
❌ docker-compose.prod.yml:51 комментарий устарел: "SENTRY_RELEASE=$IMAGE_TAG"
❌ README.md:109 упоминает sed SENTRY_RELEASE
❌ .claude/rules/deploy.md "Sentry release tracking" секция устарела (но scope-rule, обновляется параллельно)
✅ app.workers.lifecycle import — не сломан (PR не трогает worker hooks)
✅ Все 33 logger.error/exception теперь шлют events → estimated volume ~10-50 events/час on healthy prod, всплеск до тысяч при scraper outage. Для self-hosted GlitchTip приемлемо, но traces_sample_rate=0.05 оставит достаточный budget. Per-task rate-limit на GlitchTip side стоит выставить.
✅ Frontend PR (#205-related) — независим, separately reviewed (vault decisions/decision-glitchtip-frontend-sentry.md)
✅ Beat-monitor: build_beat_schedule() зарегистрирует 4 periodic tasks (scrape_kn, refresh_analytics, objective_sync, geo) — все будут видны в GlitchTip Crons после первых тиков. ✅

Vault cross-check

decisions/decision-glitchtip-frontend-sentry.md — frontend pair, consistent design (no-op без DSN, Sentry-compatible)
fixes/fixes-MOC.md содержит entry под этот PR с упоминанием "SENTRY_RELEASE удалено — теперь читается из os.getenv('GIT_SHA', 'unknown')" — то есть design intent был именно перейти с .env.runtime на CI-env. Но CI-env механизм не реализован в deploy.yml — это и есть 🟠#1. Либо vault впереди кода, либо PR неполный.
Нет related entries в limitations/ или code/patterns/ по error-tracking — это первая GlitchTip migration, нормально.

Performance findings

LoggingIntegration(level=INFO, event_level=ERROR) — breadcrumbs от INFO + events от ERROR. На worker'е с 8 concurrency возможен memory pressure при event burst'е (sentry-sdk буферизует до 30 events по умолчанию × 8 workers = 240 in-flight). Default acceptable, но мониторить worker RSS первую неделю.
SqlalchemyIntegration — каждый query как span при traced transaction. С traces_sample_rate=0.05 → 5% запросов получат SQL spans. Negligible overhead.
HttpxIntegration — wraps httpx.AsyncClient.send → +1 context manager per call. Тоже negligible.

Security 🔒

✅ send_default_pii=False — IP/cookies/headers не уходят
🟡 HttpxIntegration + objective_api_key в URL → потенциальная утечка API key в trace (см. 🟡#4)
✅ Нет hardcoded DSN / token в коде
✅ DSN читается из env via pydantic — корректно
✅ Нет SQL injection / path traversal в diff

Positive observations

✅ SDK init до FastAPI app creation — правильное место для StarletteIntegration (если init в lifespan, middleware не оборачивается)
✅ Двойной init (web + worker) выполнен корректно — каждый процесс независим
✅ profiles_sample_rate=0.0 — соответствует GlitchTip limitations
✅ monitor_beat_tasks=True — beat-уровень мониторинга подключен
✅ send_default_pii=False по умолчанию — секурно
✅ Pure feature branch, single commit, без --no-verify/--amend/--force маркеров
✅ Test file присутствует (test_sentry_init.py) — пусть и слабый (см. 🟢)
✅ uv.lock updated consistently — нет risk of import failure из-за missing extras

Pre-flight

✅ Branch: feat/204-glitchtip-backend — не main
✅ Single commit, no force/amend/no-verify markers
✅ Mergeable=true, no conflicts
✅ GPG: not signed (как и весь репо)
⚠️ CI status: не проверил (нет access к GHA через MCP в этой сессии)

Recommended next steps

BLOCKER fix: Worker (backend-engineer) — добавить fallback os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown" (минимум) ИЛИ обновить .forgejo/workflows/deploy.yml чтобы писать GIT_SHA= в .env.runtime (+ обновить .env.runtime.example, docker-compose.prod.yml:51, README.md:109)
Security: добавить before_send_transaction redacting apiKey= / api_key= query params (🟡#4)
DX: обновить backend/.env.example — оставить SENTRY_DSN= (deprecated) рядом с GLITCHTIP_DSN= на 1-2 deploy-цикла или добавить runtime DeprecationWarning
После merge: вручную проверить через celery_app.send_task('test') что event из worker fork'а доходит в GlitchTip
После merge: обновить .claude/rules/deploy.md секцию "Sentry release tracking"
Vault: дополнить fixes/fixes-MOC.md entry с финальным механизмом передачи release tag (CI env vs .env.runtime)

Complexity / blast radius

Risk: Medium — broken release tracking немедленно после merge, security risk от Objective apiKey в spans проявится постепенно при scraper runs
Reversibility: Easy revert (single commit, no schema changes)
Recommended merge window: после fix 🟠#1, в рабочее окно — нужно подтвердить test event приходит после deploy

Auto-merge policy

❌ BLOCKED for auto-merge per scope policy:

Trogает backend/app/main.py + backend/app/workers/celery_app.py + backend/app/core/config.py — backend core
Touches backend/pyproject.toml (dependency growth — docker image rebuild required)
Plus 🟠 HIGH findings (#1, #2) — даже при scope-allowed не следовало бы мержить

Action: human merge required после fix 🟠#1 (release-tracking) + 🟡#4 (apiKey scrubbing). 🟡#3 и 🟡#5 можно follow-up PR.

## Deep Code Review — verdict ### Summary - **Status**: 🟠 HIGH — два регрессии deploy-pipeline, остальное OK - **Files reviewed**: 7 (P1: `main.py`, `celery_app.py`, `config.py`, `pyproject.toml` · P2: `.env.example` · P3: `test_sentry_init.py` · ⚪ `uv.lock`) - **Lines**: +146 / -18 - **Head SHA**: `2f79f7c` · **Mergeable**: true · **Conflicts**: none - **Time**: ~12 min --- ### 🟠 High (должен быть пофиксен до merge) **1. Release tracking сломан в production — `GIT_SHA` нигде не выставляется** Новый код в `main.py:37` и `celery_app.py:25`: ```python release=os.getenv("GIT_SHA", "unknown"), ``` Но `GIT_SHA` **не задан нигде**: - `.forgejo/workflows/deploy.yml` пишет `SENTRY_RELEASE=$IMAGE_TAG` в `backend/.env.runtime` (lines 157, 164, 180-184) — НЕ `GIT_SHA` - `docker-compose.prod.yml` env_file подгружает `.env.runtime`, в котором `SENTRY_RELEASE=...` — это значение становится `os.environ["SENTRY_RELEASE"]`, а не `GIT_SHA` - `backend/.env.runtime.example:6` всё ещё содержит `SENTRY_RELEASE=` (не обновлён) - `.claude/rules/deploy.md` явно фиксирует контракт `SENTRY_RELEASE=$IMAGE_TAG` в `.env.runtime` **Результат после merge**: каждый event в GlitchTip будет помечен `release=unknown` — теряется возможность фильтра по версии, regression detection, deploy markers. Это тихая поломка: SDK инициализируется, события идут, но release-tag всегда "unknown". **Fix (выбрать один)**: - **Вариант A — минимальное вмешательство** (рекомендуется): прочитать обе env-vars в `main.py` и `celery_app.py`: ```python release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown", ``` - **Вариант B — обновить deploy.yml**: переименовать `SENTRY_RELEASE_VAL` → `GIT_SHA_VAL`, в `sed` писать `GIT_SHA=...` в `.env.runtime`. Также обновить `backend/.env.runtime.example` (`GIT_SHA=`) и `.claude/rules/deploy.md`. Туда же `README.md:109` и `docker-compose.prod.yml:51` (комментарии). Без любого из них release tracking remains broken — это не fail-deploy, но обесценивает половину ценности GlitchTip (deploy markers / version filtering). --- **2. Init-блок не отрабатывает в `beat` процессе** `docker-compose.prod.yml:122`: ```yaml command: ["celery", "-A", "app.workers.celery_app", "beat", ...] ``` `celery beat -A app.workers.celery_app` импортирует `celery_app.py` — module-level `if settings.glitchtip_dsn: sentry_sdk.init(...)` отработает, ✅ beat покрыт. Но `worker` командуется аналогично (`docker-compose.prod.yml:109`) → SDK init происходит в **родительском процессе** worker'а до fork'а. С `prefork`-pool по умолчанию child-процессы наследуют SDK state через fork — это работает для error capture, но **`CeleryIntegration(monitor_beat_tasks=True)` heartbeat-events отправляются из beat-процесса, не из worker fork'а** — то есть pattern корректен. ⚠️ Однако: если в `app.workers.lifecycle` есть `@worker_process_init.connect` hook (vault упоминает Bug_Worker_Ready_EarlyReturn) — нужно убедиться, что после fork SDK переинициализируется или transport reopen'ится. Стандартный `sentry-sdk` это делает автоматически через `multiprocessing.util.register_after_fork` (с 2.x). На версии `>=2.18.0` — OK. 🤔 **Question**: проверить руками после deploy, что test-event из worker task (`celery_app.send_task(...)`) реально доходит до GlitchTip — fork-safety багов в `sentry-sdk[celery]` 2.x исторически было несколько. --- ### 🟡 Medium (желательно fix, но не блокирующее) **3. PII в логах через `LoggingIntegration(level=INFO, event_level=ERROR)`** `send_default_pii=False` — корректно, IP / cookies / headers фильтруются. НО: - 33 call site `logger.error/exception` в 18 файлах — каждый теперь шлёт event - В `backend/app/workers/tasks/nspd_geo.py:140-148`, `nspd_geo.py:225-238` логируются `cad_num` через f-string в WARNING/INFO. Per CLAUDE.md «cad_num as PII» в задаче — это spec-level concern. Reality check: cad_num по 152-ФЗ скорее всего НЕ персональные данные (это публичный кадастровый номер, не привязан к физлицу до запроса в ЕГРН), но если у нас лиды связаны с конкретным cad_num — это уже бизнес-PII. - `domrf_kn.py:620-627` — уже специально подавляет логирование в Sentry/Docker для одного класса ошибок (comment подтверждает учёт). Хороший знак — codebase осведомлён о volume. **Fix-suggestion** (опционально): в `LoggingIntegration` поднять `event_level=logging.WARNING` или оставить ERROR — но добавить `before_send` фильтр, scrubbing'ующий поля `inn`, `phone`, `email`, `password`, `token` из event `extra`. Канонически: ```python def _scrub(event, hint): SENSITIVE = ("inn", "phone", "email", "password", "token", "api_key", "lead_phone") for section in ("extra", "contexts"): data = event.get(section, {}) if isinstance(data, dict): for k in list(data): if any(s in k.lower() for s in SENSITIVE): data[k] = "[Filtered]" return event sentry_sdk.init(..., before_send=_scrub) ``` **4. `traces_sample_rate=0.05` × `HttpxIntegration` → утечка outbound URLs** `HttpxIntegration` пишет URL outbound запросов в spans. У нас scrapers ходят на: - `pkk.rosreestr.ru/api/features/1?...` — содержит `cad_num` в querystring - `api.objctv.ru/...` — содержит **`apiKey=<KEY>` в querystring** (`config.py:48-50`) - `domrf-kn` — токены в URL не передаются, но search params могут содержать ИНН/cad_num **При `traces_sample_rate=0.05`** 5% всех outbound httpx-запросов отправят их URL целиком в GlitchTip. **`apiKey=` в URL → token leakage в трейсы.** Это уже security-relevant. **Fix**: добавить `before_send_transaction` (или `before_breadcrumb`) которая redact'ит query params для известных доменов. Минимально: ```python def _redact_tx(event, hint): for span in event.get("spans", []): url = span.get("data", {}).get("url") or span.get("description", "") if "apiKey=" in url or "api_key=" in url: import re redacted = re.sub(r"(api[_-]?[Kk]ey=)[^&]+", r"\1[REDACTED]", url) if "data" in span: span["data"]["url"] = redacted span["description"] = redacted return event ``` Альтернатива (proще): передать `HttpxIntegration(...)` с whitelist `tracing_status_codes` или вообще `traces_sample_rate=0` для httpx через отдельный фильтр. Самое чистое — обернуть httpx-вызовы Objective в `urlsplit` и держать `apiKey` в header'е вместо querystring (но это уже scope-creep для отдельного PR). **5. `SENTRY_DSN=` в `.env.runtime.example`/`backend/.env.example` orphaned** После переименования `sentry_dsn` → `glitchtip_dsn`, любой существующий `.env` или `.env.runtime` на VPS с заданным `SENTRY_DSN=https://...` **молча игнорируется** (pydantic `extra="ignore"`). Пользователь думает SDK работает, по факту no-op. **Fix**: в `config.py` добавить migration warning: ```python @field_validator("glitchtip_dsn", mode="before") @classmethod def _warn_legacy_sentry(cls, v, info): import os, warnings if not v and os.getenv("SENTRY_DSN"): warnings.warn("SENTRY_DSN deprecated, use GLITCHTIP_DSN", DeprecationWarning, 2) return os.environ["SENTRY_DSN"] return v ``` Либо — короче — в `backend/.env.example` оставить **обе** переменные с пометкой "SENTRY_DSN deprecated, prefer GLITCHTIP_DSN" на 1-2 deploy-цикла. --- ### 🟢 Low / nits - **`celery_app.py:13`** — `logger = logging.getLogger(__name__)` определён ПОСЛЕ import-блока, но `logger.info(...)` в init-блоке (line 38) вызывается ДО создания `celery_app`. Это работает (logger создан line 13, init line 16-39), но порядок странный — комментарий "SDK инициализируется в обоих процессах" между import-блоком и `celery_app = Celery(...)`. Ничего критичного. - **Тесты `test_sentry_init.py:30-37`** — тестируют логику внутри `with patch` (locally-defined `glitchtip_dsn = None`), а не реальный модуль. То есть unit-тест **не покрывает фактический guard в `main.py:30`** — если кто-то удалит `if settings.glitchtip_dsn:` в main.py, тесты не упадут. Рекомендую: `importlib.reload(app.main)` с monkey-patched `settings.glitchtip_dsn` → проверить `mock_init.called`. Не блокирует merge. - **Dependency upper bound**: `sentry-sdk[...]>=2.18.0` без upper bound. sentry-sdk 3.x ожидается и может содержать breaking changes (e.g., legacy SDK API removal). Реальная установлена `2.58.0` (`uv.lock:1907`) — OK сейчас, но при `uv lock --upgrade` без awareness можно подтянуть 3.x. Обычно для prod рекомендую `>=2.18.0,<3.0`. Минор. --- ### Cross-file impact analysis - ❌ **`.forgejo/workflows/deploy.yml:157-184`** не обновлён — пишет `SENTRY_RELEASE` который теперь nobody читает (см. 🟠#1) - ❌ **`backend/.env.runtime.example:6`** не обновлён — orphaned `SENTRY_RELEASE=` - ❌ **`docker-compose.prod.yml:51`** комментарий устарел: "SENTRY_RELEASE=$IMAGE_TAG" - ❌ **`README.md:109`** упоминает sed SENTRY_RELEASE - ❌ **`.claude/rules/deploy.md`** "Sentry release tracking" секция устарела (но scope-rule, обновляется параллельно) - ✅ `app.workers.lifecycle` import — не сломан (PR не трогает worker hooks) - ✅ Все 33 `logger.error/exception` теперь шлют events → estimated volume ~10-50 events/час on healthy prod, всплеск до тысяч при scraper outage. Для self-hosted GlitchTip приемлемо, но `traces_sample_rate=0.05` оставит достаточный budget. Per-task rate-limit на GlitchTip side стоит выставить. - ✅ Frontend PR (#205-related) — независим, separately reviewed (vault `decisions/decision-glitchtip-frontend-sentry.md`) - ✅ Beat-monitor: `build_beat_schedule()` зарегистрирует 4 periodic tasks (scrape_kn, refresh_analytics, objective_sync, geo) — все будут видны в GlitchTip Crons после первых тиков. ✅ --- ### Vault cross-check - **`decisions/decision-glitchtip-frontend-sentry.md`** — frontend pair, consistent design (no-op без DSN, Sentry-compatible) - **`fixes/fixes-MOC.md`** содержит entry под этот PR с упоминанием "SENTRY_RELEASE удалено — теперь читается из os.getenv('GIT_SHA', 'unknown')" — то есть design intent был именно перейти с `.env.runtime` на CI-env. Но **CI-env механизм не реализован в deploy.yml** — это и есть 🟠#1. Либо vault впереди кода, либо PR неполный. - Нет related entries в `limitations/` или `code/patterns/` по error-tracking — это первая GlitchTip migration, нормально. --- ### Performance findings - `LoggingIntegration(level=INFO, event_level=ERROR)` — breadcrumbs от INFO + events от ERROR. На worker'е с 8 concurrency возможен memory pressure при event burst'е (sentry-sdk буферизует до 30 events по умолчанию × 8 workers = 240 in-flight). Default acceptable, но мониторить worker RSS первую неделю. - `SqlalchemyIntegration` — каждый query как span при traced transaction. С `traces_sample_rate=0.05` → 5% запросов получат SQL spans. Negligible overhead. - `HttpxIntegration` — wraps `httpx.AsyncClient.send` → +1 context manager per call. Тоже negligible. --- ### Security 🔒 - ✅ `send_default_pii=False` — IP/cookies/headers не уходят - 🟡 **HttpxIntegration + objective_api_key в URL** → потенциальная утечка API key в trace (см. 🟡#4) - ✅ Нет hardcoded DSN / token в коде - ✅ DSN читается из env via pydantic — корректно - ✅ Нет SQL injection / path traversal в diff --- ### Positive observations - ✅ SDK init **до** FastAPI app creation — правильное место для `StarletteIntegration` (если init в lifespan, middleware не оборачивается) - ✅ Двойной init (web + worker) выполнен корректно — каждый процесс независим - ✅ `profiles_sample_rate=0.0` — соответствует GlitchTip limitations - ✅ `monitor_beat_tasks=True` — beat-уровень мониторинга подключен - ✅ `send_default_pii=False` по умолчанию — секурно - ✅ Pure feature branch, single commit, без `--no-verify`/`--amend`/`--force` маркеров - ✅ Test file присутствует (test_sentry_init.py) — пусть и слабый (см. 🟢) - ✅ `uv.lock` updated consistently — нет risk of import failure из-за missing extras --- ### Pre-flight - ✅ Branch: `feat/204-glitchtip-backend` — не main - ✅ Single commit, no force/amend/no-verify markers - ✅ Mergeable=true, no conflicts - ✅ GPG: not signed (как и весь репо) - ⚠️ CI status: не проверил (нет access к GHA через MCP в этой сессии) --- ### Recommended next steps 1. **BLOCKER fix**: Worker (backend-engineer) — добавить fallback `os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown"` (минимум) ИЛИ обновить `.forgejo/workflows/deploy.yml` чтобы писать `GIT_SHA=` в `.env.runtime` (+ обновить `.env.runtime.example`, `docker-compose.prod.yml:51`, `README.md:109`) 2. **Security**: добавить `before_send_transaction` redacting `apiKey=` / `api_key=` query params (🟡#4) 3. **DX**: обновить `backend/.env.example` — оставить `SENTRY_DSN=` (deprecated) рядом с `GLITCHTIP_DSN=` на 1-2 deploy-цикла или добавить runtime DeprecationWarning 4. **После merge**: вручную проверить через `celery_app.send_task('test')` что event из worker fork'а доходит в GlitchTip 5. **После merge**: обновить `.claude/rules/deploy.md` секцию "Sentry release tracking" 6. **Vault**: дополнить `fixes/fixes-MOC.md` entry с финальным механизмом передачи release tag (CI env vs .env.runtime) --- ### Complexity / blast radius - **Risk**: Medium — broken release tracking немедленно после merge, security risk от Objective apiKey в spans проявится постепенно при scraper runs - **Reversibility**: Easy revert (single commit, no schema changes) - **Recommended merge window**: после fix 🟠#1, в рабочее окно — нужно подтвердить test event приходит после deploy --- ### Auto-merge policy ❌ **BLOCKED for auto-merge** per scope policy: - Trogает `backend/app/main.py` + `backend/app/workers/celery_app.py` + `backend/app/core/config.py` — backend core - Touches `backend/pyproject.toml` (dependency growth — docker image rebuild required) - Plus 🟠 HIGH findings (#1, #2) — даже при scope-allowed не следовало бы мержить **Action**: human merge required после fix 🟠#1 (release-tracking) + 🟡#4 (apiKey scrubbing). 🟡#3 и 🟡#5 можно follow-up PR.

lekss361 referenced this pull request

2026-05-16 13:15:16 +00:00

feat(nspd): TIER 3 risk zones (#94 PR1 of 4) #217

lekss361 added 1 commit 2026-05-16 15:04:14 +00:00

fix(backend): GlitchTip release fallback + apiKey scrub (#204 review fixes) 8a04af5da3

- release= now: GIT_SHA or SENTRY_RELEASE or "unknown" (covers existing
  deploy.yml which writes SENTRY_RELEASE=$IMAGE_TAG, not GIT_SHA)
- new module app/observability/sentry_scrub.py: before_send_transaction
  hook that redacts apiKey/token/access_token/secret query params from
  HttpxIntegration spans before they reach GlitchTip
- config.py: model_validator promotes legacy SENTRY_DSN -> glitchtip_dsn
  with DeprecationWarning for backward compat on existing VPS env
- .env.example: added deprecated SENTRY_DSN entry with comment
- tests: 8 new cases covering release fallback + scrub module (11 total)

lekss361 commented

2026-05-16 15:04:39 +00:00

Author

Owner

Review fixes pushed in commit 8a04af5.

Закрытые пункты:

Release tracking (HIGH) — исправлен fallback:
release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown"
в обоих файлах. Покрывает существующий deploy.yml (пишет SENTRY_RELEASE=$IMAGE_TAG).
apiKey scrub (HIGH) — новый модуль app/observability/sentry_scrub.py
с scrub_sensitive_query (before_send_transaction hook). Redact-ит
apiKey/api_key/token/access_token/secret из span data и request URL.
Импортируется обоими entrypoint'ами (main.py + celery_app.py).
Orphan SENTRY_DSN (HIGH) — model_validator(mode="after") в config.py
читает os.getenv("SENTRY_DSN") если glitchtip_dsn пуст, продвигает
значение с DeprecationWarning. .env.example дополнен deprecated-записью.

Отложено в follow-up PR:

Переименование SENTRY_RELEASE -> GIT_SHA в deploy.yml + .env.runtime.example
(отдельный blast radius, согласовано).

Тесты: 11/11 passed. Ruff: ok. Mypy на наших файлах: 0 ошибок (129 pre-existing в других модулях).

Review fixes pushed in commit 8a04af5. **Закрытые пункты:** 1. **Release tracking (HIGH)** — исправлен fallback: `release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown"` в обоих файлах. Покрывает существующий deploy.yml (пишет SENTRY_RELEASE=$IMAGE_TAG). 2. **apiKey scrub (HIGH)** — новый модуль `app/observability/sentry_scrub.py` с `scrub_sensitive_query` (before_send_transaction hook). Redact-ит apiKey/api_key/token/access_token/secret из span data и request URL. Импортируется обоими entrypoint'ами (main.py + celery_app.py). 3. **Orphan SENTRY_DSN (HIGH)** — `model_validator(mode="after")` в config.py читает os.getenv("SENTRY_DSN") если glitchtip_dsn пуст, продвигает значение с DeprecationWarning. .env.example дополнен deprecated-записью. **Отложено в follow-up PR:** - Переименование SENTRY_RELEASE -> GIT_SHA в deploy.yml + .env.runtime.example (отдельный blast radius, согласовано). **Тесты:** 11/11 passed. Ruff: ok. Mypy на наших файлах: 0 ошибок (129 pre-existing в других модулях).

lekss361 commented

2026-05-16 15:09:10 +00:00

Author

Owner

Deep Code Review — re-review verdict (SHA `8a04af5`)

Summary

Status: APPROVE — все 🟠 HIGH из предыдущего review закрыты, 🟡 medium #4 (apiKey leak) полноценно зафиксен, #5 (legacy DSN migration) — закрыт runtime DeprecationWarning + автопродвижением
Files reviewed: 9 (+323 / -17)
Head SHA: 8a04af5 · Mergeable: true · No conflicts
Diff vs previous review: added app/observability/sentry_scrub.py, __init__.py, model_validator в config, before_send_transaction=scrub_sensitive_query hook, release fallback chain, extended test coverage

Fixup verification (per previous comment #388)

1. 🟠 GIT_SHA release tracking → FIXED

main.py:37 и celery_app.py:25 теперь:

release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown"

Покрывает существующий .forgejo/workflows/deploy.yml (пишет SENTRY_RELEASE=$IMAGE_TAG в .env.runtime) — release будет корректно проставлен сразу после merge без изменений deploy.yml. Future cleanup (переименование SENTRY_RELEASE → GIT_SHA в deploy.yml + .env.runtime.example + docker-compose.prod.yml:51 + README.md:109 + .claude/rules/deploy.md:31) — отдельный PR, согласовано.
Тесты test_release_uses_git_sha_when_set / falls_back_to_sentry_release / falls_back_to_unknown корректно покрывают цепочку.

2. 🟠 Beat-monitor / fork SDK inheritance — Resolved (prefork inheritance + module-level init в celery_app.py покрывает оба процесса). Runtime verify после deploy остаётся в check-list PR.

3. 🟡 PII в логах via LoggingIntegration — Не покрыто в этом PR (логично — send_default_pii=False уже фильтрует canonical PII fields; кастомный extra-scrubbing вынесен в follow-up если volume / sensitivity станут проблемой). Acceptable.

4. 🟡 apiKey leak в HttpxIntegration traces → FIXED

Новый app/observability/sentry_scrub.py с scrub_sensitive_query(event, hint) подключён через before_send_transaction=scrub_sensitive_query в обоих entrypoint'ах.
Regex ((?:api[_-]?[Kk]ey|token|access[_-]?token|secret)=)([^&\s]+) покрывает все известные query-секреты (Objective apiKey=, generic token=, access_token=, api_key=, secret=). Case-insensitive.
Обходит: span.data["url"/"http.url"/"http.target"], span["description"], event["request"]["url"] — это покрытие всех мест, куда HttpxIntegration / FastApiIntegration кладут URL.
Mutating через shared reference (span.get("data") возвращает живой dict) — корректно: mutation in-place + return event.
5 unit тестов (apikey/token/request_url/clean passthrough/missing spans) — позитивные и негативные кейсы покрыты.

5. 🟡 Legacy SENTRY_DSN migration → FIXED

model_validator(mode="after")._promote_legacy_sentry_dsn в config.py:23-41 читает os.getenv("SENTRY_DSN") напрямую (минуя pydantic extra="ignore"), выдаёт DeprecationWarning и автопродвигает в glitchtip_dsn.
.env.example обновлён с явной DEPRECATED-меткой и пояснением механизма. Корректный path для smooth миграции на VPS.

Дополнительные findings (новый код)

🟢 Low / nits — none blocking

sentry_scrub.py:35 — event.get("spans") or [] корректно обрабатывает None (transaction events vs error events).
Regex покрывает substring matches типа customtoken= (будет matched как token=) — это OK, defensive scrubbing: false-positive redacts non-sensitive value, false-negative leaks secret. Trade-off правильный.
_redact возвращает non-string значения as-is — graceful для dict с smешанными типами.
Type hint Event | None корректен — позволяет filter-out events (хотя текущая реализация всегда возвращает event).
test_sentry_init.py — from sentry_sdk.types import Event опосредованно проверен через тест import-блока.
Mypy не падает на новых файлах (worker подтверждает 0 errors в touched files).

⚪ Не fixed (acceptable / follow-up)

Cleanup deploy.yml + .env.runtime.example + README + rules — согласован follow-up PR
Upper bound sentry-sdk<3.0 — минорный nit из прошлого review
LoggingIntegration extra-field scrubbing для inn/phone/email — отложен (low priority pending volume/sensitivity observation post-deploy)

Security 🔒

✅ send_default_pii=False (оба процесса)
✅ before_send_transaction=scrub_sensitive_query redacts API keys из span URLs — Objective apiKey= больше не утечёт в трейсы
✅ Нет hardcoded DSN / token в коде
✅ DSN через env via pydantic + safe fallback
✅ Pure additive change в __init__.py (empty) — нет import-side-effects

Conventions (backend.md / git-pr.md)

✅ No psycopg2 / no requests / no print() / no f-string SQL
✅ Line length ≤100
✅ No --no-verify / --amend / --force markers
✅ Feature branch, conventional commit
✅ Tests добавлены

Cross-file impact

✅ pyproject.toml + uv.lock consistent (extras добавлены в обе стороны)
✅ main.py init до FastAPI app — middleware покрыт
✅ celery_app.py init до Celery(...) — корректный порядок
✅ scrub_sensitive_query импортируется в обоих entrypoint'ах — единая логика
⚠️ Cleanup deploy.yml — известный follow-up

Verdict

✅ APPROVE — все блокирующие findings закрыты, security-критичная утечка apiKey предотвращена, legacy migration path прописан, тесты покрывают новую логику. Готов к merge.

Per user override 2026-05-16 ("после approve можешь мерджить любой scope") — мержу сам.

## Deep Code Review — re-review verdict (SHA `8a04af5`) ### Summary - **Status**: APPROVE — все 🟠 HIGH из предыдущего review закрыты, 🟡 medium #4 (apiKey leak) полноценно зафиксен, #5 (legacy DSN migration) — закрыт runtime DeprecationWarning + автопродвижением - **Files reviewed**: 9 (+323 / -17) - **Head SHA**: `8a04af5` · **Mergeable**: true · **No conflicts** - **Diff vs previous review**: added `app/observability/sentry_scrub.py`, `__init__.py`, `model_validator` в config, `before_send_transaction=scrub_sensitive_query` hook, release fallback chain, extended test coverage --- ### Fixup verification (per previous comment #388) **1. 🟠 GIT_SHA release tracking → FIXED** - `main.py:37` и `celery_app.py:25` теперь: ```python release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown" ``` - Покрывает существующий `.forgejo/workflows/deploy.yml` (пишет `SENTRY_RELEASE=$IMAGE_TAG` в `.env.runtime`) — release будет корректно проставлен сразу после merge без изменений deploy.yml. Future cleanup (переименование `SENTRY_RELEASE → GIT_SHA` в deploy.yml + `.env.runtime.example` + `docker-compose.prod.yml:51` + `README.md:109` + `.claude/rules/deploy.md:31`) — отдельный PR, согласовано. - Тесты `test_release_uses_git_sha_when_set / falls_back_to_sentry_release / falls_back_to_unknown` корректно покрывают цепочку. **2. 🟠 Beat-monitor / fork SDK inheritance** — Resolved (prefork inheritance + module-level init в celery_app.py покрывает оба процесса). Runtime verify после deploy остаётся в check-list PR. **3. 🟡 PII в логах via LoggingIntegration** — Не покрыто в этом PR (логично — `send_default_pii=False` уже фильтрует canonical PII fields; кастомный extra-scrubbing вынесен в follow-up если volume / sensitivity станут проблемой). Acceptable. **4. 🟡 apiKey leak в HttpxIntegration traces → FIXED** - Новый `app/observability/sentry_scrub.py` с `scrub_sensitive_query(event, hint)` подключён через `before_send_transaction=scrub_sensitive_query` в обоих entrypoint'ах. - Regex `((?:api[_-]?[Kk]ey|token|access[_-]?token|secret)=)([^&\s]+)` покрывает все известные query-секреты (Objective `apiKey=`, generic `token=`, `access_token=`, `api_key=`, `secret=`). Case-insensitive. - Обходит: `span.data["url"/"http.url"/"http.target"]`, `span["description"]`, `event["request"]["url"]` — это покрытие всех мест, куда HttpxIntegration / FastApiIntegration кладут URL. - Mutating через shared reference (`span.get("data")` возвращает живой dict) — корректно: mutation in-place + return event. - 5 unit тестов (apikey/token/request_url/clean passthrough/missing spans) — позитивные и негативные кейсы покрыты. **5. 🟡 Legacy SENTRY_DSN migration → FIXED** - `model_validator(mode="after")._promote_legacy_sentry_dsn` в `config.py:23-41` читает `os.getenv("SENTRY_DSN")` напрямую (минуя pydantic `extra="ignore"`), выдаёт `DeprecationWarning` и автопродвигает в `glitchtip_dsn`. - `.env.example` обновлён с явной DEPRECATED-меткой и пояснением механизма. Корректный path для smooth миграции на VPS. --- ### Дополнительные findings (новый код) 🟢 **Low / nits — none blocking** - `sentry_scrub.py:35` — `event.get("spans") or []` корректно обрабатывает None (transaction events vs error events). - Regex покрывает substring matches типа `customtoken=` (будет matched как `token=`) — это OK, defensive scrubbing: false-positive redacts non-sensitive value, false-negative leaks secret. Trade-off правильный. - `_redact` возвращает non-string значения as-is — graceful для dict с smешанными типами. - Type hint `Event | None` корректен — позволяет filter-out events (хотя текущая реализация всегда возвращает event). - `test_sentry_init.py` — `from sentry_sdk.types import Event` опосредованно проверен через тест import-блока. - Mypy не падает на новых файлах (worker подтверждает 0 errors в touched files). ⚪ **Не fixed (acceptable / follow-up)** - Cleanup deploy.yml + .env.runtime.example + README + rules — согласован follow-up PR - Upper bound `sentry-sdk<3.0` — минорный nit из прошлого review - `LoggingIntegration` extra-field scrubbing для inn/phone/email — отложен (low priority pending volume/sensitivity observation post-deploy) --- ### Security 🔒 - ✅ `send_default_pii=False` (оба процесса) - ✅ `before_send_transaction=scrub_sensitive_query` redacts API keys из span URLs — Objective `apiKey=` больше не утечёт в трейсы - ✅ Нет hardcoded DSN / token в коде - ✅ DSN через env via pydantic + safe fallback - ✅ Pure additive change в `__init__.py` (empty) — нет import-side-effects --- ### Conventions (backend.md / git-pr.md) - ✅ No `psycopg2` / no `requests` / no `print()` / no f-string SQL - ✅ Line length ≤100 - ✅ No `--no-verify` / `--amend` / `--force` markers - ✅ Feature branch, conventional commit - ✅ Tests добавлены --- ### Cross-file impact - ✅ `pyproject.toml` + `uv.lock` consistent (extras добавлены в обе стороны) - ✅ `main.py` init **до** FastAPI app — middleware покрыт - ✅ `celery_app.py` init **до** `Celery(...)` — корректный порядок - ✅ `scrub_sensitive_query` импортируется в обоих entrypoint'ах — единая логика - ⚠️ Cleanup deploy.yml — известный follow-up --- ### Verdict ✅ **APPROVE** — все блокирующие findings закрыты, security-критичная утечка apiKey предотвращена, legacy migration path прописан, тесты покрывают новую логику. Готов к merge. Per user override 2026-05-16 ("после approve можешь мерджить любой scope") — мержу сам.

lekss361 merged commit b61f025673 into main

2026-05-16 15:09:17 +00:00

lekss361 referenced this pull request from a commit

2026-05-16 15:09:19 +00:00

feat(backend): sentry-sdk init для FastAPI + Celery (#204 backend) (#207)

lekss361 referenced this pull request

2026-05-16 15:09:25 +00:00

feat(frontend): @sentry/nextjs init для GlitchTip (#204 frontend) #208

lekss361 referenced this pull request from a commit

2026-05-16 18:48:38 +00:00

fix(infra): wire GlitchTip DSNs (build-arg + env_file) + reject sentry.io promote

lekss361 referenced this pull request