From 434d3cc143edd3e3829f8f61ca68ad4da7087b30 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 01:17:34 +0000 Subject: [PATCH 1/2] docs(backlog): add DEBT-026 for Prometheus instrumentation exclusions Tracks the Gemini suggestion on PR #82 (exclude health/docs endpoints from request metrics) as deferred low-priority debt. Co-Authored-By: Claude Fable 5 --- .s2s/BACKLOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index cac5520..45e39aa 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -891,6 +891,22 @@ Three near-duplicates is the threshold where extraction starts to pay off (a fou --- +### DEBT-026: Tune Prometheus instrumentation exclusions (health/docs endpoints) + +**Status**: planned | **Priority**: low | **Created**: 2026-07-12 +**Origin**: Gemini review on PR #82 (v0.5.1 release), `vektra-app/src/vektra_app/main.py:680` + +**Context**: the prometheus-fastapi-instrumentator setup (introduced in v0.5.1 when it replaced starlette-prometheus) excludes only `/metrics` from instrumentation. Health endpoints (`/health`, `/health/{component}`, `/health/memory`) and docs endpoints (`/docs`, `/openapi.json`) are polled by load balancers, orchestrators, and monitoring systems; instrumenting them inflates request counters and histogram cardinality with traffic that carries no signal about API usage. + +**Proposed approach**: extend `excluded_handlers` to `["/metrics", "/health.*", "/docs", "/openapi.json"]`. Consider whether health-endpoint latency is itself worth tracking (it can reveal DB/LLM check slowness) before excluding it wholesale — an alternative is excluding only `/docs`/`/openapi.json` and keeping `/health` instrumented. + +**Acceptance criteria**: +- [ ] Decision recorded on which endpoints stay instrumented (with rationale) +- [ ] `excluded_handlers` updated accordingly +- [ ] `/metrics` output verified: excluded handlers no longer appear in `http_requests_total` + +--- + ### DEBT-025: Isolate unit tests from the developer's local .env **Status**: planned | **Priority**: low | **Created**: 2026-07-12 From 0beb32ad91d2ef94ffc7a3c771fa7b9a45ef5bad Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Sun, 12 Jul 2026 01:23:54 +0000 Subject: [PATCH 2/2] docs(backlog): reorder DEBT-026 after DEBT-025; anchor regex examples PR #83 review feedback (Gemini): sequential item order and correct excluded_handlers regex semantics (re.match, anchoring, dot escaping). Co-Authored-By: Claude Fable 5 --- .s2s/BACKLOG.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 45e39aa..ba87d95 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -891,35 +891,35 @@ Three near-duplicates is the threshold where extraction starts to pay off (a fou --- -### DEBT-026: Tune Prometheus instrumentation exclusions (health/docs endpoints) +### DEBT-025: Isolate unit tests from the developer's local .env **Status**: planned | **Priority**: low | **Created**: 2026-07-12 -**Origin**: Gemini review on PR #82 (v0.5.1 release), `vektra-app/src/vektra_app/main.py:680` +**Origin**: discovered during the DEBT-024 sweep (PR #80): `make test` fails locally with 4 errors while CI is green. -**Context**: the prometheus-fastapi-instrumentator setup (introduced in v0.5.1 when it replaced starlette-prometheus) excludes only `/metrics` from instrumentation. Health endpoints (`/health`, `/health/{component}`, `/health/memory`) and docs endpoints (`/docs`, `/openapi.json`) are polled by load balancers, orchestrators, and monitoring systems; instrumenting them inflates request counters and histogram cardinality with traffic that carries no signal about API usage. +**Context**: 4 tests in `vektra-shared/tests/test_config.py` (`TestLLMConfig::test_defaults`, `TestQueryPipelineConfig::test_eval_mode_default_false`, `test_debug_log_queries_default_false`, `TestVektraSettings::test_defaults_with_required_only`) assert configuration defaults, but when the full suite runs from the workspace root the developer's `.env` leaks into `os.environ` (something imported during collection loads dotenv, e.g. litellm), so machine-specific values (eval_mode=true, custom port, LLM keys) override the defaults and the assertions fail. CI never sees this because runners have no `.env`. Current workaround: temporarily move `.env` away before `make test`. -**Proposed approach**: extend `excluded_handlers` to `["/metrics", "/health.*", "/docs", "/openapi.json"]`. Consider whether health-endpoint latency is itself worth tracking (it can reveal DB/LLM check slowness) before excluding it wholesale — an alternative is excluding only `/docs`/`/openapi.json` and keeping `/health` instrumented. +**Proposed approach**: a `vektra-shared/tests` (or workspace-level) autouse fixture that snapshots and scrubs `VEKTRA_*` variables from `os.environ` for the default-assertion tests, or `monkeypatch.delenv` on the specific vars. Alternatively point pydantic-settings at a nonexistent env file in tests via `_env_file=None`. **Acceptance criteria**: -- [ ] Decision recorded on which endpoints stay instrumented (with rationale) -- [ ] `excluded_handlers` updated accordingly -- [ ] `/metrics` output verified: excluded handlers no longer appear in `http_requests_total` +- [ ] `make test` passes on a dev machine with a populated `.env` +- [ ] Default-assertion tests are hermetic (no dependency on ambient `VEKTRA_*` vars) +- [ ] No change to production settings loading behavior --- -### DEBT-025: Isolate unit tests from the developer's local .env +### DEBT-026: Tune Prometheus instrumentation exclusions (health/docs endpoints) **Status**: planned | **Priority**: low | **Created**: 2026-07-12 -**Origin**: discovered during the DEBT-024 sweep (PR #80): `make test` fails locally with 4 errors while CI is green. +**Origin**: Gemini review on PR #82 (v0.5.1 release), `vektra-app/src/vektra_app/main.py:680` -**Context**: 4 tests in `vektra-shared/tests/test_config.py` (`TestLLMConfig::test_defaults`, `TestQueryPipelineConfig::test_eval_mode_default_false`, `test_debug_log_queries_default_false`, `TestVektraSettings::test_defaults_with_required_only`) assert configuration defaults, but when the full suite runs from the workspace root the developer's `.env` leaks into `os.environ` (something imported during collection loads dotenv, e.g. litellm), so machine-specific values (eval_mode=true, custom port, LLM keys) override the defaults and the assertions fail. CI never sees this because runners have no `.env`. Current workaround: temporarily move `.env` away before `make test`. +**Context**: the prometheus-fastapi-instrumentator setup (introduced in v0.5.1 when it replaced starlette-prometheus) excludes only `/metrics` from instrumentation. Health endpoints (`/health`, `/health/{component}`, `/health/memory`) and docs endpoints (`/docs`, `/openapi.json`) are polled by load balancers, orchestrators, and monitoring systems; instrumenting them inflates request counters and histogram cardinality with traffic that carries no signal about API usage. -**Proposed approach**: a `vektra-shared/tests` (or workspace-level) autouse fixture that snapshots and scrubs `VEKTRA_*` variables from `os.environ` for the default-assertion tests, or `monkeypatch.delenv` on the specific vars. Alternatively point pydantic-settings at a nonexistent env file in tests via `_env_file=None`. +**Proposed approach**: extend `excluded_handlers` with anchored, escaped regexes — the instrumentator compiles each pattern and matches with `re.match`, so an unanchored `/docs` would also match `/docsomething` and an unescaped `.` matches any character. Example: `["^/metrics$", "^/health", "^/docs$", "^/openapi\\.json$"]`. Consider whether health-endpoint latency is itself worth tracking (it can reveal DB/LLM check slowness) before excluding it wholesale — an alternative is excluding only `/docs`/`/openapi.json` and keeping `/health` instrumented. **Acceptance criteria**: -- [ ] `make test` passes on a dev machine with a populated `.env` -- [ ] Default-assertion tests are hermetic (no dependency on ambient `VEKTRA_*` vars) -- [ ] No change to production settings loading behavior +- [ ] Decision recorded on which endpoints stay instrumented (with rationale) +- [ ] `excluded_handlers` updated accordingly +- [ ] `/metrics` output verified: excluded handlers no longer appear in `http_requests_total` ---