diff --git a/.forgejo/workflows/deploy-metrics.yml b/.forgejo/workflows/deploy-metrics.yml index 559784aa..86a702b9 100644 --- a/.forgejo/workflows/deploy-metrics.yml +++ b/.forgejo/workflows/deploy-metrics.yml @@ -90,8 +90,9 @@ jobs: METRICS_TELEGRAM_TOPIC_ID: ${{ secrets.METRICS_TELEGRAM_TOPIC_ID }} METRICS_TELEGRAM_INFRA_TOPIC_ID: ${{ secrets.METRICS_TELEGRAM_INFRA_TOPIC_ID }} METRICS_TELEGRAM_ONCALL: ${{ secrets.METRICS_TELEGRAM_ONCALL }} + ALERT_ACK_GLITCHTIP_SECRET: ${{ secrets.ALERT_ACK_GLITCHTIP_SECRET }} with: - envs: METRICS_TELEGRAM_BOT_TOKEN,METRICS_TELEGRAM_CHAT_ID,METRICS_TELEGRAM_TOPIC_ID,METRICS_TELEGRAM_INFRA_TOPIC_ID,METRICS_TELEGRAM_ONCALL + envs: METRICS_TELEGRAM_BOT_TOKEN,METRICS_TELEGRAM_CHAT_ID,METRICS_TELEGRAM_TOPIC_ID,METRICS_TELEGRAM_INFRA_TOPIC_ID,METRICS_TELEGRAM_ONCALL,ALERT_ACK_GLITCHTIP_SECRET host: ${{ secrets.INFRA_DEPLOY_HOST || secrets.DEPLOY_HOST }} username: ${{ secrets.INFRA_DEPLOY_USER || secrets.DEPLOY_USER }} key: ${{ secrets.INFRA_DEPLOY_SSH_KEY || secrets.DEPLOY_SSH_KEY }} @@ -188,6 +189,14 @@ jobs: echo "Инфраструктура: тема ${INFRA_TOPIC_ID} по умолчанию (METRICS_TELEGRAM_INFRA_TOPIC_ID не задана)." fi + # Резервный приёмник GlitchTip (#3471) отвечает 503 на любой + # запрос, пока секрет пуст: тихо принимать чужие алерты настежь + # хуже, чем не принимать вовсе. Молчаливого отказа тут быть не + # должно — деплой обязан сказать, что канал не поднялся. + if [ -z "${ALERT_ACK_GLITCHTIP_SECRET:-}" ]; then + echo "::warning title=Резервный канал GlitchTip выключен::ALERT_ACK_GLITCHTIP_SECRET пуст — alert-ack отвечает 503 на /glitchtip, и при падении продуктового бэкенда его ошибки доставлять будет нечем." + fi + if [ -n "${METRICS_TELEGRAM_ONCALL:-}" ]; then echo "Клиентские инциденты: зовём ${METRICS_TELEGRAM_ONCALL} поимённо." else @@ -344,6 +353,51 @@ jobs: done docker compose -p gendesign-metrics -f docker-compose.metrics.yml ps + # ── Prometheus: конфиг/правила лежат на диске, `up -d` их не + # перечитывает ──────────────────────────────────────────────── + # Тот же класс бага, что у Caddyfile и alertmanager.yml выше: + # docker compose сравнивает описание сервиса, а НЕ содержимое + # бинд-маунта, поэтому уже работающий контейнер продолжает жить + # со старым конфигом сколько угодно — на проде дошло до 16 суток + # незамеченными (#3467): lastConfigTime совпадал со startTime + # контейнера при каждом зелёном деплое, менявшем ops/metrics/prometheus/**. + # + # У Prometheus, в отличие от Alertmanager (см. комментарий выше), + # /-/reload переоткрывает файлы ПО ПУТИ заново, поэтому новый инод + # после `git reset --hard` подхватывается без пересоздания + # контейнера. --web.enable-lifecycle уже включён в compose ради + # этого шага (см. docker-compose.metrics.yml) — просто раньше + # никто не звал сам reload. + # + # promtool проверяет ОБА файла ДО reload: битый конфиг не должен + # положить работающий Prometheus молчаливым откатом на дефолты. + if docker exec gendesign-prometheus promtool check config /etc/prometheus/prometheus.yml \ + && docker exec gendesign-prometheus sh -c 'promtool check rules /etc/prometheus/rules/*.yml'; then + LAST_CONFIG_BEFORE="$(docker exec gendesign-prometheus wget -qO- http://localhost:9090/api/v1/status/runtimeinfo | grep -oE '"lastConfigTime":"[^"]*"')" + + docker exec gendesign-prometheus wget -q -O /dev/null --post-data='' http://localhost:9090/-/reload + + # lastConfigTime обновляется на КАЖДЫЙ успешный reload, даже + # если содержимое конфига не поменялось — значит сравнение + # "было/стало" надёжно ловит и несостоявшийся reload, и + # изменившиеся правила. + LAST_CONFIG_AFTER="" + for i in $(seq 1 10); do + LAST_CONFIG_AFTER="$(docker exec gendesign-prometheus wget -qO- http://localhost:9090/api/v1/status/runtimeinfo | grep -oE '"lastConfigTime":"[^"]*"')" + [ -n "$LAST_CONFIG_AFTER" ] && [ "$LAST_CONFIG_AFTER" != "$LAST_CONFIG_BEFORE" ] && break + sleep 1 + done + + if [ -z "$LAST_CONFIG_AFTER" ] || [ "$LAST_CONFIG_AFTER" = "$LAST_CONFIG_BEFORE" ]; then + echo "ОШИБКА: reload Prometheus не подтверждён — lastConfigTime не изменился ($LAST_CONFIG_BEFORE)." + exit 1 + fi + echo "Prometheus: конфиг и правила проверены, reload подтверждён ($LAST_CONFIG_BEFORE -> $LAST_CONFIG_AFTER)." + else + echo "ОШИБКА: конфиг/правила Prometheus не проходят promtool — reload НЕ выполнен, работающий Prometheus остаётся на прежнем конфиге." + exit 1 + fi + # ═══ АГЕНТЫ — оба хоста ═══════════════════════════════════════════════════ agent-apps: runs-on: ubuntu-latest diff --git a/backend/tests/ops/test_3467_prometheus_reload.py b/backend/tests/ops/test_3467_prometheus_reload.py new file mode 100644 index 00000000..d27e5673 --- /dev/null +++ b/backend/tests/ops/test_3467_prometheus_reload.py @@ -0,0 +1,112 @@ +"""Правки Prometheus-конфига/правил обязаны доезжать до работающего процесса. + +ЧТО СЛУЧИЛОСЬ НА ПРОДЕ. `GET /api/v1/status/runtimeinfo` внутри +`gendesign-prometheus` 12.09 отдавал `lastConfigTime`, совпадающий со +`startTime` контейнера, — конфиг и правила не перечитывались 16 суток. +Деплой при этом был зелёный: `docker compose up -d` не пересоздаёт +контейнер из-за изменения содержимого бинд-маунта (он сравнивает только +описание сервиса), а `--web.enable-lifecycle` был включён в +docker-compose.metrics.yml, но эндпоинт `/-/reload` никто не вызывал. + +Тот же класс бага, что уже пойман и починен для Caddy (`caddy reload`) +и для Alertmanager (`--force-recreate`, см. test_3xxx_alertmanager_inode.py) +в этом же workflow — только для Prometheus починка не пересоздание +контейнера, а именно `POST /-/reload`: он переоткрывает файлы конфига по +пути заново, так что новый инод после `git reset --hard` подхватывается +без даунтайма. + +Проверяется здесь: (1) валидация promtool ЕСТЬ, (2) reload вызывается +ТОЛЬКО после успешной валидации, (3) шаг обязан упасть, если reload не +подтверждён сменой lastConfigTime. +""" + +from __future__ import annotations + +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +WORKFLOW = REPO_ROOT / ".forgejo" / "workflows" / "deploy-metrics.yml" + + +def _text() -> str: + return WORKFLOW.read_text(encoding="utf-8") + + +def test_promtool_checks_config_and_rules() -> None: + """promtool обязан проверять и конфиг, и правила — не только один файл.""" + text = _text() + assert "promtool check config /etc/prometheus/prometheus.yml" in text, ( + "нет проверки конфига promtool'ом — битый prometheus.yml долетит до reload" + ) + assert "promtool check rules" in text, ( + "нет проверки правил promtool'ом — битое правило долетит до reload" + ) + + +def test_reload_endpoint_is_called() -> None: + """Сам reload обязан вызываться — иначе валидация ничего не решает.""" + text = _text() + assert "localhost:9090/-/reload" in text, ( + "нет вызова POST /-/reload — конфиг/правила проверяются, но в силу не вступают " + "(#3467: lastConfigTime не менялся 16 суток при зелёном деплое)" + ) + + +def test_reload_happens_after_validation_not_before() -> None: + """Reload обязан идти ПОСЛЕ promtool, а не до/вместо него.""" + text = _text() + check_pos = text.index("promtool check config /etc/prometheus/prometheus.yml") + reload_pos = text.index("localhost:9090/-/reload") + assert check_pos < reload_pos, ( + "reload стоит раньше проверки конфига — битый конфиг мог бы применяться вслепую" + ) + + +def test_reload_is_guarded_by_the_promtool_check() -> None: + """Reload обязан быть ВНУТРИ `if promtool ...; then`, а не безусловным.""" + text = _text() + guard_start = text.index("if docker exec gendesign-prometheus promtool check config") + else_pos = text.index("else", guard_start) + reload_pos = text.index("localhost:9090/-/reload") + assert guard_start < reload_pos < else_pos, ( + "вызов reload лежит вне ветки успешной проверки promtool — " + "битый конфиг всё равно приведёт к reload, либо reload вообще не защищён проверкой" + ) + + +def test_failed_validation_skips_reload_and_fails_the_step() -> None: + """При провале promtool — reload НЕ вызывается, и шаг падает (exit 1).""" + text = _text() + guard_start = text.index("if docker exec gendesign-prometheus promtool check config") + else_pos = text.index("else", guard_start) + fi_pos = text.index("fi", else_pos) + else_branch = text[else_pos:fi_pos] + assert "localhost:9090/-/reload" not in else_branch, ( + "reload вызывается даже в ветке провалившейся проверки" + ) + assert "exit 1" in else_branch, ( + "провал promtool не роняет шаг — деплой останется зелёным при битом конфиге" + ) + + +def test_acceptance_checks_last_config_time_actually_changed() -> None: + """Приёмка обязана сверять `lastConfigTime` до/после, а не доверять коду ответа reload. + + `wget` на POST /-/reload может отрапортовать успех, даже если Prometheus + молча остался на старом конфиге (например, если бинарь внутри образа не + поддерживает --post-data так, как ожидалось) — единственное надёжное + подтверждение реального перечитывания конфига это смена таймстемпа. + """ + text = _text() + assert text.count("lastConfigTime") >= 2, ( + "нет сравнения lastConfigTime до/после — reload не проверяется по факту" + ) + assert "LAST_CONFIG_BEFORE" in text and "LAST_CONFIG_AFTER" in text, ( + "нет явного до/после сравнения таймстемпа последней перезагрузки конфига" + ) + verify_start = text.index("LAST_CONFIG_AFTER") + verify_block_end = text.index("Prometheus: конфиг и правила проверены", verify_start) + verify_block = text[verify_start:verify_block_end] + assert "exit 1" in verify_block, ( + "если lastConfigTime не изменился, шаг обязан падать, а не считаться успешным" + ) diff --git a/caddy/sites/infra.caddy b/caddy/sites/infra.caddy index 15788ff9..4d22fa65 100644 --- a/caddy/sites/infra.caddy +++ b/caddy/sites/infra.caddy @@ -105,11 +105,24 @@ metrics.gendsgn.ru { # угадавший, — ложная отметка «принято» в чате, где сразу видно, что её # поставил не человек. Прав в системе токен не даёт никаких. # - # Сервис отвечает только на /ack/* и /healthz; всё прочее — 404. + # Сервис отвечает только на /ack/*, /glitchtip и /healthz; всё прочее — 404. handle /ack/* { reverse_proxy alert-ack:8080 } + # Резервный приёмник алертов GlitchTip (#3471). Основной получатель — + # продуктовый бэкенд на Selectel, то есть тот самый сервис, за которым эти + # алерты и следят: пока он лежит, его собственные ошибки доставлять некому. + # Этот путь живёт у другого провайдера и с чистой сетью до Telegram, поэтому + # переживает падение Selectel целиком. + # + # Секрет — в значении query-параметра, а не в пути: путь сам по себе не + # секрет, и его попадание в access-лог безопасно. Само значение вырезает + # scrub_credentials в Alloy до записи в Loki (#3154). + handle /glitchtip* { + reverse_proxy alert-ack:8080 + } + # Вход ОДИН — собственный вход Grafana (#3078). Внешний basic_auth снят по # решению владельца: два запроса пароля подряд мешали работе, а Grafana имеет # собственную аутентификацию с ролями и `GF_USERS_ALLOW_SIGN_UP=false`. diff --git a/docker-compose.metrics-agent.yml b/docker-compose.metrics-agent.yml index 506be509..7755e2cd 100644 --- a/docker-compose.metrics-agent.yml +++ b/docker-compose.metrics-agent.yml @@ -231,6 +231,71 @@ services: mem_limit: 128m logging: *default-logging + # ── redis-exporter: здоровье общего Redis (только на Poincare) ─────────────── + # #3471. Redis — один инстанс на три потребителя: db0 celery-брокер Site + # Finder, db1 SearchCache trade-in, db2 glitchtip (см. комментарий у сервиса + # `redis` в docker-compose.prod.yml). Один `redis_up` покрывает риск для всех + # трёх разом — до этой правки Redis не измерялся вообще, переполнение + # брокера и обычная недоступность снаружи выглядели одинаково — тишиной. + # + # Адрес — через alias `gendesign-redis`, который `redis` регистрирует на + # сети `shared` (см. #2709 в docker-compose.prod.yml) — джойнить ещё и + # `product` не нужно, тем же путём уже идёт postgres-exporter-tradein. + # + # Пароль — из окружения, не хардкод: сегодня на Redis нет requirepass (нет + # переменной ни в docker-compose.prod.yml, ни здесь), но если он появится, + # значение подставляется через METRICS_REDIS_PASSWORD в /opt/gendesign/.env + # на хосте, а не в этот файл. + redis-exporter: + image: oliver006/redis_exporter:v1.65.0 + container_name: gendesign-redis-exporter + restart: unless-stopped + profiles: ["apps"] + environment: + REDIS_ADDR: ${METRICS_REDIS_ADDR:-redis://gendesign-redis:6379} + REDIS_PASSWORD: ${METRICS_REDIS_PASSWORD:-} + expose: + - "9121" + networks: + - shared + mem_limit: 64m + logging: *default-logging + + # ── celery-exporter: очередь Site Finder (только на Poincare) ──────────────── + # #3471. Слепая зона: глубина очереди, число живых воркеров и счётчик + # неуспешных задач нигде не измерялись — залипший воркер и переполненная + # очередь снаружи неотличимы от тишины. + # + # ПОЧЕМУ ОТДЕЛЬНЫЙ ОБРАЗ, А НЕ redis-exporter --check-keys. check-keys дал + # бы LLEN дефолтной очереди "celery" (в backend/app/workers/celery_app.py + # НЕТ task_routes — все таски идут в один дефолтный queue, имя буквально + # "celery") без нового образа вообще. Но он НЕ умеет считать живых + # воркеров и неуспешные таски — то есть закрыл бы только треть минимума + # из задачи. celery-exporter слушает событийную шину Celery через тот же + # брокер и даёт все три метрики разом, поэтому выбран он, а не комбинация + # check-keys + что-то ещё для остальных двух чисел. + # + # ⚠️ ИМЕНА МЕТРИК НИЖЕ (celery_queue_length, celery_worker_up, + # celery_task_failed_total) — по документации проекта на момент правки, БЕЗ + # прогона на реальном брокере (агент писал этот файл без доступа к проду). + # Сверить с `curl http://gendesign-celery-exporter:9808/metrics` на хосте + # после первого деплоя и поправить `ops/metrics/prometheus/rules/infra.yml` + # при расхождении — иначе алерты будут молча ничего не ловить. + celery-exporter: + image: danihodovic/celery-exporter:0.13.0 + container_name: gendesign-celery-exporter + restart: unless-stopped + profiles: ["apps"] + command: + - "--broker-url=${METRICS_CELERY_BROKER_URL:-redis://gendesign-redis:6379/0}" + - "--queue=celery" + expose: + - "9808" + networks: + - shared + mem_limit: 128m + logging: *default-logging + # ── postgres-exporter: инфраструктурная БД (только на Beget) ───────────────── # forgejo + glitchtip. Нужен и сам по себе, и как страховка: рост базы glitchtip # ничем не ограничен — политики ретенции у GlitchTip нет вообще. diff --git a/docker-compose.metrics.yml b/docker-compose.metrics.yml index 856deede..631b519d 100644 --- a/docker-compose.metrics.yml +++ b/docker-compose.metrics.yml @@ -196,6 +196,12 @@ services: # Внешний адрес попадает в кнопку. Пустой — сообщение уйдёт без кнопки, # но уйдёт: алерт важнее подтверждения. ALERT_ACK_PUBLIC_URL: ${ALERT_ACK_PUBLIC_URL:-https://metrics.gendsgn.ru} + # Резервный получатель GlitchTip-алертов (#3471, POST /glitchtip) — второй + # получатель наряду с основным вебхуком в продуктовый бэкенд на Selectel. + # Секрет СВОЙ, не общий с продуктовым TRADEIN_INTERNAL_AUTH_SECRET: разные + # хосты/домены безопасности. Пусто — эндпоинт отвечает 503, остальной + # функционал сервиса не затронут. + ALERT_ACK_GLITCHTIP_SECRET: ${ALERT_ACK_GLITCHTIP_SECRET:-} volumes: - ./ops/metrics/alert-ack/app.py:/app/app.py:ro expose: @@ -211,6 +217,22 @@ services: retries: 5 # ── Grafana: витрина ───────────────────────────────────────────────────────── + # Grafana здесь ТОЛЬКО рисует — не решает, что считать инцидентом и куда его + # слать. Тревоги живут в Prometheus (правила) и Alertmanager (маршрутизация, + # Telegram); это единственный путь доставки (#3158). + # + # Встроенный Alerting выключен ЯВНО, а не просто «не настроен». Проверка на + # живом API 12.09.2026 нашла: 0 правил, единственный контакт-поинт — + # стоковый grafana-default-email на example@email.com, GF_SMTP_* не заданы. + # То есть кнопка «New alert rule» в интерфейсе есть и работает, а результат + # молча уходит в никуда — ровно та ситуация, из-за которой никто не проверяет + # второй, настоящий путь. Дублирующий движок на том же датасорсе Prometheus + # надёжности всё равно не прибавляет (общая точка отказа), только даёт второе + # место, где правило может быть заведено и забыто. + # + # Если это когда-нибудь понадобится включить обратно — сначала подключить + # реальный SMTP или другой contact point и завести хотя бы одно тестовое + # правило руками, иначе вернётся тот же капкан. grafana: image: grafana/grafana:11.5.1 container_name: gendesign-grafana @@ -230,6 +252,12 @@ services: GF_SECURITY_ADMIN_PASSWORD: ${GRAFANA_ADMIN_PASSWORD:-} GF_SERVER_ROOT_URL: https://metrics.gendsgn.ru/ GF_SERVER_SERVE_FROM_SUB_PATH: "false" + # Единственный официальный переключатель Grafana Alerting в 11.x — секция + # [unified_alerting], легаси-[alerting] удалён из Grafana ещё в 9.0 и в + # 11.5 в конфиге отсутствует (сверено с grafana.com/docs/grafana/v11.5/ + # setup-grafana/configure-grafana/#unified_alerting). false здесь убирает + # раздел Alerting из UI и глушит движок правил целиком — см. #3158 выше. + GF_UNIFIED_ALERTING_ENABLED: "false" # Телеметрия наружу — выключена. Отдельный хост, отдельный провайдер, и не # хочется, чтобы наблюдатель сам ходил в интернет без нужды. GF_ANALYTICS_REPORTING_ENABLED: "false" diff --git a/ops/metrics/alert-ack/app.py b/ops/metrics/alert-ack/app.py index 323f077d..a809fe6d 100644 --- a/ops/metrics/alert-ack/app.py +++ b/ops/metrics/alert-ack/app.py @@ -27,6 +27,28 @@ METRICS_TELEGRAM_ONCALL кого звать поимённо (необязательна) ALERT_ACK_PUBLIC_URL внешний адрес сервиса, попадает в кнопку ALERT_ACK_TTL_MIN сколько минут живёт токен (по умолчанию 1440) + ALERT_ACK_GLITCHTIP_SECRET секрет резервного вебхука GlitchTip (#3471, + см. POST /glitchtip ниже); пусто — 503 + +РЕЗЕРВНЫЙ КАНАЛ GLITCHTIP (#3471). Все три alert-правила GlitchTip (backend, +frontend, Trade-In) шлют основной вебхук в продуктовый бэкенд на Selectel — +тот самый хост, за которым они следят. Если там упал сам бэкенд или Caddy, +алерт об этом теряется именно тогда, когда нужнее всего. `POST /glitchtip` +— второй получатель того же алерта, зарегистрированный в GlitchTip отдельной +строкой; живёт на ЭТОМ (инфраструктурном, Beget) хосте и не зависит от +здоровья продукта. Формат тела — тот же Slack-совместимый payload, что и у +продуктового приёмника (`tradein-mvp/backend/app/api/v1/glitchtip.py`): +``{"text": str, "attachments": [{"title","title_link","text","color", +"fields":[{"title","value"}]}]}``, GlitchTip заголовков не шлёт вовсе — +аутентификация только через секрет в query (``?secret=``) или в заголовке +``X-GlitchTip-Secret`` (тот же выбор, что там же и по той же причине: заголовок +не течёт в access-log, query остаётся, т.к. сам GlitchTip 6.1.6 заголовков не +добавляет). Секрет намеренно СВОЙ (``ALERT_ACK_GLITCHTIP_SECRET``), а не общий +с продуктовым ``TRADEIN_INTERNAL_AUTH_SECRET`` — секреты разных хостов/доменов +безопасности компрометировать вместе незачем. Сообщение уходит в ту же тему +клиентских инцидентов (``METRICS_TELEGRAM_CHAT_ID``/``_TOPIC_ID``), что и +Alertmanager-алерты через alert-ack, с явной пометкой «резервный канал», чтобы +не спутать с основным путём. """ from __future__ import annotations @@ -51,6 +73,7 @@ TOPIC_ID = os.environ.get("METRICS_TELEGRAM_TOPIC_ID", "") ONCALL = os.environ.get("METRICS_TELEGRAM_ONCALL", "") PUBLIC_URL = os.environ.get("ALERT_ACK_PUBLIC_URL", "").rstrip("/") TTL_SEC = int(os.environ.get("ALERT_ACK_TTL_MIN", "1440")) * 60 +GLITCHTIP_SECRET = os.environ.get("ALERT_ACK_GLITCHTIP_SECRET", "") API = "https://api.telegram.org/bot{}/{}" # token -> {"message_id": int, "title": str, "created": float, "acked_by": str|None} @@ -153,6 +176,79 @@ def _send_alert(payload: dict) -> None: _tg("sendMessage", msg) +_GLITCHTIP_TEXT_LIMIT = 3500 # запас под баннер+имя проекта до лимита Telegram 4096 + + +def _verify_glitchtip_secret(provided: str) -> bool: + """Constant-time сравнение — длина/префикс секрета не утекают через время + ответа (тот же приём, что у продуктового приёмника, см. docstring модуля).""" + return bool(GLITCHTIP_SECRET) and secrets.compare_digest(provided or "", GLITCHTIP_SECRET) + + +def _glitchtip_field(attachment: dict, label: str) -> str | None: + for field in attachment.get("fields") or []: + if not isinstance(field, dict): + continue + if str(field.get("title", "")).strip().lower() == label.lower(): + value = field.get("value") + return str(value) if value is not None else None + return None + + +def _render_glitchtip(payload: dict) -> str: + """Собрать текст сообщения из Slack-совместимого payload GlitchTip. + + Максимально терпимо к форме тела: GlitchTip шлёт ОДИНАКОВУЮ структуру для + issue- и uptime-алертов, но поля внутри attachments опциональны, а тестовое + сообщение из UI GlitchTip может не иметь attachments вовсе. Ничего в теле + не считаем обязательным — падать сервису на резервном канале нельзя. + """ + lines = ["⚠️ РЕЗЕРВНЫЙ КАНАЛ (GlitchTip → alert-ack)"] + lines.append("Основной путь через продуктовый бэкенд мог быть недоступен.") + lines.append("") + text = payload.get("text") + lines.append(html.escape(str(text)) if text else "GlitchTip alert") + + attachments = payload.get("attachments") + for attachment in attachments if isinstance(attachments, list) else []: + if not isinstance(attachment, dict): + continue + block: list[str] = [] + project = _glitchtip_field(attachment, "Project") + if project: + block.append(f"Проект: {html.escape(project)}") + if attachment.get("title"): + block.append(html.escape(str(attachment["title"]))) + if attachment.get("text"): + block.append(html.escape(str(attachment["text"]))) + if attachment.get("title_link"): + block.append(f"Ссылка: {html.escape(str(attachment['title_link']))}") + if block: + lines.append("") + lines.extend(block) + + out = "\n".join(lines) + if len(out) > _GLITCHTIP_TEXT_LIMIT: + out = out[:_GLITCHTIP_TEXT_LIMIT] + "\n… (обрезано)" + return out + + +def _send_glitchtip_alert(payload: dict) -> None: + """Переслать вебхук GlitchTip в ту же тему клиентских инцидентов, что и + Alertmanager через этот сервис. Без кнопки подтверждения — это не + firing/resolved инцидент с состоянием, а разовое уведомление резервного + канала.""" + msg = { + "chat_id": CHAT_ID, + "text": _render_glitchtip(payload), + "parse_mode": "HTML", + "disable_web_page_preview": "true", + } + if TOPIC_ID: + msg["message_thread_id"] = TOPIC_ID + _tg("sendMessage", msg) + + _PAGE = ( "" "{t}" @@ -243,11 +339,24 @@ class Handler(BaseHTTPRequestHandler): self._reply(code, page.encode()) def do_POST(self) -> None: # noqa: N802 — имя из stdlib - if self.path != "/alertmanager": - self._reply(404, b"not found", "text/plain; charset=utf-8") - return + # Тело читается ДО любой развилки и ветки отказа. protocol_version = + # HTTP/1.1, то есть соединение переиспользуется, а Caddy перед нами + # держит пул к апстриму. Ответить 401/404/503, не вычитав тело, значит + # оставить его в сокете — и следующий запрос по тому же соединению + # начнётся с чужих байт. Проверено на проде 12.09.2026: неавторизованный + # зонд на /glitchtip, а следом законный алерт получил + # 501 Unsupported method ('{"text":"probe"}POST'). То есть один + # отказ ронял следующий НАСТОЯЩИЙ алерт — ровно то, ради чего этот + # резервный канал и заводился. + parsed = urllib.parse.urlsplit(self.path) length = int(self.headers.get("Content-Length") or 0) raw = self.rfile.read(length) if length else b"{}" + if parsed.path == "/glitchtip": + self._handle_glitchtip(parsed, raw) + return + if parsed.path != "/alertmanager": + self._reply(404, b"not found", "text/plain; charset=utf-8") + return try: payload = json.loads(raw.decode() or "{}") except Exception: # noqa: BLE001 @@ -261,6 +370,36 @@ class Handler(BaseHTTPRequestHandler): self._reply(200, b"accepted", "text/plain; charset=utf-8") threading.Thread(target=_send_alert, args=(payload,), daemon=True).start() + def _handle_glitchtip(self, parsed: urllib.parse.SplitResult, raw: bytes) -> None: + """POST /glitchtip — резервный получатель GlitchTip-алертов (#3471). + + Секрет — из заголовка ``X-GlitchTip-Secret`` (предпочтительно, не течёт + в access-log) либо из query ``?secret=`` (fallback: GlitchTip 6.1.6 + заголовков не шлёт вовсе). Несконфигурированный секрет → 503, а не + тихий приём без проверки. Неразобранное/нестандартное тело НЕ роняет + запрос — это резервный канал, теряться на кривом JSON ему нельзя. + """ + if not GLITCHTIP_SECRET: + self._reply(503, b"glitchtip webhook not configured", "text/plain; charset=utf-8") + return + + header_secret = self.headers.get("X-GlitchTip-Secret", "") + query_secret = urllib.parse.parse_qs(parsed.query).get("secret", [""])[0] + if not _verify_glitchtip_secret(header_secret or query_secret): + log.warning("glitchtip webhook: invalid or missing secret") + self._reply(401, b"invalid or missing secret", "text/plain; charset=utf-8") + return + + try: + payload = json.loads(raw.decode() or "{}") + if not isinstance(payload, dict): + payload = {"text": raw.decode(errors="replace")} + except Exception: # noqa: BLE001 — резервный канал не роняем на кривом теле + payload = {"text": raw.decode(errors="replace")} + + self._reply(200, b"accepted", "text/plain; charset=utf-8") + threading.Thread(target=_send_glitchtip_alert, args=(payload,), daemon=True).start() + def main() -> None: missing = [n for n, v in (("BOT_TOKEN", BOT_TOKEN), ("CHAT_ID", CHAT_ID)) if not v] @@ -268,6 +407,11 @@ def main() -> None: raise SystemExit(f"не заданы обязательные переменные: {', '.join(missing)}") if not PUBLIC_URL: log.warning("ALERT_ACK_PUBLIC_URL пуст — сообщения уйдут БЕЗ кнопки подтверждения") + if not GLITCHTIP_SECRET: + log.warning( + "ALERT_ACK_GLITCHTIP_SECRET пуст — резервный канал GlitchTip (#3471) " + "отключён, POST /glitchtip будет отвечать 503" + ) port = int(os.environ.get("ALERT_ACK_PORT", "8080")) log.info("alert-ack слушает :%d, тема=%s, дежурный=%s", port, TOPIC_ID or "—", ONCALL or "—") ThreadingHTTPServer(("", port), Handler).serve_forever() diff --git a/ops/metrics/alert-ack/test_app.py b/ops/metrics/alert-ack/test_app.py new file mode 100644 index 00000000..52213917 --- /dev/null +++ b/ops/metrics/alert-ack/test_app.py @@ -0,0 +1,256 @@ +"""Тесты для резервного канала GlitchTip → alert-ack (#3471). + +Продукт (tradein-backend на Selectel) — САМ объект наблюдения GlitchTip. Если +он лежит, основной вебхук-получатель лежит вместе с ним, и алерт об этом не +доходит именно тогда, когда нужнее всего. `POST /glitchtip` — второй +получатель на ДРУГОМ хосте (Beget, рядом с этим сервисом), не зависящий от +здоровья продукта. + +Проверяем на уровне функций, а не полного HTTP-транспорта (тот же приём, что +`ops/glitchtip-auth-forwarder/test_forwarder.py`): `BaseHTTPRequestHandler` +неудобно поднимать без реального сокета, а бизнес-логика — секрет, рендер, +отправка — целиком вынесена в чистые функции модуля. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent)) + +# Env — ДО импорта app.py: BOT_TOKEN/CHAT_ID читаются на уровне модуля. +os.environ.setdefault("METRICS_TELEGRAM_BOT_TOKEN", "test-bot-token") +os.environ.setdefault("METRICS_TELEGRAM_CHAT_ID", "-1001234567890") +os.environ.setdefault("METRICS_TELEGRAM_TOPIC_ID", "158") +os.environ["ALERT_ACK_GLITCHTIP_SECRET"] = "correct-secret" + +import app as alert_ack # noqa: E402 + + +def test_verify_glitchtip_secret_accepts_correct_value() -> None: + assert alert_ack._verify_glitchtip_secret("correct-secret") is True + + +def test_verify_glitchtip_secret_rejects_wrong_value() -> None: + """Неверный секрет — отказ.""" + assert alert_ack._verify_glitchtip_secret("wrong-secret") is False + + +def test_verify_glitchtip_secret_rejects_empty_value() -> None: + assert alert_ack._verify_glitchtip_secret("") is False + + +def test_verify_glitchtip_secret_fails_closed_when_unconfigured(monkeypatch) -> None: + """Пустой ALERT_ACK_GLITCHTIP_SECRET — отказ всем, а не тихий fail-open.""" + monkeypatch.setattr(alert_ack, "GLITCHTIP_SECRET", "") + assert alert_ack._verify_glitchtip_secret("correct-secret") is False + assert alert_ack._verify_glitchtip_secret("") is False + + +def test_render_glitchtip_full_payload_marks_fallback_channel() -> None: + payload = { + "text": "GlitchTip Alert: Something broke", + "attachments": [ + { + "title": "TypeError: boom", + "title_link": "https://errors.gendsgn.ru/issue/1", + "text": "подробности ошибки", + "color": "#ff0000", + "fields": [{"title": "Project", "value": "tradein-backend"}], + } + ], + } + text = alert_ack._render_glitchtip(payload) + assert "РЕЗЕРВНЫЙ КАНАЛ" in text + assert "Проект: tradein-backend" in text + assert "TypeError: boom" in text + assert "https://errors.gendsgn.ru/issue/1" in text + + +def test_render_glitchtip_survives_payload_without_attachments() -> None: + """Тело без attachments не роняет сервис — только текст.""" + text = alert_ack._render_glitchtip({"text": "просто текст без вложений"}) + assert "РЕЗЕРВНЫЙ КАНАЛ" in text + assert "просто текст без вложений" in text + + +def test_render_glitchtip_survives_empty_payload() -> None: + """Пустой словарь (нет ни text, ни attachments) — тоже не должен падать.""" + text = alert_ack._render_glitchtip({}) + assert "РЕЗЕРВНЫЙ КАНАЛ" in text + assert "GlitchTip alert" in text + + +def test_render_glitchtip_survives_malformed_attachments() -> None: + """attachments/fields неожиданной формы (не список, не словарь, битые + типы) — резервный канал не должен падать на кривом теле.""" + payload = { + "text": "test", + "attachments": [ + "not-a-dict", + {"fields": "not-a-list"}, + {"fields": [{"title": "Project"}]}, # value отсутствует + None, + ], + } + text = alert_ack._render_glitchtip(payload) + assert "РЕЗЕРВНЫЙ КАНАЛ" in text + + +def test_send_glitchtip_alert_forwards_via_tg(monkeypatch) -> None: + """Верный секрет уже проверен вызывающей стороной (do_POST) — здесь + проверяем, что отрендеренное сообщение реально уходит в тот же chat/topic, + что и Alertmanager-алерты этого сервиса, без кнопки подтверждения.""" + calls = [] + monkeypatch.setattr(alert_ack, "_tg", lambda method, payload: calls.append((method, payload))) + + alert_ack._send_glitchtip_alert({"text": "boom", "attachments": []}) + + assert len(calls) == 1 + method, sent = calls[0] + assert method == "sendMessage" + assert sent["chat_id"] == alert_ack.CHAT_ID + assert sent["message_thread_id"] == alert_ack.TOPIC_ID + assert "reply_markup" not in sent + assert "boom" in sent["text"] + + +# ── Keep-alive: отказ не должен ронять СЛЕДУЮЩИЙ запрос ────────────────────── +# Эти четыре теста — единственные, что поднимают настоящий сокет. Дефект, +# который они стерегут, живёт именно в транспорте и на уровне функций невидим: +# ветка отказа отвечала, не вычитав тело запроса, а `protocol_version` здесь +# HTTP/1.1, то есть соединение переиспользуется (и Caddy перед сервисом держит +# пул к апстриму). Непрочитанное тело оставалось в сокете, и следующий запрос +# по тому же соединению начинался с чужих байт. +# +# Поймано на проде 12.09.2026: зонд без секрета получил 401, а следующий — +# уже с верным секретом — вернул 501 Unsupported method ('{"text":"probe"}POST'). +# То есть один отказ съедал следующий НАСТОЯЩИЙ алерт. + + +def _serve_in_background(monkeypatch): + """Поднимает Handler на эфемерном порту, глушит отправку в Telegram.""" + import threading as _threading + from http.server import ThreadingHTTPServer + + sent: list[dict] = [] + monkeypatch.setattr(alert_ack, "_send_glitchtip_alert", sent.append) + monkeypatch.setattr(alert_ack, "_send_alert", sent.append) + + srv = ThreadingHTTPServer(("127.0.0.1", 0), alert_ack.Handler) + thread = _threading.Thread(target=srv.serve_forever, daemon=True) + thread.start() + return srv, sent + + +def _raw_post(sock, path: str, body: bytes, headers: str = "") -> str: + """Шлёт POST по уже открытому сокету и возвращает статусную строку.""" + req = ( + f"POST {path} HTTP/1.1\r\n" + f"Host: localhost\r\n" + f"Content-Type: application/json\r\n" + f"Content-Length: {len(body)}\r\n" + f"{headers}" + f"\r\n" + ).encode() + body + sock.sendall(req) + # Читаем ровно заголовки: тела короткие, Content-Length всегда проставлен. + buf = b"" + while b"\r\n\r\n" not in buf: + chunk = sock.recv(4096) + if not chunk: + break + buf += chunk + head, _, rest = buf.partition(b"\r\n\r\n") + length = 0 + for line in head.split(b"\r\n")[1:]: + if line.lower().startswith(b"content-length:"): + length = int(line.split(b":")[1]) + while len(rest) < length: + rest += sock.recv(4096) + return head.split(b"\r\n")[0].decode() + + +def _pair_on_one_connection(monkeypatch, first_headers: str, first_path: str = "/glitchtip"): + import socket + + srv, sent = _serve_in_background(monkeypatch) + try: + sock = socket.create_connection(srv.server_address, timeout=5) + try: + first = _raw_post(sock, first_path, b'{"text":"probe"}', first_headers) + second = _raw_post( + sock, + "/glitchtip", + b'{"text":"real alert"}', + "X-GlitchTip-Secret: correct-secret\r\n", + ) + finally: + sock.close() + finally: + srv.shutdown() + srv.server_close() + return first, second, sent + + +def test_rejected_request_does_not_break_next_one_on_same_connection(monkeypatch) -> None: + """401 без секрета, следом законный алерт по ТОМУ ЖЕ соединению — 200.""" + first, second, sent = _pair_on_one_connection(monkeypatch, "") + assert "401" in first + assert "200" in second, f"второй запрос испорчен первым: {second}" + assert sent == [{"text": "real alert"}] + + +def test_unconfigured_secret_does_not_break_next_request(monkeypatch) -> None: + """503 при пустом секрете тоже обязан вычитать тело.""" + import socket + + monkeypatch.setattr(alert_ack, "GLITCHTIP_SECRET", "") + srv, _sent = _serve_in_background(monkeypatch) + try: + sock = socket.create_connection(srv.server_address, timeout=5) + try: + first = _raw_post(sock, "/glitchtip", b'{"text":"probe"}') + second = _raw_post(sock, "/glitchtip", b'{"text":"again"}') + finally: + sock.close() + finally: + srv.shutdown() + srv.server_close() + assert "503" in first + assert "503" in second, f"второй запрос испорчен первым: {second}" + + +def test_unknown_path_does_not_break_next_request(monkeypatch) -> None: + """404 на чужом пути — та же ветка раннего ответа, то же требование.""" + first, second, sent = _pair_on_one_connection(monkeypatch, "", first_path="/nope") + assert "404" in first + assert "200" in second, f"второй запрос испорчен первым: {second}" + assert sent == [{"text": "real alert"}] + + +def test_bad_json_on_alertmanager_does_not_break_next_request(monkeypatch) -> None: + """400 на неразобранном теле /alertmanager — тело уже вычитано, связь цела.""" + import socket + + srv, sent = _serve_in_background(monkeypatch) + try: + sock = socket.create_connection(srv.server_address, timeout=5) + try: + first = _raw_post(sock, "/alertmanager", b"{not json") + second = _raw_post( + sock, + "/glitchtip", + b'{"text":"real alert"}', + "X-GlitchTip-Secret: correct-secret\r\n", + ) + finally: + sock.close() + finally: + srv.shutdown() + srv.server_close() + assert "400" in first + assert "200" in second, f"второй запрос испорчен первым: {second}" + assert sent == [{"text": "real alert"}] diff --git a/ops/metrics/alloy/alloy-apps.alloy b/ops/metrics/alloy/alloy-apps.alloy index 709d621f..5b66559a 100644 --- a/ops/metrics/alloy/alloy-apps.alloy +++ b/ops/metrics/alloy/alloy-apps.alloy @@ -119,6 +119,27 @@ prometheus.scrape "postgres" { scrape_interval = "60s" } +// ═══ REDIS И ОЧЕРЕДЬ CELERY (#3471) ═════════════════════════════════════════════ +// До этой правки ни одной серии redis_* / celery_* в Prometheus не было: глубина +// очереди, число живых воркеров и потеря соединения с брокером были невидимы — +// переполнение очереди и залипший воркер снаружи выглядели одинаково, тишиной. + +prometheus.scrape "redis" { + targets = [ + { __address__ = "gendesign-redis-exporter:9121", job = "redis" }, + ] + forward_to = [prometheus.remote_write.central.receiver] + scrape_interval = "30s" +} + +prometheus.scrape "celery" { + targets = [ + { __address__ = "gendesign-celery-exporter:9808", job = "celery" }, + ] + forward_to = [prometheus.remote_write.central.receiver] + scrape_interval = "30s" +} + // ═══ МЕТРИКИ ПРИЛОЖЕНИЙ ════════════════════════════════════════════════════════ // Эндпоинты появляются в части 3. До этого скрейп просто отдаёт `up 0` — и это // правильно: цель видна как недоступная, а не отсутствует молча. diff --git a/ops/metrics/grafana/provisioning/alerting/README.md b/ops/metrics/grafana/provisioning/alerting/README.md new file mode 100644 index 00000000..ad2b8f7e --- /dev/null +++ b/ops/metrics/grafana/provisioning/alerting/README.md @@ -0,0 +1,16 @@ +# Эта папка сознательно пустая + +Grafana умеет провижинить contact points, notification policies и alert rules +файлами отсюда (`/etc/grafana/provisioning/alerting`). Не клади их сюда. + +Решение (#3158, 12.09.2026): единственный путь доставки тревог — Prometheus +(правила) + Alertmanager (маршрутизация, Telegram). Grafana только рисует. +Встроенный Alerting выключен явно (`GF_UNIFIED_ALERTING_ENABLED: "false"` в +`docker-compose.metrics.yml`, секция `grafana`) — при живом API 12.09.2026 +единственным контакт-поинтом был стоковый `grafana-default-email` на +`example@email.com`, `GF_SMTP_*` не задан, правил ноль. Файл сюда работать не +заставит: движок alerting выключен на уровне сервиса, провижининг в эту папку +Grafana просто не читает. + +Если понадобится включить обратно — сначала пересмотреть само решение в +`docker-compose.metrics.yml`, а не просто добавить файл в эту папку. diff --git a/ops/metrics/prometheus/rules/infra.yml b/ops/metrics/prometheus/rules/infra.yml index ee906111..c064cd4a 100644 --- a/ops/metrics/prometheus/rules/infra.yml +++ b/ops/metrics/prometheus/rules/infra.yml @@ -232,6 +232,63 @@ groups: summary: "p95 задержки ответа выше 5 секунд" description: "{{ $labels.app }}: p95 за 10 минут — {{ $value | humanizeDuration }}." + # ── Redis и очередь Celery (#3471) ─────────────────────────────────────────── + # Слепая зона: до этих правил ни redis_*, ни celery_* не собирались вовсе. + # Redis — общий инстанс на три потребителя (celery-брокер Site Finder, кэш + # trade-in, glitchtip — см. docker-compose.prod.yml), поэтому его смерть + # клиентская, отсюда severity: critical без явного host: apps — серия + # приходит только с продуктового alloy (alloy-apps.alloy), host в неё + # проставляется через external_labels уже на месте. + # + # ⚠️ Имена метрик celery_queue_length / celery_worker_up / + # celery_task_failed_total — по документации celery-exporter на момент + # написания правил, без проверки на реальном брокере (см. комментарий у + # сервиса celery-exporter в docker-compose.metrics-agent.yml). Сверить после + # первого деплоя. + - name: redis-celery + interval: 60s + rules: + - alert: RedisDown + expr: up{job="redis"} == 0 or redis_up == 0 + for: 5m + labels: + severity: critical + annotations: + summary: "Redis недоступен" + description: "redis_exporter не может достучаться до Redis (или сам процесс лёг). Разом теряют связь celery-брокер Site Finder, SearchCache trade-in и glitchtip." + + # `absent()` — как у CadvisorDown: если сам celery-exporter не поднялся, + # серии celery_worker_up не будет вообще, а не будет со значением 0. + - alert: NoActiveCeleryWorkers + expr: count(celery_worker_up == 1) == 0 or absent(celery_worker_up) + for: 5m + labels: + severity: critical + annotations: + summary: "Ни одного живого воркера Celery" + description: "celery-exporter не видит ни одного heartbeat от воркера Site Finder. Все periodic-таски (парсинг, аналитика, синк слоёв) встали." + + # Порог 150 ПРЕДВАРИТЕЛЬНЫЙ: реальных данных по глубине очереди нет (до + # этой правки метрика не собиралась). beat_schedule.py на момент правки + # содержит 44 periodic-задачи с разным временем срабатывания — даже + # маловероятный залп всех разом даёт кратно меньше 150. Порог взят с + # запасом сознательно и требует пересмотра через неделю наблюдений по + # факту `celery_queue_length`. + # + # `delta(...) >= 0` — очередь не УМЕНЬШАЕТСЯ за 15 минут (тот же приём, + # что и "растёт и не разгребается" в тексте задачи): просто высокое + # значение без этого условия поймало бы и здоровый кратковременный всплеск. + - alert: CeleryQueueGrowing + expr: | + celery_queue_length{queue_name="celery"} > 150 + and delta(celery_queue_length{queue_name="celery"}[15m]) >= 0 + for: 15m + labels: + severity: warning + annotations: + summary: "Очередь Celery растёт и не разгребается" + description: "В очереди {{ $value }} задач, за 15 минут меньше не стало. Похоже на залипший воркер или устойчивый рост нагрузки." + # ── Postgres ──────────────────────────────────────────────────────────────── - name: postgres interval: 60s diff --git a/tradein-mvp/backend/app/api/v1/glitchtip.py b/tradein-mvp/backend/app/api/v1/glitchtip.py index 9cf64598..0539698e 100644 --- a/tradein-mvp/backend/app/api/v1/glitchtip.py +++ b/tradein-mvp/backend/app/api/v1/glitchtip.py @@ -50,11 +50,14 @@ from datetime import UTC, datetime from typing import Annotated, Any from fastapi import APIRouter, Header, HTTPException, Query, Request +from fastapi.responses import JSONResponse from pydantic import BaseModel, ConfigDict, ValidationError +from starlette.background import BackgroundTask from app.core.config import settings from app.services.tgbot.client import TelegramError from app.services.tgbot.shared import get_telegram_client +from app.tasks.glitchtip_alert_retry import retry_forward_alert logger = logging.getLogger(__name__) @@ -184,16 +187,28 @@ def _verify_secret(provided: str) -> None: raise HTTPException(status_code=401, detail="invalid or missing secret") -@router.post("/ops/glitchtip-webhook") +@router.post("/ops/glitchtip-webhook", response_model=None) async def glitchtip_webhook( request: Request, secret: Annotated[str, Query()] = "", header_secret: Annotated[str, Header(alias="X-GlitchTip-Secret")] = "", -) -> dict[str, str]: +) -> dict[str, str] | JSONResponse: """Приёмник GlitchTip webhook-алертов (issue + uptime) → пересылка в Telegram-тему алертов (``TELEGRAM_ALERTS_CHAT_ID``/``TELEGRAM_ALERTS_TOPIC_ID`` — ОТДЕЛЬНАЯ тема от support-топика, см. docstring модуля). + Отказ синхронной попытки (#3471) отвечает 502 как и раньше (#3456 — честный + сигнал отправителю), но ставит доставку в фон + (``app.tasks.glitchtip_alert_retry.retry_forward_alert`` через + ``starlette.background.BackgroundTask`` на самом ответе) — GlitchTip вебхуки + не ретраит (#3157), без этого текст алерта терялся бы безвозвратно. + ``BackgroundTask`` привязан НАПРЯМУЮ к возвращаемому ``JSONResponse``, а не + к ``BackgroundTasks``-зависимости: FastAPI прикрепляет задачи из + ``BackgroundTasks`` только к ответу, который вернул сам хендлер, а `raise + HTTPException` строит ОТДЕЛЬНЫЙ ответ в exception-мидлваре — задачи, + поставленные до `raise`, в реальности молча терялись бы вместе с ним (это + воспроизведено тестом, не гипотеза). + Путь публичный в ``rbac_guard`` (``app.core.rbac._PUBLIC_PATHS``) — этот хендлер сам делает единственную проверку секрета. @@ -231,8 +246,27 @@ async def glitchtip_webhook( except TelegramError: # Ловим общий предок, а не `TelegramApiError`: недоступность Telegram — # тоже «переслать не смогли», и отвечать на неё надо задуманным 502, а не - # 500 из необработанного исключения (#3456). + # 500 из необработанного исключения (#3456). 502 ОСТАЁТСЯ — это честный + # сигнал отправителю. Но GlitchTip вебхуки не ретраит (#3157) — без этого + # текст алерта пропал бы бесследно, поэтому доставку ставим в фон + # (#3471, см. app.tasks.glitchtip_alert_retry). + # + # `raise HTTPException` здесь НЕ подходит: FastAPI прикрепляет + # background-задачи только к ответу, который вернул сам хендлер, а + # исключение строит СВОЙ отдельный JSONResponse в exception-мидлваре — + # задача, поставленная до `raise`, никогда бы не выполнилась. Поэтому + # 502 собран и возвращён вручную, с задачей на этом же объекте ответа. logger.exception("glitchtip webhook: не удалось переслать алерт в Telegram") - raise HTTPException(status_code=502, detail="failed to forward alert to telegram") from None + return JSONResponse( + status_code=502, + content={"detail": "failed to forward alert to telegram"}, + background=BackgroundTask( + retry_forward_alert, + client, + chat_id=settings.telegram_alerts_chat_id, + text=text, + message_thread_id=settings.telegram_alerts_topic_id or None, + ), + ) return {"status": "ok"} diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 1e48a282..ab62bddd 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -19,6 +19,7 @@ from sentry_sdk.integrations.httpx import HttpxIntegration from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration from sentry_sdk.integrations.starlette import StarletteIntegration +from sentry_sdk.types import Event, Hint from app.api.public import mera as public_mera from app.api.v1 import ( @@ -79,31 +80,43 @@ install_query_secret_filter() # frontend), отдельного broker нет → мониторить нечего. if settings.glitchtip_dsn: from app.observability.sentry_scrub import ( + drop_payments_disabled_event, redact_telegram_bot_token, scrub_payment_request_body, scrub_public_address, stabilize_retry_error_fingerprint, ) - def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None: - """Композиция платёжный body-wipe + PII-scrub + Telegram bot-токен redaction + - RetryError fingerprint-стабилизация (#tgsupport-web, PR-D2, glitchtip-noise) — - см. app/tgbot_main.py._before_send (идентичная композиция без последнего шага, - тот бот geocoder не зовёт). Тот же риск: теперь этот процесс тоже держит - TelegramClient в стек-фреймах при ошибке sendMessage, а + def _before_send(event: Event, hint: Hint) -> Event | None: + """Композиция payments-disabled drop + платёжный body-wipe + PII-scrub + + Telegram bot-токен redaction + RetryError fingerprint-стабилизация + (#tgsupport-web, PR-D2, glitchtip-noise, #3471) — см. + app/tgbot_main.py._before_send (идентичная композиция без последнего + шага, тот бот geocoder не зовёт). Тот же риск: теперь этот процесс тоже + держит TelegramClient в стек-фреймах при ошибке sendMessage, а include_local_variables=False ниже — первый рубеж защиты. - PR-D2: платёжный body-wipe идёт ПЕРВЫМ шагом, а не заменяет остальные — - режет `request.data` целиком только для `/payments/*`, остальные пути - (extra/contexts/traceback) по-прежнему проходят ключ-based scrub и - token-redaction. Тот же обработчик передан ОБОИМ каналам ниже - (before_send и before_send_transaction) — вчерашний баг в Птице закрыл - только error-канал, transaction-канал остался вообще без обработчика. + #3471: payments-disabled drop идёт ПЕРВЫМ шагом — это единственный + процесс из трёх entrypoint'ов, который реально держит ASGI-роут + `/api/v1/payments/*`, поэтому именно здесь события возникают; ранний + return None экономит остальную композицию на заведомо отбрасываемом + событии. + + PR-D2: платёжный body-wipe идёт следующим шагом, а не заменяет + остальные — режет `request.data` целиком только для `/payments/*`, + остальные пути (extra/contexts/traceback) по-прежнему проходят + ключ-based scrub и token-redaction. Тот же обработчик передан ОБОИМ + каналам ниже (before_send и before_send_transaction) — вчерашний баг в + Птице закрыл только error-канал, transaction-канал остался вообще без + обработчика. RetryError-стабилизация — этот процесс обслуживает /api/v1/geocode/* (suggest/lookup/reverse), которые ретраят Nominatim через tenacity; см. sentry_scrub.stabilize_retry_error_fingerprint.""" - scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type] + dropped = drop_payments_disabled_event(event, hint) # type: ignore[arg-type] + if dropped is None: + return None + scrubbed = scrub_payment_request_body(dropped, hint) # type: ignore[arg-type] if scrubbed is None: return None # Публичный периметр МЕРЫ: тело запроса — это ровно введённый адрес, а diff --git a/tradein-mvp/backend/app/observability/sentry_scrub.py b/tradein-mvp/backend/app/observability/sentry_scrub.py index 742e14ab..02b34d79 100644 --- a/tradein-mvp/backend/app/observability/sentry_scrub.py +++ b/tradein-mvp/backend/app/observability/sentry_scrub.py @@ -25,6 +25,7 @@ import re from typing import Any from sentry_sdk.types import Event +from starlette.exceptions import HTTPException as _StarletteHTTPException from tenacity import RetryError _REDACTED = "[REDACTED]" @@ -256,6 +257,41 @@ def scrub_payment_request_body(event: Event, _hint: dict[str, Any]) -> Event | N return event +_PAYMENTS_DISABLED_DETAIL = "payments are disabled" + + +def drop_payments_disabled_event(event: Event, hint: dict[str, Any]) -> Event | None: + """before_send-хук: роняет 503 "payments are disabled" из + `payments.py._require_enabled` (issue #3471, GlitchTip-группа TRADE-IN-3GG). + + Источник — внутренний IP смоук-проверки: кнопки оплаты во фронте нет, + клиентского трафика на эти пути нет вообще, а выключенный платёжный контур + (`settings.payments_enabled=False`) штатно отвечает 503 на каждый такой + запрос — 167 событий за 29.08-12.09 размывали ленту, на этом фоне терялась + настоящая ошибка. Само поведение ручки НЕ меняется (503 остаётся) — + фильтруется только репортинг в трекер: sentry_sdk `StarletteIntegration` + репортит любой `HTTPException` с кодом из `failed_request_status_codes` + (по умолчанию весь диапазон 5xx) как error-событие, даже когда исключение + штатно обработано FastAPI и превращено в корректный HTTP-ответ. + + Матчим `isinstance` реального объекта исключения из `hint["exc_info"]` (тот + же контракт, что `stabilize_retry_error_fingerprint` ниже) + точный текст + `detail` — НЕ код 503 сам по себе, чтобы не проглотить другие 503 (напр. + будущий maintenance-режим другого роутера). + """ + if not isinstance(event, dict): + return event + exc_info = hint.get("exc_info") if isinstance(hint, dict) else None + exc_value = exc_info[1] if exc_info and len(exc_info) > 1 else None + if ( + isinstance(exc_value, _StarletteHTTPException) + and exc_value.status_code == 503 + and exc_value.detail == _PAYMENTS_DISABLED_DETAIL + ): + return None + return event + + _PUBLIC_API_URL_SEGMENT = "/api/public/" #: Хосты геокодеров: их URL несёт введённый адрес прямо в query. diff --git a/tradein-mvp/backend/app/scheduler_main.py b/tradein-mvp/backend/app/scheduler_main.py index 57e17551..e279e59a 100644 --- a/tradein-mvp/backend/app/scheduler_main.py +++ b/tradein-mvp/backend/app/scheduler_main.py @@ -43,14 +43,16 @@ if settings.glitchtip_dsn: from sentry_sdk.integrations.httpx import HttpxIntegration from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration + from sentry_sdk.types import Event, Hint from app.observability.sentry_scrub import ( + drop_payments_disabled_event, scrub_payment_request_body, scrub_pii_event, stabilize_retry_error_fingerprint, ) - def _before_send(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg] + def _before_send(event: Event, hint: Hint) -> Event | None: """PR-D2: этот процесс не держит ASGI-приложения (нет `request` в event сегодня), но payments_confirm/payments_reconcile (PR-E, тот же `tradein-scraper` контейнер) будут звать Т-Банк API отсюда — belt-and- @@ -58,6 +60,13 @@ if settings.glitchtip_dsn: `request`/`extra`. Тот же обработчик на оба канала ниже — см. app/main.py._before_send (идентичный мотив, не дублировать без причины). + #3471: payments-disabled drop — тот же belt-and-suspenders мотив, что и + payment body-wipe выше по докстрингу: этот процесс сегодня не отвечает + 503 из `_require_enabled` (нет ASGI/роутов), реальный источник шума — + `app/main.py`, но фильтр держим одинаковым во всех трёх entrypoint'ах, + чтобы поведение не разошлось, если payments-код когда-нибудь переедет + сюда же. + PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise) идут следом за платёжным body-wipe: этот процесс гоняет `geocode_missing_listings` (ночной batch, сотни адресов за прогон) — @@ -66,13 +75,16 @@ if settings.glitchtip_dsn: на КАЖДЫЙ адрес (RetryError.__str__() тащит нестабильный repr() Future). См. sentry_scrub docstring. """ - scrubbed = scrub_payment_request_body(event, hint) # type: ignore[arg-type] + dropped = drop_payments_disabled_event(event, hint) # type: ignore[arg-type] + if dropped is None: + return None + scrubbed = scrub_payment_request_body(dropped, hint) # type: ignore[arg-type] if scrubbed is None: return None - scrubbed = scrub_pii_event(scrubbed, hint) + scrubbed = scrub_pii_event(scrubbed, hint) # type: ignore[arg-type] if scrubbed is None: return None - return stabilize_retry_error_fingerprint(scrubbed, hint) + return stabilize_retry_error_fingerprint(scrubbed, hint) # type: ignore[arg-type,return-value] sentry_sdk.init( dsn=settings.glitchtip_dsn, diff --git a/tradein-mvp/backend/app/services/proxy_pool.py b/tradein-mvp/backend/app/services/proxy_pool.py index dcdd4bb1..437e937c 100644 --- a/tradein-mvp/backend/app/services/proxy_pool.py +++ b/tradein-mvp/backend/app/services/proxy_pool.py @@ -1309,6 +1309,8 @@ async def _probe_proxy(url: str) -> tuple[bool, str | None, int | None, str | No транзиентный сбой узла ≠ перманентный бан, используется пока только для логов): - "timeout" — сеть недоступна/медленная (httpx.TimeoutException) - "connect_error" — прокси не поднят/не слушает/DNS (httpx.ConnectError) + - "proxy_error" — сам прокси отверг соединение (httpx.ProxyError, напр. 407 от + провайдера — это состояние пула, а не инцидент; #3471) - "http_error" — ipify ответил ошибкой через прокси (auth/upstream) - "other" — прочее @@ -1328,6 +1330,14 @@ async def _probe_proxy(url: str) -> tuple[bool, str | None, int | None, str | No except httpx.ConnectError: logger.warning("proxy_pool: health probe connect_error proxy=%s", _mask(url)) return False, None, None, "connect_error" + except httpx.ProxyError as exc: + # #3471: сам прокси-провайдер отверг соединение (чаще всего 407 — + # исчерпан лимит/просрочен пакет) — штатный исход health-пробы, не + # инцидент приложения. Одна строка без трейса: узел + причина текстом + # исключения, полный traceback здесь не несёт новой информации и только + # засорял логи (184 строки/сутки, #3471). + logger.warning("proxy_pool: health probe proxy_error proxy=%s reason=%s", _mask(url), exc) + return False, None, None, "proxy_error" except httpx.HTTPStatusError as exc: logger.warning( "proxy_pool: health probe http_error proxy=%s status=%s", diff --git a/tradein-mvp/backend/app/services/tgbot/bridge.py b/tradein-mvp/backend/app/services/tgbot/bridge.py index 1dfe9449..d23c3307 100644 --- a/tradein-mvp/backend/app/services/tgbot/bridge.py +++ b/tradein-mvp/backend/app/services/tgbot/bridge.py @@ -223,7 +223,13 @@ class BridgeStorage(Protocol): ) -> int | None: ... def record_web_out_message( - self, *, thread_id: int, text_body: str, operator_tg_id: int | None + self, + *, + thread_id: int, + text_body: str, + operator_tg_id: int | None, + topic_message_id: int | None = None, + support_chat_id: int | None = None, ) -> None: ... @@ -376,14 +382,20 @@ class SqlBridgeStorage: со ЧУЖИМ (не NULL, не текущим) support_chat_id — исторический артефакт ротации support-группы, не валидный маршрут сегодня. NULL (строки до миграции 188, если есть) — лениентный wildcard-матч (единственный - действовавший чат на тот момент).""" + действовавший чат на тот момент). + + БЕЗ фильтра по direction (#3471 P0, было `AND direction = 'in'`): с тех + пор как `record_message` на исходящем ответе тоже сохраняет + `topic_message_id` (id сообщения оператора В ТОПИКЕ), реплай оператора + на СВОЙ предыдущий ответ обязан резолвиться так же, как реплай на + зеркало клиента — иначе продолжение диалога без повторного цитирования + клиента тихо проваливалось в orphan-check.""" row = self._db.execute( text( """ SELECT chat_id FROM tg_support_messages WHERE topic_message_id = CAST(:topic_message_id AS bigint) - AND direction = 'in' AND (support_chat_id = CAST(:support_chat_id AS bigint) OR support_chat_id IS NULL) ORDER BY created_at DESC @@ -413,13 +425,21 @@ class SqlBridgeStorage: ) def record_web_out_message( - self, *, thread_id: int, text_body: str, operator_tg_id: int | None + self, + *, + thread_id: int, + text_body: str, + operator_tg_id: int | None, + topic_message_id: int | None = None, + support_chat_id: int | None = None, ) -> None: web_support_storage.record_outbound( self._db, thread_id=thread_id, text_body=text_body, operator_tg_id=operator_tg_id, + topic_message_id=topic_message_id, + support_chat_id=support_chat_id, ) @@ -448,7 +468,7 @@ async def _notify_topic( text: str, reply_to_message_id: int | None, context: str, -) -> None: +) -> bool: """Служебное уведомление оператору в support-топик (вторичное действие). Два свойства, которых не было у прямых `client.send_message` вызовов: @@ -459,6 +479,12 @@ async def _notify_topic( и не решает судьбу апдейта. `context` — только технические идентификаторы (chat_id/thread_id), НЕ текст переписки: логи моста принципиально не содержат ПДн. + + Возвращает True, если уведомление реально ушло, False — если само + уведомление тоже упало (напр. Telegram недоступен). Вызывающий, для + которого проваленное уведомление означает ПОЛНУЮ тишину (ни клиенту, ни + оператору), обязан на False залогировать `logger.error` с идентификаторами + (#3471 P0) — иначе единственный след остаётся только в этом WARNING. """ try: await client.send_message( @@ -470,6 +496,7 @@ async def _notify_topic( max_retries=_NOTIFY_SEND_MAX_RETRIES, max_backoff=_NOTIFY_SEND_MAX_BACKOFF_S, ) + return True except Exception: logger.warning( "tgbot bridge: не удалось отправить уведомление оператору в топик (%s) — " @@ -477,6 +504,7 @@ async def _notify_topic( context, exc_info=True, ) + return False # ── Update routing ──────────────────────────────────────────────────────────── @@ -649,10 +677,22 @@ async def _handle_group_reply( chat_id=target_chat_id, direction="out", tg_message_id=tg_message_id, - topic_message_id=None, + # #3471 P0: id ЭТОГО сообщения оператора В ТОПИКЕ (было безусловно + # None) — без него реплай оператора на СВОЙ предыдущий ответ не + # резолвился (искать было нечего), маршрут держался только на + # зеркале клиента. Совпадение с `in`-записью структурно исключено: + # `message_id` — id реплая оператора, а зеркало клиента уже занимает + # другой message_id в том же чате. + topic_message_id=message_id, kind=_infer_kind(message), text_body=message.get("text") or message.get("caption"), operator_tg_id=operator_id, + # Deep review PR #3479: без этого out-строка была бы вечным + # wildcard для `find_chat_by_topic_message` (матчит support_chat_id + # IS NULL под ЛЮБЫМ текущим чатом) — при ротации support-группы + # (188) новый message_id мог бы совпасть со старой out-строкой и + # увести ответ ЧУЖОМУ клиенту. Симметрично in-ветке выше (строка ~601). + support_chat_id=settings.telegram_support_chat_id, ) return @@ -686,11 +726,64 @@ async def _handle_group_reply( operator = message.get("from") or {} operator_id = operator.get("id") - storage.record_web_out_message( - thread_id=web_thread_id, - text_body=text_body, - operator_tg_id=operator_id, - ) + try: + storage.record_web_out_message( + thread_id=web_thread_id, + text_body=text_body, + operator_tg_id=operator_id, + # #3471 P0: id ЭТОГО сообщения оператора в топике — без него + # реплай оператора на СВОЙ предыдущий веб-ответ не резолвится + # (см. `find_thread_by_topic_message`, direction-фильтр снят). + topic_message_id=message_id if isinstance(message_id, int) else None, + # Deep review PR #3479: БЕЗ этого out-строка писалась бы с + # support_chat_id=NULL — `find_thread_by_topic_message` матчит + # NULL под ЛЮБЫМ текущим чатом (лениентный wildcard для легаси + # строк до 187/188), т.е. каждая out-строка стала бы вечным + # wildcard. При ротации support-группы новый message_id мог бы + # совпасть со старой out-строкой и увести ответ в ЧУЖОЙ тред — + # ровно то, от чего защищала скоупинг-миграция 187/188. + support_chat_id=settings.telegram_support_chat_id, + ) + except SQLAlchemyError: + # #3471 P0: для веб-треда ЭТА запись — и есть доставка клиенту (веб- + # фронт вычитывает ответ обычным polling'ом web_support_messages). + # Откат без уведомления означал бы: оператор уверен, что ответил, + # клиент ждёт молча. Offset ниже всё равно сдвигается — НЕ потому, + # что апдейт "частично применён в Telegram" (в этой ветке до сбоя в + # Telegram ничего не уходило вообще: сам реплай оператора Telegram + # уже полностью доставил ДО того, как мы начали его разбирать, + # ретраить на стороне площадки нечего), а потому что действует общая + # политика `process_update` для `SQLAlchemyError` — сбой БД не + # переигрывается (в отличие от `TelegramNetworkError`), а + # сигнализируется громко; здесь это explicit-просьба оператору + # прислать ответ заново — human-in-the-loop retry вместо + # технического. rollback() ОБЯЗАН отработать ДО уведомления — + # сессия в failed-transaction state, а `_notify_topic` шлёт через + # `client`, не через `storage`, поэтому сам rollback тут не нужен для + # отправки, но нужен, чтобы process_update дальше не упал на + # save_offset/commit тем же PendingRollbackError (см. #3 review). + storage.rollback() + notified = await _notify_topic( + client, + text=( + "Не удалось сохранить ваш ответ из-за сбоя базы данных — клиенту " + "он НЕ доставлен. Пожалуйста, отправьте ответ ещё раз." + ), + reply_to_message_id=message_id if isinstance(message_id, int) else None, + context=f"сбой БД на доставке веб-ответа thread_id={web_thread_id}", + ) + if not notified: + # Оба канала молчат (БД и уведомление) — единственный след, + # который останется, это эта строка. Идентификаторы, НЕ текст + # (ПДн в лог не идёт) — по ним человек найдёт ответ оператора в + # топике и перешлёт его руками (#3471 P0). + logger.error( + "tgbot bridge: сбой БД на веб-ответе И не удалось уведомить " + "оператора (thread_id=%d, message_id=%s) — ответ клиенту " + "потерян молча, требуется ручной разбор support-топика", + web_thread_id, + message_id, + ) return # Обычная болтовня в топике (реплай на чьё-то ещё сообщение) — не логируем, @@ -741,7 +834,12 @@ async def process_update( `PendingRollbackError`, `process_update` вылетит без сохранения offset'а, следующая итерация получит СТАРЫЙ offset от `get_offset()` и переиграет тот же апдейт — copyMessage задублирует зеркало клиента в топике на - каждый повтор поллинга (#3 review, воспроизведено). + каждый повтор поллинга (#3 review, воспроизведено). Для веб-ветки + (`_handle_group_reply` → `record_web_out_message`) этот `SQLAlchemyError` + перехватывается ЛОКАЛЬНО, до этого места: там запись в БД И ЕСТЬ + доставка клиенту, поэтому rollback сопровождается уведомлением оператору + в топике, что ответ НЕ доставлен (#3471 P0) — сюда, на верхний уровень, + это исключение уже не долетает. - любое прочее исключение (в т.ч. `TelegramApiError` — площадка ОТВЕТИЛА отказом, повтор ничего не изменит) — offset двигаем, «ядовитый» апдейт не блокирует поток. diff --git a/tradein-mvp/backend/app/services/tgbot/web_support_storage.py b/tradein-mvp/backend/app/services/tgbot/web_support_storage.py index 42334482..e1b87a23 100644 --- a/tradein-mvp/backend/app/services/tgbot/web_support_storage.py +++ b/tradein-mvp/backend/app/services/tgbot/web_support_storage.py @@ -106,8 +106,17 @@ def record_inbound( def find_thread_by_topic_message( db: Session, topic_message_id: int, support_chat_id: int ) -> int | None: - """Резолвит id зеркала (сообщения оператора reply_to) в thread_id — только - среди direction='in' записей, зеркало-конвенция как в tg_support_messages (186). + """Резолвит id зеркала/ответа (reply_to) в thread_id. + + БЕЗ фильтра по direction (#3471 P0, было `AND direction = 'in'`): с тех пор + как `record_outbound` тоже сохраняет `topic_message_id` (id ответа оператора + В ТОПИКЕ), реплай оператора на СВОЙ предыдущий ответ обязан резолвиться так + же, как реплай на inbound-зеркало клиента — иначе продолжение диалога без + повторного цитирования клиента тихо проваливалось в orphan-check + (`_handle_group_reply` в bridge.py). Коллизий topic_message_id между + inbound- и outbound-строками одного треда быть не может: Telegram выдаёт + каждому сообщению в чате свой возрастающий id, `web_support_messages_topic_message_id_uq` + (partial unique, 187) это же и гарантирует на уровне БД. Скоупим к ТЕКУЩЕМУ `support_chat_id` (#tgsupport-web review M1): строка со ЧУЖИМ (не NULL и не текущим) support_chat_id — это исторический артефакт @@ -120,7 +129,6 @@ def find_thread_by_topic_message( SELECT thread_id FROM web_support_messages WHERE topic_message_id = CAST(:topic_message_id AS bigint) - AND direction = 'in' AND (support_chat_id = CAST(:support_chat_id AS bigint) OR support_chat_id IS NULL) ORDER BY created_at DESC LIMIT 1 @@ -132,18 +140,40 @@ def find_thread_by_topic_message( def record_outbound( - db: Session, *, thread_id: int, text_body: str, operator_tg_id: int | None + db: Session, + *, + thread_id: int, + text_body: str, + operator_tg_id: int | None, + topic_message_id: int | None = None, + support_chat_id: int | None = None, ) -> int | None: """Записывает ответ оператора (реплай на веб-зеркало) как direction='out'. - `topic_message_id` всегда NULL — маршрутизирующий ключ живёт только на - inbound-записи (см. tg_support_messages-конвенцию, 186).""" + + `topic_message_id` (#3471 P0, раньше был безусловно NULL) — id ЭТОГО + сообщения оператора в топике. Раньше маршрутизирующий ключ жил только на + inbound-записи (конвенция 186/187), из-за чего реплай оператора на СВОЙ + предыдущий ответ был нерезолвим — искать было нечего, а `reply_to_message_id` + указывал на строку без ключа. `find_thread_by_topic_message` теперь матчит + обе стороны (direction-фильтр там снят). + + `support_chat_id` (deep review PR #3479) — ОБЯЗАТЕЛЕН при заполненном + `topic_message_id`: `find_thread_by_topic_message` матчит `support_chat_id + IS NULL` как лениентный wildcard "под любым текущим чатом" (легаси-строки + до 187/188). Без этого поля КАЖДАЯ out-строка была бы таким wildcard — при + ротации support-группы новый message_id мог бы совпасть со старой + out-строкой и увести ответ в ЧУЖОЙ тред (ровно то, от чего защищала + скоупинг-миграция 187/188, см. review M1 там же).""" row = db.execute( text( """ INSERT INTO web_support_messages - (thread_id, direction, text_body, topic_message_id, operator_tg_id, created_at) + (thread_id, direction, text_body, topic_message_id, + support_chat_id, operator_tg_id, created_at) VALUES - (CAST(:thread_id AS bigint), 'out', :text_body, NULL, + (CAST(:thread_id AS bigint), 'out', :text_body, + CAST(:topic_message_id AS bigint), + CAST(:support_chat_id AS bigint), CAST(:operator_tg_id AS bigint), NOW()) RETURNING id """ @@ -151,6 +181,8 @@ def record_outbound( { "thread_id": thread_id, "text_body": text_body, + "topic_message_id": topic_message_id, + "support_chat_id": support_chat_id, "operator_tg_id": operator_tg_id, }, ).fetchone() diff --git a/tradein-mvp/backend/app/tasks/glitchtip_alert_retry.py b/tradein-mvp/backend/app/tasks/glitchtip_alert_retry.py new file mode 100644 index 00000000..3a698937 --- /dev/null +++ b/tradein-mvp/backend/app/tasks/glitchtip_alert_retry.py @@ -0,0 +1,105 @@ +"""Фоновая пересылка GlitchTip-алерта в Telegram после отказа синхронной попытки. + +Контекст (#3471, #3157, #3456). GlitchTip-вебхуки НЕ ретраятся — сам GlitchTip +безусловно помечает уведомление ``is_sent`` сразу после HTTP-ответа приёмника +(upstream-поведение, см. #3157), поэтому если синхронная пересылка в Telegram +(``app.api.v1.glitchtip``) не удалась, повторной доставки от GlitchTip не будет +никогда — текст алерта исчезает бесследно. Ответ 502 на отказ Telegram остаётся +как есть (задуман осознанно, #3456: честный сигнал отправителю, а не тихий +проглот) — меняется то, что происходит С ТЕКСТОМ алерта после этого отказа. + +Почему не Celery. В tradein-mvp нет очереди с воркером: ни ``app/celery_app.py``, +ни зависимости ``celery`` в ``backend/pyproject.toml`` не существует (проверено +при работе над #3471) — попытка ``from celery import ...`` здесь упала бы +``ModuleNotFoundError``. Бутстрап полноценного Celery-воркера — новый контейнер и +брокер, инфраструктурное решение вне границ этой задачи. Единственный доступный +внутри границ задачи (``app/api/v1/glitchtip.py`` + ``app/tasks/**``) механизм +«не блокировать интерактивный ответ, но не потерять текст» — Starlette +``BackgroundTasks``: выполняется ПОСЛЕ отправки HTTP-ответа тем же процессом, вне +узкого интерактивного бюджета (``_INTERACTIVE_SEND_TIMEOUT_S=8s`` в glitchtip.py), +поэтому здесь можно позволить себе штатную "воркерную" ретрай-политику клиента +(``TelegramClient.send_message`` без явных ``timeout``/``max_retries`` — 5 попыток, +backoff до 30s, см. ``app.services.tgbot.client``), плюс собственный внешний +потолок ниже. + +Компромисс, честно: BackgroundTasks не переживает рестарт процесса (это не +персистентная очередь) — если tradein-backend упадёт ровно между отказом +синхронной попытки и завершением фоновой, текст всё-таки потеряется. Событие +редкое (одно с начала эксплуатации, TRADE-IN-3F7, 28.08.2026), а сеть до Telegram +теряет отдельные запросы, а не рвётся на минуты (замер 12.09: 9/12 успешных +``getMe``) — штатной ретрай-политики клиента обычно достаточно без внешнего +потолка вовсе. Персистентная очередь (переживающая рестарт) требует +Celery/Redis-воркера — отдельное инфраструктурное решение. + +Идемпотентность настолько, насколько дёшево. Текст между попытками не +пересобирается (переиспользуется уже отформатированный ``text`` из +``glitchtip.py`` — никакого дублирования форматирования). Полной идемпотентности +нет и быть не может дёшево: Telegram ``sendMessage`` не идемпотентен сам по себе +(повтор создаёт НОВОЕ сообщение, не апдейтит старое) — именно поэтому внешний +потолок попыток мал (``_MAX_ATTEMPTS``), а не «ретраить пока не получится». +""" + +from __future__ import annotations + +import asyncio +import logging + +from app.services.tgbot.client import TelegramClient, TelegramError + +logger = logging.getLogger(__name__) + +__all__ = ["retry_forward_alert"] + +# Внешний потолок ПОВЕРХ штатной ретрай-политики клиента (5 попыток внутри одного +# send_message с backoff до 30s) — защита от «недоставляемый алерт крутится в фоне +# вечно»: если сеть до Telegram не восстановилась за это время, сдаёмся и логируем +# ERROR с текстом, а не повторяем бесконечно. +_MAX_ATTEMPTS = 3 +_RETRY_DELAY_S = 30.0 + + +async def retry_forward_alert( + client: TelegramClient, + *, + chat_id: int, + text: str, + message_thread_id: int | None, +) -> None: + """Досылает уже отформатированный текст алерта после отказа синхронной попытки. + + ``client`` — ТОТ ЖЕ общий клиент приложения, что и в синхронном пути + (``get_telegram_client()`` в ``glitchtip.py``), а не новый инстанс: он живёт в + lifespan ради keep-alive-соединения (см. docstring ``glitchtip.py``). + """ + for attempt in range(1, _MAX_ATTEMPTS + 1): + try: + await client.send_message( + chat_id=chat_id, + text=text, + message_thread_id=message_thread_id, + # Без явных timeout/max_retries — штатная "воркерная" политика + # клиента (см. докстринг модуля). + ) + except TelegramError: + if attempt == _MAX_ATTEMPTS: + logger.error( + "glitchtip alert retry: не удалось доставить алерт в Telegram " + "после %d попыток — текст потерян: %r", + _MAX_ATTEMPTS, + text[:200], + exc_info=True, + ) + return + logger.warning( + "glitchtip alert retry: попытка %d/%d не удалась, повтор через %.0fs", + attempt, + _MAX_ATTEMPTS, + _RETRY_DELAY_S, + exc_info=True, + ) + await asyncio.sleep(_RETRY_DELAY_S) + else: + logger.info( + "glitchtip alert retry: доставлено фоном с попытки %d/%d", attempt, _MAX_ATTEMPTS + ) + return diff --git a/tradein-mvp/backend/app/tgbot_main.py b/tradein-mvp/backend/app/tgbot_main.py index be555a25..7a82e0d0 100644 --- a/tradein-mvp/backend/app/tgbot_main.py +++ b/tradein-mvp/backend/app/tgbot_main.py @@ -26,7 +26,6 @@ import logging import os import signal from contextlib import suppress -from typing import Any from app.core.config import settings from app.core.db import SessionLocal @@ -57,18 +56,20 @@ if settings.glitchtip_dsn: import sentry_sdk from sentry_sdk.integrations.httpx import HttpxIntegration from sentry_sdk.integrations.logging import LoggingIntegration + from sentry_sdk.types import Event, Hint from app.observability.sentry_scrub import ( + drop_payments_disabled_event, redact_telegram_bot_token, scrub_payment_request_body, scrub_pii_event, ) - def _before_send(event: Any, hint: dict[str, Any]) -> Any: - """Композиция платёжный body-wipe (PR-D2) + PII-scrub (form-данные) + - Telegram bot-токен redaction (#tgsupport review). Токен утекает ДВУМЯ - независимыми векторами, которые `include_local_variables=False` ниже и - этот хук закрывают вместе: + def _before_send(event: Event, hint: Hint) -> Event | None: + """Композиция payments-disabled drop (#3471) + платёжный body-wipe (PR-D2) + + PII-scrub (form-данные) + Telegram bot-токен redaction (#tgsupport review). + Токен утекает ДВУМЯ независимыми векторами, которые + `include_local_variables=False` ниже и этот хук закрывают вместе: 1. `include_local_variables=True` (sentry_sdk default) кладёт stack-frame locals (`self._base`/`url` в `TelegramClient._request`) в traceback — закрыто через `include_local_variables=False` в `sentry_sdk.init`. @@ -78,18 +79,22 @@ if settings.glitchtip_dsn: — belt-and-suspenders на случай #1 (если include_local_variables случайно вернут) И на span data. - Платёжный body-wipe — belt-and-suspenders: этот процесс не держит ASGI- - приложения (нет `request` в event сегодня), но тот же обработчик передан - ОБОИМ каналам ниже (before_send/before_send_transaction) ради единообразия - со всеми точками инициализации sentry_sdk в проекте (см. app/main.py). + Payments-disabled drop и платёжный body-wipe — belt-and-suspenders: этот + процесс не держит ASGI-приложения (нет `request`/HTTPException в event + сегодня, реальный источник 503 — app/main.py), но тот же обработчик + передан ОБОИМ каналам ниже (before_send/before_send_transaction) ради + единообразия со всеми точками инициализации sentry_sdk в проекте. """ - scrubbed = scrub_payment_request_body(event, hint) + dropped = drop_payments_disabled_event(event, hint) # type: ignore[arg-type] + if dropped is None: + return None + scrubbed = scrub_payment_request_body(dropped, hint) # type: ignore[arg-type] if scrubbed is None: return None - scrubbed = scrub_pii_event(scrubbed, hint) + scrubbed = scrub_pii_event(scrubbed, hint) # type: ignore[arg-type] if scrubbed is None: return None - return redact_telegram_bot_token(scrubbed, hint) + return redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type,return-value] sentry_sdk.init( dsn=settings.glitchtip_dsn, diff --git a/tradein-mvp/backend/tests/services/test_proxy_pool.py b/tradein-mvp/backend/tests/services/test_proxy_pool.py index 1a40a8aa..c5466431 100644 --- a/tradein-mvp/backend/tests/services/test_proxy_pool.py +++ b/tradein-mvp/backend/tests/services/test_proxy_pool.py @@ -50,6 +50,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost: from datetime import UTC, datetime, timedelta from typing import Any +import httpx import pytest from app.services import proxy_pool @@ -1527,3 +1528,74 @@ def test_clear_source_bans_resets_escalation() -> None: assert ban["ban_count"] == 1 expected = datetime.now(UTC) + timedelta(hours=SOURCE_BAN_BASE_HOURS) assert abs((ban["banned_until"] - expected).total_seconds()) < 60 + + +# ── health-probe failure logging (#3471 — GlitchTip/log noise) ────────────── +# httpx.ProxyError (типично 407 от провайдера) раньше падал в generic +# `except Exception: ... exc_info=True` внутри `_probe_proxy` — полный traceback +# на КАЖДЫЙ провал, хотя это штатное состояние пула (184 строки/сутки на +# проде), а не инцидент приложения. Тесты ниже бьют по РЕАЛЬНОМУ `_probe_proxy` +# (не монки-заглушке, как в тестах `run_proxy_healthcheck` выше) — только так +# видно, что осталось от логирования при живом httpx-исключении. + + +async def test_probe_proxy_proxy_error_logs_single_line_without_traceback( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + async def _broken_get(self: httpx.AsyncClient, *args: Any, **kwargs: Any) -> httpx.Response: + raise httpx.ProxyError("407 Proxy Authentication Required") + + monkeypatch.setattr(httpx.AsyncClient, "get", _broken_get) + + with caplog.at_level("WARNING", logger="app.services.proxy_pool"): + ok, exit_ip, latency_ms, fail_kind = await proxy_pool._probe_proxy( + "http://u:p@h1:8080" + ) + + assert ok is False + assert exit_ip is None + assert latency_ms is None + assert fail_kind == "proxy_error" + + records = [r for r in caplog.records if r.name == "app.services.proxy_pool"] + assert len(records) == 1, "провал ipify-пробы обязан лечь ОДНОЙ строкой, не пачкой" + record = records[0] + assert record.exc_info is None, "проба — штатная операция, полный traceback не нужен" + assert "proxy_error" in record.message + assert "407" in record.message # причина (текст исключения) видна без трейса + + +async def test_healthcheck_counts_proxy_error_as_failed_without_traceback( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Итоговая строка `checked=.../ok=.../failed=...` не ломается провалом + вида ProxyError, а сам провал не тащит traceback в лог прогона.""" + db = FakeSession([_proxy(1, fails=0)]) + + async def _broken_get(self: httpx.AsyncClient, *args: Any, **kwargs: Any) -> httpx.Response: + raise httpx.ProxyError("407 Proxy Authentication Required") + + monkeypatch.setattr(httpx.AsyncClient, "get", _broken_get) + + # INFO (не WARNING): итоговая сводка `healthcheck done` логируется на INFO — + # порог ниже WARNING нужен, чтобы её тоже поймать в этом же прогоне. + with caplog.at_level("INFO", logger="app.services.proxy_pool"): + counters = await proxy_pool.run_proxy_healthcheck(db) # type: ignore[arg-type] + + assert counters["checked"] == 1 + assert counters["ok"] == 0 + assert counters["failed"] == 1 + assert db._by_id(1)["consecutive_fails"] == 1 + + summary = [ + r + for r in caplog.records + if r.name == "app.services.proxy_pool" and "healthcheck done" in r.message + ] + assert len(summary) == 1 + assert "checked=1" in summary[0].message + assert "ok=0" in summary[0].message + assert "failed=1" in summary[0].message + assert not any(r.exc_info for r in caplog.records if r.name == "app.services.proxy_pool") diff --git a/tradein-mvp/backend/tests/services/tgbot/test_bridge.py b/tradein-mvp/backend/tests/services/tgbot/test_bridge.py index 312e8cb5..bb42dd49 100644 --- a/tradein-mvp/backend/tests/services/tgbot/test_bridge.py +++ b/tradein-mvp/backend/tests/services/tgbot/test_bridge.py @@ -80,6 +80,10 @@ class FakeBridgeStorage: self._next_id = 1 self.clock_s: float = 0.0 self.fail_next_record_message = False + # #3471 P0: следующий вызов record_web_out_message кидает SQLAlchemyError + # (симулирует обрыв коннекта к БД на веб-ветке — для веб-треда эта запись + # И ЕСТЬ доставка клиенту) и сбрасывается в False. + self.fail_next_record_web_out_message = False # #tgsupport-web: web_support_messages-эквивалент, topic_message_id -> # (thread_id, support_chat_id) — второй элемент моделирует колонку # web_support_messages.support_chat_id (review M1); None = легаси wildcard. @@ -158,9 +162,11 @@ class FakeBridgeStorage: def find_chat_by_topic_message(self, topic_message_id: int, support_chat_id: int) -> int | None: """support_chat_id-скоуп (review M1): запись со ЧУЖИМ (не None, не текущим) - support_chat_id не матчится — None (легаси/дефолт) матчится всегда.""" + support_chat_id не матчится — None (легаси/дефолт) матчится всегда. + БЕЗ фильтра по direction (#3471 P0) — реплай на СВОЙ предыдущий ответ + (direction='out') резолвится так же, как реплай на зеркало клиента.""" for m in reversed(self.messages): - if m["direction"] != "in" or m["topic_message_id"] != topic_message_id: + if m["topic_message_id"] != topic_message_id: continue entry_chat_id = m.get("support_chat_id") if entry_chat_id is not None and entry_chat_id != support_chat_id: @@ -185,15 +191,32 @@ class FakeBridgeStorage: return thread_id def record_web_out_message( - self, *, thread_id: int, text_body: str, operator_tg_id: int | None + self, + *, + thread_id: int, + text_body: str, + operator_tg_id: int | None, + topic_message_id: int | None = None, + support_chat_id: int | None = None, ) -> None: + if self.fail_next_record_web_out_message: + self.fail_next_record_web_out_message = False + raise SQLAlchemyError("simulated DB failure (deploy connection reset)") self.web_out_messages.append( { "thread_id": thread_id, "text_body": text_body, "operator_tg_id": operator_tg_id, + "topic_message_id": topic_message_id, + "support_chat_id": support_chat_id, } ) + # Зеркалим в `web_topic_to_thread` (deep review PR #3479) — реальный + # `record_outbound` пишет ту же строку в web_support_messages, которую + # потом читает `find_thread_by_topic_message`; без этого фейк не мог бы + # поймать баг "out-строка с support_chat_id=NULL — вечный wildcard". + if topic_message_id is not None: + self.web_topic_to_thread[topic_message_id] = (thread_id, support_chat_id) # ── httpx mocking helpers (mirrors tests/services/test_dadata.py) ─────────── @@ -855,6 +878,146 @@ async def test_group_reply_refuses_delivery_when_both_tg_and_web_match( assert storage.get_offset() == 62 +async def test_group_reply_to_web_mirror_db_failure_notifies_operator_and_advances_offset() -> None: + """#3471 P0: сбой БД на `record_web_out_message` — для веб-треда эта запись И + ЕСТЬ доставка клиенту (веб-фронт читает её polling'ом), поэтому тихий откат + означал бы навсегда потерянный ответ оператора (воспроизведено на проде + 31.08.2026 — клиент kopylov). Теперь: rollback → уведомление оператору + реплаем в топик, что ответ НЕ доставлен → offset всё равно сдвигается (та же + политика, что у любого другого `SQLAlchemyError` в `process_update` — сбой + БД не переигрывается, human-in-the-loop retry заменяет технический) → + исключение наружу НЕ улетает.""" + calls: list[tuple[str, dict[str, Any]]] = [] + client = _make_client({}, calls) + storage = FakeBridgeStorage() + storage.web_topic_to_thread[310] = (50, SUPPORT_CHAT_ID) + storage.fail_next_record_web_out_message = True + + update = { + "update_id": 70, + "message": _group_reply_message(reply_to_message_id=310, message_id=210), + } + await bridge.process_update(update, client, storage) + + # Ответ НЕ попал в web_out_messages (запись упала), но и не потерян молча. + assert storage.web_out_messages == [] + assert storage.rollbacks == 1 + methods = [m for m, _ in calls] + assert methods == ["sendMessage"] + notice = calls[0][1] + assert notice["chat_id"] == SUPPORT_CHAT_ID + assert notice["reply_to_message_id"] == 210 + assert "НЕ доставлен" in notice["text"] + # Сбой БД не переигрывается (общая политика SQLAlchemyError) — offset сдвинут и закоммичен. + assert storage.get_offset() == 70 + assert storage.commits == 1 + + +async def test_group_reply_to_web_mirror_db_failure_and_notify_failure_logs_error( + caplog: pytest.LogCaptureFixture, +) -> None: + """#3471 P0: сбой БД НА веб-ответе, а следом ещё и уведомление оператору не + ушло (Telegram недоступен) — полная тишина по обоим каналам. `process_update` + всё равно не падает и offset сдвигает, но остаётся `logger.error` с + идентификаторами (thread_id/message_id), НЕ текстом — по нему человек найдёт + ответ оператора в топике вручную.""" + calls: list[tuple[str, dict[str, Any]]] = [] + # 403 (не 5xx) — не ретраится клиентом, `_notify_topic` падает быстро и + # детерминированно (без реальных retry-пауз). + client = _make_client({"sendMessage": 403}, calls) + + storage = FakeBridgeStorage() + storage.web_topic_to_thread[311] = (51, SUPPORT_CHAT_ID) + storage.fail_next_record_web_out_message = True + + update = { + "update_id": 71, + "message": _group_reply_message(reply_to_message_id=311, message_id=211), + } + with caplog.at_level(logging.ERROR, logger="app.services.tgbot.bridge"): + await bridge.process_update(update, client, storage) + + assert storage.web_out_messages == [] + assert "потерян молча" in caplog.text + assert "51" in caplog.text # thread_id узнаваем в логе + assert storage.get_offset() == 71 # сбой БД не переигрывается — offset сдвинут + assert storage.commits == 1 + + +async def test_group_reply_to_own_previous_web_reply_resolves_thread() -> None: + """#3471 P0 (пункт 3): реплай оператора на СВОЙ предыдущий веб-ответ (не на + исходное зеркало клиента) теперь тоже резолвится — `record_outbound` + сохраняет topic_message_id исходящей записи, `find_thread_by_topic_message` + больше не фильтрует по direction. + + Deep review PR #3479: новая out-строка ОБЯЗАНА писаться с ТЕКУЩИМ + `support_chat_id`, а не NULL — NULL матчится `find_thread_by_topic_message` + как лениентный wildcard "любой чат" (легаси до 187/188), т.е. NULL сделал бы + КАЖДУЮ out-строку вечным wildcard-совпадением при ротации support-группы. + Эта проверка падает на дефектной реализации (support_chat_id не передавался + в `record_web_out_message`), даже когда resolve выше внешне "работает".""" + calls: list[tuple[str, dict[str, Any]]] = [] + client = _make_client({}, calls) + storage = FakeBridgeStorage() + # Симулируем уже сохранённый предыдущий ответ оператора (topic_message_id=320) + # так, как это сделал бы реальный record_outbound после фикса. + storage.web_topic_to_thread[320] = (52, SUPPORT_CHAT_ID) + + update = { + "update_id": 72, + "message": _group_reply_message( + reply_to_message_id=320, message_id=220, text="Продолжение" + ), + } + await bridge.process_update(update, client, storage) + + assert len(storage.web_out_messages) == 1 + assert storage.web_out_messages[0]["thread_id"] == 52 + assert storage.web_out_messages[0]["topic_message_id"] == 220 + # Deep review PR #3479: НЕ NULL — иначе эта строка стала бы вечным wildcard. + assert storage.web_out_messages[0]["support_chat_id"] == SUPPORT_CHAT_ID + + +async def test_group_reply_to_own_previous_tg_reply_resolves_target_chat() -> None: + """#3471 P0 (пункт 3): та же история для Telegram-пути — реплай оператора на + СВОЙ предыдущий ответ клиенту (direction='out', topic_message_id теперь + заполнен) резолвится в chat_id, а не проваливается в orphan-check. + + Deep review PR #3479: новая out-строка ОБЯЗАНА писаться с ТЕКУЩИМ + `support_chat_id` (симметрично in-ветке) — иначе она стала бы вечным + wildcard в `find_chat_by_topic_message` при ротации support-группы, и + reply мог бы увести ответ ЧУЖОМУ клиенту.""" + calls: list[tuple[str, dict[str, Any]]] = [] + client = _make_client({"copyMessage": {"message_id": 601}}, calls) + storage = FakeBridgeStorage() + # Предыдущий ответ оператора клиенту, зафиксированный с topic_message_id + # (id этого ответа В ТОПИКЕ) — то, что теперь пишет TG-путь `_handle_group_reply`. + storage.record_message( + chat_id=555, + direction="out", + tg_message_id=500, + topic_message_id=330, + kind="text", + text_body="Первый ответ оператора", + operator_tg_id=777, + support_chat_id=SUPPORT_CHAT_ID, + ) + + update = { + "update_id": 73, + "message": _group_reply_message(reply_to_message_id=330, message_id=230, text="Уточнение"), + } + await bridge.process_update(update, client, storage) + + methods = [m for m, _ in calls] + assert methods == ["copyMessage"] + assert calls[0][1]["chat_id"] == 555 + assert len(storage.messages) == 2 # исходный 'out' + новый 'out' + assert storage.messages[-1]["topic_message_id"] == 230 + # Deep review PR #3479: НЕ NULL — иначе эта строка стала бы вечным wildcard. + assert storage.messages[-1]["support_chat_id"] == SUPPORT_CHAT_ID + + # ── C) дедуп ────────────────────────────────────────────────────────────────── diff --git a/tradein-mvp/backend/tests/test_glitchtip_alert_retry.py b/tradein-mvp/backend/tests/test_glitchtip_alert_retry.py new file mode 100644 index 00000000..48b3fe3e --- /dev/null +++ b/tradein-mvp/backend/tests/test_glitchtip_alert_retry.py @@ -0,0 +1,150 @@ +"""Тесты фоновой пересылки GlitchTip-алерта в Telegram после отказа синхронной +попытки — app/api/v1/glitchtip.py (BackgroundTasks) + app/tasks/glitchtip_alert_retry.py. + +Контекст (#3471, #3157): GlitchTip вебхуки не ретраит, поэтому отказ синхронной +попытки не должен терять текст алерта. 502 на отказ Telegram остаётся как есть +(#3456) — проверяем, что он остаётся ОДНОВРЕМЕННО с постановкой фоновой доставки. + +NEVER touches real DB / real Telegram API. +""" + +from __future__ import annotations + +import os + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +import asyncio +from typing import Any, ClassVar + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.api.v1 import glitchtip as glitchtip_module +from app.services.tgbot.client import TelegramNetworkError +from app.tasks import glitchtip_alert_retry as retry_module + +_SECRET = "test-shared-secret" +_ENDPOINT = "/api/v1/trade-in/ops/glitchtip-webhook" + +_ISSUE_PAYLOAD = { + "text": "GlitchTip Alert", + "attachments": [{"title": "ValueError: something broke", "text": "app/services/foo.py"}], +} + + +@pytest.fixture(autouse=True) +def _configured(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(glitchtip_module.settings, "tradein_internal_auth_secret", _SECRET) + monkeypatch.setattr(glitchtip_module.settings, "telegram_bot_token", "fake-token") + monkeypatch.setattr(glitchtip_module.settings, "telegram_alerts_chat_id", -1004443088679) + monkeypatch.setattr(glitchtip_module.settings, "telegram_alerts_topic_id", 158) + + +class _FakeTelegramClient: + """Подменяет `TelegramClient` внутри модуля `glitchtip` — никакого httpx/сети. + + `responses` — очередь: каждый вызов `send_message` берёт следующий элемент + (dict = успех, Exception = отказ), позволяя смоделировать «первая попытка не + удалась, повторная фоном прошла». + """ + + calls: ClassVar[list[dict[str, Any]]] = [] + responses: ClassVar[list[dict[str, Any] | Exception]] = [] + + def __init__(self, _token: str = "fake-token") -> None: + pass + + async def send_message(self, **kwargs: Any) -> dict[str, Any]: + _FakeTelegramClient.calls.append(kwargs) + outcome = _FakeTelegramClient.responses.pop(0) + if isinstance(outcome, Exception): + raise outcome + return outcome + + +@pytest.fixture(autouse=True) +def _fake_telegram_client(monkeypatch: pytest.MonkeyPatch) -> Any: + _FakeTelegramClient.calls = [] + _FakeTelegramClient.responses = [{"message_id": 1}] + monkeypatch.setattr(glitchtip_module, "get_telegram_client", lambda: _FakeTelegramClient()) + # Ретрай-модуль спит между попытками (_RETRY_DELAY_S=30s) — в тестах не ждём. + monkeypatch.setattr(retry_module, "_RETRY_DELAY_S", 0.0) + return _FakeTelegramClient + + +@pytest.fixture +def client() -> TestClient: + app = FastAPI() + app.include_router(glitchtip_module.router, prefix="/api/v1/trade-in") + return TestClient(app) + + +def _network_error() -> TelegramNetworkError: + return TelegramNetworkError("sendMessage", "ConnectTimeout", 4) + + +# ── отказ синхронной попытки → фон + 502 ──────────────────────────────────── + + +def test_sync_failure_queues_background_retry_and_still_returns_502( + client: TestClient, _fake_telegram_client: Any +) -> None: + """Синхронная попытка не удалась → задача уходит в фон, ответ ОСТАЁТСЯ 502 + (#3456 — 502 задуман осознанно, не подменяется молчаливым 200).""" + _fake_telegram_client.responses = [_network_error(), {"message_id": 2}] + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 502 + # TestClient прогоняет BackgroundTasks синхронно перед возвратом ответа — + # к этому моменту фоновая попытка уже отработала: 2 вызова (sync + retry). + assert len(_fake_telegram_client.calls) == 2 + for call in _fake_telegram_client.calls: + assert call["chat_id"] == -1004443088679 + assert call["message_thread_id"] == 158 + assert "ValueError: something broke" in call["text"] + # Переиспользован тот же уже отформатированный текст — не пересобран заново. + assert _fake_telegram_client.calls[0]["text"] == _fake_telegram_client.calls[1]["text"] + + +def test_sync_success_does_not_queue_background_retry( + client: TestClient, _fake_telegram_client: Any +) -> None: + """Успешная синхронная отправка НЕ ставит фоновую задачу — ровно один вызов.""" + _fake_telegram_client.responses = [{"message_id": 1}] + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 200, r.text + assert len(_fake_telegram_client.calls) == 1 + + +# ── retry_forward_alert напрямую: потолок ретраев ─────────────────────────── + + +async def _run_retry(fake_client_cls: Any, responses: list[Any]) -> None: + fake_client_cls.responses = list(responses) + await retry_module.retry_forward_alert( + fake_client_cls(), chat_id=-1, text="алерт", message_thread_id=158 + ) + + +def test_retry_succeeds_after_transient_failure(_fake_telegram_client: Any) -> None: + asyncio.run(_run_retry(_fake_telegram_client, [_network_error(), {"message_id": 9}])) + + assert len(_fake_telegram_client.calls) == 2 + + +def test_retry_gives_up_after_max_attempts_and_logs( + _fake_telegram_client: Any, caplog: pytest.LogCaptureFixture +) -> None: + """На потолке ретраев — сдаётся с ERROR-логом, а не молча и не бесконечно.""" + responses = [_network_error() for _ in range(retry_module._MAX_ATTEMPTS)] + + with caplog.at_level("ERROR", logger=retry_module.logger.name): + asyncio.run(_run_retry(_fake_telegram_client, responses)) + + assert len(_fake_telegram_client.calls) == retry_module._MAX_ATTEMPTS + assert any("не удалось доставить" in rec.message for rec in caplog.records) diff --git a/tradein-mvp/backend/tests/test_sentry_scrub.py b/tradein-mvp/backend/tests/test_sentry_scrub.py index 4925c48d..7abfb4ca 100644 --- a/tradein-mvp/backend/tests/test_sentry_scrub.py +++ b/tradein-mvp/backend/tests/test_sentry_scrub.py @@ -13,6 +13,7 @@ import pytest os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") from app.observability.sentry_scrub import ( + drop_payments_disabled_event, redact_telegram_bot_token, scrub_payment_request_body, scrub_pii_event, @@ -627,3 +628,63 @@ def test_scrub_pii_event_httpx_url_query_stabilization_leaves_unrelated_text_unt out = scrub_pii_event(event, {}) assert out is not None assert out["extra"]["note"] == benign + + +# ── payments-disabled drop (issue #3471, GlitchTip-группа TRADE-IN-3GG) ───── +# 503 из `payments.py._require_enabled` — штатный ответ выключенного +# kill-switch'а, а не инцидент: sentry_sdk `StarletteIntegration` репортит его +# как error-событие только потому, что 503 попадает в дефолтный диапазон +# `failed_request_status_codes` (5xx), хотя FastAPI обработал исключение +# штатно. 167 событий/2 недели от внутреннего IP смоук-проверки (кнопки оплаты +# во фронте нет) размывали ленту. Фильтр не должен трогать поведение самой +# ручки (503 остаётся) и не должен глотать другие ошибки — включая другие 503. + +from fastapi import HTTPException # noqa: E402 + + +def _exc_info_hint(exc: BaseException) -> dict: + """Та же форма hint, что sentry_sdk реально передаёт в before_send — + `exc_info = (type, value, traceback)` (см. `_hint_for` выше по файлу).""" + return {"exc_info": (type(exc), exc, exc.__traceback__)} + + +def test_drop_payments_disabled_event_drops_the_503() -> None: + exc = HTTPException(status_code=503, detail="payments are disabled") + event = {"level": "error", "exception": {"values": [{"type": "HTTPException"}]}} + assert drop_payments_disabled_event(event, _exc_info_hint(exc)) is None + + +def test_drop_payments_disabled_event_leaves_other_errors_untouched() -> None: + """Обычная ошибка (не платёжный kill-switch) должна долетать до GlitchTip + без изменений — фильтр специфичен по (status_code, detail), а не по 5xx.""" + exc = ValueError("boom") + event = {"level": "error"} + out = drop_payments_disabled_event(event, _exc_info_hint(exc)) + assert out is event + + +def test_drop_payments_disabled_event_leaves_other_503s_untouched() -> None: + """Другой 503 с другим текстом (напр. будущий maintenance-режим другого + роутера) не должен ложно схлопнуться с платёжным kill-switch'ем.""" + exc = HTTPException(status_code=503, detail="service temporarily unavailable") + event = {"level": "error"} + out = drop_payments_disabled_event(event, _exc_info_hint(exc)) + assert out is event + + +def test_drop_payments_disabled_event_leaves_matching_detail_wrong_status_untouched() -> None: + """Тот же текст detail, но другой status_code — не платёжный kill-switch.""" + exc = HTTPException(status_code=500, detail="payments are disabled") + event = {"level": "error"} + out = drop_payments_disabled_event(event, _exc_info_hint(exc)) + assert out is event + + +def test_drop_payments_disabled_event_no_exc_info_untouched() -> None: + event = {"level": "error"} + out = drop_payments_disabled_event(event, {}) + assert out is event + + +def test_drop_payments_disabled_event_handles_non_dict_event() -> None: + assert drop_payments_disabled_event(None, {}) is None # type: ignore[arg-type]