Merge pull request 'fix(observability): убрать 83% мусора из трекера ошибок' (#2906) from fix/tradein-glitchtip-noise into main
All checks were successful
Deploy / changes (push) Successful in 10s
Deploy Trade-In / changes (push) Successful in 12s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy / build-backend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy / build-worker (push) Has been skipped
Deploy / build-frontend (push) Has been skipped
Deploy / deploy (push) Successful in 1m7s
Deploy / deploy-status (push) Successful in 1s
Deploy Trade-In / test (push) Successful in 3m41s
Deploy Trade-In / build-backend (push) Successful in 1m17s
Deploy Trade-In / deploy (push) Successful in 6m53s
Deploy Trade-In / deploy-status (push) Successful in 1s
All checks were successful
Deploy / changes (push) Successful in 10s
Deploy Trade-In / changes (push) Successful in 12s
Deploy Trade-In / build-frontend (push) Has been skipped
Deploy / build-backend (push) Has been skipped
Deploy Trade-In / build-browser (push) Has been skipped
Deploy / build-worker (push) Has been skipped
Deploy / build-frontend (push) Has been skipped
Deploy / deploy (push) Successful in 1m7s
Deploy / deploy-status (push) Successful in 1s
Deploy Trade-In / test (push) Successful in 3m41s
Deploy Trade-In / build-backend (push) Successful in 1m17s
Deploy Trade-In / deploy (push) Successful in 6m53s
Deploy Trade-In / deploy-status (push) Successful in 1s
This commit is contained in:
commit
7def4973bd
7 changed files with 503 additions and 31 deletions
|
|
@ -8,6 +8,13 @@ Persistent offset в /state/offset.json — не дублируем при resta
|
|||
Throttle: при >10 401 events за 60s — однократный digest event
|
||||
(чтобы не флудить GlitchTip storm'ом); индивидуальные events во время storm пропускаются.
|
||||
|
||||
before_send=_drop_basic_auth_noise (glitchtip-noise фикс): все события отсюда
|
||||
дропаются перед отправкой в GlitchTip — 401 от неаутентифицированного запроса
|
||||
не ошибка сервиса, это боты сканируют закрытый basic_auth'ом сайт. Раньше это
|
||||
был крупнейший источник шума в трекере (3 738 issue). Скрипт по-прежнему тэйлит
|
||||
лог и печатает `[forwarder] 401 event sent: ...` в stdout (docker logs) — просто
|
||||
больше не шлёт эти события в issue-трекер. Смотри `_drop_basic_auth_noise` docstring.
|
||||
|
||||
Реальный Caddy JSON access log (v2) структура:
|
||||
{
|
||||
"level": "info",
|
||||
|
|
@ -73,6 +80,41 @@ _shutdown = False
|
|||
_last_exc_sent: float = 0.0
|
||||
_EXC_THROTTLE_S: float = 300.0
|
||||
|
||||
# event_type-теги, которыми emit_event/emit_digest помечают КАЖДОЕ отправляемое
|
||||
# событие (см. scope.set_tag("event_type", ...) ниже) — используются как ключ
|
||||
# для before_send-фильтра.
|
||||
_BASIC_AUTH_EVENT_TYPES = frozenset({"basic_auth_failed", "basic_auth_storm"})
|
||||
|
||||
|
||||
def _drop_basic_auth_noise(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg]
|
||||
"""before_send-фильтр: 401 неаутентифицированного basic_auth-запроса — НЕ
|
||||
ошибка сервиса, а expected-поведение сканеров-ботов, ломящихся в закрытый
|
||||
basic_auth'ом gendsgn.ru (`GET /wp-admin/install.php` и подобное). До этого
|
||||
фикса emit_event/emit_digest слали КАЖДЫЙ такой 401 individual-событием (или
|
||||
storm-digest) в GlitchTip — remote_ip в message/тегах раздувал кардинальность
|
||||
(3 738 issue, 2 019 различных заголовков, топ — 222 события на «GET
|
||||
/wp-admin/install.p…»), топя содержательные алерты (OperationalError, sweep
|
||||
failures) в шуме сканеров.
|
||||
|
||||
Дропаем НА ИСТОЧНИКЕ (before_send), не постфактум-чисткой issue-трекера —
|
||||
так шум не появляется вообще, а не изредка удаляется руками. Фильтруем по
|
||||
тегу `event_type`, который ставят ТОЛЬКО emit_event/emit_digest — необработанные
|
||||
исключения самого форвардера (`capture_exception` в конце `main()`, реальный
|
||||
баг скрипта) этот тег не несут и проходят фильтр как есть (см. `except
|
||||
Exception` ниже в `main()`).
|
||||
"""
|
||||
tags = event.get("tags")
|
||||
event_type = None
|
||||
if isinstance(tags, dict):
|
||||
event_type = tags.get("event_type")
|
||||
elif isinstance(tags, list):
|
||||
# sentry_sdk в некоторых версиях сериализует tags как list[tuple[str, str]]
|
||||
# вместо dict — на всякий случай поддерживаем обе формы.
|
||||
event_type = dict(tags).get("event_type") if tags else None
|
||||
if event_type in _BASIC_AUTH_EVENT_TYPES:
|
||||
return None
|
||||
return event
|
||||
|
||||
|
||||
def _signal_handler(signum: int, frame: object) -> None:
|
||||
global _shutdown
|
||||
|
|
@ -221,6 +263,7 @@ def main() -> None:
|
|||
traces_sample_rate=0.0,
|
||||
attach_stacktrace=False,
|
||||
send_default_pii=False,
|
||||
before_send=_drop_basic_auth_noise,
|
||||
# Отключаем интеграции которые не нужны тонкому sidecar
|
||||
default_integrations=False,
|
||||
)
|
||||
|
|
|
|||
77
ops/glitchtip-auth-forwarder/test_forwarder.py
Normal file
77
ops/glitchtip-auth-forwarder/test_forwarder.py
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
"""Тесты для `_drop_basic_auth_noise` (before_send-фильтр, glitchtip-noise).
|
||||
|
||||
Раньше форвардер слал КАЖДЫЙ basic_auth 401 (сканеры-боты, ломящиеся в закрытый
|
||||
basic_auth'ом gendsgn.ru) individual-событием в GlitchTip — 3 738 issue, 2 019
|
||||
различных заголовков (remote_ip раздувал кардинальность), топя содержательный
|
||||
сигнал. `_drop_basic_auth_noise` дропает эти события НА ИСТОЧНИКЕ (before_send),
|
||||
но НЕ должен трогать unhandled-ошибки самого форвардера (реальный баг скрипта —
|
||||
`capture_exception` без `event_type`-тега, аналог "500 должен пройти").
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
# DSN обязателен на module-level (`os.environ["GLITCHTIP_DSN"]`, fail-fast) — задаём
|
||||
# ДО импорта forwarder.py, иначе импорт падает KeyError.
|
||||
os.environ.setdefault("GLITCHTIP_DSN", "http://test@localhost/1")
|
||||
|
||||
from forwarder import _BASIC_AUTH_EVENT_TYPES, _drop_basic_auth_noise
|
||||
|
||||
|
||||
def test_drops_individual_basic_auth_401() -> None:
|
||||
"""emit_event() тегирует event_type=basic_auth_failed — 401 от бота-сканера,
|
||||
не ошибка сервиса, должен быть отброшен (return None)."""
|
||||
event = {
|
||||
"tags": {"event_type": "basic_auth_failed", "remote_ip": "95.165.147.218"},
|
||||
"message": "basic_auth 401 — GET /wp-admin/install.php from 95.165.147.218",
|
||||
}
|
||||
assert _drop_basic_auth_noise(event, {}) is None
|
||||
|
||||
|
||||
def test_drops_basic_auth_storm_digest() -> None:
|
||||
"""emit_digest() тегирует event_type=basic_auth_storm — тоже 401-класс, тоже
|
||||
не ошибка сервиса, дропаем."""
|
||||
event = {
|
||||
"tags": {"event_type": "basic_auth_storm"},
|
||||
"message": "basic_auth storm — 15 failed attempts in 60s",
|
||||
}
|
||||
assert _drop_basic_auth_noise(event, {}) is None
|
||||
|
||||
|
||||
def test_drops_when_tags_serialized_as_list_of_tuples() -> None:
|
||||
"""Некоторые версии sentry_sdk сериализуют tags как list[tuple[str, str]]
|
||||
вместо dict — фильтр обязан поддерживать обе формы."""
|
||||
event = {"tags": [("event_type", "basic_auth_failed")]}
|
||||
assert _drop_basic_auth_noise(event, {}) is None
|
||||
|
||||
|
||||
def test_passes_through_forwarder_own_crash() -> None:
|
||||
"""500-аналог: unhandled exception самого форвардера (capture_exception в
|
||||
конце main(), реальный баг скрипта — напр. PermissionError на STATE_FILE) не
|
||||
несёт event_type-тег → должен пройти НЕТРОНУТЫМ, не быть молча проглоченным
|
||||
вместе с ботовым шумом."""
|
||||
event = {
|
||||
"level": "error",
|
||||
"exception": {"values": [{"type": "PermissionError", "value": "denied"}]},
|
||||
}
|
||||
out = _drop_basic_auth_noise(dict(event), {})
|
||||
assert out == event
|
||||
|
||||
|
||||
def test_passes_through_event_without_tags() -> None:
|
||||
event: dict = {"message": "something unrelated"}
|
||||
out = _drop_basic_auth_noise(dict(event), {})
|
||||
assert out == event
|
||||
|
||||
|
||||
def test_passes_through_unrelated_tag_value() -> None:
|
||||
event = {"tags": {"event_type": "something_else"}}
|
||||
out = _drop_basic_auth_noise(dict(event), {})
|
||||
assert out == event
|
||||
|
||||
|
||||
def test_basic_auth_event_types_are_exactly_the_two_emitters_use() -> None:
|
||||
"""Явная фиксация словаря — emit_event → basic_auth_failed,
|
||||
emit_digest → basic_auth_storm (см. forwarder.py)."""
|
||||
assert _BASIC_AUTH_EVENT_TYPES == frozenset({"basic_auth_failed", "basic_auth_storm"})
|
||||
|
|
@ -66,17 +66,28 @@ logging.getLogger("httpx").setLevel(logging.WARNING)
|
|||
# worker (in-app scheduler зовёт task-функции напрямую; compose = postgres/backend/
|
||||
# frontend), отдельного broker нет → мониторить нечего.
|
||||
if settings.glitchtip_dsn:
|
||||
from app.observability.sentry_scrub import redact_telegram_bot_token
|
||||
from app.observability.sentry_scrub import (
|
||||
redact_telegram_bot_token,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
||||
def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None:
|
||||
"""Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) —
|
||||
см. app/tgbot_main.py._before_send (идентичная композиция, тот же риск:
|
||||
теперь этот процесс тоже держит TelegramClient в стек-фреймах при ошибке
|
||||
sendMessage, а include_local_variables=False ниже — первый рубеж защиты)."""
|
||||
"""Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) +
|
||||
RetryError fingerprint-стабилизация (glitchtip-noise) — см.
|
||||
app/tgbot_main.py._before_send (та же композиция без последнего шага,
|
||||
тот бот geocoder не зовёт). PII/token — тот же риск: теперь этот процесс
|
||||
тоже держит TelegramClient в стек-фреймах при ошибке sendMessage, а
|
||||
include_local_variables=False ниже — первый рубеж защиты. RetryError —
|
||||
этот процесс обслуживает /api/v1/geocode/* (suggest/lookup/reverse),
|
||||
которые ретраят Nominatim через tenacity; см.
|
||||
sentry_scrub.stabilize_retry_error_fingerprint."""
|
||||
scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type]
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type,return-value]
|
||||
detokened = redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type]
|
||||
if detokened is None:
|
||||
return None
|
||||
return stabilize_retry_error_fingerprint(detokened, hint) # type: ignore[arg-type,return-value]
|
||||
|
||||
sentry_sdk.init(
|
||||
dsn=settings.glitchtip_dsn,
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import re
|
|||
from typing import Any
|
||||
|
||||
from sentry_sdk.types import Event
|
||||
from tenacity import RetryError
|
||||
|
||||
_REDACTED = "[REDACTED]"
|
||||
# Ключи consumer-PII (нижний регистр; сверка case-insensitive).
|
||||
|
|
@ -76,6 +77,31 @@ _URL_SECRET_QUERY_RE = re.compile(
|
|||
)
|
||||
_URL_SECRET_QUERY_REPLACEMENT = r"\g<1>" + _REDACTED
|
||||
|
||||
# httpx error-message URL query stabilization (GlitchTip-noise review round 2,
|
||||
# claim #1). `httpx.HTTPStatusError.__str__()` (raised by `response.raise_for_status()`)
|
||||
# bakes the FULL request URL — INCLUDING query string — into the exception message:
|
||||
# "Client error '403 Forbidden' for url 'https://nominatim.openstreetmap.org/
|
||||
# search?q=<адрес>&format=json&limit=3'" (воспроизведено эмпирически: httpx.Response
|
||||
# с params={"q": "<адрес>"} → raise_for_status() → именно этот текст). После
|
||||
# app/services/geocoder.py `reraise=True` (стабилизирует ТИП исключения — RetryError
|
||||
# → httpx.HTTPStatusError, см. комментарий у `_nominatim_lookup`) ИМЕННО этот текст
|
||||
# становится GlitchTip title/value каждого события. `q=<адрес>` — переменная часть
|
||||
# на КАЖДЫЙ вызов (ночной `geocode_missing_listings` — сотни разных адресов за
|
||||
# прогон), значит per-address issue-explosion не устранён `reraise=True`, а просто
|
||||
# переехал с RetryError на HTTPStatusError (тот же механизм: GlitchTip группирует по
|
||||
# нестабильному тексту сообщения — это же подтверждают исходные 2 462 RetryError-issue,
|
||||
# невозможные при группировке чисто по stacktrace/culprit).
|
||||
#
|
||||
# Отдельная регулярка от `_URL_SECRET_QUERY_RE` намеренно: та бьёт по ИМЕНИ известных
|
||||
# secret-параметров (security-редактор), здесь — ЛЮБОЙ query string в httpx-стиле
|
||||
# сообщении "for url '...'" (grouping-стабильность, не секретность — `q` не секрет).
|
||||
# Режем query целиком (не только конкретные параметры) — host+path остаются
|
||||
# стабильными для группировки, "for url '...'" — единственная форма, которую бьёт
|
||||
# regex (не трогает произвольные строки с `?`, см. тест
|
||||
# test_scrub_pii_event_httpx_url_query_stabilization_leaves_unrelated_text_untouched).
|
||||
_HTTPX_ERROR_URL_QUERY_RE = re.compile(r"(for url '[^'?]*)\?[^']*(')")
|
||||
_HTTPX_ERROR_URL_QUERY_REPLACEMENT = r"\g<1>?" + _REDACTED + r"\g<2>"
|
||||
|
||||
|
||||
def _scrub(obj: Any) -> None:
|
||||
"""Рекурсивно заменить значения PII-ключей в dict на [REDACTED] (in-place)."""
|
||||
|
|
@ -90,32 +116,34 @@ def _scrub(obj: Any) -> None:
|
|||
_scrub(item)
|
||||
|
||||
|
||||
def _redact_url_secrets_inplace(obj: Any) -> None:
|
||||
"""Рекурсивно (IN-PLACE, как `_scrub`) заменяет значения секрет-подобных
|
||||
query-параметров (`?token=...`, `?proxy_key=...` и т.п.) на [REDACTED] в
|
||||
КАЖДОЙ строке event — не ключ-based: секрет утекает через httpx span
|
||||
`url`/`query` data и через текст исключений (`str(exc)` httpx содержит полный
|
||||
request URL), а не только через известные PII-поля формы. Мутирует dict/list
|
||||
на месте (НЕ пересоздаёт структуру, в отличие от `_redact_strings`) —
|
||||
сохраняет identity верхнеуровневого `event`, на что опирается контракт
|
||||
`scrub_pii_event`/`before_send` и существующие тесты (`out is event`).
|
||||
def _regex_redact_inplace(obj: Any, pattern: re.Pattern[str], replacement: str) -> None:
|
||||
"""Рекурсивно (IN-PLACE, как `_scrub`) прогоняет `pattern.sub(replacement, ...)`
|
||||
по КАЖДОЙ строке event (не ключ-based) — общий обход, переиспользуемый и для
|
||||
URL-секретов (`_URL_SECRET_QUERY_RE`), и для стабилизации httpx error-message
|
||||
URL (`_HTTPX_ERROR_URL_QUERY_RE`): в обоих случаях переменные данные утекают
|
||||
через httpx span `url`/`query` data и через текст исключений (`str(exc)` httpx
|
||||
содержит полный request URL), а не только через известные PII-поля формы.
|
||||
Мутирует dict/list на месте (НЕ пересоздаёт структуру, в отличие от
|
||||
`_redact_strings`) — сохраняет identity верхнеуровневого `event`, на что
|
||||
опирается контракт `scrub_pii_event`/`before_send` и существующие тесты
|
||||
(`out is event`).
|
||||
"""
|
||||
if isinstance(obj, dict):
|
||||
for key, value in obj.items():
|
||||
if isinstance(value, str):
|
||||
redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value)
|
||||
redacted = pattern.sub(replacement, value)
|
||||
if redacted != value:
|
||||
obj[key] = redacted
|
||||
else:
|
||||
_redact_url_secrets_inplace(value)
|
||||
_regex_redact_inplace(value, pattern, replacement)
|
||||
elif isinstance(obj, list):
|
||||
for i, value in enumerate(obj):
|
||||
if isinstance(value, str):
|
||||
redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value)
|
||||
redacted = pattern.sub(replacement, value)
|
||||
if redacted != value:
|
||||
obj[i] = redacted
|
||||
else:
|
||||
_redact_url_secrets_inplace(value)
|
||||
_regex_redact_inplace(value, pattern, replacement)
|
||||
# tuple намеренно не обрабатываем: sentry_sdk event — это JSON-совместимая
|
||||
# структура (dict/list/str/int/...), tuple там не встречается, а даже если бы
|
||||
# встретился — он immutable, in-place правка невозможна (см. `_scrub`, тот же
|
||||
|
|
@ -123,16 +151,21 @@ def _redact_url_secrets_inplace(obj: Any) -> None:
|
|||
|
||||
|
||||
def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
||||
"""Redact consumer-PII + URL query-string секретов из error event перед отправкой.
|
||||
"""Redact consumer-PII + URL query-string секретов/nondeterministic-данных из
|
||||
error event перед отправкой.
|
||||
|
||||
Композиция (обе — in-place, сохраняют identity `event`): (1) ключ-based
|
||||
Композиция (все — in-place, сохраняют identity `event`): (1) ключ-based
|
||||
dict-scrub consumer-PII полей формы (как раньше), (2) full-text regex-проход
|
||||
по ВСЕМУ event, вырезающий значения секрет-подобных query-параметров в любой
|
||||
строке (proxy/API-ключи в исходящих URL сторонних сервисов, напр. mobileproxy
|
||||
changeip — #security-audit). Второй шаг не завязан на конкретные ключи полей —
|
||||
ловит секрет в frame locals, breadcrumb, exception message и т.д., где он может
|
||||
оказаться независимо от include_local_variables/traces_sample_rate. Возвращает
|
||||
event (не None).
|
||||
changeip — #security-audit), (3) full-text regex-проход, стабилизирующий httpx
|
||||
error-message URL (`for url '...?...'`) — убирает переменный query string
|
||||
(адрес геокодинга и т.п.), от которого GlitchTip group-title плодит issue на
|
||||
каждый вызов (GlitchTip-noise review round 2, claim #1; см. комментарий у
|
||||
`_HTTPX_ERROR_URL_QUERY_RE`). (2) и (3) не завязаны на конкретные ключи полей —
|
||||
ловят секрет/переменные данные в frame locals, breadcrumb, exception message
|
||||
и т.д., где они могут оказаться независимо от
|
||||
include_local_variables/traces_sample_rate. Возвращает event (не None).
|
||||
"""
|
||||
if not isinstance(event, dict):
|
||||
return event
|
||||
|
|
@ -141,7 +174,8 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None:
|
|||
_scrub(request.get("data"))
|
||||
_scrub(event.get("extra"))
|
||||
_scrub(event.get("contexts"))
|
||||
_redact_url_secrets_inplace(event)
|
||||
_regex_redact_inplace(event, _URL_SECRET_QUERY_RE, _URL_SECRET_QUERY_REPLACEMENT)
|
||||
_regex_redact_inplace(event, _HTTPX_ERROR_URL_QUERY_RE, _HTTPX_ERROR_URL_QUERY_REPLACEMENT)
|
||||
return event
|
||||
|
||||
|
||||
|
|
@ -174,3 +208,61 @@ def redact_telegram_bot_token(event: Event, _hint: dict[str, Any]) -> Event | No
|
|||
if not isinstance(event, dict):
|
||||
return event
|
||||
return _redact_strings(event) # type: ignore[return-value]
|
||||
|
||||
|
||||
# ── RetryError fingerprint stabilization (GlitchTip noise-reduction) ────────
|
||||
# tenacity.RetryError.__str__() тащит repr() последнего Future
|
||||
# (`RetryError[<Future at 0x7f... state=finished raised HTTPStatusError>]`) —
|
||||
# memory address объекта, случайный на каждый вызов процесса. Пока geocoder.py
|
||||
# ретраил Nominatim без `reraise=True`, каждое исчерпание ретраев (Nominatim
|
||||
# недоступен/rate-limit/403) улетало в GlitchTip как RetryError с этим
|
||||
# нестабильным текстом → одна и та же причина плодила отдельный issue на КАЖДОЕ
|
||||
# исчерпание (2 462 issue из 7 461 в трекере на момент фикса). `reraise=True`
|
||||
# в app/services/geocoder.py устраняет RetryError на этом пути (пробрасывает
|
||||
# реальное исключение) — но реальное исключение (httpx.HTTPStatusError) само
|
||||
# несёт нестабильный текст (URL с адресом в query), поэтому group-стабильность
|
||||
# для geocoder держит НЕ эта функция, а `_HTTPX_ERROR_URL_QUERY_RE` в
|
||||
# `scrub_pii_event` (см. её комментарий, GlitchTip-noise review round 2 claim #1).
|
||||
#
|
||||
# Функция ниже — belt-and-suspenders для ЛЮБОГО кода, который ретраит через
|
||||
# tenacity БЕЗ `reraise=True` (живой пример на момент фикса: `BaseScraper._http_get`
|
||||
# в packages/scraper-kit — retry-декоратор НЕ reraise'ит, сознательно оставлен на
|
||||
# этот фолбэк, а не на URL-стабилизацию: ретраятся listing detail URL БЕЗ query
|
||||
# string — переменная часть там в ПУТИ (offer id), которую `_HTTPX_ERROR_URL_QUERY_RE`
|
||||
# не покрывает; см. review round 2 claim #3). Схлопывает RetryError в ОДИН
|
||||
# persistent issue per (culprit, класс исключения-причины) — culprit обязателен:
|
||||
# БЕЗ него RetryError с одинаковым типом причины из НЕСВЯЗАННЫХ подсистем (напр.
|
||||
# geocoder и scraper_kit одновременно ретраят httpx и оба ловят HTTPStatusError)
|
||||
# схлопнулись бы в ОДИН issue — потеря сигнала хуже исходного шума (review round 2
|
||||
# claim #2). Источник culprit — `event["logger"]`: sentry_sdk `LoggingIntegration`
|
||||
# ставит его в имя logger'а (`logging.getLogger(__name__)`, напр.
|
||||
# "app.services.geocoder" vs "scraper_kit.providers.yandex.detail") на КАЖДОМ
|
||||
# `logger.exception(...)`/`logger.error(...)` — стабильно per-модуль, не зависит от
|
||||
# конкретного запроса. Остальная часть fingerprint собрана ТОЛЬКО из стабильных
|
||||
# данных — имя типа исключения-причины (небольшой фиксированный словарь вроде
|
||||
# "HTTPStatusError"/"ConnectTimeout") — НИКАКИХ переменных данных запроса (адрес,
|
||||
# IP, id объявления и т.п.), иначе проблема повторится в других терминах.
|
||||
def stabilize_retry_error_fingerprint(event: Event, hint: dict[str, Any]) -> Event | None:
|
||||
"""before_send-хук: схлопывает tenacity.RetryError в один persistent issue per
|
||||
(источник, тип причины) — РАЗНЫЕ источники (geocoder / scraper_kit / будущий
|
||||
retry-код) НЕ схлопываются друг с другом, даже если тип причины совпадает.
|
||||
|
||||
Определяет тип exception через `hint["exc_info"]` (реальный объект
|
||||
исключения, тот же контракт что sentry_sdk передаёт в before_send) — не
|
||||
парсит уже сериализованный event dict, надёжнее к изменениям формата SDK.
|
||||
`isinstance` (не сравнение `type(...).__name__` со строкой) — иначе любой
|
||||
посторонний класс с совпадающим именем ложно матчился бы, а подкласс
|
||||
`tenacity.RetryError` — промахивался бы. Не-RetryError события возвращает без
|
||||
изменений (OperationalError, алерты scraper sweep'ов и т.п. фильтр не трогает).
|
||||
"""
|
||||
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 not isinstance(exc_value, RetryError):
|
||||
return event
|
||||
cause = exc_value.__cause__ or exc_value.__context__
|
||||
cause_type = type(cause).__name__ if cause is not None else "Unknown"
|
||||
culprit = event.get("logger") or event.get("transaction") or "unknown"
|
||||
event["fingerprint"] = ["retry-exhausted", str(culprit), cause_type]
|
||||
return event
|
||||
|
|
|
|||
|
|
@ -44,7 +44,24 @@ if settings.glitchtip_dsn:
|
|||
from sentry_sdk.integrations.logging import LoggingIntegration
|
||||
from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration
|
||||
|
||||
from app.observability.sentry_scrub import scrub_pii_event
|
||||
from app.observability.sentry_scrub import (
|
||||
scrub_pii_event,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
||||
def _before_send(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg]
|
||||
"""PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise).
|
||||
|
||||
Этот процесс гоняет `geocode_missing_listings` (ночной batch, сотни
|
||||
адресов за прогон) — @retry-декорированные Nominatim-хелперы
|
||||
(app/services/geocoder.py) на исчерпанных ретраях исторически плодили
|
||||
по отдельному GlitchTip issue на КАЖДЫЙ адрес (RetryError.__str__()
|
||||
тащит нестабильный repr() Future). См. sentry_scrub docstring.
|
||||
"""
|
||||
scrubbed = scrub_pii_event(event, hint)
|
||||
if scrubbed is None:
|
||||
return None
|
||||
return stabilize_retry_error_fingerprint(scrubbed, hint)
|
||||
|
||||
sentry_sdk.init(
|
||||
dsn=settings.glitchtip_dsn,
|
||||
|
|
@ -52,7 +69,7 @@ if settings.glitchtip_dsn:
|
|||
release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown",
|
||||
traces_sample_rate=0.0,
|
||||
send_default_pii=False,
|
||||
before_send=scrub_pii_event,
|
||||
before_send=_before_send,
|
||||
integrations=[
|
||||
SqlalchemyIntegration(),
|
||||
HttpxIntegration(),
|
||||
|
|
|
|||
|
|
@ -742,7 +742,23 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No
|
|||
return oblast_fallback
|
||||
|
||||
|
||||
@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8))
|
||||
# reraise=True (GlitchTip-noise fix): без него tenacity на исчерпанных ретраях
|
||||
# бросает СВОЙ tenacity.RetryError, чей str() тащит repr() последнего Future
|
||||
# (`<Future at 0x...>` — адрес объекта в памяти, разный на КАЖДЫЙ вызов). GlitchTip
|
||||
# группирует по этому нестабильному тексту → одна и та же причина (Nominatim
|
||||
# недоступен/rate-limit) плодила отдельный issue на каждое исчерпание ретраев
|
||||
# (2 462 issue из 7 461 в трекере). reraise=True пробрасывает РЕАЛЬНОЕ исключение
|
||||
# (httpx.HTTPStatusError/TimeoutException) — стабильный ТИП+стек. НО httpx.HTTPStatusError
|
||||
# сам несёт нестабильный ТЕКСТ (str() содержит полный request URL, включая query
|
||||
# string с адресом — `for url '...search?q=<адрес>&...'`) — group-стабильность на
|
||||
# ЭТОМ пути держит `_HTTPX_ERROR_URL_QUERY_RE` в app/observability/sentry_scrub.py
|
||||
# (`scrub_pii_event`, часть before_send-композиции обоих entrypoint), которая режет
|
||||
# query string из httpx-style "for url '...'" сообщений (GlitchTip-noise review
|
||||
# round 2, claim #1 — reraise=True сам по себе НЕ закрывает per-address explosion).
|
||||
# Отдельно — `stabilize_retry_error_fingerprint` (та же sentry_scrub.py) на случай
|
||||
# если голый tenacity.RetryError (не httpx-исключение) всплывёт откуда-то ещё
|
||||
# (belt-and-suspenders для retry-кода без reraise=True, напр. scraper_kit).
|
||||
@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8), reraise=True)
|
||||
async def _nominatim_lookup(address: str, city_hint: str | None = None) -> GeocodeResult | None:
|
||||
"""OSM Nominatim — бесплатно, без ключа, 1 req/sec policy.
|
||||
|
||||
|
|
@ -941,7 +957,8 @@ async def _nominatim_query_city_aware(
|
|||
return _dedupe_nominatim_items(ekb_data, bare_data)[:limit]
|
||||
|
||||
|
||||
@retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4))
|
||||
# reraise=True — см. комментарий у `_nominatim_lookup` (GlitchTip RetryError-шум).
|
||||
@retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4), reraise=True)
|
||||
async def _nominatim_suggest(
|
||||
query: str, limit: int = 8, city_hint: str | None = None
|
||||
) -> list[GeocodeSuggestion]:
|
||||
|
|
@ -1987,7 +2004,8 @@ def _format_reverse_address(addr: dict) -> str | None:
|
|||
return ", ".join(parts)
|
||||
|
||||
|
||||
@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8))
|
||||
# reraise=True — см. комментарий у `_nominatim_lookup` (GlitchTip RetryError-шум).
|
||||
@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8), reraise=True)
|
||||
async def _nominatim_reverse(lat: float, lon: float) -> ReverseGeocodeResult | None:
|
||||
"""Nominatim /reverse → ReverseGeocodeResult с snapped coords из item.lat/lon.
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:
|
|||
from app.observability.sentry_scrub import (
|
||||
redact_telegram_bot_token,
|
||||
scrub_pii_event,
|
||||
stabilize_retry_error_fingerprint,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -259,3 +260,216 @@ def test_composed_before_send_scrubs_pii_and_token_together() -> None:
|
|||
assert out["request"]["data"]["client_phone"] == "[REDACTED]"
|
||||
frame_url = out["exception"]["values"][0]["stacktrace"]["frames"][0]["vars"]["url"]
|
||||
assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url
|
||||
|
||||
|
||||
# ── RetryError fingerprint stabilization (glitchtip-noise, #<GlitchTip triage>) ─
|
||||
#
|
||||
# tenacity.RetryError.__str__() тащит repr() последнего Future — memory address
|
||||
# объекта, случайный на каждый вызов процесса. Раньше (без `reraise=True` в
|
||||
# app/services/geocoder.py) каждое исчерпание ретраев Nominatim улетало в
|
||||
# GlitchTip как RetryError с этим нестабильным текстом → одна и та же причина
|
||||
# плодила отдельный issue на КАЖДОЕ исчерпание (2 462 issue из 7 461 в трекере).
|
||||
# Тесты ниже бьют по `stabilize_retry_error_fingerprint` напрямую — belt-and-
|
||||
# suspenders слой для retry-кода БЕЗ reraise=True (напр. scraper_kit —
|
||||
# geocoder.py `reraise=True` устраняет RetryError на своём пути, но остаётся
|
||||
# фолбэком общего назначения), и по контракту before_send: 401-класс (RetryError)
|
||||
# схлопывается ПО ИСТОЧНИКУ (не глобально — review round 2 claim #2: разные
|
||||
# подсистемы с совпавшим типом причины НЕ сливаются), содержательные категории
|
||||
# (500-подобный generic Exception, OperationalError) проходят НЕТРОНУТЫМИ.
|
||||
|
||||
from tenacity import RetryError # noqa: E402
|
||||
|
||||
|
||||
def _hint_for(exc: BaseException) -> dict:
|
||||
"""Строит hint в форме, которую sentry_sdk реально передаёт в before_send —
|
||||
`exc_info = (type, value, traceback)` (contract stabilize_retry_error_fingerprint
|
||||
полагается именно на эту форму, не на уже сериализованный event dict)."""
|
||||
return {"exc_info": (type(exc), exc, exc.__traceback__)}
|
||||
|
||||
|
||||
def _raise_retry_error_from(cause: BaseException) -> RetryError:
|
||||
try:
|
||||
raise cause
|
||||
except type(cause) as caught:
|
||||
try:
|
||||
raise RetryError(None) from caught
|
||||
except RetryError as retry_exc:
|
||||
return retry_exc
|
||||
|
||||
|
||||
def test_stabilize_retry_error_sets_stable_fingerprint() -> None:
|
||||
"""RetryError коллапсится в persistent issue по (culprit, имени типа причины) —
|
||||
НЕ по нестабильному str(RetryError) (repr() Future с memory address). Без
|
||||
`event["logger"]` (напр. capture_exception без LoggingIntegration) culprit
|
||||
падает на явный "unknown", а не пропадает из fingerprint молча."""
|
||||
exc = _raise_retry_error_from(TimeoutError("Nominatim timed out"))
|
||||
out = stabilize_retry_error_fingerprint({"level": "error"}, _hint_for(exc))
|
||||
assert out is not None
|
||||
assert out["fingerprint"] == ["retry-exhausted", "unknown", "TimeoutError"]
|
||||
|
||||
|
||||
def test_stabilize_retry_error_fingerprint_has_no_variable_data() -> None:
|
||||
"""Fingerprint не должен содержать IP/id объявления/адрес и т.п. — только
|
||||
culprit (logger-имя модуля) + фиксированное имя типа исключения-причины
|
||||
(маленький словарь: HTTPStatusError/ConnectTimeout/TimeoutError/...)."""
|
||||
exc = _raise_retry_error_from(
|
||||
ValueError("addr='ул. Ленина 1', ip=95.165.147.218, listing_id=12345")
|
||||
)
|
||||
out = stabilize_retry_error_fingerprint({}, _hint_for(exc))
|
||||
assert out is not None
|
||||
fingerprint_text = " ".join(out["fingerprint"])
|
||||
assert "95.165.147.218" not in fingerprint_text
|
||||
assert "12345" not in fingerprint_text
|
||||
assert out["fingerprint"] == ["retry-exhausted", "unknown", "ValueError"]
|
||||
|
||||
|
||||
def test_stabilize_retry_error_fingerprint_uses_logger_as_culprit() -> None:
|
||||
"""`event["logger"]` (sentry_sdk LoggingIntegration ставит его = имя модуля,
|
||||
вызвавшего logger.exception/.error) идёт в fingerprint как culprit — стабильно
|
||||
per-модуль, не переменные данные запроса."""
|
||||
exc = _raise_retry_error_from(TimeoutError("timed out"))
|
||||
out = stabilize_retry_error_fingerprint({"logger": "app.services.geocoder"}, _hint_for(exc))
|
||||
assert out is not None
|
||||
assert out["fingerprint"] == ["retry-exhausted", "app.services.geocoder", "TimeoutError"]
|
||||
|
||||
|
||||
def test_stabilize_retry_error_fingerprint_does_not_collapse_unrelated_subsystems() -> None:
|
||||
"""Review round 2 claim #2: RetryError с ОДИНАКОВЫМ типом причины из
|
||||
НЕСВЯЗАННЫХ подсистем (geocoder vs scraper_kit) НЕ должны схлопнуться в один
|
||||
issue — разные проблемы не сливаются, даже если типы причины совпали."""
|
||||
exc_geocoder = _raise_retry_error_from(TimeoutError("nominatim timed out"))
|
||||
exc_scraper = _raise_retry_error_from(TimeoutError("yandex detail timed out"))
|
||||
out_geocoder = stabilize_retry_error_fingerprint(
|
||||
{"logger": "app.services.geocoder"}, _hint_for(exc_geocoder)
|
||||
)
|
||||
out_scraper = stabilize_retry_error_fingerprint(
|
||||
{"logger": "scraper_kit.providers.yandex.detail"}, _hint_for(exc_scraper)
|
||||
)
|
||||
assert out_geocoder is not None
|
||||
assert out_scraper is not None
|
||||
assert out_geocoder["fingerprint"] != out_scraper["fingerprint"]
|
||||
|
||||
|
||||
class _DecoyRetryError(Exception):
|
||||
"""Посторонний класс, СЛУЧАЙНО названный так же, как tenacity.RetryError —
|
||||
но НЕ его подкласс. Строковое сравнение имён (старый баг, review round 2
|
||||
claim #4) ложно матчило бы такое; isinstance — нет."""
|
||||
|
||||
|
||||
_DecoyRetryError.__name__ = "RetryError" # type: ignore[misc]
|
||||
|
||||
|
||||
def test_stabilize_retry_error_ignores_lookalike_class_by_name() -> None:
|
||||
"""type(exc).__name__ == "RetryError" НЕ должно быть достаточно — только
|
||||
реальный tenacity.RetryError (или его подкласс) триггерит fingerprint-хук."""
|
||||
exc = _DecoyRetryError("unrelated exception, same class __name__ by accident")
|
||||
event = {"level": "error"}
|
||||
out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc))
|
||||
assert out == event
|
||||
assert "fingerprint" not in out
|
||||
|
||||
|
||||
def test_stabilize_retry_error_leaves_operational_error_untouched() -> None:
|
||||
"""401-аналог задачи: OperationalError — содержательная категория (реальный
|
||||
сбой БД), фильтр её НЕ трогает (см. задачу #4 — не выключить сигнал вместе с
|
||||
шумом)."""
|
||||
from sqlalchemy.exc import OperationalError
|
||||
|
||||
exc = OperationalError("SELECT 1", {}, Exception("connection refused"))
|
||||
event = {"level": "error", "message": "db connection failed"}
|
||||
out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc))
|
||||
assert out == event
|
||||
assert "fingerprint" not in out
|
||||
|
||||
|
||||
def test_stabilize_retry_error_leaves_generic_exception_untouched() -> None:
|
||||
"""500-аналог задачи: обычное необработанное исключение (не RetryError)
|
||||
проходит без изменений."""
|
||||
exc = RuntimeError("scraper city-sweep failed")
|
||||
event = {"level": "error"}
|
||||
out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc))
|
||||
assert out == event
|
||||
assert "fingerprint" not in out
|
||||
|
||||
|
||||
def test_stabilize_retry_error_no_exc_info_untouched() -> None:
|
||||
"""capture_message-based события (нет exc_info) — фильтр не трогает, напр.
|
||||
scrape_runs.py consecutive-failure алерты (content-ful, должны доходить)."""
|
||||
event = {"level": "error", "message": "Scraper source 'avito' has 5 consecutive failed runs"}
|
||||
out = stabilize_retry_error_fingerprint(dict(event), {})
|
||||
assert out == event
|
||||
|
||||
|
||||
def test_stabilize_retry_error_handles_non_dict_event() -> None:
|
||||
assert stabilize_retry_error_fingerprint(None, {}) is None # type: ignore[arg-type]
|
||||
|
||||
|
||||
# ── httpx error-message URL query stabilization (glitchtip-noise review round 2,
|
||||
# claim #1) ────────────────────────────────────────────────────────────────
|
||||
#
|
||||
# `httpx.HTTPStatusError.__str__()` (raised by `response.raise_for_status()`) bakes
|
||||
# the FULL request URL — including query string — into the exception message.
|
||||
# Воспроизведено эмпирически (httpx.Response(403, request=...).raise_for_status()):
|
||||
# "Client error '403 Forbidden' for url 'https://nominatim.openstreetmap.org/
|
||||
# search?q=<адрес>&format=json&limit=3'". После `reraise=True` в geocoder.py
|
||||
# (устраняет RetryError, но НЕ этот текст) именно ЭТА строка становится GlitchTip
|
||||
# title/value — переменный `q=<адрес>` на каждый вызов воспроизводит тот же
|
||||
# per-address issue-explosion, который reraise=True должен был устранить, просто
|
||||
# сменивший класс исключения (RetryError → HTTPStatusError). Тесты бьют по
|
||||
# `scrub_pii_event` напрямую (композиция, реально применяемая в before_send).
|
||||
|
||||
_NOMINATIM_403_TEMPLATE = (
|
||||
"Client error '403 Forbidden' for url "
|
||||
"'https://nominatim.openstreetmap.org/search?q={query}&format=json&limit=3'\n"
|
||||
"For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403"
|
||||
)
|
||||
|
||||
|
||||
def _httpx_error_event(message: str) -> dict:
|
||||
return {"exception": {"values": [{"type": "HTTPStatusError", "value": message}]}}
|
||||
|
||||
|
||||
def test_scrub_pii_event_stabilizes_httpx_error_url_query() -> None:
|
||||
"""Query string режется целиком из httpx-style 'for url' сообщения — host+path
|
||||
остаются стабильными для группировки."""
|
||||
encoded_ekb = "%D0%95%D0%BA%D0%B0%D1%82%D0%B5%D1%80%D0%B8%D0%BD%D0%B1%D1%83%D1%80%D0%B3"
|
||||
event = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query=encoded_ekb))
|
||||
out = scrub_pii_event(event, {})
|
||||
assert out is not None
|
||||
value = out["exception"]["values"][0]["value"]
|
||||
assert "search?[REDACTED]'" in value
|
||||
assert "%D0%95" not in value
|
||||
assert "nominatim.openstreetmap.org/search" in value # host+path сохранены
|
||||
|
||||
|
||||
def test_scrub_pii_event_httpx_url_query_stabilization_collapses_different_addresses() -> None:
|
||||
"""Два РАЗНЫХ адреса (переменная часть query) после редактора дают
|
||||
ИДЕНТИЧНЫЙ текст сообщения — GlitchTip group-title больше не плодит issue
|
||||
на каждый адрес (review round 2 claim #1)."""
|
||||
event_a = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query="ул.+Ленина+1"))
|
||||
event_b = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query="ул.+Мира+42%2C+кв.+5"))
|
||||
out_a = scrub_pii_event(event_a, {})
|
||||
out_b = scrub_pii_event(event_b, {})
|
||||
assert out_a is not None
|
||||
assert out_b is not None
|
||||
assert out_a["exception"]["values"][0]["value"] == out_b["exception"]["values"][0]["value"]
|
||||
|
||||
|
||||
def test_scrub_pii_event_httpx_url_without_query_untouched() -> None:
|
||||
"""URL без query string (напр. scraper detail page — переменная часть в
|
||||
ПУТИ, не в query) остаётся нетронутым — regex матчит только `?...`."""
|
||||
message = "Client error '404 Not Found' for url 'https://realty.yandex.ru/offer/12345/'"
|
||||
event = _httpx_error_event(message)
|
||||
out = scrub_pii_event(event, {})
|
||||
assert out is not None
|
||||
assert out["exception"]["values"][0]["value"] == message
|
||||
|
||||
|
||||
def test_scrub_pii_event_httpx_url_query_stabilization_leaves_unrelated_text_untouched() -> None:
|
||||
"""Regex бьёт только по 'for url \\'...?...\\'' — произвольный текст с `?` и
|
||||
кавычками не должен ложно матчиться."""
|
||||
benign = "Вопрос: 'что такое ЖК \"Солнечный\"?' — уточните адрес"
|
||||
event = {"extra": {"note": benign}}
|
||||
out = scrub_pii_event(event, {})
|
||||
assert out is not None
|
||||
assert out["extra"]["note"] == benign
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue