mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-14 05:18:54 +00:00
655bbcbf2c90677715f30c7d6f3e1fbbff7ead53
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 lukaszraczylo
|
5cc681496c |
perf(limits): single-batch counter writes, bounded fallback retention
Review fix round (2 Important + 5 folded) on 08cfdb4/b3fb705.
1. counterStore gains incrMulti(entries []counterIncr) ([]int64, error).
redisStore.incrMulti sends every entry's INCRBY+EXPIRE pair as ONE
pipeline (ttlToSeconds extracted from incrBy, now shared);
memoryStore.incrMulti loops incrBy for interface conformance.
checkAndCount batches every scope's req:min/req:day/req:hour into one
storeIncrMulti call (down from up to 9 round trips for a 3-scope
request) and reads req:min/req:day back from the reply, no extra
GETs. account batches every scope's nonzero tok-in/tok-out/cost
entries into one call (down from up to 27). Violation math and
fail-open/fail-closed/latched semantics are unchanged — only the
round-trip count moves.
2. memoryStoreMaxTTL (48h) caps every TTL memoryStore actually applies,
clamped inside incrBy so incrMulti's loop inherits it for free. The
in-process fallback is continuity, not history: applying the real
hour/day/month TTLs there was growing live-key count ~30x, multiplying
sweepEvery's own documented "167x incrBy cliff" by the same factor.
Sweep cadence is unchanged (still 30s-gated); the clamp shrinks what
there is to sweep instead.
Folded: historyMaxSpan's margin comment corrected (~4 days, not a
month); checkAndCount's godoc names req:hour; the integration
current-hour assertion sums every point instead of reading only the
last one (an hour-rollover race the suite cannot control); a new
exact-boundary test proves tokin+tokout landing exactly on a
TokensPerDay limit still violates; README documents the history route
as unthrottled (admin-gate is its only trust boundary) and the
fallback's 48h retention ceiling.
Tests: countingIncrStore proves exactly one incrMulti call per
checkAndCount and per account, with correct per-scope/metric/window
values and unchanged violation math; a fake-RESP-server test pins
incrMulti's wire format (one pipeline, entry-ordered replies); TTL
clamp tests cover both the ceiling and the pass-through case at the
boundary.
Gates: go test ./... -count=1, -race, gofmt, vet, golangci-lint,
make yaegi-check all green. make integration: every test passes
except the pre-existing TestUsersFileHotReload flake, which is NOT
caused by this change — verified via an isolated worktree at the
prior commit (
|
||
|
lukaszraczylo
|
08cfdb4d99 |
feat: split token counters and hour-resolution usage history
Data layer for the upcoming Vue admin charts (v0.2 data-layer task).
Tokens-in/out split (limits.go): metricTok replaced by metricTokIn
("tokin") and metricTokOut ("tokout"). account() records usage.prompt
and usage.completion independently, skipping a direction only when its
own value is zero. A TokensPerDay/TokensPerMonth limit still enforces
one TOTAL budget: tokenBudgetViolation reads both counters in one
storeGetMulti batch and sums them; the violation message still says
"tokens". Estimated usage (the request-body-size fallback) lands
entirely in tokin, documented at its call site.
Hour window (limits.go): windowHour ("hour"), bucket format
"2006010215" UTC, hourWindowTTL (48h). checkAndCount/account both write
it on every metered route; no LimitsConfig field ever names it, so
evaluateScope never evaluates it. dayWindowTTL/monthWindowTTL bumped to
35d/400d so the usage-history API's largest span always finds a live
key.
Total scope (limits.go, routes_unified.go): a synthetic
{kind:"total", id:"all"} scope, limits always nil. withTotalScope
wraps buildLimitScopes' result at every metered route (unified,
media, passthrough, mcp/a2a) so none can forget it; handleAdminAPI's
own accounting stays on buildLimitScopes alone, so admin traffic never
contributes to it.
History API (admin.go): GET /admin/api/usage/history?scope=&metric=&
window=&span= — admin-gated like the other two JSON routes, but
skips checkAndCount ahead of that same change landing for every
/admin/api/* route in a later task. Validates scope kind/metric/window
(400), span bounds per window (400), and unknown user/group id (404).
Computes every bucket key up front and reads them in ONE storeGetMulti
batch; a storeDown read is 503, never a silently-zero series. Works
against memoryStore too.
Admin usage API (admin.go): scopeUsage/adminUsageEntryView now expose
tokensInPerDay/tokensOutPerDay and their per-month counterparts in
place of the combined tokensPerDay/tokensPerMonth pair; buildAdminUsage
adds a "total" row to the response, still one storeGetMulti round trip
overall. usageKeysPerScope names the per-scope key stride (was a bare
6, now 8), closing a deferred review follow-up while the count changed
anyway.
Tests: TDD throughout — bucket/window-key math incl. UTC and hour
boundaries, the month step-back day-31 AddDate trap, split accounting,
token budget summing both directions, total-scope counting-without-
evaluating on every metered route, and the full history API (shape,
oldest-first, span bounds, validation table, 404, 503, one-batch
assertion via a counting stub). Integration: TestAdminDashboard now
fetches an hour history series for user alice and for total and
asserts a non-zero current-hour bucket.
Migration: none — old "tok" counter keys simply expire on their
existing TTL; nothing is deployed yet.
|
||
|
lukaszraczylo
|
c1bb9fe87a |
fix(hot-reload): honest reload docs, unconditional settle, isolated probe budget
Review response (5 Important + 3 folded):
1. README's cacheTTL row/bullet implied per-group scoping; cacheTTL sets
the TTL written when a group's own request populates a shared cache
entry, it never scopes the entry itself. Reworded only, cacheKey
unchanged.
2. README's fallback-counter claim ("used only when redis is not
configured") was wrong: limits.go also falls back during a Redis
outage whenever failOpen is true (the default). Reworded against
limiter.storeIncrBy/storeGet.
3. Dropped the unevidenced "in-flight requests finish on the old
instance" claim; replaced with what TestConfigHotReload actually
observes (zero non-200s across the rewrite window, every run).
4. Real bug: cleanup's settle poll checked for the injected alias to
disappear, which trivially no-ops on the skip path (the alias was
never live, so "absent" was already true on the first check) —
exactly the unguarded window that caused the earlier 404 cascade.
Replaced with settleToBaseline: captures the baseline /v1/models id
set up front, then polls unconditionally (both paths) for 3
consecutive matching polls, bounded 20s. Verified against a live run
that actually hit the skip path.
5. The ~20rps probe loop shared aliceKey/"eng" (1000/min) with most of
this suite's other tests; a budget-exhaustion 429 would look identical
to a reload blip. Added a dedicated hotreload group
(requestsPerMinute: 100000) and hotreload-probe user to
dynamic.yml.tmpl; every request TestConfigHotReload issues now uses
that key.
6. examples/docker-compose.yml still shipped providers.file.filename,
contradicting the new guidance; switched to providers.file.directory.
Verified by actually running the demo stack.
7. waitForModelAliasBounded and the new settle poll now use
doJSONNonFatal (returns an error instead of calling t.Fatalf), so a
transient transport error resets that poll's retry state instead of
aborting the whole test.
8. Reworded the skip-path Skip message: zero failures there proves only
that the stack survived an unreloaded rewrite attempt, not that a
successful reload is blip-free. Re-added the old single-file path to
.gitignore alongside the new directory entry.
Gates: go build/vet/gofmt clean on integration/; root go test -race,
gofmt, vet, yaegi-check all clean; make integration run cold twice, both
exit 0, plus a verbose run that landed on the skip path (the exact
scenario item 4 fixes) with the full suite, including
TestUsersFileHotReload, passing immediately after.
|
||
|
lukaszraczylo
|
657480348b |
fix: v0.2 final review wave
Transcriptions Yaegi coverage via integration mock, flattened admin usage batch, hardened CSP, sanitized provider lastErr, source-archive prefix template, vendor export-ignore warning, release paths. |
||
|
lukaszraczylo
|
f0730a70d7 |
fix(aliases): deterministic integration gate, warn-first fast path, deduped listing
Review fix, task 6 (1 Important + 3 folded): 1. TestModelAliases now polls GET /v1/models (waitForModelAlias) until "aliased/mock" appears, bounded 30s, before asserting - removes the stale-dynamic-config propagation window a plain healthy-200 check cannot rule out. Confirmed config-propagation, not alias-logic, per reviewer's config+error-text analysis. 2. registry.go resolve(): check the warned-set (aliasShadowWarned) before paying bareWinner's O(providerNames) scan on every aliased request - only the first request for a colliding alias pays that cost now. 3. listFor: skip an alias entry whose id already exists in owners (a real model already owns it) - previously produced duplicate ids when a discovered model collided with a configured alias. resolve's own alias-wins precedence is unchanged; only the listing is deduped. New test: TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision. 4. newAliasTestRegistry doc comment: "grp's given aliases" -> "the given aliases" (no grp parameter exists). |
||
|
lukaszraczylo
|
2090b31ae2 |
feat: operator-defined model aliases
Spec §5 (v0.2): Config.ModelAliases map[string]string, alias-first resolution precedence ahead of provider-prefix splitting, alias-OR-target authorization, lazy target resolution with a 404 naming both ids, /v1/models listing, admin overview alias table (JSON + dashboard), and construction-time validation (shape, prefix-shadow, explicit-model collision, chain, empty target). routes_unified.go/routes_media.go needed no resolution-call-site changes - both already pass the client's raw requested id through to registry.resolve, so aliases inherit the __alias echo and multipart rewrite machinery for free. cache.go needed no changes either - cacheKey already hashes requestedModel, so two aliases resolving to the same target already produce distinct cache entries. Adds registry_test.go validation table + precedence/authz/lazy-404/ listing coverage, a dedicated cache-key split test, chat/embeddings/ images/audio-transcriptions end-to-end tests, an extended admin overview test, and an integration test covering the openai-type vs anthropic-type echo asymmetry under real Traefik+Yaegi. |
||
|
lukaszraczylo
|
f49c51b341 |
fix(docs,integration): example env completeness, flaky reset, cache key assertion
Task 5 re-review fixes: 1 Important + 3 folded. 1. (Important) examples/kubernetes.yaml was construction-broken on copy-paste: the inline dashboard-admin user resolves env:LLMGW_ADMIN_KEY at construction time regardless of admin.enabled, but the Helm env snippet never listed it. Added it there, alongside a comment on the admin user explaining the env var is required either way. Fixed the two reversed above/below cross-references (kubernetes.yaml:90/:115) and reworded the misleading "self-contained" comment on the inline admin user. Verified with a throwaway round-trip test covering: no env vars set fails, every documented env var set succeeds (admin.enabled staying false as committed), and — the actual bug — every var set EXCEPT LLMGW_ADMIN_KEY still fails even with admin disabled. 2. README: Admin section's "(disabled still 404s)" now points at the same 404/passthroughUnknown wording the section already uses above it. Caching section's "block absent entirely is a construction error" now says "not enabled" — auth.go's actual check is !cfg.Cache.Enabled, which is identically true whether the block is omitted or present with enabled: false. 3. integration/mock/main.go: added POST /flaky/reset (handleFlakyReset), reachable through the "flaky" provider's own native passthrough route, resetting the atomic hit counter on demand. TestRetryFlakyRecovers now calls it at the start of the test, removing the make integration-keep re-run trap the old doc comment warned about instead of fixing. dynamic.yml.tmpl's stale "test 8" comment now reads "task 5". 4. integration_test.go: added redisKeys (docker compose exec redis-cli KEYS, mirroring flushRedis's own pattern) and a TestResponseCache assertion that redis holds at least one llmgw:cache:* key immediately after the miss request. go test ./... -count=1, gofmt, vet, make yaegi-check all pass; make integration exits 0 across two fresh full runs. |
||
|
lukaszraczylo
|
25578fb282 |
test: v0.2 integration coverage
Extends the integration suite (task 5) to exercise retry, cache, images/audio, and admin under real Traefik+Yaegi:
- mock/main.go: a /flaky/v1/chat/completions endpoint failing the first
request per process with 503 then succeeding (atomic counter, response
carries mock_attempt for verification); /v1/images/generations returning
a canned OpenAI images response; /v1/audio/speech returning binary bytes
under audio/mpeg; gemini mode's :predict returning canned Imagen
predictions.
- dynamic.yml.tmpl: retry{enabled:true,attempts:1}, cache{enabled:true,
ttl:2m}, admin{enabled:true}; a flaky provider pointed at the new mock
endpoint; an admin:true inline user; alice and admin both given a
generous-but-real per-user limits block so their own request counters
are trackable via the usage API.
- integration_test.go: TestRetryFlakyRecovers (client 200, exactly two
upstream hits proven via mock_attempt), TestResponseCache (miss then
hit, tokensPerDay unchanged across the hit via the admin usage API),
TestImagesGenerations (openai native forward + gemini Imagen
translation), TestAudioSpeech (binary round trip, Content-Type
forwarded), TestAdminDashboard (unauthenticated shell, admin vs
non-admin gate, usage reflecting this suite's own traffic).
make integration exits 0 (verified across multiple fresh runs).
|
||
|
lukaszraczylo
|
ae4d6bab96 |
fix: config validation hardening and integration polish
- providers.go: validate provider names against ^[a-zA-Z0-9._-]+$ and reject reserved names (v1, mcp, a2a) that would shadow gateway routes; reject nil ProviderConfig map values at construction instead of panicking. - mcp_a2a.go: apply the same name-pattern and reserved-name validation to mcpServers/agents keys, and reject nil TargetConfig/AgentConfig map values at construction. - auth.go: reject nil GroupConfig map values at construction. - provider_openai_test.go, mcp_a2a_test.go, auth_test.go: table-driven constructor-error coverage for the above. - integration/integration_test.go: point the package doc at `make integration-up`/`integration-keep` instead of a bare `docker compose up -d --build`, which skips the dynamic.yml.tmpl render step and can run against a stale or absent dynamic.yml; add max_tokens to the gemini chat request so translate_gemini.go's hasMaxTokens=true branch is actually exercised under real Yaegi. - translate_gemini.go: soften the max_tokens comment's verification claim to match what the integration suite actually now exercises. |
||
|
lukaszraczylo
|
730583d98e |
fix(integration): green gate with tracked streaming skip, gemini coverage, robust probe
Addresses coordinator review of the Task 15 integration suite: 1 critical ruling, 5 important, 3 folded. make integration now exits 0. - integration_test.go: TestStreamingIncrementalityProbe restructured — content assertion (5 deltas + [DONE], in order) always runs; the raw-TCP timing assertion (unchanged, >=3 gaps >=100ms) is gated behind INTEGRATION_STREAMING=1 with t.Skipf naming traefik/traefik#10269, verified to still genuinely fail when re-armed (not silently disabled). - Makefile: integration-up/integration-wait moved from prerequisites into the integration recipe body as go test ./... -count=1 ok github.com/lukaszraczylo/traefik-llmgateway 6.065s sub-invocations with per-stage status capture, so integration-down always runs even when bring-up or the health wait fails; verified with a synthetic failure. Added .NOTPARALLEL (the integration targets are stateful). - translate_gemini.go: fixed the same yaegi multi-value-assignment panic shape as the prior sse.go fix, on the live max_tokens path. Added a gemini chat completion assertion to integration_test.go (renamed TestUnifiedChatAllProviders) so this path now runs under real Yaegi. - providers.go, translate_anthropic.go: documented the never-wrap contract on providerHTTPError/translateError (routes_unified.go asserts their concrete type directly; errors.As panics under yaegi on these). - integration_test.go: TestCrossReplicaLimits now FLUSHALLs Redis first, for idempotence under integration-keep. - mock/main.go: probe's header-terminator search now accumulates a rolling buffer across reads instead of only checking the latest one. - Folded: hot-reload deadline 10s -> 20s; dynamic.yml's real-upstream baseUrl is now a template (dynamic.yml.tmpl) rendered via INTEGRATION_REAL_BASEURL (default https://llmgw.example.com), not a hardcoded-only hostname. See task-15-report.md's addendum for full verification detail. |
||
|
lukaszraczylo
|
7939c3e74b |
test: integration suite with real Traefik, redis-shared limits, streaming assertions
Adds integration/ (separate module): docker compose stack with real Traefik v3.5 + localPlugins, mock openai/anthropic/gemini/tool providers, shared Redis, and a raw-TCP streaming timing probe. 6/7 tests pass; streaming incrementality fails on a confirmed external yaegi/Traefik bug (see task-15-report.md), not a plugin defect. Fixes 4 real Yaegi-interpretation bugs surfaced by running the plugin under real Traefik for the first time (none caught by go test or yaegi-check, which never exercises chat/embeddings/streaming/Redis): - .traefik.yml: add basePkg (Traefik derives the yaegi binding identifier from the module path, not the package's own declared name; without this override the plugin never loads at all). - registry.go: resolveAgainst's multi-value return panicked yaegi on an inline string concatenation; assign to a local first. - routes_unified.go: errors.As panicked yaegi on interpreted pointer error types; use plain type assertions (both types are always returned bare, never wrapped). - sse.go: readSSE's dispatch closure hit the same multi-value-assignment panic as registry.go; split into separate statements. - resp.go: respErr as a defined string type was indistinguishable from a plain string in a yaegi type assertion, misreading every successful Redis SELECT reply as failed and silently disabling rate limiting; changed to a struct. |