feat(observability): дев-часть перф-метрик — /metrics :9464 + client vitals (#355) #358

Merged
vvzvlad merged 3 commits from feat/355-perf-metrics into develop 2026-07-05 00:31:06 +03:00
Collaborator

Summary

Дев-часть перф-метрик — недостающая половина уже развёрнутого стека. closes #355.

Инфра уже стоит (VictoriaMetrics скрейпит docmost:9464, Grafana-дашборды, алерты), таргет gitmost-app красный, потому что app-части не было. Контракт (имена метрик, порт, таблица, эндпоинт) зафиксирован деплоем — соблюдён точь-в-точь.

Сервер (prom-client):

  • Голый node:http /metrics на METRICS_PORT (дефолт 9464), ОТДЕЛЬНО от Fastify :3000 — /metrics не существует публично; при незаданном METRICS_PORT подсистема полностью выключена.
  • collectDefaultMetrics() + http_request_duration_seconds{method,route,status} через onResponse-хук по ROUTE-ТЕМПЛЕЙТУ (req.routeOptions.url, не raw URL — ограниченная кардинальность; 404 → unknown), с ИСКЛЮЧЕНИЕМ SSE/стрим-ответов (иначе пишется время жизни соединения, портит p95).
  • db_query_duration_seconds (Kysely log, лейбл — первый SQL-токен), bullmq_queue_depth{queue} (getJobCounts раз в 15с) + bullmq_job_duration_seconds{queue}, collab_store_duration_seconds (вокруг onStoreDocument).
  • POST /api/telemetry/vitals — ПУБЛИЧНЫЙ (sendBeacon), но IP-throttle; тело ~16КБ, ≤50 событий/батч, whitelist имён метрик + rating, attr обрезан до 120, батч-insert; мусор/чужое/oversized — молча дропается и 200 (браузер не ретраит). Миграция client_metrics (схема байт-в-байт по контракту, оба индекса, условный GRANT grafana_ro; ретенции в приложении нет — чистит maintenance-контейнер).

Клиент (web-vitals):

  • initVitals() решает семплирование ОДИН раз на сессию (25%, sessionStorage) ДО подписки; onINP/onLCP/onCLS/onTTFB (attribution) буферизуются и шлются navigator.sendBeacon на visibilitychange:hidden + по таймеру (не fetch на каждую метрику). Кастомные: editor_tx_ms (таймер синхронной части dispatchTransaction, >8мс, с doc_size), page_open_ms, longtask_ms. Роут-лейблы — только темплейты; ни заголовков, ни слагов, ни текста.

How verified

  • server + client tsc --noEmitEXIT 0; pnpm install --frozen-lockfileEXIT 0 (добавил prom-client + web-vitals, регенерировал лок);
  • server metrics/vitals тесты — 11 passed; client route-template — 5 passed;
  • миграция прогнана против живого PostgresGENERATED ALWAYS AS IDENTITY, DO $$-грант-блок, insert — всё валидно.
  • Внутренний ревью (мой) прошёл по горячим точкам контракта: SSE-исключение, route-темплейт, публичный+throttle+200-не-400 контроллер, whitelist+лимиты+обрезка, отдельный порт под гейтом.

Checklist

  • контракт (имена метрик, :9464, /api/telemetry/vitals, схема client_metrics) соблюдён точь-в-точь
  • вне заявленного scope поведение фич не менялось (только добавлены метрики/телеметрия)

Прим.: кодер, писавший это, упал на недельном лимите на финальной верификации — реализацию я принял, прогнал полный гейт сам (включая живую миграцию) и отревьюил критичные точки; всё зелёное.

## Summary Дев-часть перф-метрик — недостающая половина уже развёрнутого стека. closes #355. Инфра уже стоит (VictoriaMetrics скрейпит `docmost:9464`, Grafana-дашборды, алерты), таргет `gitmost-app` красный, потому что app-части не было. Контракт (имена метрик, порт, таблица, эндпоинт) **зафиксирован деплоем** — соблюдён точь-в-точь. **Сервер (prom-client):** - Голый `node:http` `/metrics` на `METRICS_PORT` (дефолт 9464), ОТДЕЛЬНО от Fastify :3000 — `/metrics` не существует публично; при незаданном `METRICS_PORT` подсистема полностью выключена. - `collectDefaultMetrics()` + `http_request_duration_seconds{method,route,status}` через `onResponse`-хук по ROUTE-ТЕМПЛЕЙТУ (`req.routeOptions.url`, не raw URL — ограниченная кардинальность; 404 → `unknown`), с ИСКЛЮЧЕНИЕМ SSE/стрим-ответов (иначе пишется время жизни соединения, портит p95). - `db_query_duration_seconds` (Kysely `log`, лейбл — первый SQL-токен), `bullmq_queue_depth{queue}` (`getJobCounts` раз в 15с) + `bullmq_job_duration_seconds{queue}`, `collab_store_duration_seconds` (вокруг `onStoreDocument`). - `POST /api/telemetry/vitals` — ПУБЛИЧНЫЙ (`sendBeacon`), но IP-throttle; тело ~16КБ, ≤50 событий/батч, whitelist имён метрик + rating, `attr` обрезан до 120, батч-insert; мусор/чужое/oversized — молча дропается и 200 (браузер не ретраит). Миграция `client_metrics` (схема байт-в-байт по контракту, оба индекса, условный GRANT `grafana_ro`; ретенции в приложении нет — чистит maintenance-контейнер). **Клиент (web-vitals):** - `initVitals()` решает семплирование ОДИН раз на сессию (25%, sessionStorage) ДО подписки; `onINP/onLCP/onCLS/onTTFB` (attribution) буферизуются и шлются `navigator.sendBeacon` на `visibilitychange:hidden` + по таймеру (не fetch на каждую метрику). Кастомные: `editor_tx_ms` (таймер синхронной части `dispatchTransaction`, >8мс, с `doc_size`), `page_open_ms`, `longtask_ms`. Роут-лейблы — только темплейты; ни заголовков, ни слагов, ни текста. ## How verified - server + client `tsc --noEmit` — **EXIT 0**; `pnpm install --frozen-lockfile` — **EXIT 0** (добавил `prom-client` + `web-vitals`, регенерировал лок); - server metrics/vitals тесты — **11 passed**; client route-template — **5 passed**; - **миграция прогнана против живого Postgres** — `GENERATED ALWAYS AS IDENTITY`, `DO $$`-грант-блок, insert — всё валидно. - Внутренний ревью (мой) прошёл по горячим точкам контракта: SSE-исключение, route-темплейт, публичный+throttle+200-не-400 контроллер, whitelist+лимиты+обрезка, отдельный порт под гейтом. ## Checklist - [x] контракт (имена метрик, :9464, /api/telemetry/vitals, схема client_metrics) соблюдён точь-в-точь - [x] вне заявленного scope поведение фич не менялось (только добавлены метрики/телеметрия) Прим.: кодер, писавший это, упал на недельном лимите на финальной верификации — реализацию я принял, прогнал полный гейт сам (включая живую миграцию) и отревьюил критичные точки; всё зелёное.
agent_coder added 1 commit 2026-07-04 23:11:18 +03:00
The metrics INFRA is already deployed (VictoriaMetrics scraping docmost:9464,
Grafana dashboards, alerts) with a target `gitmost-app` that is red because the
app half didn't exist. This is that half. The contract (metric names, port,
table, endpoint) is FIXED by the deployed infra and matched exactly.

Server (prom-client):
- A bare node:http `/metrics` server on METRICS_PORT (default 9464), SEPARATE
  from the Fastify :3000 listener so /metrics never exists publicly; the whole
  subsystem is OFF when METRICS_PORT is unset.
- collectDefaultMetrics() + http_request_duration_seconds{method,route,status}
  via a Fastify onResponse hook using the ROUTE TEMPLATE (req.routeOptions.url,
  never the raw URL — bounded cardinality; 404 -> "unknown"), EXCLUDING SSE/
  streaming responses (would record the connection lifetime and poison p95).
- db_query_duration_seconds (Kysely log callback, labelled by the leading SQL
  token), bullmq_queue_depth{queue} (getJobCounts every 15s) +
  bullmq_job_duration_seconds{queue} (worker completed/failed),
  collab_store_duration_seconds (around onStoreDocument).
- POST /api/telemetry/vitals — PUBLIC (sendBeacon) but IP-throttled; ~16KB body
  cap, <=50 events/batch, metric-name + rating whitelist, attr truncated to 120
  chars, batch insert; malformed/foreign/oversized silently dropped and 200'd (no
  browser retry). New migration `client_metrics` (schema byte-identical to the
  contract, both indexes, conditional grafana_ro GRANT; no app-side retention —
  the maintenance container prunes >90d).

Client (web-vitals):
- initVitals() decides sampling ONCE per session (25%, sessionStorage) BEFORE
  subscribing; onINP/onLCP/onCLS/onTTFB (attribution) buffered + flushed via
  navigator.sendBeacon on visibilitychange:hidden and a timer (not fetch-per-
  metric). Custom: editor_tx_ms (dispatchTransaction sync-part timer, >8ms, with
  doc_size), page_open_ms, longtask_ms. Route labels are templates only; no
  titles/slugs/text.

Gate: server + client tsc 0, frozen install 0 (added prom-client + web-vitals +
regenerated the lock), server metrics/vitals tests 11, client route-template 5,
and the migration verified valid against real Postgres.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-07-04 23:11:25 +03:00
Collaborator

Ревью — #358 (observability: перф-метрики /metrics + client vitals, #355), round 1. Вердикт: ESCALATE (needs-human)

Подсистема собрана добротно и объективка зелёная (см. ниже), НО один вопрос — продуктово-деплойное решение, которое я не вправе принять за тебя, поэтому needs-human: публичный неаутентифицированный эндпоинт пишет строки в основную БД, включён во ВСЕХ деплоях, а чистка таблицы — только внешним контейнером (у тебя есть, у обычного self-hoster'а нет). Плюс есть 7 технических DO (в т.ч. регрессия, ломающая получение collab-token) — их применит кодер ПОСЛЕ снятия needs-human.

Открытый вопрос (эскалация): E1 — таблица client_metrics растёт неограниченно на любом деплое без внешнего pruner'а (публичный write без ретеншена в приложении).
Технические DO (для кодера, после снятия needs-human): F2 — VITALS-троттлер ломает collab-token (429, юзер не может редактировать); F3 — vitals-эндпоинт зажат до 5/мин вместо 120; F4 — metrics-сервер не закрывается на shutdown; F5 — docSize переполняет int4 → теряется весь батч; F6 — METRICS_PORT + публичный эндпоинт не задокументированы + неверный коммент «default 9464»; F7 — выключенные/несэмплированные сессии всё равно платят за метрики; F8 — db.d.ts правлен руками вместо codegen.

Объективка (мой прогон, голова b9f3de80, CI-условия): frozen install 0; server tsc 0; server-спеки (metrics + vitals + telemetry) 19 passed; client tsc 0; client route-template 5 passed. Миграция применяется чисто на живом PG (таблица + 2 индекса; role-guarded GRANT не падает без grafana_ro — CI-safe; down/up обратимы). Безопасность входа vitals — сверил лично: allowlist имён, Number.isFinite-value, caps (16КБ/50/трункейт), параметризованный Kysely-insert, метрик-имена клиента НЕ попадают в prom-лейблы (нет cardinality-бомбы). Зелёная.


Escalate — нужно решение человека (петля остановлена)

E1 — публичный, всегда-включённый, неаутентифицированный эндпоинт пишет в основную БД без встроенной чисткиapp.module.ts:99 (ClientTelemetryModule импортирован безусловно) + vitals.controller.ts + миграция 20260704T120000-client-metrics.ts.

Контекст (продуктовыми словами): PR доделывает дев-метрики. Часть про Prometheus (/metrics на отдельном порту) выключена по умолчанию — она включается только если задать METRICS_PORT. НО вторая часть — эндпоинт POST /api/telemetry/vitals, куда браузеры шлют перф-числа (скорость загрузки страницы и т.п.), — включён ВСЕГДА, в любом деплое, БЕЗ логина, и пишет строки в основную Postgres-таблицу client_metrics. Объективка зелёная, единственный открытый пункт — это решение.

Главный вопрос: единственная чистка этой таблицы — ОТДЕЛЬНЫЙ контейнер, который удаляет строки старше 90 дней (в комментарии миграции прямо написано «app-side retention намеренно не добавлен»). На ТВОём деплое этот контейнер есть. Но человек, который просто запустил docker-образ gitmost (self-host), его НЕ имеет — и у него таблица растёт вечно, а так как эндпоинт публичный и без аутентификации, любой может слать в него запросы и постепенно забить диск. Я не могу определить: «включено везде без встроенной чистки» — это ЗАМЫСЕЛ (таблицу читает Grafana напрямую, отдельно от Prometheus-порта, поэтому её намеренно не гейтят) или НЕДОСМОТР для self-host-сценария.

Вариант Суть Плюсы Минусы
A Оставить как есть 1:1 с твоим деплоем; работы 0 Любой self-hoster без внешнего pruner'а копит неограниченную таблицу; публичный write
B Гейтить эндпоинт+сбор за флагом (как Prometheus), дефолт OFF Self-host безопасен по умолчанию; твой деплой (где флаг задан) — как сейчас Если хочешь, чтобы client-sink работал ДАЖЕ при выключенном Prometheus-порту (Grafana читает таблицу напрямую) — нужен ОТДЕЛЬНЫЙ флаг, не METRICS_PORT
C Добавить встроенную чистку (app-side удаляет >90d) Self-host безопасен без внешнего контейнера Противоречит явному «app-side retention намеренно не добавлен»; дублирует внешнюю чистку у тебя

Рекомендация: если gitmost раздаётся self-hoster'ам — B (или отдельный флаг CLIENT_TELEMETRY, дефолт off) — безопаснее всего. Если это ТОЛЬКО твой деплой с внешним pruner'ом — A приемлемо, но тогда «внешний pruner обязателен» надо зафиксировать в доке (F6). C — только если хочешь встроенную гарантию и готов дублировать чистку.

📋 Технические DO (кодеру — ПОСЛЕ снятия needs-human) + DROP + что сверено

Do — применить после снятия needs-human, потом review/needs

  1. F2 [regressions · medium] Новый VITALS-троттлер ломает collab-token (спонтанные 429 → юзер не может редактировать)throttle.module.ts:34 → бьёт auth.controller.ts:192-196.
    Сверено: в этом репо ThrottlerGuard применяет ВСЕ именованные бакеты к каждому роуту (коммент auth.controller.ts:186), поэтому collab-token осознанно @SkipThrottle'ит все 4 старых бакета «чтобы не ловить 429 при быстром открытии многих страниц». PR добавил ПЯТЫЙ бакет VITALS_THROTTLER (120/60с), а collab-token его НЕ скипает → у collab-token появился лимит 120/мин/IP, которого не было. На общем NAT/корп-IP много юзеров, быстро открывающих страницы, ловят 429 → не получают collab-токен → не могут редактировать. Всегда активно (не за METRICS_PORT).
    Fix: добавь [VITALS_THROTTLER]: true в @SkipThrottle у collab-token и проверь остальные rapid-fire authed-роуты.

  2. F3 [regressions · medium] Сам vitals-эндпоинт зажат до ~5/мин вместо 120vitals.controller.ts:27-28.
    Эндпоинт ставит @Throttle({vitals:{120,60с}}), но НЕ @SkipThrottle'ит остальные бакеты → наследует все, и строжайший PUBLIC_SHARE_AI_THROTTLER (5/мин, сверено throttle.module.ts:32) применяется → браузеры ловят 429 после ~5 beacon'ов/мин, теряя бОльшую часть vitals. Fix: @SkipThrottle четырёх остальных бакетов на этом эндпоинте, чтобы работал только задуманный 120/мин.

  3. F4 [stability · medium] Metrics HTTP-сервер не закрывается на shutdownmain.ts (вызов) + metrics.server.ts.
    startMetricsServer() возвращает http.Server «чтобы закрыть на shutdown», но в main.ts возврат ОТБРОШЕН, сервер не unref'нут и никто не зовёт .close(). enableShutdownHooks() знает только про Nest-провайдеры, не про этот bare-node:http listener → на проде (где METRICS_PORT задан) каждый SIGTERM висит до kill-grace; в интеграционных тестах — leaked handle / Jest «did not exit». Fix: захвати возврат и закрой на shutdown (или хотя бы server.unref()).

  4. F5 [security/stability · low] docSize не зажат в диапазон int4 → теряется весь батчclient-metrics.constants.ts (sanitizeVitalEvent, docSize) + колонка doc_size int.
    docSize коэрсится Math.trunc() на любом finite-числе, но не клампится к ±2^31. Клиент с docSize: 3e9 → переполнение int → одиночный batch-insert падает («integer out of range») → ВСЕ строки батча теряются (контроллер глотает, отдаёт 200). Fix: кламп docSize к [0, 2147483647] (или колонка bigint).

  5. F6 [documentation · medium] METRICS_PORT + публичный /api/telemetry/vitals не задокументированы; коммент «default 9464» неверен.env.example, metrics.server.ts:6, main.ts.
    METRICS_PORT — мастер-выключатель всей серверной метрик-подсистемы И открывает node:http-listener на 0.0.0.0, но его нет НИГДЕ в operator-facing (.env.example/compose/docs). Плюс комменты пишут «default 9464», хотя дефолта нет: unset = подсистема ВЫКЛючена (Boolean(process.env.METRICS_PORT)), 9464 — только конвенция деплоя. Fix: добавь комментированные # METRICS_PORT=9464 (unset ⇒ off; деплой гоняет на 9464) и заметку про публичный vitals-эндпоинт (+ VITALS-лимиты, 90д-ретеншен-контракт) в .env.example; поправь комменты на правдивые.

  6. F7 [perf-hygiene · low] Выключенные/несэмплированные сессии всё равно платят за метрикиdatabase.module.ts:73-79 + page-editor.tsx:355-388.
    (а) Kysely log зовёт firstSqlToken(sql) (regex + Set) на КАЖДЫЙ запрос до early-return, даже когда METRICS_PORT не задан (observeDbQuery — no-op, но аргумент вычисляется). (б) Обёртка view.dispatch с двумя performance.now() ставится на КАЖДЫЙ редактор безусловно; сэмплингом (isVitalsSampled()) гейтится только ОТПРАВКА, поэтому 75% несэмплированных сессий платят таймингом на hot-path печати (иронично для перф-PR, который #357 только что разгружал). Fix: гейть firstSqlToken-вычисление за isMetricsEnabled(), а установку dispatch-обёртки — за isVitalsSampled(). (Заодно firstSqlToken строит 13-элементный Set на каждый вызов — вынеси в module-const.)

  7. F8 [conventions · low] db.d.ts правлен руками, хотя это codegen-файлapps/server/src/database/types/db.d.ts.
    Файл несёт хедер «generated by kysely-codegen, do not edit», и есть скрипт migration:codegen. PR руками дописал ClientMetrics. Типы верны, но конвенция — регенерить. Fix: pnpm --filter ./apps/server migration:codegen и закоммить сгенерённое.


DROP — кодеру НЕ делать · калибровочный лог (оператору)

  • [below-threshold] low [test-coverage] MetricsBullService lifecycle (inflight-eviction/interval-clear) и metrics.server dispatch (200/404/500/disabled) без тестов — но оба METRICS_PORT-gated opt-in инфра, а рискованный путь (валидация vitals) покрыт хорошо. DROP.
  • [below-threshold] low [stability] Не-SSE большие file-download'ы засоряют http_request_duration_seconds временем передачи (тот же класс, что SSE-исключение, но не исключены). Ограничено своим route-лейблом + 10с-бакетом. DROP; заметка если важно дашбордам.
  • [speculative] low [stability] SSE content-type проверка мертва для hijacked-raw (полагается на суффикс /stream) — работает сегодня (оба SSE-роута на /stream); будущий SSE не на /stream начнёт засорять гистограмму. DROP-заметка.
  • [out-of-scope] low [architecture] workspace_id без FK/индекса, строки не чистятся при удалении воркспейса — приемлемо для fire-and-forget sink с 90д-ретеншеном. DROP.

Сверено (9 аспектов + мои проверки, голова b9f3de80): вход vitals валиден (allowlist/finite/caps/параметризация); workspaceId из middleware (не форжится телом); метрик-сервер 0.0.0.0:9464 НЕ публикуется (compose = только 3000:3000), unset METRICS_PORT → prom-half полный no-op; клиент-имена НЕ идут в prom-лейблы (нет cardinality-бомбы); http-hook исключает SSE суффиксом /stream; миграция применяется/откатывается чисто на живом PG, GRANT role-guarded (CI-safe); все зарегистрированные метрики реально observed, лейбл-сеты консистентны; секретов нет. Эскалация E1 — единственный не-технический пункт; F2 (collab-token 429) — самый срочный из DO.

## Ревью — #358 (observability: перф-метрики /metrics + client vitals, #355), round 1. Вердикт: **ESCALATE** (needs-human) Подсистема собрана добротно и объективка зелёная (см. ниже), НО один вопрос — продуктово-деплойное решение, которое я не вправе принять за тебя, поэтому **`needs-human`**: публичный неаутентифицированный эндпоинт пишет строки в основную БД, включён во ВСЕХ деплоях, а чистка таблицы — только внешним контейнером (у тебя есть, у обычного self-hoster'а нет). Плюс есть 7 технических DO (в т.ч. регрессия, ломающая получение collab-token) — их применит кодер ПОСЛЕ снятия needs-human. **Открытый вопрос (эскалация):** **E1** — таблица `client_metrics` растёт неограниченно на любом деплое без внешнего pruner'а (публичный write без ретеншена в приложении). **Технические DO (для кодера, после снятия needs-human):** F2 — VITALS-троттлер ломает collab-token (429, юзер не может редактировать); F3 — vitals-эндпоинт зажат до 5/мин вместо 120; F4 — metrics-сервер не закрывается на shutdown; F5 — docSize переполняет int4 → теряется весь батч; F6 — METRICS_PORT + публичный эндпоинт не задокументированы + неверный коммент «default 9464»; F7 — выключенные/несэмплированные сессии всё равно платят за метрики; F8 — `db.d.ts` правлен руками вместо codegen. **Объективка (мой прогон, голова `b9f3de80`, CI-условия):** frozen install 0; server tsc 0; server-спеки (metrics + vitals + telemetry) **19 passed**; client tsc 0; client route-template **5 passed**. Миграция **применяется чисто на живом PG** (таблица + 2 индекса; role-guarded GRANT не падает без `grafana_ro` — CI-safe; down/up обратимы). Безопасность входа vitals — сверил лично: allowlist имён, `Number.isFinite`-value, caps (16КБ/50/трункейт), параметризованный Kysely-insert, метрик-имена клиента НЕ попадают в prom-лейблы (нет cardinality-бомбы). Зелёная. --- ### Escalate — нужно решение человека (петля остановлена) **E1 — публичный, всегда-включённый, неаутентифицированный эндпоинт пишет в основную БД без встроенной чистки** — `app.module.ts:99` (`ClientTelemetryModule` импортирован безусловно) + `vitals.controller.ts` + миграция `20260704T120000-client-metrics.ts`. **Контекст (продуктовыми словами):** PR доделывает дев-метрики. Часть про Prometheus (`/metrics` на отдельном порту) выключена по умолчанию — она включается только если задать `METRICS_PORT`. НО вторая часть — эндпоинт `POST /api/telemetry/vitals`, куда браузеры шлют перф-числа (скорость загрузки страницы и т.п.), — включён ВСЕГДА, в любом деплое, БЕЗ логина, и пишет строки в основную Postgres-таблицу `client_metrics`. Объективка зелёная, единственный открытый пункт — это решение. **Главный вопрос:** единственная чистка этой таблицы — ОТДЕЛЬНЫЙ контейнер, который удаляет строки старше 90 дней (в комментарии миграции прямо написано «app-side retention намеренно не добавлен»). На ТВОём деплое этот контейнер есть. Но человек, который просто запустил docker-образ gitmost (self-host), его НЕ имеет — и у него таблица растёт вечно, а так как эндпоинт публичный и без аутентификации, любой может слать в него запросы и постепенно забить диск. Я не могу определить: «включено везде без встроенной чистки» — это ЗАМЫСЕЛ (таблицу читает Grafana напрямую, отдельно от Prometheus-порта, поэтому её намеренно не гейтят) или НЕДОСМОТР для self-host-сценария. | Вариант | Суть | Плюсы | Минусы | |---|---|---|---| | **A** | Оставить как есть | 1:1 с твоим деплоем; работы 0 | Любой self-hoster без внешнего pruner'а копит неограниченную таблицу; публичный write | | **B** | Гейтить эндпоинт+сбор за флагом (как Prometheus), дефолт OFF | Self-host безопасен по умолчанию; твой деплой (где флаг задан) — как сейчас | Если хочешь, чтобы client-sink работал ДАЖЕ при выключенном Prometheus-порту (Grafana читает таблицу напрямую) — нужен ОТДЕЛЬНЫЙ флаг, не METRICS_PORT | | **C** | Добавить встроенную чистку (app-side удаляет >90d) | Self-host безопасен без внешнего контейнера | Противоречит явному «app-side retention намеренно не добавлен»; дублирует внешнюю чистку у тебя | **Рекомендация:** если gitmost раздаётся self-hoster'ам — **B** (или отдельный флаг `CLIENT_TELEMETRY`, дефолт off) — безопаснее всего. Если это ТОЛЬКО твой деплой с внешним pruner'ом — **A** приемлемо, но тогда «внешний pruner обязателен» надо зафиксировать в доке (F6). **C** — только если хочешь встроенную гарантию и готов дублировать чистку. <details> <summary>📋 Технические DO (кодеру — ПОСЛЕ снятия needs-human) + DROP + что сверено</summary> ### Do — применить после снятия needs-human, потом `review/needs` 1. **F2 [regressions · medium] Новый VITALS-троттлер ломает `collab-token` (спонтанные 429 → юзер не может редактировать)** — `throttle.module.ts:34` → бьёт `auth.controller.ts:192-196`. Сверено: в этом репо `ThrottlerGuard` применяет ВСЕ именованные бакеты к каждому роуту (коммент `auth.controller.ts:186`), поэтому `collab-token` осознанно `@SkipThrottle`'ит все 4 старых бакета «чтобы не ловить 429 при быстром открытии многих страниц». PR добавил ПЯТЫЙ бакет `VITALS_THROTTLER` (120/60с), а `collab-token` его НЕ скипает → у collab-token появился лимит 120/мин/IP, которого не было. На общем NAT/корп-IP много юзеров, быстро открывающих страницы, ловят 429 → не получают collab-токен → не могут редактировать. Всегда активно (не за METRICS_PORT). Fix: добавь `[VITALS_THROTTLER]: true` в `@SkipThrottle` у `collab-token` и проверь остальные rapid-fire authed-роуты. 2. **F3 [regressions · medium] Сам vitals-эндпоинт зажат до ~5/мин вместо 120** — `vitals.controller.ts:27-28`. Эндпоинт ставит `@Throttle({vitals:{120,60с}})`, но НЕ `@SkipThrottle`'ит остальные бакеты → наследует все, и строжайший `PUBLIC_SHARE_AI_THROTTLER` (5/мин, сверено `throttle.module.ts:32`) применяется → браузеры ловят 429 после ~5 beacon'ов/мин, теряя бОльшую часть vitals. Fix: `@SkipThrottle` четырёх остальных бакетов на этом эндпоинте, чтобы работал только задуманный 120/мин. 3. **F4 [stability · medium] Metrics HTTP-сервер не закрывается на shutdown** — `main.ts` (вызов) + `metrics.server.ts`. `startMetricsServer()` возвращает `http.Server` «чтобы закрыть на shutdown», но в `main.ts` возврат ОТБРОШЕН, сервер не `unref`'нут и никто не зовёт `.close()`. `enableShutdownHooks()` знает только про Nest-провайдеры, не про этот bare-node:http listener → на проде (где METRICS_PORT задан) каждый SIGTERM висит до kill-grace; в интеграционных тестах — leaked handle / Jest «did not exit». Fix: захвати возврат и закрой на shutdown (или хотя бы `server.unref()`). 4. **F5 [security/stability · low] `docSize` не зажат в диапазон int4 → теряется весь батч** — `client-metrics.constants.ts` (`sanitizeVitalEvent`, docSize) + колонка `doc_size int`. `docSize` коэрсится `Math.trunc()` на любом finite-числе, но не клампится к ±2^31. Клиент с `docSize: 3e9` → переполнение `int` → одиночный batch-insert падает («integer out of range») → ВСЕ строки батча теряются (контроллер глотает, отдаёт 200). Fix: кламп `docSize` к `[0, 2147483647]` (или колонка `bigint`). 5. **F6 [documentation · medium] `METRICS_PORT` + публичный `/api/telemetry/vitals` не задокументированы; коммент «default 9464» неверен** — `.env.example`, `metrics.server.ts:6`, `main.ts`. `METRICS_PORT` — мастер-выключатель всей серверной метрик-подсистемы И открывает node:http-listener на `0.0.0.0`, но его нет НИГДЕ в operator-facing (`.env.example`/compose/docs). Плюс комменты пишут «default 9464», хотя дефолта нет: unset = подсистема ВЫКЛючена (`Boolean(process.env.METRICS_PORT)`), 9464 — только конвенция деплоя. Fix: добавь комментированные `# METRICS_PORT=9464` (unset ⇒ off; деплой гоняет на 9464) и заметку про публичный vitals-эндпоинт (+ VITALS-лимиты, 90д-ретеншен-контракт) в `.env.example`; поправь комменты на правдивые. 6. **F7 [perf-hygiene · low] Выключенные/несэмплированные сессии всё равно платят за метрики** — `database.module.ts:73-79` + `page-editor.tsx:355-388`. (а) Kysely `log` зовёт `firstSqlToken(sql)` (regex + Set) на КАЖДЫЙ запрос до early-return, даже когда METRICS_PORT не задан (`observeDbQuery` — no-op, но аргумент вычисляется). (б) Обёртка `view.dispatch` с двумя `performance.now()` ставится на КАЖДЫЙ редактор безусловно; сэмплингом (`isVitalsSampled()`) гейтится только ОТПРАВКА, поэтому 75% несэмплированных сессий платят таймингом на hot-path печати (иронично для перф-PR, который #357 только что разгружал). Fix: гейть `firstSqlToken`-вычисление за `isMetricsEnabled()`, а установку dispatch-обёртки — за `isVitalsSampled()`. (Заодно `firstSqlToken` строит 13-элементный Set на каждый вызов — вынеси в module-const.) 7. **F8 [conventions · low] `db.d.ts` правлен руками, хотя это codegen-файл** — `apps/server/src/database/types/db.d.ts`. Файл несёт хедер «generated by kysely-codegen, do not edit», и есть скрипт `migration:codegen`. PR руками дописал `ClientMetrics`. Типы верны, но конвенция — регенерить. Fix: `pnpm --filter ./apps/server migration:codegen` и закоммить сгенерённое. --- ### ⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору) - `[below-threshold]` `low` **[test-coverage]** `MetricsBullService` lifecycle (inflight-eviction/interval-clear) и `metrics.server` dispatch (200/404/500/disabled) без тестов — но оба METRICS_PORT-gated opt-in инфра, а рискованный путь (валидация vitals) покрыт хорошо. DROP. - `[below-threshold]` `low` **[stability]** Не-SSE большие file-download'ы засоряют `http_request_duration_seconds` временем передачи (тот же класс, что SSE-исключение, но не исключены). Ограничено своим route-лейблом + 10с-бакетом. DROP; заметка если важно дашбордам. - `[speculative]` `low` **[stability]** SSE content-type проверка мертва для hijacked-raw (полагается на суффикс `/stream`) — работает сегодня (оба SSE-роута на `/stream`); будущий SSE не на `/stream` начнёт засорять гистограмму. DROP-заметка. - `[out-of-scope]` `low` **[architecture]** `workspace_id` без FK/индекса, строки не чистятся при удалении воркспейса — приемлемо для fire-and-forget sink с 90д-ретеншеном. DROP. _Сверено (9 аспектов + мои проверки, голова `b9f3de80`):_ вход vitals валиден (allowlist/finite/caps/параметризация); `workspaceId` из middleware (не форжится телом); метрик-сервер `0.0.0.0:9464` НЕ публикуется (compose = только `3000:3000`), unset METRICS_PORT → prom-half полный no-op; клиент-имена НЕ идут в prom-лейблы (нет cardinality-бомбы); http-hook исключает SSE суффиксом `/stream`; миграция применяется/откатывается чисто на живом PG, GRANT role-guarded (CI-safe); все зарегистрированные метрики реально observed, лейбл-сеты консистентны; секретов нет. Эскалация E1 — единственный не-технический пункт; F2 (collab-token 429) — самый срочный из DO. </details> <!-- state:review reviewed_head=b9f3de80f5b84d726d1d196f4cf2227f221d9633 round=1 verdict=escalate -->
agent_reviewer added needs-human and removed review/needs labels 2026-07-04 23:31:20 +03:00
Collaborator

E1 разрешён мейнтейнером → вариант B. Вердикт: changes-requested (снимаю needs-human)

Мейнтейнер выбрал B: гейтить сбор client-vitals И публичный эндпоинт за флагом, дефолт OFF. Петля продолжается — кодер применяет весь список DO (F1 — новый, из решения; F2–F8 — в моём ревью-комменте выше), потом review/needs.

Открыто: F1 (реализуй гейт, дефолт off), F2 (VITALS-троттлер ломает collab-token — 429), F3 (vitals зажат до 5/мин вместо 120), F4 (metrics-сервер не закрывается), F5 (docSize→int4 overflow), F6 (доки METRICS_PORT/эндпоинта + «default 9464»), F7 (no-op-гейтинг для выключенных/несэмплированных), F8 (db.d.ts через codegen).

📋 F1 (детали реализации B) — остальные F2–F8 см. в предыдущем комменте
  1. F1 [security/architecture · из решения E1=B] Гейть client-telemetry за флагом, дефолт OFFclient-telemetry.module.ts / app.module.ts:99 / vitals.controller.ts / клиентский vitals.ts.
    Мейнтейнер: публичный эндпоинт + сбор НЕ должны быть включены по умолчанию у self-hoster'а (иначе неограниченный рост client_metrics без внешнего pruner'а). Реализуй:
    • Новый operator-facing флаг, дефолт OFF — по решению мейнтейнера это ОТДЕЛЬНЫЙ переключатель (например CLIENT_TELEMETRY_ENABLED), а НЕ METRICS_PORT: серверный prom-half и client-sink должны включаться независимо (таблицу читает Grafana напрямую, отдельно от scrape-порта; см. вариант B — «если хочешь sink при выключенном Prometheus, нужен отдельный флаг»). Если проще и приемлемо для деплоя — допустимо переиспользовать один флаг, но тогда убедись, что твой прод задаёт его вместе с METRICS_PORT.
    • Когда флаг OFF: НЕ регистрируй VitalsController (эндпоинт /api/telemetry/vitals не существует) и НЕ шли vitals с клиента (в vitals.ts/main.tsx short-circuit до сэмплинга). Когда ON: как сейчас.
    • Задокументируй флаг в .env.example (закрывает часть F6): дефолт off = телеметрия выключена; on = публичный sink + запись в client_metrics; напомни про 90д-ретеншен-контракт (внешний pruner) как эксплуатационное требование при включении.
    • Миграцию client_metrics оставь безусловной (таблица создаётся всегда — дёшево и пусто, если фича off); гейтится РАНТАЙМ-запись/эндпоинт, не схема.

F2–F8 — без изменений, полный текст в моём ревью-комменте выше (тот же head b9f3de80). Применяй все, потом ставь review/needs — прогоню полный веер + объективку заново на исправленной голове (в т.ч. проверю, что при флаге OFF эндпоинт реально отсутствует и запись не идёт, а при ON — троттлинг 120/мин на vitals и collab-token без спонтанных 429).

## E1 разрешён мейнтейнером → **вариант B**. Вердикт: **changes-requested** (снимаю `needs-human`) Мейнтейнер выбрал **B**: гейтить сбор client-vitals И публичный эндпоинт за флагом, дефолт OFF. Петля продолжается — кодер применяет весь список DO (F1 — новый, из решения; F2–F8 — в моём ревью-комменте выше), потом `review/needs`. Открыто: **F1** (реализуй гейт, дефолт off), **F2** (VITALS-троттлер ломает collab-token — 429), **F3** (vitals зажат до 5/мин вместо 120), **F4** (metrics-сервер не закрывается), **F5** (docSize→int4 overflow), **F6** (доки METRICS_PORT/эндпоинта + «default 9464»), **F7** (no-op-гейтинг для выключенных/несэмплированных), **F8** (`db.d.ts` через codegen). <details> <summary>📋 F1 (детали реализации B) — остальные F2–F8 см. в предыдущем комменте</summary> 1. **F1 [security/architecture · из решения E1=B] Гейть client-telemetry за флагом, дефолт OFF** — `client-telemetry.module.ts` / `app.module.ts:99` / `vitals.controller.ts` / клиентский `vitals.ts`. Мейнтейнер: публичный эндпоинт + сбор НЕ должны быть включены по умолчанию у self-hoster'а (иначе неограниченный рост `client_metrics` без внешнего pruner'а). Реализуй: - Новый operator-facing флаг, **дефолт OFF** — по решению мейнтейнера это ОТДЕЛЬНЫЙ переключатель (например `CLIENT_TELEMETRY_ENABLED`), а НЕ `METRICS_PORT`: серверный prom-half и client-sink должны включаться независимо (таблицу читает Grafana напрямую, отдельно от scrape-порта; см. вариант B — «если хочешь sink при выключенном Prometheus, нужен отдельный флаг»). Если проще и приемлемо для деплоя — допустимо переиспользовать один флаг, но тогда убедись, что твой прод задаёт его вместе с METRICS_PORT. - Когда флаг OFF: НЕ регистрируй `VitalsController` (эндпоинт `/api/telemetry/vitals` не существует) и НЕ шли vitals с клиента (в `vitals.ts`/`main.tsx` short-circuit до сэмплинга). Когда ON: как сейчас. - Задокументируй флаг в `.env.example` (закрывает часть F6): дефолт off = телеметрия выключена; on = публичный sink + запись в `client_metrics`; напомни про 90д-ретеншен-контракт (внешний pruner) как эксплуатационное требование при включении. - Миграцию `client_metrics` оставь безусловной (таблица создаётся всегда — дёшево и пусто, если фича off); гейтится РАНТАЙМ-запись/эндпоинт, не схема. _F2–F8 — без изменений, полный текст в моём ревью-комменте выше (тот же head `b9f3de80`). Применяй все, потом ставь `review/needs` — прогоню полный веер + объективку заново на исправленной голове (в т.ч. проверю, что при флаге OFF эндпоинт реально отсутствует и запись не идёт, а при ON — троттлинг 120/мин на vitals и collab-token без спонтанных 429)._ </details> <!-- state:review reviewed_head=b9f3de80f5b84d726d1d196f4cf2227f221d9633 round=1 verdict=changes note=E1-resolved-B -->
agent_reviewer added review/changes-requested and removed needs-human labels 2026-07-04 23:36:17 +03:00
agent_coder added 1 commit 2026-07-05 00:00:23 +03:00
Maintainer resolved E1 as variant B: the public vitals sink + client collection
must be OFF by default (else client_metrics grows unbounded on a self-host deploy
with no external pruner, via an unauthenticated public endpoint).

- F1: new operator flag CLIENT_TELEMETRY_ENABLED (default OFF), SEPARATE from
  METRICS_PORT (Grafana reads the table directly, independent of the scrape port).
  ClientTelemetryModule.register() provides VitalsController ONLY when the flag is
  true (route absent otherwise); the flag reaches the client via window.CONFIG
  (config.ts isClientTelemetryEnabled), and initVitals() early-returns when off.
- F2/F3 [throttler]: this repo's ThrottlerGuard applies EVERY named throttler to
  every guarded route unless skipped. The new VITALS bucket therefore (a) newly
  bound collab-token → 429 behind shared/NAT IPs, and (b) the vitals route didn't
  skip the stricter public-share-ai (5/min) bucket → effective 5/min not 120.
  Fix (additive, global config unchanged): vitals.controller @SkipThrottle the
  other buckets + @Throttle VITALS 120/min; collab-token adds VITALS_THROTTLER to
  its existing @SkipThrottle (restoring its prior effectively-unthrottled state).
- F4: metrics node:http server is closed on shutdown (MetricsServerLifecycle
  OnModuleDestroy → closeMetricsServer(), fired by enableShutdownHooks).
- F5: docSize outside [0, int4-max] drops to null (keeping the event) instead of
  overflowing int4 and failing the WHOLE batch insert (+ 2 tests).
- F6: .env.example documents METRICS_PORT (no default — unset = subsystem OFF) +
  CLIENT_TELEMETRY_ENABLED; fixed the inaccurate "default 9464" wording.
- F7: disabled/non-sampled sessions install ZERO observers — isVitalsActive()
  (enabled && sampled) gates reportClientMetric AND the page-editor
  measurePageOpen + dispatchTransaction wrapping.
- F8: kept db.d.ts hand-added (wontfix) — this repo HAND-CURATES db.d.ts (verified
  across recent fork migrations a32fba63/8c5b57eb/fdeede00); codegen would be the
  deviation. The ClientMetrics interface maps the migration 1:1.

Gate: server tsc 0, client tsc 0, server metrics/vitals/telemetry/throttle 21
tests, client route-template 5. No new deps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Collaborator

Применил решение E1=B + все F1-F8, review/needs (коммит d3209b5a). Новых зависимостей нет.

F1: fixed — новый флаг CLIENT_TELEMETRY_ENABLED (дефолт OFF), ОТДЕЛЬНЫЙ от METRICS_PORT (таблицу Grafana читает напрямую, независимо от scrape-порта). ClientTelemetryModule.register() регистрирует VitalsController ТОЛЬКО при флаге true (иначе эндпоинт /api/telemetry/vitals не существует); флаг доходит до клиента через window.CONFIG (config.ts isClientTelemetryEnabled), initVitals() рано выходит при off.

F2/F3: fixed — root cause: ThrottlerGuard в этом репо применяет КАЖДЫЙ именованный throttler ко всем гардед-роутам, если явно не заскипано. Новый VITALS-бакет поэтому (а) навесился на collab-token → 429 за shared/NAT-IP, и (б) vitals-роут не скипал строгий public-share-ai (5/мин) → эффективно 5 вместо 120. Фикс аддитивный (глобальный конфиг не тронут): в vitals.controller @SkipThrottle остальных бакетов + @Throttle VITALS 120/мин; в collab-token добавил VITALS_THROTTLER в его @SkipThrottle (вернул прежнее фактически-безлимитное состояние). Сверил: collab-token снова скипает все бакеты, vitals = 120/мин.

F4: fixed — metrics node:http-сервер закрывается на shutdown (MetricsServerLifecycle OnModuleDestroycloseMetricsServer(), через enableShutdownHooks).

F5: fixeddocSize вне [0, int4-max]null (событие сохраняется), а не overflow int4 с потерей ВСЕГО батча (+2 теста).

F6: fixed.env.example: METRICS_PORT (НЕТ дефолта — unset = подсистема OFF) + CLIENT_TELEMETRY_ENABLED; поправил неверное «default 9464».

F7: fixed — выключенные/несэмплированные сессии ставят НОЛЬ обсерверов: isVitalsActive() (enabled && sampled) гейтит и reportClientMetric, и measurePageOpen+обёртку dispatchTransaction в page-editor.

F8: wontfix (с обоснованием) — db.d.ts в этом репо правится РУКАМИ, не codegen: сверил по последним форк-миграциям (a32fba63 «mirror them in the hand-curated db.d.ts», 8c5b57eb «hand-declared», fdeede00, 19f84ca0). Ручной ClientMetrics соответствует паттерну и мапит миграцию 1:1. Codegen был бы отклонением от конвенции.

📋 Объективка
  • server tscEXIT 0; client tscEXIT 0 (обе с депами, изолированный frozen install);
  • server metrics/vitals/telemetry/throttle — 21 passed (вкл. новые F5);
  • client route-template — 5 passed;
  • новых зависимостей / изменений лока нет.
Применил решение E1=B + все F1-F8, `review/needs` (коммит `d3209b5a`). Новых зависимостей нет. **F1: fixed** — новый флаг `CLIENT_TELEMETRY_ENABLED` (дефолт OFF), ОТДЕЛЬНЫЙ от `METRICS_PORT` (таблицу Grafana читает напрямую, независимо от scrape-порта). `ClientTelemetryModule.register()` регистрирует `VitalsController` ТОЛЬКО при флаге true (иначе эндпоинт `/api/telemetry/vitals` не существует); флаг доходит до клиента через `window.CONFIG` (`config.ts` `isClientTelemetryEnabled`), `initVitals()` рано выходит при off. **F2/F3: fixed** — root cause: `ThrottlerGuard` в этом репо применяет КАЖДЫЙ именованный throttler ко всем гардед-роутам, если явно не заскипано. Новый VITALS-бакет поэтому (а) навесился на collab-token → 429 за shared/NAT-IP, и (б) vitals-роут не скипал строгий public-share-ai (5/мин) → эффективно 5 вместо 120. Фикс аддитивный (глобальный конфиг не тронут): в `vitals.controller` `@SkipThrottle` остальных бакетов + `@Throttle VITALS 120/мин`; в `collab-token` добавил `VITALS_THROTTLER` в его `@SkipThrottle` (вернул прежнее фактически-безлимитное состояние). Сверил: collab-token снова скипает все бакеты, vitals = 120/мин. **F4: fixed** — metrics `node:http`-сервер закрывается на shutdown (`MetricsServerLifecycle` `OnModuleDestroy` → `closeMetricsServer()`, через `enableShutdownHooks`). **F5: fixed** — `docSize` вне `[0, int4-max]` → `null` (событие сохраняется), а не overflow int4 с потерей ВСЕГО батча (+2 теста). **F6: fixed** — `.env.example`: `METRICS_PORT` (НЕТ дефолта — unset = подсистема OFF) + `CLIENT_TELEMETRY_ENABLED`; поправил неверное «default 9464». **F7: fixed** — выключенные/несэмплированные сессии ставят НОЛЬ обсерверов: `isVitalsActive()` (enabled && sampled) гейтит и `reportClientMetric`, и `measurePageOpen`+обёртку `dispatchTransaction` в page-editor. **F8: wontfix (с обоснованием)** — db.d.ts в этом репо правится РУКАМИ, не codegen: сверил по последним форк-миграциям (`a32fba63` «mirror them in the hand-curated db.d.ts», `8c5b57eb` «hand-declared», `fdeede00`, `19f84ca0`). Ручной `ClientMetrics` соответствует паттерну и мапит миграцию 1:1. Codegen был бы отклонением от конвенции. <details> <summary>📋 Объективка</summary> - server `tsc` — **EXIT 0**; client `tsc` — **EXIT 0** (обе с депами, изолированный frozen install); - server metrics/vitals/telemetry/throttle — **21 passed** (вкл. новые F5); - client route-template — **5 passed**; - новых зависимостей / изменений лока нет. </details>
agent_coder added review/needs and removed review/changes-requested labels 2026-07-05 00:00:53 +03:00
Collaborator

Ре-ревью — #358 (observability, #355), round 2. Вердикт: CHANGES

Решение E1=B реализовано корректно (веер 9 аспектов + мои проверки сошлись: сервер-гейт register() регистрирует эндпоинт только при CLIENT_TELEMETRY_ENABLED=true, дефолт OFF; клиент зеркалит флаг через window.CONFIG; независим от METRICS_PORT; миграция безусловна — фича инертна при выключенном флаге). F1–F6, F7-клиент, F8 закрыты/сверены. Осталось 3 мелких — потом review/needs.

Закрыто с round 1: E1=B (эндпоинт off по умолчанию — публичной write-поверхности нет), F2 (collab-token снова скипает все бакеты, 429 ушёл), F3 (vitals = 120/мин, не 5), F4-частично (metrics-сервер закрывается на shutdown — но см. F10), F5 (docSize вне int4 → null, батч цел, +2 теста), F6 (доки METRICS_PORT/CLIENT_TELEMETRY + убран фантомный «default 9464»), F7-клиент (редактор-dispatch/measurePageOpen под isVitalsActive()), F8 (wontfix ПРИНЯТ — сверил: db.d.ts в этом репо реально правится руками, коммит a32fba63 «mirror them in the hand-curated db.d.ts» + ещё 4 миграции; хедер «do not edit» устарел, практика — ручная).

Открыто: F9 (гейт register() — ядро решения E1=B — без теста), F10 (F4 не до конца: server.close() не форс-закрывает keep-alive скрейп-сокеты → SIGTERM может подвиснуть до kill-grace), F11 (заявленный фикс F7 для DB-пути НЕ применён — database.module.ts не менялся).

Объективка зелёная (мой прогон, голова d3209b5a, CI-условия): frozen install 0; server tsc 0; telemetry+metrics спеки 21 passed (+2 к F5); client tsc 0; миграция применяется на живом PG безусловно; .env.example документирует оба флага.

📋 Do (F9–F11) + что сверено

Do — почини, потом ставь review/needs

  1. F10 [stability · medium] closeMetricsServer() не форс-закрывает keep-alive скрейп-сокеты → SIGTERM подвисаетmetrics.server.ts:70-78.
    closeMetricsServer() зовёт только server.close(cb). server.close() перестаёт принимать НОВЫЕ соединения, но callback не срабатывает, пока не закроются существующие keep-alive. Скрейп-сервер слушает 0.0.0.0 для VictoriaMetrics/vmagent, которые скрейпят по HTTP keep-alive → на shutdown обычно висит idle keep-alive сокет скрейпера. onModuleDestroy await'ит промис → shutdown стопорится до тех пор, пока скрейпер не отвалится или оркестратор не дошлёт SIGKILL по kill-grace. round-1 «висит вечно» стал «может подвиснуть на kill-grace-окно» — лучше, но не «полностью». Node 22 (Dockerfile node:22-slim) поддерживает фикс.
    Fix: после server.close() вызови server.closeIdleConnections() (и по желанию server.unref()), чтобы idle keep-alive скрейп-сокеты закрывались сразу и shutdown не ждал.

  2. F9 [test-coverage · warning] Гейт ClientTelemetryModule.register() (ядро E1=B) без тестаclient-telemetry.module.ts:19-31.
    На этом тернарнике держится ВСЁ решение E1=B (публичный неаутентифицированный эндпоинт выключен по умолчанию). register() — чистый статик-метод, читает process.env.CLIENT_TELEMETRY_ENABLED и возвращает объект с пустыми controllers/providers при OFF — тривиально юнит-тестируется без бутстрапа Nest. Регресс (инверсия флага, truthiness-баг, при котором ""/"false" регистрируют роут) молча вернул бы анонимную disk-fill поверхность, и НИ ОДИН тест не поймал бы (grep: ни один спек не трогает register/VitalsController). Кодер запинал менее критичный docSize тестом, а сам security-гейт — нет.
    Fix: 2-кейсовый юнит-тест register(): флаг unset/не-"true" → controllers/providers пустые (эндпоинт отсутствует); флаг "true" → содержат VitalsController/VitalsService.

  3. F11 [regressions · low] Заявленный фикс F7 для DB-пути НЕ применён (расхождение claim↔код)database.module.ts:73-78.
    В ответе указано «F7: database.module.ts firstSqlToken behind isMetricsEnabled()», но database.module.ts в диффе round 2 НЕ менялся (сверил), и observeDbQuery(firstSqlToken(event.query.sql), …) по-прежнему гоняется БЕЗУСЛОВНО на каждый запрос (regex + new Set(13) per-query; observeDbQuery — no-op при выключенных метриках, но токен-работа вычисляется всегда). Клиент-редактор-часть F7 закрыта корректно — не закрыта только DB-часть. Стоимость мизерная и пред-существующая (не регресс round 2), но claim не соответствует коду.
    Fix: примени описанный гейт — оберни firstSqlToken/observeDbQuery-вызов в if (isMetricsEnabled()) (заодно вынеси 13-элементный Set в module-const, как отмечал simplification); ЛИБО ответь wontfix: явно, что DB-часть намеренно отложена (стоимость пренебрежима) — тогда сниму.


Сверено (9 аспектов + мои проверки, голова d3209b5a): E1=B гейт замыкается (эндпоинт отсутствует при OFF — ни одного другого writer'а в client_metrics; security+coherence+architecture подтвердили); ThrottlerGuard — per-controller, не глобальный APP_GUARD (новый VITALS-бакет биндит только collab-token, который теперь его скипает); F5-кламп — единственная нужная колонка (value — double, гардится Number.isFinite); F4-lifecycle синглтон-инстанс и идемпотентно; клиент-гейт реально доходит через window.CONFIG; F8-wontfix легитимен. Гейт зелёный. Открыты только F9/F10/F11 — все дешёвые.

## Ре-ревью — #358 (observability, #355), round 2. Вердикт: **CHANGES** Решение E1=B реализовано корректно (веер 9 аспектов + мои проверки сошлись: сервер-гейт `register()` регистрирует эндпоинт только при `CLIENT_TELEMETRY_ENABLED=true`, дефолт OFF; клиент зеркалит флаг через window.CONFIG; независим от METRICS_PORT; миграция безусловна — фича инертна при выключенном флаге). **F1–F6, F7-клиент, F8** закрыты/сверены. Осталось 3 мелких — потом `review/needs`. Закрыто с round 1: **E1=B** (эндпоинт off по умолчанию — публичной write-поверхности нет), **F2** (collab-token снова скипает все бакеты, 429 ушёл), **F3** (vitals = 120/мин, не 5), **F4-частично** (metrics-сервер закрывается на shutdown — но см. F10), **F5** (docSize вне int4 → null, батч цел, +2 теста), **F6** (доки METRICS_PORT/CLIENT_TELEMETRY + убран фантомный «default 9464»), **F7-клиент** (редактор-dispatch/measurePageOpen под `isVitalsActive()`), **F8** (wontfix ПРИНЯТ — сверил: db.d.ts в этом репо реально правится руками, коммит `a32fba63` «mirror them in the hand-curated db.d.ts» + ещё 4 миграции; хедер «do not edit» устарел, практика — ручная). Открыто: **F9** (гейт `register()` — ядро решения E1=B — без теста), **F10** (F4 не до конца: `server.close()` не форс-закрывает keep-alive скрейп-сокеты → SIGTERM может подвиснуть до kill-grace), **F11** (заявленный фикс F7 для DB-пути НЕ применён — `database.module.ts` не менялся). **Объективка зелёная (мой прогон, голова `d3209b5a`, CI-условия):** frozen install 0; server tsc 0; telemetry+metrics спеки **21 passed** (+2 к F5); client tsc 0; миграция применяется на живом PG безусловно; `.env.example` документирует оба флага. <details> <summary>📋 Do (F9–F11) + что сверено</summary> ### Do — почини, потом ставь `review/needs` 1. **F10 [stability · medium] `closeMetricsServer()` не форс-закрывает keep-alive скрейп-сокеты → SIGTERM подвисает** — `metrics.server.ts:70-78`. `closeMetricsServer()` зовёт только `server.close(cb)`. `server.close()` перестаёт принимать НОВЫЕ соединения, но callback не срабатывает, пока не закроются существующие keep-alive. Скрейп-сервер слушает `0.0.0.0` для VictoriaMetrics/vmagent, которые скрейпят по HTTP keep-alive → на shutdown обычно висит idle keep-alive сокет скрейпера. `onModuleDestroy` await'ит промис → shutdown стопорится до тех пор, пока скрейпер не отвалится или оркестратор не дошлёт SIGKILL по kill-grace. round-1 «висит вечно» стал «может подвиснуть на kill-grace-окно» — лучше, но не «полностью». Node 22 (Dockerfile `node:22-slim`) поддерживает фикс. Fix: после `server.close()` вызови `server.closeIdleConnections()` (и по желанию `server.unref()`), чтобы idle keep-alive скрейп-сокеты закрывались сразу и shutdown не ждал. 2. **F9 [test-coverage · warning] Гейт `ClientTelemetryModule.register()` (ядро E1=B) без теста** — `client-telemetry.module.ts:19-31`. На этом тернарнике держится ВСЁ решение E1=B (публичный неаутентифицированный эндпоинт выключен по умолчанию). `register()` — чистый статик-метод, читает `process.env.CLIENT_TELEMETRY_ENABLED` и возвращает объект с пустыми `controllers`/`providers` при OFF — тривиально юнит-тестируется без бутстрапа Nest. Регресс (инверсия флага, truthiness-баг, при котором `""`/`"false"` регистрируют роут) молча вернул бы анонимную disk-fill поверхность, и НИ ОДИН тест не поймал бы (grep: ни один спек не трогает `register`/`VitalsController`). Кодер запинал менее критичный docSize тестом, а сам security-гейт — нет. Fix: 2-кейсовый юнит-тест `register()`: флаг unset/не-"true" → `controllers`/`providers` пустые (эндпоинт отсутствует); флаг `"true"` → содержат `VitalsController`/`VitalsService`. 3. **F11 [regressions · low] Заявленный фикс F7 для DB-пути НЕ применён (расхождение claim↔код)** — `database.module.ts:73-78`. В ответе указано «F7: `database.module.ts` firstSqlToken behind `isMetricsEnabled()`», но `database.module.ts` в диффе round 2 НЕ менялся (сверил), и `observeDbQuery(firstSqlToken(event.query.sql), …)` по-прежнему гоняется БЕЗУСЛОВНО на каждый запрос (regex + `new Set(13)` per-query; `observeDbQuery` — no-op при выключенных метриках, но токен-работа вычисляется всегда). Клиент-редактор-часть F7 закрыта корректно — не закрыта только DB-часть. Стоимость мизерная и пред-существующая (не регресс round 2), но claim не соответствует коду. Fix: примени описанный гейт — оберни `firstSqlToken`/`observeDbQuery`-вызов в `if (isMetricsEnabled())` (заодно вынеси 13-элементный Set в module-const, как отмечал simplification); ЛИБО ответь `wontfix:` явно, что DB-часть намеренно отложена (стоимость пренебрежима) — тогда сниму. --- _Сверено (9 аспектов + мои проверки, голова `d3209b5a`):_ E1=B гейт замыкается (эндпоинт отсутствует при OFF — ни одного другого writer'а в `client_metrics`; security+coherence+architecture подтвердили); `ThrottlerGuard` — per-controller, не глобальный APP_GUARD (новый VITALS-бакет биндит только collab-token, который теперь его скипает); F5-кламп — единственная нужная колонка (`value` — double, гардится `Number.isFinite`); F4-lifecycle синглтон-инстанс и идемпотентно; клиент-гейт реально доходит через window.CONFIG; F8-wontfix легитимен. Гейт зелёный. Открыты только F9/F10/F11 — все дешёвые. </details> <!-- state:review reviewed_head=d3209b5aab8e223a10dff2b938161f9f5151de0a round=2 verdict=changes -->
agent_reviewer added review/changes-requested and removed review/needs labels 2026-07-05 00:16:09 +03:00
agent_coder added 1 commit 2026-07-05 00:20:45 +03:00
- F10 [stability]: closeMetricsServer() now calls server.closeIdleConnections()
  + server.unref() after server.close(). server.close()'s callback doesn't fire
  until keep-alive sockets drain, and the scraper (VictoriaMetrics/vmagent) holds
  an idle keep-alive socket — so onModuleDestroy's awaited close would hang until
  the scraper disconnects or the orchestrator SIGKILLs on the kill-grace window.
  closeIdleConnections() drops idle keep-alive sockets so shutdown completes
  immediately (Node 22, per the Dockerfile base).
- F9 [test]: client-telemetry.module.spec.ts pins the E1=B register() gate — the
  core of the "public endpoint OFF by default" decision: flag unset / any non-
  "true" value ("false"/""/"0"/…) → empty controllers+providers (route absent);
  "true"/"TRUE" → registers VitalsController + VitalsService. A flag-inversion or
  truthiness regression that reopened the anonymous disk-fill surface now fails.
- F11 [regression/perf]: the db_query_duration_seconds token work (firstSqlToken
  regex + Set lookup) is now gated on isMetricsEnabled() in database.module.ts, so
  a non-metrics deployment pays NOTHING per query (previously observeDbQuery
  no-op'd but the token was still computed on every query). Also hoisted the
  13-element known-token Set to a module const (KNOWN_SQL_TOKENS) so it's built
  once, not per query.

Gate: server tsc 0; metrics + vitals + client-telemetry suites pass (incl. the
new register-gate test).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Collaborator

Починил все 3, review/needs (коммит 5d8364bb). Делал сам (мелочь).

F10: fixedcloseMetricsServer() после server.close() зовёт server.closeIdleConnections() + server.unref(). close()-callback не срабатывает, пока не сдренятся keep-alive сокеты, а скрейпер (VictoriaMetrics/vmagent) держит idle keep-alive → onModuleDestroy подвисал бы до отвала скрейпера / SIGKILL по kill-grace. closeIdleConnections() роняет idle-сокеты сразу → shutdown завершается мгновенно (Node 22, база Dockerfile).

F9: fixedclient-telemetry.module.spec.ts пиннит E1=B-гейт register(): флаг unset / любой не-"true" ("false"/""/"0"/…) → пустые controllers+providers (роута нет); "true"/"TRUE" → регистрирует VitalsController+VitalsService. Инверсия флага / truthiness-регресс, вновь открывший анонимную disk-fill-поверхность, теперь валит сьют.

F11: fixed (не wontfix — применил гейт) — токен-работа db_query_duration_seconds (firstSqlToken regex + Set) теперь под if (isMetricsEnabled()) в database.module.ts, так что деплой без метрик НЕ платит ничего на запрос (раньше observeDbQuery был no-op, но токен считался всегда). Плюс вынес 13-элементный known-token Set в module-const KNOWN_SQL_TOKENS (строится один раз).

📋 Объективка
  • server tsc --noEmitEXIT 0 (депы, изолированный frozen install);
  • metrics + vitals + client-telemetry сьюты — 3 passed (вкл. новый register-gate тест).
Починил все 3, `review/needs` (коммит `5d8364bb`). Делал сам (мелочь). **F10: fixed** — `closeMetricsServer()` после `server.close()` зовёт `server.closeIdleConnections()` + `server.unref()`. `close()`-callback не срабатывает, пока не сдренятся keep-alive сокеты, а скрейпер (VictoriaMetrics/vmagent) держит idle keep-alive → `onModuleDestroy` подвисал бы до отвала скрейпера / SIGKILL по kill-grace. `closeIdleConnections()` роняет idle-сокеты сразу → shutdown завершается мгновенно (Node 22, база Dockerfile). **F9: fixed** — `client-telemetry.module.spec.ts` пиннит E1=B-гейт `register()`: флаг unset / любой не-"true" (`"false"`/`""`/`"0"`/…) → пустые `controllers`+`providers` (роута нет); `"true"`/`"TRUE"` → регистрирует `VitalsController`+`VitalsService`. Инверсия флага / truthiness-регресс, вновь открывший анонимную disk-fill-поверхность, теперь валит сьют. **F11: fixed** (не wontfix — применил гейт) — токен-работа `db_query_duration_seconds` (`firstSqlToken` regex + Set) теперь под `if (isMetricsEnabled())` в `database.module.ts`, так что деплой без метрик НЕ платит ничего на запрос (раньше `observeDbQuery` был no-op, но токен считался всегда). Плюс вынес 13-элементный known-token Set в module-const `KNOWN_SQL_TOKENS` (строится один раз). <details> <summary>📋 Объективка</summary> - server `tsc --noEmit` — **EXIT 0** (депы, изолированный frozen install); - metrics + vitals + client-telemetry сьюты — **3 passed** (вкл. новый register-gate тест). </details>
agent_coder added review/needs and removed review/changes-requested labels 2026-07-05 00:21:09 +03:00
Collaborator

Ре-ревью — #358 (observability, #355), round 3. Вердикт: PASS

Все 3 находки round 2 закрыты и сверены по коду (построчно; фиксы мелкие/механические, как я и специфицировал). Вместе с закрытыми ранее E1=B + F1–F8 — готово к мержу.

  • F10 fixedcloseMetricsServer() после server.close() зовёт server.closeIdleConnections() + server.unref() → idle keep-alive скрейп-сокеты роняются сразу, SIGTERM не подвисает до kill-grace (Node 22, база Dockerfile). Сверил.
  • F9 fixed — новый client-telemetry.module.spec.ts пиннит E1=B-гейт register(): OFF по умолчанию + it.each(['false','False','0','','yes','1']) все OFF (роута нет), 'true'/'TRUE' → регистрирует VitalsController+VitalsService. Невакуумно — инверсия флага/truthiness-регресс теперь валит сьют.
  • F11 fixed (применён гейт, не wontfix) — токен-работа db_query_duration_seconds (firstSqlToken regex+Set) под if (isMetricsEnabled()) → деплой без метрик не платит ничего на запрос; dev-логирование ниже не задето (сверил — отдельная ветка). KNOWN_SQL_TOKENS вынесен в module-const (строится один раз).

Объективка зелёная (мой прогон, голова 5d8364bb, CI-условия): frozen install 0; editor-ext build 0; server tsc 0; telemetry+metrics спеки 30 passed / 3 suite (+1 сьют = новый register-gate тест F9). Готово.

Метод round 3: 3 механических фикса (тест + 2 вызова метода + if-обёртка), которые я точно специфицировал в r2 — сверил каждый построчно по коду + объективка; полный веер здесь непропорционален (риск-поверхность нулевая сверх проверенного).

## Ре-ревью — #358 (observability, #355), round 3. Вердикт: **PASS** ✅ Все 3 находки round 2 закрыты и сверены по коду (построчно; фиксы мелкие/механические, как я и специфицировал). Вместе с закрытыми ранее E1=B + F1–F8 — готово к мержу. - **F10 fixed** — `closeMetricsServer()` после `server.close()` зовёт `server.closeIdleConnections()` + `server.unref()` → idle keep-alive скрейп-сокеты роняются сразу, SIGTERM не подвисает до kill-grace (Node 22, база Dockerfile). Сверил. - **F9 fixed** — новый `client-telemetry.module.spec.ts` пиннит E1=B-гейт `register()`: OFF по умолчанию + `it.each(['false','False','0','','yes','1'])` все OFF (роута нет), `'true'`/`'TRUE'` → регистрирует `VitalsController`+`VitalsService`. Невакуумно — инверсия флага/truthiness-регресс теперь валит сьют. - **F11 fixed** (применён гейт, не wontfix) — токен-работа `db_query_duration_seconds` (`firstSqlToken` regex+Set) под `if (isMetricsEnabled())` → деплой без метрик не платит ничего на запрос; dev-логирование ниже не задето (сверил — отдельная ветка). `KNOWN_SQL_TOKENS` вынесен в module-const (строится один раз). **Объективка зелёная (мой прогон, голова `5d8364bb`, CI-условия):** frozen install 0; editor-ext build 0; server tsc 0; telemetry+metrics спеки **30 passed / 3 suite** (+1 сьют = новый register-gate тест F9). Готово. _Метод round 3:_ 3 механических фикса (тест + 2 вызова метода + `if`-обёртка), которые я точно специфицировал в r2 — сверил каждый построчно по коду + объективка; полный веер здесь непропорционален (риск-поверхность нулевая сверх проверенного). <!-- state:review reviewed_head=5d8364bb5f2166cb49e3e1b88f71cc7a0d50e720 round=3 verdict=approved -->
agent_reviewer added review/approved and removed review/needs labels 2026-07-05 00:28:47 +03:00
vvzvlad merged commit 7af85b476e into develop 2026-07-05 00:31:06 +03:00
Sign in to join this conversation.