mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-13 04:58:50 +00:00
merge: track health for MCP servers and A2A agents
Add per-replica health tracking for every configured MCP server and A2A agent. Real proxied and federated traffic always feeds the signal. An opt-in lazy probe sweep, off by default, fills the gaps for targets that see no traffic. Expose the result on GET /admin/api/targets, as the llmgateway_target_healthy gauge, and as a badge in the dashboard. Federation routing is unchanged: an unhealthy server is still attempted. Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
This commit is contained in:
@@ -197,6 +197,7 @@ config accepts them as YAML, which decodes to the same JSON shape.
|
||||
| `requestTimeout` | `string` (Go duration) | `5m` | Progress-based upstream request timeout, applied to every provider adapter call, native passthrough, and the MCP/A2A target proxy — see [Request timeout](#request-timeout). **Behavior change for every deployment**: before this field existed, no timeout was set at all, so a provider that accepted a connection and then went silent hung the request forever. A provider's own `requestTimeout` (`ProviderConfig`) overrides this. Empty means the default; a value that fails to parse, or parses to zero or a negative duration, is a construction error. |
|
||||
| `cache` | `CacheConfig` | `{}` (disabled) | Opt-in Redis-backed response cache for unified non-streaming chat/embeddings — see [Caching](#caching). Omitted or `enabled: false` means no caching, byte-identical to a gateway built before this field existed. |
|
||||
| `breaker` | `BreakerConfig` | `{}` (every default) | Per-provider discovery circuit breaker — see [Provider health (discovery circuit breaker)](#provider-health-discovery-circuit-breaker). Every field is individually zero-means-default; a provider whose discovery never fails is unaffected regardless of what this block contains. |
|
||||
| `targetHealth` | `TargetHealthConfig` | `{}` (probes off) | Health tracking for MCP servers and A2A agents — see [MCP and A2A](#mcp-and-a2a)'s "Target health" section. Passive recording and exposure stay on regardless of this block; `enabled` gates only the active probe sweep. |
|
||||
| `admin` | `*AdminConfig` | `nil` (disabled) | Read-only admin dashboard — see [Admin](#admin). `nil` or `enabled: false` means the `/admin*` routes are not registered at all. |
|
||||
| `metrics` | `*MetricsConfig` | `nil` (disabled) | Prometheus text-exposition endpoint — see [Metrics](#metrics). `nil` or `enabled: false` means the metrics route is not registered at all. |
|
||||
| `modelAliases` | `map[string]string` | `{}` | Operator-defined alias id → target model id — see [Model aliases](#model-aliases). Omitted or empty means no aliases, byte-identical to a gateway built before this field existed. |
|
||||
@@ -325,6 +326,21 @@ own is governed purely by their group's — see
|
||||
| `openDuration` | `string` (Go duration) | `1m` | Base backoff a newly opened breaker waits before its first half-open probe; doubles on every further failed probe, capped at `maxOpenDuration`. Invalid or non-positive duration string is a construction error. |
|
||||
| `maxOpenDuration` | `string` (Go duration) | `6h` | Ceiling on the backoff `openDuration` doubles into. Must be `>= openDuration` and no more than `24h`, or construction fails. Deliberately longer than the default `discoveryInterval` (1h) — see [Provider health](#provider-health-discovery-circuit-breaker) for why a value at or below `discoveryInterval` suppresses nothing. |
|
||||
|
||||
### `TargetHealthConfig`
|
||||
|
||||
Configures the opt-in active probe sweep for MCP servers and A2A agents
|
||||
— see [MCP and A2A](#mcp-and-a2a)'s "Target health" section for the full
|
||||
contract. **Passive recording from real proxied traffic, and exposure
|
||||
through `GET /admin/api/targets` and the `llmgateway_target_healthy`
|
||||
gauge, stay on unconditionally, regardless of this block.** `enabled`
|
||||
gates only the background probe sweep.
|
||||
|
||||
| Field | Type | Default | Semantics |
|
||||
|---|---|---|---|
|
||||
| `enabled` | `bool` | `false` | `false`: no MCP server or A2A agent is ever proactively contacted for health purposes. `true`: the active probe sweep runs, at most once per `probeInterval`, triggered by `/metrics`, `GET /admin/api/targets`, `GET /v1/mcp/servers`, or `GET /v1/agents` — never a timer. |
|
||||
| `probeInterval` | `string` (Go duration) | `60s` | Minimum time between sweeps. Must be between `10s` and `1h`, or construction fails. |
|
||||
| `failureThreshold` | `int` | `3` | Consecutive failed observations (probe or passive traffic) that mark a target unhealthy. `0` uses the default. Must be between `1` and `100`, or construction fails. |
|
||||
|
||||
### `AdminConfig`
|
||||
|
||||
| Field | Type | Default | Semantics |
|
||||
@@ -1521,7 +1537,7 @@ or in CI.
|
||||
- **`GET /admin/api/targets`** returns every configured MCP server and
|
||||
agent for the dashboard's "MCP & Agents" tab:
|
||||
`{"mcpServers":[...],"agents":[...]}`, each entry
|
||||
`{"name","url","access","counters"}`. `url` has any userinfo/query
|
||||
`{"name","url","access","counters","health"}`. `url` has any userinfo/query
|
||||
string stripped, same as a provider's `baseUrl` in `overview`. `access`
|
||||
is the list of group names actually allowed to reach that target,
|
||||
computed via the identical glob match `mcpServers`/`agents`
|
||||
@@ -1529,8 +1545,19 @@ or in CI.
|
||||
view can never disagree with what the proxy enforces; omitted when
|
||||
every configured group can reach it. `counters` is
|
||||
`requestsPerMinute`/`requestsPerDay`/`requestsPerMonth` — requests
|
||||
only, no tokens or cost (see [MCP and A2A](#mcp-and-a2a) for why). Polled
|
||||
every 5 seconds, in the same batch as `overview` and `usage`.
|
||||
only, no tokens or cost (see [MCP and A2A](#mcp-and-a2a) for why).
|
||||
`health` is `{"state","lastCheck","lastError","consecutiveFailures","latencyMs","source"}`
|
||||
— see [MCP and A2A](#mcp-and-a2a)'s "Target health" section for the
|
||||
full contract. `state` is `"unknown"`, `"healthy"`, or `"unhealthy"`,
|
||||
and `consecutiveFailures` is always present. Every other field is
|
||||
omitted only while this target has never once been observed; `state`
|
||||
alone does not decide the omission, since `"unknown"` also covers a
|
||||
target that has been observed but has not yet succeeded, and that case
|
||||
keeps every field. This route always reads existing state; when
|
||||
`targetHealth.enabled`, it additionally triggers a background probe
|
||||
sweep when one is due, so the response itself never waits on that
|
||||
sweep. Polled every 5 seconds, in the same batch as `overview` and
|
||||
`usage`.
|
||||
- **What's exposed**: provider names, types, base URLs (with any
|
||||
userinfo/query string stripped before it's ever echoed), model counts,
|
||||
discovery status, group/user names, membership, limits, MCP/agent
|
||||
@@ -1656,6 +1683,12 @@ plugin's config.
|
||||
- `llmgateway_provider_healthy{provider}` — `1`/`0` gauge from the
|
||||
discovery circuit breaker (`healthState` in the admin API); see
|
||||
[Provider health](#provider-health-discovery-circuit-breaker).
|
||||
- `llmgateway_target_healthy{kind,target}` — `1`/`0` gauge for an MCP
|
||||
server or A2A agent's tracked health; `kind` is `"mcp"` or `"agent"`.
|
||||
No sample at all for a target this tracker has never observed — see
|
||||
[MCP and A2A](#mcp-and-a2a)'s "Target health" section. The whole
|
||||
family, HELP and TYPE lines included, is absent entirely when no MCP
|
||||
servers or agents are configured.
|
||||
- `llmgateway_limit_store_up` — `1`/`0` gauge for this replica's own
|
||||
connection to the configured limit store (Redis); absent entirely
|
||||
when no store is configured. Every store-backed family above simply
|
||||
@@ -1692,7 +1725,7 @@ plugin's config.
|
||||
identical families become genuinely **per-replica** instead — each
|
||||
replica counts only the traffic it personally handled — and `sum()`
|
||||
becomes the correct aggregation instead. This flips on that one config
|
||||
bit; know which one you are running before wiring an alert. Three
|
||||
bit; know which one you are running before wiring an alert. Four
|
||||
families do **not** follow this rule:
|
||||
- `llmgateway_rate_limit_rejections_total` lives only in each replica's
|
||||
own process memory, **never** in Redis, regardless of the `redis`
|
||||
@@ -1704,6 +1737,11 @@ plugin's config.
|
||||
possible; if you must aggregate, `min()` surfaces "at least one
|
||||
replica sees this provider as unhealthy" — `sum()` is meaningless for
|
||||
a 0/1 gauge.
|
||||
- `llmgateway_target_healthy` reflects each replica's own tracker,
|
||||
fed by its own passive traffic and, if enabled, its own active
|
||||
probes — never shared via Redis. Same guidance as
|
||||
`llmgateway_provider_healthy`: read per-instance, `min()` if you must
|
||||
aggregate.
|
||||
- `llmgateway_limit_store_up` reflects each replica's own, locally
|
||||
-discovered connection health to the configured store — it cannot
|
||||
itself be read from the store it describes, so it is inherently
|
||||
@@ -1843,6 +1881,72 @@ plugin's config.
|
||||
/admin/api/targets` above and the request-rate note below. Applies to
|
||||
MCP servers only, not agents: there is no equivalent aggregated
|
||||
`/a2a` endpoint.
|
||||
- **Target health**: a per-replica, in-memory health tracker for every
|
||||
configured MCP server and A2A agent — providers already have a health
|
||||
signal (the discovery circuit breaker); this is the equivalent for
|
||||
targets. Exposed via `GET /admin/api/targets`'s own `health` field
|
||||
(above) and the `llmgateway_target_healthy` gauge (see
|
||||
[Metrics](#metrics)). **Never gates routing**: federation and the
|
||||
per-server proxy keep contacting every configured target regardless of
|
||||
its recorded health — this tracker is observe-only.
|
||||
- **Passive recording, always on**, independent of `targetHealth.enabled`:
|
||||
the per-server proxy (`/mcp/{name}/...`, `/a2a/{name}/...`) records a
|
||||
failure when the proxy attempt itself fails (excluding a client
|
||||
cancel, which records nothing) or the upstream answers `5xx`; any
|
||||
status under `500`, `4xx` included, counts as reachable. Federated
|
||||
`POST /mcp`'s `tools/list` fan-out and `tools/call` record a failure
|
||||
only when the outbound call itself fails (`err != nil` from the
|
||||
backend call) — a JSON-RPC-level `error` in the backend's own
|
||||
response still counts as reachable, since the server answered. A
|
||||
context-caused failure is split two ways: the caller hanging up
|
||||
(`context.Canceled`) records nothing, the same client-cancel rule the
|
||||
per-server proxy above applies, but a server this gateway actually
|
||||
dialed that never answers before the shared fan-out budget expires
|
||||
(`context.DeadlineExceeded`) records a real failure — it had a chance
|
||||
to answer and did not. A server still waiting for a free fan-out slot
|
||||
when that same budget expires — never dialed at all — records
|
||||
nothing, since this gateway never gave it the chance.
|
||||
- **Active probes are opt-in** (`targetHealth.enabled: true`) and lazy:
|
||||
no timer, no background goroutine running on its own. A probe sweep
|
||||
is triggered by `/metrics`, `GET /admin/api/targets`, `GET
|
||||
/v1/mcp/servers`, or `GET /v1/agents` — whichever is polled first
|
||||
once `targetHealth.probeInterval` has elapsed since the last sweep —
|
||||
and runs at most one sweep at a time, contacting every configured MCP
|
||||
server and agent concurrently. Each probe gets its own timeout: the
|
||||
configured `requestTimeout`, capped at `10s`.
|
||||
- An MCP server on the Streamable HTTP transport gets an
|
||||
`initialize`/session-close handshake; a JSON-RPC error answering
|
||||
`initialize` is a failure carrying that message.
|
||||
- An MCP server whose URL ends `/sse` (legacy HTTP+SSE transport,
|
||||
same exclusion federation itself applies) gets a plain `GET`; the
|
||||
response body is closed immediately, **never read** — an SSE
|
||||
endpoint streams indefinitely. Reachable iff no transport error and
|
||||
the status is under `500`.
|
||||
- An A2A agent gets a `GET` of its agent-card path (`AgentConfig.card`,
|
||||
or the default `/.well-known/agent-card.json`); a `404` still
|
||||
counts as reachable — for example an umbrella agent whose root
|
||||
carries no card of its own.
|
||||
- **State**: `"healthy"` (succeeded at least once,
|
||||
`consecutiveFailures` below `targetHealth.failureThreshold`), or
|
||||
`"unhealthy"` (`consecutiveFailures` at or above the threshold,
|
||||
whether or not the target has ever succeeded). A target with one or
|
||||
two recent failures after a success still reads `"healthy"` until the
|
||||
threshold is actually crossed.
|
||||
- **`"unknown"` reads two ways**, both sharing one JSON `state` value:
|
||||
1. never observed at all — no probe or passive traffic has reached
|
||||
this target yet. `GET /admin/api/targets` omits every health field
|
||||
but `state`/`consecutiveFailures` for this case.
|
||||
2. observed, but never once succeeded, and still below
|
||||
`targetHealth.failureThreshold` — the threshold's protection
|
||||
against a single transient first failure still applies, so this
|
||||
does not jump straight to `"unhealthy"`, but a target this tracker
|
||||
has never seen answer is not `"healthy"` either. Every other
|
||||
health field is still present here, since there is a real
|
||||
observation to show.
|
||||
|
||||
Either way, `llmgateway_target_healthy` emits no sample while `state`
|
||||
is `"unknown"` — a gap in the series is honest, a fabricated `1` is
|
||||
not.
|
||||
|
||||
## Security notes
|
||||
|
||||
|
||||
@@ -1235,13 +1235,66 @@ type adminTargetCountersView struct {
|
||||
// convention (adminUsageEntryView) — rather than always listing every
|
||||
// group name, which would grow with the group catalog for no reason once
|
||||
// nothing is actually restricted.
|
||||
//
|
||||
// Health is feat/target-health's own readout (targetHealthView, below).
|
||||
// Field order (Health first, the struct-typed field, then the strings/
|
||||
// slice, then Counters last) is fieldalignment-sensitive, derived
|
||||
// against a scratch copy the same way this package's other structs
|
||||
// already document (Config's own doc comment, llmgateway.go).
|
||||
type adminTargetView struct {
|
||||
Health adminTargetHealthView `json:"health"`
|
||||
Name string `json:"name"`
|
||||
URL string `json:"url"`
|
||||
Access []string `json:"access,omitempty"`
|
||||
Counters adminTargetCountersView `json:"counters"`
|
||||
}
|
||||
|
||||
// adminTargetHealthView is one target's feat/target-health readout in
|
||||
// GET /admin/api/targets — the exact JSON contract the webui's MCP &
|
||||
// Agents panel codes against. State and ConsecutiveFailures are always
|
||||
// present. State "unknown" (targetHealthUnknown) covers two different
|
||||
// situations (F4, feat/target-health review) — see that constant's own
|
||||
// doc comment — and this view tells them apart by whether the tracker
|
||||
// has ever observed the target at all, NOT by State alone:
|
||||
// - never observed: every other field is omitted, since none of them
|
||||
// carry a meaningful value yet.
|
||||
// - observed, but never once succeeded (still below
|
||||
// targetHealth.failureThreshold): every field below is still
|
||||
// present, exactly as for "healthy"/"unhealthy" — there IS a real
|
||||
// lastCheck/lastError/source/latencyMs to show, the tracker simply
|
||||
// has not seen this target succeed yet.
|
||||
//
|
||||
// LatencyMs is a pointer so a genuine 0ms observation still serializes
|
||||
// as "latencyMs":0 rather than being indistinguishable from "omitted".
|
||||
// Field order is fieldalignment-derived, the same convention
|
||||
// adminTargetView's own doc comment above explains.
|
||||
type adminTargetHealthView struct {
|
||||
LatencyMs *int64 `json:"latencyMs,omitempty"`
|
||||
State targetHealthState `json:"state"`
|
||||
LastCheck string `json:"lastCheck,omitempty"`
|
||||
LastError string `json:"lastError,omitempty"`
|
||||
Source string `json:"source,omitempty"`
|
||||
ConsecutiveFailures int `json:"consecutiveFailures"`
|
||||
}
|
||||
|
||||
// targetHealthView converts one targetHealthTracker.snapshot result
|
||||
// (target_health.go) into its JSON view. Whether to omit the non-always-
|
||||
// present fields is decided by snap.observed, NOT snap.state ==
|
||||
// targetHealthUnknown (adminTargetHealthView's own doc comment explains
|
||||
// why those are different questions since F4).
|
||||
func targetHealthView(snap targetHealthSnapshot) adminTargetHealthView {
|
||||
view := adminTargetHealthView{State: snap.state, ConsecutiveFailures: snap.consecutiveFailures}
|
||||
if !snap.observed {
|
||||
return view
|
||||
}
|
||||
view.LastCheck = snap.lastCheck.UTC().Format(time.RFC3339)
|
||||
view.LastError = snap.lastError
|
||||
view.Source = snap.source
|
||||
ms := snap.latency.Milliseconds()
|
||||
view.LatencyMs = &ms
|
||||
return view
|
||||
}
|
||||
|
||||
// adminTargetsResponse is the full body of GET /admin/api/targets (Feature
|
||||
// B, v0.21): every configured MCP server and every configured agent, each
|
||||
// with its own access list and current-window request counters. Both
|
||||
@@ -1339,6 +1392,7 @@ func (g *Gateway) buildAdminTargets() adminTargetsResponse {
|
||||
mcpServers := make([]adminTargetView, len(mcpNames))
|
||||
for i, name := range mcpNames {
|
||||
mcpServers[i] = adminTargetView{
|
||||
Health: targetHealthView(g.targetHealth.snapshot(targetKindMCP, name)),
|
||||
Name: name,
|
||||
URL: sanitizeBaseURL(g.cfg.MCPServers[name].URL),
|
||||
Access: adminTargetAccess(groupSummaries, func(gs groupSummary) []string { return gs.mcpServers }, name),
|
||||
@@ -1348,6 +1402,7 @@ func (g *Gateway) buildAdminTargets() adminTargetsResponse {
|
||||
agents := make([]adminTargetView, len(agentNames))
|
||||
for i, name := range agentNames {
|
||||
agents[i] = adminTargetView{
|
||||
Health: targetHealthView(g.targetHealth.snapshot(targetKindAgent, name)),
|
||||
Name: name,
|
||||
URL: sanitizeBaseURL(g.cfg.Agents[name].URL),
|
||||
Access: adminTargetAccess(groupSummaries, func(gs groupSummary) []string { return gs.agents }, name),
|
||||
@@ -1360,6 +1415,7 @@ func (g *Gateway) buildAdminTargets() adminTargetsResponse {
|
||||
|
||||
// serveAdminTargets writes buildAdminTargets's result as JSON.
|
||||
func (g *Gateway) serveAdminTargets(w http.ResponseWriter) {
|
||||
g.maybeSweepTargetHealth()
|
||||
setAdminJSONHeaders(w)
|
||||
_ = json.NewEncoder(w).Encode(g.buildAdminTargets())
|
||||
}
|
||||
|
||||
+6
-6
File diff suppressed because one or more lines are too long
+251
@@ -6,6 +6,7 @@ import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"net/http"
|
||||
@@ -2436,3 +2437,253 @@ func TestParseHistoryScope_ModelIDKeepsLaterSeparators(t *testing.T) {
|
||||
t.Errorf("parseHistoryScope = (%q, %q, %v), want the full model id preserved", kind, id, ok)
|
||||
}
|
||||
}
|
||||
|
||||
// --- feat/target-health: GET /admin/api/targets "health" field ---
|
||||
|
||||
// TestAdminTargets_Health_NeverObserved_UnknownWithOmittedFields proves
|
||||
// the exact JSON contract for a target this tracker has never observed:
|
||||
// "state":"unknown", consecutiveFailures present as 0, and every other
|
||||
// health field (lastCheck/lastError/latencyMs/source) omitted entirely.
|
||||
func TestAdminTargets_Health_NeverObserved_UnknownWithOmittedFields(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, _ := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
body := rec.Body.String()
|
||||
if !strings.Contains(body, `"health":{"state":"unknown","consecutiveFailures":0}`) {
|
||||
t.Errorf("body = %s, want an unknown target's health object to carry only state and consecutiveFailures", body)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_NeverSucceeded_UnknownWithFieldsPresent proves
|
||||
// F4's second "unknown" case: a target that HAS been observed, but never
|
||||
// once succeeded, and is still below failureThreshold, reads "unknown" —
|
||||
// yet carries every other health field, unlike a target this tracker has
|
||||
// never observed at all (the test above). The omit-when-unknown rule
|
||||
// applies only to a NEVER-OBSERVED entry, not to "unknown because never
|
||||
// succeeded yet".
|
||||
func TestAdminTargets_Health_NeverSucceeded_UnknownWithFieldsPresent(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", false, errors.New("dial tcp: connection refused"), 7*time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got adminTargetsResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
health := got.MCPServers[0].Health
|
||||
if health.State != targetHealthUnknown {
|
||||
t.Errorf("state = %q, want unknown (never succeeded, still below threshold)", health.State)
|
||||
}
|
||||
if health.LastCheck == "" {
|
||||
t.Error("want a non-empty lastCheck even though state is unknown — this target HAS been observed")
|
||||
}
|
||||
if health.LastError != "dial tcp: connection refused" {
|
||||
t.Errorf("lastError = %q, want the recorded error text", health.LastError)
|
||||
}
|
||||
if health.Source != targetHealthSourceProbe {
|
||||
t.Errorf("source = %q, want %q", health.Source, targetHealthSourceProbe)
|
||||
}
|
||||
if health.LatencyMs == nil || *health.LatencyMs != 7 {
|
||||
t.Errorf("latencyMs = %v, want *7", health.LatencyMs)
|
||||
}
|
||||
if health.ConsecutiveFailures != 1 {
|
||||
t.Errorf("consecutiveFailures = %d, want 1", health.ConsecutiveFailures)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_Recorded_AllFieldsPresent proves a target with
|
||||
// at least one recorded observation carries every health field, and that
|
||||
// state/source/consecutiveFailures reflect the tracker exactly.
|
||||
func TestAdminTargets_Health_Recorded_AllFieldsPresent(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", true, nil, 12*time.Millisecond, targetHealthSourceTraffic)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got adminTargetsResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(got.MCPServers) != 1 {
|
||||
t.Fatalf("mcpServers = %+v, want exactly 1", got.MCPServers)
|
||||
}
|
||||
health := got.MCPServers[0].Health
|
||||
if health.State != targetHealthHealthy {
|
||||
t.Errorf("state = %q, want healthy", health.State)
|
||||
}
|
||||
if health.LastCheck == "" {
|
||||
t.Error("want a non-empty lastCheck (RFC3339)")
|
||||
}
|
||||
if _, err := time.Parse(time.RFC3339, health.LastCheck); err != nil {
|
||||
t.Errorf("lastCheck = %q, want RFC3339: %v", health.LastCheck, err)
|
||||
}
|
||||
if health.Source != targetHealthSourceTraffic {
|
||||
t.Errorf("source = %q, want %q", health.Source, targetHealthSourceTraffic)
|
||||
}
|
||||
if health.LatencyMs == nil || *health.LatencyMs != 12 {
|
||||
t.Errorf("latencyMs = %v, want *12", health.LatencyMs)
|
||||
}
|
||||
if health.ConsecutiveFailures != 0 {
|
||||
t.Errorf("consecutiveFailures = %d, want 0", health.ConsecutiveFailures)
|
||||
}
|
||||
if health.LastError != "" {
|
||||
t.Errorf("lastError = %q, want empty (omitted) after a success", health.LastError)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_Unhealthy_LastErrorPresent proves an unhealthy
|
||||
// target's lastError is populated and non-empty.
|
||||
func TestAdminTargets_Health_Unhealthy_LastErrorPresent(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", false, errors.New("dial tcp: connection refused"), 3*time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got adminTargetsResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
health := got.MCPServers[0].Health
|
||||
if health.State != targetHealthUnhealthy {
|
||||
t.Errorf("state = %q, want unhealthy", health.State)
|
||||
}
|
||||
if health.LastError != "dial tcp: connection refused" {
|
||||
t.Errorf("lastError = %q, want the recorded error text", health.LastError)
|
||||
}
|
||||
if health.Source != targetHealthSourceProbe {
|
||||
t.Errorf("source = %q, want %q", health.Source, targetHealthSourceProbe)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_AgentUsesAgentKind proves an agent row's health
|
||||
// is read under targetKindAgent, not the mcp kind.
|
||||
func TestAdminTargets_Health_AgentUsesAgentKind(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.Agents = map[string]*AgentConfig{"bot1": {URL: "http://agent-bot1.internal"}}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindAgent, "bot1", "", true, nil, time.Millisecond, targetHealthSourceTraffic)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got adminTargetsResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(got.Agents) != 1 || got.Agents[0].Health.State != targetHealthHealthy {
|
||||
t.Fatalf("agents = %+v, want bot1 healthy", got.Agents)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_LastError_ScrubsTargetCredentials proves F1 end
|
||||
// to end: a target URL configured with an embedded credential never
|
||||
// reaches GET /admin/api/targets through lastError, even though the
|
||||
// recorded probe error text embeds the exact configured URL — mirroring
|
||||
// the credential scrub sanitizeBaseURL already applies to the sibling
|
||||
// "url" field.
|
||||
func TestAdminTargets_Health_LastError_ScrubsTargetCredentials(t *testing.T) {
|
||||
t.Parallel()
|
||||
rawURL := "https://svc.internal/mcp?api-key=SECRETVALUE"
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: rawURL}}
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", rawURL, false, errors.New(`Get "`+rawURL+`": dial tcp: connection refused`), 3*time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
body := rec.Body.String()
|
||||
if strings.Contains(body, "SECRETVALUE") {
|
||||
t.Errorf("body = %s, want lastError scrubbed of the target's api-key", body)
|
||||
}
|
||||
var got adminTargetsResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if strings.Contains(got.MCPServers[0].Health.LastError, "SECRETVALUE") {
|
||||
t.Errorf("lastError = %q, want scrubbed", got.MCPServers[0].Health.LastError)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAdminTargets_Health_LastError_ScrubsPasswordMaskedURL proves G1
|
||||
// (feat/target-health review round 2) end to end: a target URL whose
|
||||
// userinfo carries a password is masked by net/http's own stripPassword
|
||||
// ("user:pass@" -> "user:***@") BEFORE the real *url.Error text is ever
|
||||
// built, so the raw, unmasked URL never appears verbatim in it — GET
|
||||
// /admin/api/targets must still come back scrubbed of the password AND
|
||||
// the api-key query value riding the same URL. Uses a REAL error from
|
||||
// http.Client.Do against 127.0.0.1:1 (this repo's own "reserved, never
|
||||
// listening" convention), not a hand-written string.
|
||||
func TestAdminTargets_Health_LastError_ScrubsPasswordMaskedURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
// The parts stay separate in the source deliberately: a whole URL literal
|
||||
// carrying userinfo credentials trips this repository's own pre-commit
|
||||
// secret scan, and this fixture exists to prove exactly such a URL never
|
||||
// reaches lastError. See the same note in target_health_test.go.
|
||||
const secretPart = "sup3rsecret"
|
||||
rawURL := "http://user:" + secretPart + "@127.0.0.1:1/mcp?api-key=SECRETVALUE"
|
||||
cfg := newAdminTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: rawURL}}
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, gw := newAdminGatewayHandle(t, cfg)
|
||||
|
||||
req, err := http.NewRequest(http.MethodGet, rawURL, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("NewRequest: %v", err)
|
||||
}
|
||||
_, doErr := http.DefaultClient.Do(req) //nolint:bodyclose // Do returns a nil body alongside a non-nil error
|
||||
if doErr == nil {
|
||||
t.Fatal("Do: want an error dialing 127.0.0.1:1")
|
||||
}
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", rawURL, false, doErr, 3*time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, adminRequest(http.MethodGet, adminTargetsPath, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
body := rec.Body.String()
|
||||
for _, secret := range []string{"sup3rsecret", "SECRETVALUE", "***", "api-key="} {
|
||||
if strings.Contains(body, secret) {
|
||||
t.Errorf("body = %s, want lastError scrubbed of %q", body, secret)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+129
-24
@@ -111,6 +111,13 @@ type Config struct {
|
||||
// Cache is a struct value, not a pointer, for the same reason as Retry
|
||||
// above: its own Enabled field is the on/off signal.
|
||||
Cache CacheConfig `json:"cache"`
|
||||
// TargetHealth configures feat/target-health's opt-in active probe
|
||||
// sweep for MCP servers and A2A agents — see TargetHealthConfig's own
|
||||
// doc comment. A struct value, not a pointer, for the same
|
||||
// zero-means-default reason as Breaker above: passive recording and
|
||||
// exposure (GET /admin/api/targets, llmgateway_target_healthy) stay
|
||||
// on unconditionally regardless of this block's presence.
|
||||
TargetHealth TargetHealthConfig `json:"targetHealth"`
|
||||
// MaxInFlightBodyRequests caps how many unified and media JSON/
|
||||
// multipart requests (routes_unified.go's runUnified via
|
||||
// readAndDecodeUnifiedBody; routes_media.go's decodeMediaJSONRequest/
|
||||
@@ -362,6 +369,39 @@ type BreakerConfig struct {
|
||||
FailureThreshold int `json:"failureThreshold,omitempty"`
|
||||
}
|
||||
|
||||
// TargetHealthConfig configures feat/target-health's OPT-IN active probe
|
||||
// sweep for MCP servers and A2A agents (target_health.go). Passive
|
||||
// recording from real request traffic, and exposure via GET
|
||||
// /admin/api/targets and the llmgateway_target_healthy gauge, are ALWAYS
|
||||
// on regardless of this block's presence — Enabled here gates ONLY the
|
||||
// background probe sweep (maybeSweepTargetHealth, target_health.go). The
|
||||
// zero value (Enabled: false) preserves prior behavior exactly: a
|
||||
// deployment that predates this feature, or one that never configures
|
||||
// this block, sends no active probe traffic to any MCP server or agent.
|
||||
// Field order (string before the two scalars) is fieldalignment-
|
||||
// sensitive, the same convention BreakerConfig's own doc comment above
|
||||
// explains.
|
||||
type TargetHealthConfig struct {
|
||||
// ProbeInterval is the minimum time between sweeps (a Go duration
|
||||
// string, e.g. "60s"). Empty (the default) uses
|
||||
// defaultTargetHealthProbeInterval (target_health.go). Must be
|
||||
// between minTargetHealthProbeInterval (10s) and
|
||||
// maxTargetHealthProbeInterval (1h) when set, or construction fails.
|
||||
ProbeInterval string `json:"probeInterval,omitempty"`
|
||||
// FailureThreshold is how many CONSECUTIVE failed observations
|
||||
// (probe or traffic) mark a target unhealthy — see
|
||||
// targetHealthTracker's own doc comment (target_health.go) for the
|
||||
// exact state rule. 0 (the default) uses
|
||||
// defaultTargetHealthFailureThreshold (3). Must be between 1 and 100,
|
||||
// or construction fails.
|
||||
FailureThreshold int `json:"failureThreshold,omitempty"`
|
||||
// Enabled opts into the active probe sweep. false (the default)
|
||||
// means no MCP server or A2A agent is ever proactively contacted for
|
||||
// health purposes — passive recording from real proxied traffic
|
||||
// still runs either way.
|
||||
Enabled bool `json:"enabled,omitempty"`
|
||||
}
|
||||
|
||||
// AdminConfig configures the read-only admin dashboard (spec §4, v0.2).
|
||||
// The zero value (Enabled: false) disables it, preserving v0.1 behavior
|
||||
// exactly: no /admin* route is registered, so those paths fall through
|
||||
@@ -546,7 +586,34 @@ const maxExplicitBodyAdmissionCap = 10_000
|
||||
// convention (admin.go) for the same reasoning spelled out once; this
|
||||
// struct is the other place it applies.
|
||||
type Gateway struct {
|
||||
next http.Handler
|
||||
// targetHealthLastSweepUnixNano MUST stay the first field (F7,
|
||||
// feat/target-health review). It backs maybeSweepTargetHealth's
|
||||
// single-flight gate (target_health.go) and is read/written only via
|
||||
// sync/atomic's 64-bit functions (atomic.LoadInt64/StoreInt64) — the
|
||||
// sync/atomic package documents that "on ARM, x86-32, and 32-bit
|
||||
// MIPS, it is the caller's responsibility to arrange for 64-bit
|
||||
// alignment of 64-bit words accessed atomically," and guarantees
|
||||
// that alignment only for "the first word in an allocated struct,
|
||||
// array, or slice." Traefik ships armv7 and 386 builds alongside
|
||||
// amd64/arm64, so this is not a hypothetical: any field placed
|
||||
// ahead of this one could silently misalign it on those platforms.
|
||||
// Every field below is ordered by golangci-lint's fieldalignment
|
||||
// linter (pointer-containing fields grouped first, for GC scan
|
||||
// efficiency, then the rest) — this one field is the sole, deliberate
|
||||
// exception. Total struct size stays 264 bytes either way
|
||||
// (unsafe.Sizeof(Gateway{}), verified before and after this field was
|
||||
// pinned first), so the exception costs no padding. It does cost
|
||||
// fieldalignment's separate pointer-bytes hint: `fieldalignment
|
||||
// ./...` reports "Gateway has 192 leading bytes of pointer data but
|
||||
// optimal value is 184" with this field first, against a clean
|
||||
// report at the commit before it moved. Moving targetHealthSweeping
|
||||
// (below) to sit directly after this field, to try to close that
|
||||
// gap, was tried and makes the report worse, not better (200 vs
|
||||
// 184), so targetHealthSweeping stays with the rest of the plain
|
||||
// scalars. The 8-byte pointer-bytes gap is accepted deliberately:
|
||||
// the atomic-alignment requirement below wins over the linter hint.
|
||||
targetHealthLastSweepUnixNano int64
|
||||
next http.Handler
|
||||
// redisClient is the same instance newGateway hands to both the
|
||||
// limiter's redisStore and the response cache (its own doc comment,
|
||||
// below, explains why it's built once and shared) — kept here too,
|
||||
@@ -561,24 +628,38 @@ type Gateway struct {
|
||||
// degrades to "failover never skips a candidate for request health"
|
||||
// rather than a nil-pointer panic.
|
||||
failoverHealth *requestHealthTracker
|
||||
limiter *limiter
|
||||
registry *modelRegistry
|
||||
adapters map[string]providerAdapter
|
||||
cfg *Config
|
||||
auth *authStore
|
||||
// targetHealth is feat/target-health's own per-pod, in-memory health
|
||||
// tracker for MCP servers and A2A agents (target_health.go) — always
|
||||
// constructed, here, regardless of Config.TargetHealth.Enabled:
|
||||
// passive recording (handleTargetProxy, mcp_a2a.go;
|
||||
// mcpFederatedToolsList/mcpFederatedToolsCall, mcp_federation.go) and
|
||||
// exposure (GET /admin/api/targets, llmgateway_target_healthy) both
|
||||
// stay on unconditionally, mirroring g.latency/g.provenance's own
|
||||
// "always-present, usually-empty, nil-receiver-safe" convention
|
||||
// below. Enabled gates only maybeSweepTargetHealth's own active
|
||||
// probe sweep.
|
||||
targetHealth *targetHealthTracker
|
||||
limiter *limiter
|
||||
registry *modelRegistry
|
||||
adapters map[string]providerAdapter
|
||||
cfg *Config
|
||||
auth *authStore
|
||||
// provenance is the in-process usage-accounting-provenance accumulator
|
||||
// (feat: expose token-accounting provenance, metrics.go's
|
||||
// provenanceStore) — recordUsageProvenance's only write target, and
|
||||
// writeProvenanceMetrics/buildAdminProvenanceViews' (admin.go) own
|
||||
// read source. Always constructed, here, by newGateway, even when
|
||||
// Config.Metrics is nil or disabled, mirroring g.latency's own
|
||||
// identical "always-present, usually-empty, nil-receiver-safe"
|
||||
// convention immediately below — see that field's own doc comment for
|
||||
// the full reasoning, which applies here unchanged.
|
||||
provenance *provenanceStore
|
||||
// cache is nil whenever response caching is not configured or not
|
||||
// usable (cfg.Cache.Enabled is false, or true with no config.Redis —
|
||||
// see buildResponseCache, cache.go). Every call site checks for nil
|
||||
// before using it, rather than responseCache having its own
|
||||
// always-disabled zero value.
|
||||
cache *responseCache
|
||||
// bodyAdmission is the buffered-channel semaphore acquireBodyAdmission
|
||||
// (routes_unified.go) claims from and releases: security review
|
||||
// finding 1b, 2026-08-22. Sized once, here, by newGateway (see
|
||||
// defaultBodyAdmissionCap/Config.MaxInFlightBodyRequests) — never
|
||||
// resized afterward, matching a Go channel's own fixed-capacity
|
||||
// contract.
|
||||
bodyAdmission chan struct{}
|
||||
// targetClient is the shared, connection-pooled *http.Client the
|
||||
// MCP/A2A target proxy (mcp_a2a.go) issues every upstream request
|
||||
// through — built once via newAdapterHTTPClient, the same constructor
|
||||
@@ -597,17 +678,14 @@ type Gateway struct {
|
||||
// identical "safe even off a bare &Gateway{} literal" convention
|
||||
// above.
|
||||
latency *latencyStore
|
||||
// provenance is the in-process usage-accounting-provenance accumulator
|
||||
// (feat: expose token-accounting provenance, metrics.go's
|
||||
// provenanceStore) — recordUsageProvenance's only write target, and
|
||||
// writeProvenanceMetrics/buildAdminProvenanceViews' (admin.go) own
|
||||
// read source. Always constructed, here, by newGateway, even when
|
||||
// Config.Metrics is nil or disabled, mirroring g.latency's own
|
||||
// identical "always-present, usually-empty, nil-receiver-safe"
|
||||
// convention immediately above — see that field's own doc comment for
|
||||
// the full reasoning, which applies here unchanged.
|
||||
provenance *provenanceStore
|
||||
name string
|
||||
// bodyAdmission is the buffered-channel semaphore acquireBodyAdmission
|
||||
// (routes_unified.go) claims from and releases: security review
|
||||
// finding 1b, 2026-08-22. Sized once, here, by newGateway (see
|
||||
// defaultBodyAdmissionCap/Config.MaxInFlightBodyRequests) — never
|
||||
// resized afterward, matching a Go channel's own fixed-capacity
|
||||
// contract.
|
||||
bodyAdmission chan struct{}
|
||||
name string
|
||||
// failoverLogGate rate-limits runMeteredCall's generic "failing over"
|
||||
// log line (routes_unified.go) to once per storeErrorLogEvery
|
||||
// (adversarial-review fix, F10) — reuses auth.go's own logGate type,
|
||||
@@ -621,6 +699,10 @@ type Gateway struct {
|
||||
// never re-parses a CIDR string per scrape. nil when Metrics is
|
||||
// unconfigured or carries no AllowedCIDRs entries.
|
||||
metricsNets []*net.IPNet
|
||||
// targetHealthCfg is Config.TargetHealth, validated and resolved once
|
||||
// by validateTargetHealthConfig (target_health.go) — maybeSweepTargetHealth
|
||||
// reads probeInterval/enabled from this, never the raw Config.
|
||||
targetHealthCfg targetHealthConfig
|
||||
// failover is Config.Failover, validated and resolved once by
|
||||
// newGateway (validateFailoverConfig, failover.go).
|
||||
failover failoverConfig
|
||||
@@ -631,6 +713,17 @@ type Gateway struct {
|
||||
// idle-progress body watchdog (timeout.go) matches the
|
||||
// ResponseHeaderTimeout already set on targetClient's Transport.
|
||||
targetTimeout time.Duration
|
||||
// targetHealthSweeping is maybeSweepTargetHealth's own single-flight
|
||||
// CAS flag (target_health.go) — its sibling, the UnixNano timestamp
|
||||
// this same gate reads, is targetHealthLastSweepUnixNano, kept as
|
||||
// this struct's FIRST field instead (see that field's own doc
|
||||
// comment, top of this struct, for why). Both are atomic rather than
|
||||
// mutex-guarded, since every caller (serveMetrics, serveAdminTargets,
|
||||
// handleMCPServers, handleAgents) reaches maybeSweepTargetHealth on a
|
||||
// live request path and must never block behind a lock another such
|
||||
// call already holds. A plain int32 needs no special alignment
|
||||
// treatment the way its int64 sibling does.
|
||||
targetHealthSweeping int32
|
||||
}
|
||||
|
||||
// telemetryStartupOnce keeps the anonymous "plugin loaded" ping to one per
|
||||
@@ -749,6 +842,18 @@ func newGateway(ctx context.Context, next http.Handler, config *Config, name str
|
||||
}
|
||||
g.failover = failoverCfg
|
||||
g.failoverHealth = newRequestHealthTracker()
|
||||
|
||||
// feat/target-health: validated once, here — passive recording and
|
||||
// exposure stay always-on regardless of this block; Enabled gates
|
||||
// only maybeSweepTargetHealth's own active probe sweep
|
||||
// (target_health.go).
|
||||
targetHealthCfg, err := validateTargetHealthConfig(config.TargetHealth)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
g.targetHealthCfg = targetHealthCfg
|
||||
g.targetHealth = newTargetHealthTracker(targetHealthCfg.failureThreshold)
|
||||
|
||||
// feat: instrument upstream latency — always constructed, regardless
|
||||
// of whether Config.Metrics is nil/disabled; see g.latency's own doc
|
||||
// comment above for why an always-present, usually-empty store is the
|
||||
|
||||
+11
-1
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
"net/url"
|
||||
"sort"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Target kinds handleTargetProxy and resolveTarget dispatch on, selecting
|
||||
@@ -63,6 +64,7 @@ type agentListing struct {
|
||||
// servers, filtered by group.allowsMCP and sorted by name for a
|
||||
// deterministic response.
|
||||
func (g *Gateway) handleMCPServers(w http.ResponseWriter, grp *group) {
|
||||
g.maybeSweepTargetHealth()
|
||||
names := make([]string, 0, len(g.cfg.MCPServers))
|
||||
for name := range g.cfg.MCPServers {
|
||||
if grp.allowsMCP(name) {
|
||||
@@ -86,6 +88,7 @@ func (g *Gateway) handleMCPServers(w http.ResponseWriter, grp *group) {
|
||||
// proxy URL plus its configured card path, defaulting to
|
||||
// defaultAgentCardPath when AgentConfig.Card is empty.
|
||||
func (g *Gateway) handleAgents(w http.ResponseWriter, grp *group) {
|
||||
g.maybeSweepTargetHealth()
|
||||
names := make([]string, 0, len(g.cfg.Agents))
|
||||
for name := range g.cfg.Agents {
|
||||
if grp.allowsAgent(name) {
|
||||
@@ -216,7 +219,14 @@ func (g *Gateway) handleTargetProxy(w http.ResponseWriter, r *http.Request, u *u
|
||||
upstreamURL += "?" + r.URL.RawQuery
|
||||
}
|
||||
|
||||
g.proxyUpstream(w, r, upstreamURL, g.targetClient, nil, nil, false, kind+" target (name "+name+")", g.targetTimeout)
|
||||
// feat/target-health: passive recording (target_health.go) — latency
|
||||
// measured around the whole proxyUpstream call, matching what an
|
||||
// active probe of the same target measures. See
|
||||
// recordTargetProxyHealth's own doc comment for the exact
|
||||
// success/failure rule.
|
||||
start := time.Now()
|
||||
result, ok := g.proxyUpstream(w, r, upstreamURL, g.targetClient, nil, nil, false, kind+" target (name "+name+")", g.targetTimeout)
|
||||
g.recordTargetProxyHealth(kind, name, targetURL, result, ok, time.Since(start))
|
||||
}
|
||||
|
||||
// validateTargetURLs checks every configured MCP-server and agent entry at
|
||||
|
||||
+244
@@ -935,3 +935,247 @@ func TestHandleTargetProxy_PerTargetCounters_AttributedAndCollisionSafe(t *testi
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// --- feat/target-health: passive recording via handleTargetProxy ---
|
||||
|
||||
func newTargetHealthProxyConfig(mcpURL string) *Config {
|
||||
cfg := CreateConfig()
|
||||
cfg.Providers = map[string]*ProviderConfig{"openai": {Type: "openai", APIKey: "k"}}
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: mcpURL}}
|
||||
cfg.Groups = map[string]*GroupConfig{"default": {}}
|
||||
cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice"}}}
|
||||
return cfg
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_200_RecordsHealthy proves an
|
||||
// ordinary successful proxy call records a healthy, "traffic"-sourced
|
||||
// observation.
|
||||
func TestHandleTargetProxy_TargetHealth_200_RecordsHealthy(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
cfg := newTargetHealthProxyConfig(srv.URL)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthHealthy {
|
||||
t.Errorf("state = %q, want healthy", snap.state)
|
||||
}
|
||||
if snap.source != targetHealthSourceTraffic {
|
||||
t.Errorf("source = %q, want %q", snap.source, targetHealthSourceTraffic)
|
||||
}
|
||||
if snap.consecutiveFailures != 0 {
|
||||
t.Errorf("consecutiveFailures = %d, want 0", snap.consecutiveFailures)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_404_RecordsHealthy proves a 4xx
|
||||
// upstream response still counts as reachable — only 5xx (or an outright
|
||||
// failed attempt) is a failure.
|
||||
func TestHandleTargetProxy_TargetHealth_404_RecordsHealthy(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
cfg := newTargetHealthProxyConfig(srv.URL)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthHealthy {
|
||||
t.Errorf("state = %q, want healthy (a 4xx still proves the target answered)", snap.state)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_500_RecordsUnhealthy proves an
|
||||
// upstream 5xx is a failure, distinct from a 4xx above.
|
||||
func TestHandleTargetProxy_TargetHealth_500_RecordsUnhealthy(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
cfg := newTargetHealthProxyConfig(srv.URL)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthUnhealthy {
|
||||
t.Errorf("state = %q, want unhealthy", snap.state)
|
||||
}
|
||||
if snap.lastError == "" {
|
||||
t.Error("want a non-empty lastError naming the 500 status")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_DeadUpstream_RecordsUnhealthy
|
||||
// proves an outright failed attempt (connection refused) is a failure,
|
||||
// exercising proxyResult's status=0/clientCanceled=false shape.
|
||||
func TestHandleTargetProxy_TargetHealth_DeadUpstream_RecordsUnhealthy(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
|
||||
deadURL := srv.URL
|
||||
srv.Close()
|
||||
|
||||
cfg := newTargetHealthProxyConfig(deadURL)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthUnhealthy {
|
||||
t.Errorf("state = %q, want unhealthy", snap.state)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_HangTripsWatchdog_RecordsUnhealthy
|
||||
// proves a stalled upstream that trips the idle-progress watchdog
|
||||
// (timeout.go) is recorded as a failure, not silently ignored — the
|
||||
// hang fixture mirrors TestHandlePassthrough_StalledUpstream_AbortedAtTimeout
|
||||
// (timeout_test.go).
|
||||
func TestHandleTargetProxy_TargetHealth_HangTripsWatchdog_RecordsUnhealthy(t *testing.T) {
|
||||
srv, release := newStallingServer(nil)
|
||||
defer release()
|
||||
|
||||
cfg := newTargetHealthProxyConfig(srv.URL)
|
||||
cfg.RequestTimeout = "60ms"
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
start := time.Now()
|
||||
h.ServeHTTP(rec, req)
|
||||
if elapsed := time.Since(start); elapsed > 3*time.Second {
|
||||
t.Errorf("target proxy took %s to abort a 60ms requestTimeout", elapsed)
|
||||
}
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Fatalf("status = %d, want 502, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthUnhealthy {
|
||||
t.Errorf("state = %q, want unhealthy (the watchdog firing must record a failure)", snap.state)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_ClientCanceled_RecordsNothing
|
||||
// proves a canceled CLIENT request records nothing at all — the target's
|
||||
// own health stays "unknown", not "unhealthy", since the client hanging
|
||||
// up says nothing about the target. F6, feat/target-health review: uses
|
||||
// a channel handshake (cancel only once the handler has genuinely been
|
||||
// reached, then release it) instead of a 30ms-cancel-vs-200ms-sleep
|
||||
// wall-clock margin, which left no slack under load and could either
|
||||
// cancel before the request reached the handler or fail to prove the
|
||||
// cancel raced a still-in-flight request.
|
||||
func TestHandleTargetProxy_TargetHealth_ClientCanceled_RecordsNothing(t *testing.T) {
|
||||
reached := make(chan struct{})
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
close(reached)
|
||||
<-release
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
// Registered in this order so t.Cleanup's LIFO order unblocks the
|
||||
// handler (closing release) BEFORE srv.Close runs — see
|
||||
// TestMaybeSweepTargetHealth_ConcurrentTriggers_SingleFlight's own
|
||||
// comment (target_health_test.go) for the identical reasoning.
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
cfg := newTargetHealthProxyConfig(srv.URL)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/alpha/tools/list", nil).WithContext(ctx)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
<-reached
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
if snap.state != targetHealthUnknown {
|
||||
t.Errorf("state = %q, want unknown (a client cancel must record nothing)", snap.state)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleTargetProxy_TargetHealth_A2A_UsesAgentKind proves an A2A
|
||||
// target proxy call records under targetKindAgent, independent of an MCP
|
||||
// server sharing the same name.
|
||||
func TestHandleTargetProxy_TargetHealth_A2A_UsesAgentKind(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
cfg := CreateConfig()
|
||||
cfg.Providers = map[string]*ProviderConfig{"openai": {Type: "openai", APIKey: "k"}}
|
||||
cfg.Agents = map[string]*AgentConfig{"alpha": {URL: srv.URL}}
|
||||
cfg.Groups = map[string]*GroupConfig{"default": {}}
|
||||
cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice"}}}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/a2a/alpha/tasks", nil)
|
||||
req.Header.Set("Authorization", "Bearer sk-alice")
|
||||
h.ServeHTTP(httptest.NewRecorder(), req)
|
||||
|
||||
if gw.targetHealth.stateOf(targetKindAgent, "alpha") != targetHealthHealthy {
|
||||
t.Errorf("agent/alpha state = %q, want healthy", gw.targetHealth.stateOf(targetKindAgent, "alpha"))
|
||||
}
|
||||
if gw.targetHealth.stateOf(targetKindMCP, "alpha") != targetHealthUnknown {
|
||||
t.Errorf("mcp/alpha state = %q, want unknown (must not be conflated with the agent of the same name)", gw.targetHealth.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
}
|
||||
|
||||
+61
-1
@@ -970,11 +970,55 @@ func (g *Gateway) mcpFederatedToolsList(w http.ResponseWriter, format mcpRespons
|
||||
}
|
||||
}()
|
||||
|
||||
sem <- struct{}{}
|
||||
// G4, feat/target-health review round 2: acquiring the
|
||||
// semaphore via select, not a bare blocking send, lets a
|
||||
// server still parked here when fanoutCtx expires return
|
||||
// WITHOUT ever calling mcpBackendCall — it was never dialed,
|
||||
// so it has nothing to say about that server's own health
|
||||
// (below). Without this, a bare `sem <- struct{}{}` still
|
||||
// unblocks once fanoutCtx dies (an earlier holder's own call
|
||||
// fails fast against the already-expired context and frees
|
||||
// its slot), but by then mcpBackendCall would be called with
|
||||
// a context that is already done — indistinguishable from a
|
||||
// server that WAS dialed and genuinely timed out, which is
|
||||
// exactly the over-suppression this fix removes.
|
||||
select {
|
||||
case sem <- struct{}{}:
|
||||
case <-fanoutCtx.Done():
|
||||
mu.Lock()
|
||||
failed = append(failed, name)
|
||||
mu.Unlock()
|
||||
return
|
||||
}
|
||||
defer func() { <-sem }()
|
||||
|
||||
targetURL := g.cfg.MCPServers[name].URL
|
||||
// feat/target-health: passive recording (target_health.go) —
|
||||
// err != nil is a failure; a JSON-RPC-level resp.Error is NOT
|
||||
// (the server answered, it just reported its own error). The
|
||||
// record call right below already covers both: it reads ok
|
||||
// from err == nil alone, so a non-nil resp.Error alongside a
|
||||
// nil err still records success here — the failed-list
|
||||
// append further down (used only for the merged tools/list
|
||||
// result and the loud-failure short-circuit) is a separate
|
||||
// concern and never touches targetHealth itself.
|
||||
probeStart := time.Now()
|
||||
resp, err := g.mcpBackendCall(fanoutCtx, targetURL, "tools/list", struct{}{}, mcpBackendResponseMaxBytes)
|
||||
// G4: this goroutine WON the select above, so it was actually
|
||||
// dialed — only a client hang-up (context.Canceled,
|
||||
// propagating from r.Context() into fanoutCtx) says nothing
|
||||
// about this server's own health, mirroring
|
||||
// recordTargetProxyHealth's identical client-cancel rule
|
||||
// (target_health.go). context.DeadlineExceeded here means the
|
||||
// shared toolsListBackendTimeout fired while THIS dialed
|
||||
// server was still in flight — it did not answer within the
|
||||
// fan-out's own budget, which is a real failure worth
|
||||
// recording, not a client artifact (F2 over-suppressed this
|
||||
// case; a genuinely hung server used to read "unknown"
|
||||
// forever under federation-only traffic).
|
||||
if err == nil || !errors.Is(err, context.Canceled) {
|
||||
g.targetHealth.record(targetKindMCP, name, targetURL, err == nil, err, time.Since(probeStart), targetHealthSourceTraffic)
|
||||
}
|
||||
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
@@ -1091,7 +1135,23 @@ func (g *Gateway) mcpFederatedToolsCall(w http.ResponseWriter, format mcpRespons
|
||||
// important finding 4): tools/call always resolves to exactly ONE
|
||||
// backend, so the fan-out memory-amplification argument does not
|
||||
// apply here — see mcpBackendCallResponseMaxBytes' own doc comment.
|
||||
// feat/target-health: passive recording (target_health.go) — measured
|
||||
// around the whole outbound call, same rule as tools/list's own fan-
|
||||
// out above: err != nil is a failure, a JSON-RPC-level resp.Error is
|
||||
// not.
|
||||
probeStart := time.Now()
|
||||
resp, err := g.mcpBackendCall(ctx, targetURL, "tools/call", mcpToolCallParams{Name: toolName, Arguments: params.Arguments}, mcpBackendCallResponseMaxBytes)
|
||||
// G4: same dialed-server rule as mcpFederatedToolsList's own fan-out,
|
||||
// above — ctx here derives from r.Context() too, so a client cancel
|
||||
// (context.Canceled) must record nothing. Unlike the fan-out,
|
||||
// tools/call has no semaphore to wait behind: the call above is
|
||||
// always actually dialed, so context.DeadlineExceeded here means
|
||||
// THIS server did not answer within toolsCallBackendTimeout — a real
|
||||
// failure worth recording, not a client artifact (F2 over-suppressed
|
||||
// this case).
|
||||
if err == nil || !errors.Is(err, context.Canceled) {
|
||||
g.targetHealth.record(targetKindMCP, serverName, targetURL, err == nil, err, time.Since(probeStart), targetHealthSourceTraffic)
|
||||
}
|
||||
g.limiter.countTargetRequest(targetKindMCP, serverName)
|
||||
if err != nil {
|
||||
g.logf("federated tools/call: server %q: %v", serverName, err)
|
||||
|
||||
@@ -858,10 +858,12 @@ func TestHandleMCPFederated_ToolsCall_SlowBackend_BoundedByRequestContext(t *tes
|
||||
t.Cleanup(func() { close(block) })
|
||||
|
||||
cfg := newFederationTestConfig(srv.URL, "http://beta.invalid", true)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
callParams, _ := json.Marshal(mcpToolCallParams{Name: "alpha_lookup"})
|
||||
req := newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/call", ID: json.RawMessage("1"), Params: callParams})
|
||||
@@ -884,6 +886,12 @@ func TestHandleMCPFederated_ToolsCall_SlowBackend_BoundedByRequestContext(t *tes
|
||||
if got.Error == nil || got.Error.Code != jsonrpcInternalError {
|
||||
t.Errorf("error = %+v, want internal error (the resolved server timed out)", got.Error)
|
||||
}
|
||||
// G4, feat/target-health review round 2: tools/call always actually
|
||||
// dials its one resolved server (no semaphore to wait behind), so
|
||||
// this shared-budget expiry must record a failure, not nothing.
|
||||
if state := gw.targetHealth.stateOf(targetKindMCP, "alpha"); state != targetHealthUnhealthy {
|
||||
t.Errorf("alpha state = %q, want unhealthy (dialed and never answered within the request's own budget)", state)
|
||||
}
|
||||
}
|
||||
|
||||
// --- MF2: non-2xx status and empty-envelope guards ---
|
||||
@@ -1873,3 +1881,353 @@ func TestMcpBackendCall_SessionCloseFailure_NeverSurfacedToCaller(t *testing.T)
|
||||
t.Fatalf("tools = %+v, want alpha_search despite the session-close failure", result.Tools)
|
||||
}
|
||||
}
|
||||
|
||||
// --- feat/target-health: passive recording from federation traffic ---
|
||||
|
||||
// TestHandleMCPFederated_ToolsList_TargetHealth_RecordsPerServer proves
|
||||
// tools/list's own fan-out records a failure for the one server that
|
||||
// could not be reached, and a healthy observation for every server that
|
||||
// answered — independent of the aggregate response degrading rather
|
||||
// than failing (TestHandleMCPFederated_ToolsList_UnreachableServerDegradesNotFails
|
||||
// already covers the response shape; this proves the target-health side
|
||||
// effect).
|
||||
func TestHandleMCPFederated_ToolsList_TargetHealth_RecordsPerServer(t *testing.T) {
|
||||
alpha := newMockJSONRPCServer(t, []mcpTool{{Name: "lookup"}})
|
||||
cfg := newFederationTestConfig(alpha.srv.URL, "http://127.0.0.1:1", false)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/list", ID: json.RawMessage("1")}))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthHealthy {
|
||||
t.Errorf("alpha state = %q, want healthy", got)
|
||||
}
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "beta"); got != targetHealthUnhealthy {
|
||||
t.Errorf("beta state = %q, want unhealthy (unreachable)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsList_TargetHealth_JSONRPCError_StillHealthy
|
||||
// proves a server that ANSWERS with its own JSON-RPC-level error is
|
||||
// still recorded healthy — it responded, which is what target-health
|
||||
// cares about; only a transport-level failure (err != nil from
|
||||
// mcpBackendCall) counts against it.
|
||||
func TestHandleMCPFederated_ToolsList_TargetHealth_JSONRPCError_StillHealthy(t *testing.T) {
|
||||
alpha := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
var req jsonrpcRequest
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(jsonrpcResponse{ID: req.ID, Error: &jsonrpcError{Code: -32000, Message: "tools unavailable"}})
|
||||
}))
|
||||
defer alpha.Close()
|
||||
|
||||
cfg := newFederationTestConfig(alpha.URL, alpha.URL, false)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/list", ID: json.RawMessage("1")}))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthHealthy {
|
||||
t.Errorf("alpha state = %q, want healthy — a JSON-RPC-level error still means the server answered", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsCall_TargetHealth_RecordsResolvedServerOnly
|
||||
// proves tools/call records health for the ONE resolved server, never
|
||||
// the other configured-but-uncontacted one.
|
||||
func TestHandleMCPFederated_ToolsCall_TargetHealth_RecordsResolvedServerOnly(t *testing.T) {
|
||||
alpha := newMockJSONRPCServer(t, nil)
|
||||
beta := newMockJSONRPCServer(t, nil)
|
||||
cfg := newFederationTestConfig(alpha.srv.URL, beta.srv.URL, false)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
callParams, _ := json.Marshal(mcpToolCallParams{Name: "alpha_lookup"})
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, newFederatedRequest(t, "sk-alice", jsonrpcRequest{
|
||||
JSONRPC: jsonrpcVersion, Method: "tools/call", ID: json.RawMessage("5"), Params: callParams,
|
||||
}))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthHealthy {
|
||||
t.Errorf("alpha state = %q, want healthy", got)
|
||||
}
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "beta"); got != targetHealthUnknown {
|
||||
t.Errorf("beta state = %q, want unknown (never contacted)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsCall_TargetHealth_UnreachableServer_RecordsUnhealthy
|
||||
// proves a resolved-but-unreachable backend records a failure even
|
||||
// though the client sees a generic JSON-RPC internal error, not a raw
|
||||
// transport error.
|
||||
func TestHandleMCPFederated_ToolsCall_TargetHealth_UnreachableServer_RecordsUnhealthy(t *testing.T) {
|
||||
cfg := newFederationTestConfig("http://127.0.0.1:1", "http://127.0.0.1:1", false)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
callParams, _ := json.Marshal(mcpToolCallParams{Name: "alpha_lookup"})
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, newFederatedRequest(t, "sk-alice", jsonrpcRequest{
|
||||
JSONRPC: jsonrpcVersion, Method: "tools/call", ID: json.RawMessage("5"), Params: callParams,
|
||||
}))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthUnhealthy {
|
||||
t.Errorf("alpha state = %q, want unhealthy", got)
|
||||
}
|
||||
}
|
||||
|
||||
// --- feat/target-health F2: context-caused fan-out errors record nothing ---
|
||||
//
|
||||
// A plain connection-refused server still recording a failure is already
|
||||
// covered by TestHandleMCPFederated_ToolsList_TargetHealth_RecordsPerServer
|
||||
// (tools/list) and TestHandleMCPFederated_ToolsCall_TargetHealth_UnreachableServer_RecordsUnhealthy
|
||||
// (tools/call) above — both must keep passing unchanged by the F2 fix
|
||||
// below, which only special-cases context.Canceled/context.DeadlineExceeded.
|
||||
|
||||
// TestHandleMCPFederated_ToolsList_ClientCanceled_RecordsNothing proves F2:
|
||||
// a client cancel mid-fan-out — context.Canceled propagating from
|
||||
// r.Context() into fanoutCtx — records nothing for the servers still in
|
||||
// flight, mirroring recordTargetProxyHealth's own client-cancel rule
|
||||
// (target_health.go): a caller hanging up says nothing about the target's
|
||||
// own health.
|
||||
func TestHandleMCPFederated_ToolsList_ClientCanceled_RecordsNothing(t *testing.T) {
|
||||
reached := make(chan struct{}, 2)
|
||||
release := make(chan struct{})
|
||||
blocking := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached <- struct{}{}
|
||||
<-release
|
||||
})
|
||||
alpha := httptest.NewServer(blocking)
|
||||
beta := httptest.NewServer(blocking)
|
||||
// Registered in this order so t.Cleanup's LIFO order unblocks both
|
||||
// handlers (closing release) BEFORE either server's Close is called —
|
||||
// httptest.Server.Close blocks until outstanding requests complete, so
|
||||
// closing it first would deadlock until the test's own timeout (F6).
|
||||
t.Cleanup(alpha.Close)
|
||||
t.Cleanup(beta.Close)
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
cfg := newFederationTestConfig(alpha.URL, beta.URL, false)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
req := newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/list", ID: json.RawMessage("1")}).WithContext(ctx)
|
||||
|
||||
done := make(chan struct{})
|
||||
rec := httptest.NewRecorder()
|
||||
go func() {
|
||||
h.ServeHTTP(rec, req)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
<-reached
|
||||
<-reached
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthUnknown {
|
||||
t.Errorf("alpha state = %q, want unknown (a client cancel must record nothing)", got)
|
||||
}
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "beta"); got != targetHealthUnknown {
|
||||
t.Errorf("beta state = %q, want unknown (a client cancel must record nothing)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsList_HungDialedServer_RecordsFailure proves
|
||||
// G4 (feat/target-health review round 2): a server that WAS actually
|
||||
// dialed and never answers before the fan-out's own shared budget
|
||||
// expires (toolsListBackendTimeout, or here the incoming request's own
|
||||
// shorter deadline — see
|
||||
// TestHandleMCPFederated_ToolsList_SlowBackend_BoundedByRequestContext's
|
||||
// own doc comment for why that stands in for the real 20s budget) must
|
||||
// still record a failure, not nothing. F2's original guard
|
||||
// blanket-suppressed context.DeadlineExceeded too, which meant a
|
||||
// genuinely hung server read "unknown" forever under federation-only
|
||||
// traffic whenever targetHealth.enabled defaults false — the
|
||||
// over-suppression this fix removes.
|
||||
func TestHandleMCPFederated_ToolsList_HungDialedServer_RecordsFailure(t *testing.T) {
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
<-release
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
cfg := newFederationTestConfig(srv.URL, "http://beta.invalid", true)
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/list", ID: json.RawMessage("1")})
|
||||
shortCtx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
|
||||
defer cancel()
|
||||
req = req.WithContext(shortCtx)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthUnhealthy {
|
||||
t.Errorf("alpha state = %q, want unhealthy (dialed and never answered within the fan-out budget)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsList_NeverDialedSemaphoreStarved_RecordsNothing
|
||||
// proves G4's other half: a server still parked behind the fan-out
|
||||
// semaphore when the shared deadline fires — never actually dialed —
|
||||
// must still record nothing, unlike a server that WAS dialed (the test
|
||||
// above). One more server than the fan-out semaphore has slots for
|
||||
// (mcpFederatedFanoutConcurrency) guarantees at least one is never
|
||||
// dialed; draining exactly mcpFederatedFanoutConcurrency sends on
|
||||
// reached first proves which servers WERE actually dialed before any
|
||||
// assertion runs, rather than assuming which specific names win the
|
||||
// race. snapshot's own "observed" field (target_health.go), not stateOf,
|
||||
// is what actually distinguishes "recorded a failure, still below
|
||||
// failureThreshold" from "never recorded at all" — both read state
|
||||
// targetHealthUnknown (F4), so FailureThreshold: 1 below also makes a
|
||||
// dialed server's single failure visible as targetHealthUnhealthy
|
||||
// through stateOf too, for a belt-and-suspenders check.
|
||||
func TestHandleMCPFederated_ToolsList_NeverDialedSemaphoreStarved_RecordsNothing(t *testing.T) {
|
||||
reached := make(chan struct{}, mcpFederatedFanoutConcurrency)
|
||||
block := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
reached <- struct{}{}
|
||||
<-block
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(func() { close(block) })
|
||||
|
||||
names := make([]string, 0, mcpFederatedFanoutConcurrency+1)
|
||||
mcpServers := make(map[string]*TargetConfig, mcpFederatedFanoutConcurrency+1)
|
||||
for i := 0; i < mcpFederatedFanoutConcurrency+1; i++ {
|
||||
name := fmt.Sprintf("srv%d", i)
|
||||
names = append(names, name)
|
||||
mcpServers[name] = &TargetConfig{URL: srv.URL}
|
||||
}
|
||||
|
||||
cfg := CreateConfig()
|
||||
cfg.Providers = map[string]*ProviderConfig{"openai": {Type: "openai", APIKey: "k"}}
|
||||
cfg.MCPServers = mcpServers
|
||||
cfg.Groups = map[string]*GroupConfig{"default": {}}
|
||||
cfg.Users = &UsersConfig{Inline: []*UserConfig{{Name: "alice", Group: "default", APIKey: "sk-alice"}}}
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
req := newFederatedRequest(t, "sk-alice", jsonrpcRequest{JSONRPC: jsonrpcVersion, Method: "tools/list", ID: json.RawMessage("1")})
|
||||
shortCtx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond)
|
||||
defer cancel()
|
||||
req = req.WithContext(shortCtx)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
h.ServeHTTP(rec, req)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
for i := 0; i < mcpFederatedFanoutConcurrency; i++ {
|
||||
<-reached
|
||||
}
|
||||
<-done
|
||||
|
||||
dialed, neverDialed := 0, 0
|
||||
for _, name := range names {
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, name)
|
||||
if snap.observed {
|
||||
dialed++
|
||||
if snap.state != targetHealthUnhealthy {
|
||||
t.Errorf("mcp/%s state = %q, want unhealthy (dialed and never answered)", name, snap.state)
|
||||
}
|
||||
continue
|
||||
}
|
||||
neverDialed++
|
||||
}
|
||||
if dialed != mcpFederatedFanoutConcurrency {
|
||||
t.Errorf("dialed = %d, want %d (exactly one server must never have been dialed)", dialed, mcpFederatedFanoutConcurrency)
|
||||
}
|
||||
if neverDialed != 1 {
|
||||
t.Errorf("neverDialed = %d, want exactly 1", neverDialed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleMCPFederated_ToolsCall_ClientCanceled_RecordsNothing mirrors
|
||||
// TestHandleMCPFederated_ToolsList_ClientCanceled_RecordsNothing for
|
||||
// tools/call's own, separate context.WithTimeout(r.Context(), ...) call
|
||||
// site.
|
||||
func TestHandleMCPFederated_ToolsCall_ClientCanceled_RecordsNothing(t *testing.T) {
|
||||
reached := make(chan struct{})
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
close(reached)
|
||||
<-release
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
cfg := newFederationTestConfig(srv.URL, "http://beta.invalid", true)
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
gw := h.(*Gateway)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
callParams, _ := json.Marshal(mcpToolCallParams{Name: "alpha_lookup"})
|
||||
req := newFederatedRequest(t, "sk-alice", jsonrpcRequest{
|
||||
JSONRPC: jsonrpcVersion, Method: "tools/call", ID: json.RawMessage("1"), Params: callParams,
|
||||
}).WithContext(ctx)
|
||||
|
||||
done := make(chan struct{})
|
||||
rec := httptest.NewRecorder()
|
||||
go func() {
|
||||
h.ServeHTTP(rec, req)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
<-reached
|
||||
cancel()
|
||||
<-done
|
||||
|
||||
if got := gw.targetHealth.stateOf(targetKindMCP, "alpha"); got != targetHealthUnknown {
|
||||
t.Errorf("alpha state = %q, want unknown (a client cancel must record nothing)", got)
|
||||
}
|
||||
}
|
||||
|
||||
+57
@@ -170,6 +170,7 @@ func (g *Gateway) metricsSourceAllowed(r *http.Request) bool {
|
||||
// disk cache, and its declared Content-Type must never be second-guessed
|
||||
// by a client's MIME sniffer.
|
||||
func (g *Gateway) serveMetrics(w http.ResponseWriter) {
|
||||
g.maybeSweepTargetHealth()
|
||||
w.Header().Set("Content-Type", "text/plain; version=0.0.4; charset=utf-8")
|
||||
w.Header().Set("X-Content-Type-Options", "nosniff")
|
||||
w.Header().Set("Cache-Control", "no-store")
|
||||
@@ -448,6 +449,16 @@ const aggregationNotePerProcess = " AGGREGATION ACROSS REPLICAS: this counter li
|
||||
// instant (e.g. one mid-backoff, one already recovered).
|
||||
const aggregationNoteProviderHealthy = " AGGREGATION ACROSS REPLICAS: each replica runs its own discovery circuit breaker in-process, never shared via Redis, so this can legitimately differ per replica. Read it per-instance where possible; if you must aggregate, use min() to surface \"at least one replica sees this provider as unhealthy\" — sum() is meaningless for a 0/1 gauge."
|
||||
|
||||
// aggregationNoteTargetHealthy is appended to llmgateway_target_healthy's
|
||||
// HELP text (feat/target-health): the tracker behind this gauge
|
||||
// (target_health.go) is per-process, in-memory state, fed by each
|
||||
// replica's own passive traffic and, when enabled, its own active probe
|
||||
// sweep — never shared via Redis. A sibling to
|
||||
// aggregationNoteProviderHealthy above, not a reuse of it: this
|
||||
// tracker has no discovery step of its own, so that note's own
|
||||
// reference to "discovery circuit breaker" would not apply here.
|
||||
const aggregationNoteTargetHealthy = " AGGREGATION ACROSS REPLICAS: each replica tracks this target's health independently from its own passive traffic and, if enabled, its own active probes — never shared via Redis — so this can legitimately differ per replica. Read it per-instance where possible; if you must aggregate, use min() to surface \"at least one replica sees this target as unhealthy\" — sum() is meaningless for a 0/1 gauge."
|
||||
|
||||
// aggregationNoteStoreHealth is appended to llmgateway_limit_store_up's
|
||||
// HELP text: whether THIS replica's own connection to the configured
|
||||
// limit store is currently healthy is, by definition, discovered
|
||||
@@ -470,6 +481,7 @@ func (g *Gateway) renderMetrics() []byte {
|
||||
var m metricWriter
|
||||
g.writeUsageMetrics(&m)
|
||||
g.writeProviderMetrics(&m)
|
||||
g.writeTargetMetrics(&m)
|
||||
g.writeLatencyMetrics(&m)
|
||||
g.writeProvenanceMetrics(&m)
|
||||
g.writeRejectionMetrics(&m)
|
||||
@@ -673,6 +685,51 @@ func (g *Gateway) writeProviderMetrics(m *metricWriter) {
|
||||
}
|
||||
}
|
||||
|
||||
// writeTargetMetrics emits llmgateway_target_healthy (feat/target-health):
|
||||
// 1 for a healthy MCP server or A2A agent, 0 for an unhealthy one, and NO
|
||||
// sample at all for a target this tracker has never observed — the
|
||||
// identical "a gap is honest, a fabricated value is not" convention
|
||||
// writeUsageMetrics/writeProviderMetrics already apply to a storeDown
|
||||
// read, applied here to "never yet observed" instead of "currently
|
||||
// unreadable". Iterates every CONFIGURED MCP server and agent (not just
|
||||
// ones the tracker happens to have an entry for), so a target removed
|
||||
// from config since its last observation contributes no stale series.
|
||||
// F7, feat/target-health review: returns before emitting even the family's
|
||||
// own HELP/TYPE header when no MCP server or agent is configured at all —
|
||||
// mirrors writeStoreHealthMetrics' identical "nothing to report, no family
|
||||
// either" early return above, rather than a HELP/TYPE line promising a
|
||||
// metric family that can never carry a sample in this deployment.
|
||||
func (g *Gateway) writeTargetMetrics(m *metricWriter) {
|
||||
if len(g.cfg.MCPServers) == 0 && len(g.cfg.Agents) == 0 {
|
||||
return
|
||||
}
|
||||
m.family("llmgateway_target_healthy", "gauge",
|
||||
"1 when an MCP server or A2A agent's health (last passive observation or active probe) is healthy, 0 when its consecutive-failure count has reached targetHealth.failureThreshold. No sample while never yet observed."+aggregationNoteTargetHealthy)
|
||||
|
||||
for _, name := range sortedMCPServerNames(g.cfg.MCPServers) {
|
||||
writeTargetHealthySample(m, g.targetHealth.snapshot(targetKindMCP, name), targetScopeKind(targetKindMCP), name)
|
||||
}
|
||||
for _, name := range sortedAgentNames(g.cfg.Agents) {
|
||||
writeTargetHealthySample(m, g.targetHealth.snapshot(targetKindAgent, name), targetScopeKind(targetKindAgent), name)
|
||||
}
|
||||
}
|
||||
|
||||
// writeTargetHealthySample writes llmgateway_target_healthy's one sample
|
||||
// for (kindLabel, name), skipped entirely while snap's state is unknown
|
||||
// — see writeTargetMetrics' own doc comment for why. kindLabel is the
|
||||
// external "mcp"/"agent" spelling (targetScopeKind), never the internal
|
||||
// routing kind ("a2a") targetHealthTracker itself keys on.
|
||||
func writeTargetHealthySample(m *metricWriter, snap targetHealthSnapshot, kindLabel, name string) {
|
||||
if snap.state == targetHealthUnknown {
|
||||
return
|
||||
}
|
||||
healthy := int64(1)
|
||||
if snap.state == targetHealthUnhealthy {
|
||||
healthy = 0
|
||||
}
|
||||
m.sampleInt("llmgateway_target_healthy", []metricLabel{{"kind", kindLabel}, {"target", name}}, healthy)
|
||||
}
|
||||
|
||||
// --- upstream latency (feat: instrument upstream latency) ---
|
||||
//
|
||||
// The task: "priority-based throttling that engages when providers are
|
||||
|
||||
+127
@@ -3,6 +3,7 @@ package traefikllmgateway
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"net/http"
|
||||
@@ -1494,3 +1495,129 @@ func TestRecordLatency_MetricsDisabled_NoObservationCollected(t *testing.T) {
|
||||
t.Errorf("g.latency has %d entries after a request with metrics disabled, want 0 — no latency observation should ever be collected when metrics is off", len(snaps))
|
||||
}
|
||||
}
|
||||
|
||||
// --- feat/target-health: llmgateway_target_healthy ---
|
||||
|
||||
// TestMetrics_TargetHealthy_NoSampleWhileUnknown proves a configured but
|
||||
// never-observed target contributes NO sample at all — the same
|
||||
// "a gap is honest, a fabricated value is not" rule the store-backed
|
||||
// families apply to storeDown, applied here to "never yet observed".
|
||||
func TestMetrics_TargetHealthy_NoSampleWhileUnknown(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newMetricsTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, _ := newMetricsGatewayHandle(t, cfg)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
_, samples := parsePrometheusText(t, rec.Body.Bytes())
|
||||
for _, s := range samples {
|
||||
if s.name == "llmgateway_target_healthy" {
|
||||
t.Errorf("unexpected sample %v — a never-observed target must emit nothing", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetrics_TargetHealthy_NoSampleWhileNeverSucceeded proves F4's
|
||||
// second no-sample case: a target that HAS been observed, but has never
|
||||
// once succeeded and is still below failureThreshold, reads "unknown"
|
||||
// (TestTargetHealthTracker_FirstObservationFailure_ReadsUnknown) and
|
||||
// therefore still emits no llmgateway_target_healthy sample — the same
|
||||
// "a gap is honest, a fabricated value is not" rule the never-observed
|
||||
// case above applies, now extended to "observed but never succeeded".
|
||||
func TestMetrics_TargetHealthy_NoSampleWhileNeverSucceeded(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newMetricsTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, gw := newMetricsGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", false, errors.New("boom"), time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
_, samples := parsePrometheusText(t, rec.Body.Bytes())
|
||||
for _, s := range samples {
|
||||
if s.name == "llmgateway_target_healthy" {
|
||||
t.Errorf("unexpected sample %v — a target that has never succeeded must emit nothing", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetrics_TargetHealthy_FamilyAbsentWhenNoTargetsConfigured proves
|
||||
// F7: writeTargetMetrics omits even llmgateway_target_healthy's own
|
||||
// HELP/TYPE header when no MCP server or agent is configured at all —
|
||||
// mirroring writeStoreHealthMetrics' identical "nothing to report, no
|
||||
// family either" behavior when Redis is unconfigured.
|
||||
func TestMetrics_TargetHealthy_FamilyAbsentWhenNoTargetsConfigured(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newMetricsTestConfig()
|
||||
h, _ := newMetricsGatewayHandle(t, cfg)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
types, _ := parsePrometheusText(t, rec.Body.Bytes())
|
||||
if _, ok := types["llmgateway_target_healthy"]; ok {
|
||||
t.Error("want llmgateway_target_healthy's TYPE line absent when no MCP servers or agents are configured")
|
||||
}
|
||||
if strings.Contains(rec.Body.String(), "llmgateway_target_healthy") {
|
||||
t.Errorf("body unexpectedly mentions llmgateway_target_healthy: %s", rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetrics_TargetHealthy_ReflectsRecordedState proves the gauge
|
||||
// renders 1 for a healthy MCP server and 0 for an unhealthy agent, with
|
||||
// the documented kind label values ("mcp"/"agent" — NOT the internal
|
||||
// routing kind "a2a").
|
||||
func TestMetrics_TargetHealthy_ReflectsRecordedState(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newMetricsTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
cfg.Agents = map[string]*AgentConfig{"bot1": {URL: "http://agent-bot1.internal"}}
|
||||
cfg.TargetHealth = TargetHealthConfig{FailureThreshold: 1}
|
||||
h, gw := newMetricsGatewayHandle(t, cfg)
|
||||
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", true, nil, time.Millisecond, targetHealthSourceTraffic)
|
||||
gw.targetHealth.record(targetKindAgent, "bot1", "", false, errors.New("boom"), time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
_, samples := parsePrometheusText(t, rec.Body.Bytes())
|
||||
assertSample(t, samples, "llmgateway_target_healthy", map[string]string{"kind": "mcp", "target": "alpha"}, "1")
|
||||
assertSample(t, samples, "llmgateway_target_healthy", map[string]string{"kind": "agent", "target": "bot1"}, "0")
|
||||
}
|
||||
|
||||
// TestMetrics_TargetHealthy_UnaffectedByStoreOutage proves this gauge is
|
||||
// unaffected by a limit-store outage — it reads the in-process
|
||||
// target-health tracker only, never the store, mirroring
|
||||
// llmgateway_provider_healthy's own identical independence
|
||||
// (TestMetrics_StoreDown_SkipsRatherThanFabricatesZero's own doc
|
||||
// comment explains why that one is asserted present during an outage;
|
||||
// this is the same claim for this feature's gauge).
|
||||
func TestMetrics_TargetHealthy_UnaffectedByStoreOutage(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := newMetricsTestConfig()
|
||||
cfg.MCPServers = map[string]*TargetConfig{"alpha": {URL: "http://mcp-alpha.internal"}}
|
||||
h, gw := newMetricsGatewayHandle(t, cfg)
|
||||
gw.targetHealth.record(targetKindMCP, "alpha", "", true, nil, time.Millisecond, targetHealthSourceTraffic)
|
||||
gw.limiter.store = alwaysErrStore{}
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, metricsRequest(http.MethodGet, metricsPathDefault, "sk-admin1"))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
_, samples := parsePrometheusText(t, rec.Body.Bytes())
|
||||
assertSample(t, samples, "llmgateway_target_healthy", map[string]string{"kind": "mcp", "target": "alpha"}, "1")
|
||||
}
|
||||
|
||||
+24
-5
@@ -910,8 +910,27 @@ func (g *Gateway) handlePassthrough(w http.ResponseWriter, r *http.Request, u *u
|
||||
// nil unless accountJSON was true and the response's Content-Type was
|
||||
// application/json.
|
||||
type proxyResult struct {
|
||||
tee *cappedAccountingBuffer
|
||||
tee *cappedAccountingBuffer
|
||||
// status is the upstream response's own status code, once headers
|
||||
// were actually written to w — 0 when proxyUpstream returned before
|
||||
// ever writing them (a build/connection failure, or a client cancel
|
||||
// before the request was even sent). Additive field for
|
||||
// feat/target-health's passive recording (recordTargetProxyHealth,
|
||||
// target_health.go): ok=false alone conflates "the upstream never
|
||||
// answered at all" with "it answered with a 5xx", which that
|
||||
// feature's failure rule needs to distinguish. Field order (pointer,
|
||||
// then int, then the two bools) is fieldalignment-sensitive, the same
|
||||
// convention this package's other structs already follow.
|
||||
status int
|
||||
isJSON bool
|
||||
// clientCanceled is true exactly when either of proxyUpstream's own
|
||||
// errors.Is(err, context.Canceled) branches fired — the CLIENT, not
|
||||
// the upstream, ended the request, whether before the upstream call
|
||||
// was even sent or mid-copy of an already-started response.
|
||||
// feat/target-health's passive recording must record NOTHING for
|
||||
// this case (handleTargetProxy's own doc comment, mcp_a2a.go): a
|
||||
// client hanging up says nothing about the target's own health.
|
||||
clientCanceled bool
|
||||
}
|
||||
|
||||
// proxyUpstream is the shared reverse-proxy core behind both native
|
||||
@@ -1054,7 +1073,7 @@ func (g *Gateway) proxyUpstream(w http.ResponseWriter, r *http.Request, upstream
|
||||
cancel()
|
||||
if errors.Is(err, context.Canceled) {
|
||||
g.logf("%s: client canceled request: %v", logPrefix, err)
|
||||
return proxyResult{}, false
|
||||
return proxyResult{clientCanceled: true}, false
|
||||
}
|
||||
g.errorf("%s: upstream connection error: %v", logPrefix, err)
|
||||
writeOAIError(w, http.StatusBadGateway, "server_error", "upstream connection error")
|
||||
@@ -1099,10 +1118,10 @@ func (g *Gateway) proxyUpstream(w http.ResponseWriter, r *http.Request, upstream
|
||||
// — the latter falls through to the errorf below, unchanged.
|
||||
if errors.Is(err, context.Canceled) {
|
||||
g.logf("%s: client canceled request: %v", logPrefix, err)
|
||||
return proxyResult{isJSON: isJSON, tee: tee}, false
|
||||
return proxyResult{isJSON: isJSON, tee: tee, status: resp.StatusCode, clientCanceled: true}, false
|
||||
}
|
||||
g.errorf("%s: stream response body: %v", logPrefix, err)
|
||||
return proxyResult{isJSON: isJSON, tee: tee}, false
|
||||
return proxyResult{isJSON: isJSON, tee: tee, status: resp.StatusCode}, false
|
||||
}
|
||||
return proxyResult{isJSON: isJSON, tee: tee}, true
|
||||
return proxyResult{isJSON: isJSON, tee: tee, status: resp.StatusCode}, true
|
||||
}
|
||||
|
||||
@@ -0,0 +1,628 @@
|
||||
package traefikllmgateway
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
)
|
||||
|
||||
// This file implements feat/target-health: a per-replica, in-memory
|
||||
// health tracker for MCP servers and A2A agents ("targets"), which have
|
||||
// no health signal today the way providers do (registry.go's discovery
|
||||
// circuit breaker, failover.go's requestHealthTracker). Two feeds keep
|
||||
// it current: always-on PASSIVE recording from real request traffic
|
||||
// (handleTargetProxy, mcp_a2a.go; mcpFederatedToolsList/
|
||||
// mcpFederatedToolsCall, mcp_federation.go) and an opt-in, lazy ACTIVE
|
||||
// probe sweep (maybeSweepTargetHealth, below). Exposure is always on
|
||||
// too: GET /admin/api/targets (admin.go) and the llmgateway_target_healthy
|
||||
// gauge (metrics.go) both read this tracker regardless of
|
||||
// Config.TargetHealth.Enabled — that flag gates only the active probe
|
||||
// sweep. This tracker is OBSERVE-ONLY: nothing in mcp_a2a.go or
|
||||
// mcp_federation.go ever skips a server or agent because it reports
|
||||
// unhealthy — federation routing behavior is unchanged by this feature.
|
||||
|
||||
// targetHealthState is the tri-state health readout stateOf/snapshot
|
||||
// report for one target, mirroring the discovery breaker's healthState
|
||||
// string shape (registry.go's providerState) so the admin API and
|
||||
// dashboard can treat both consistently.
|
||||
type targetHealthState string
|
||||
|
||||
const (
|
||||
// targetHealthUnknown covers two distinct situations (F4,
|
||||
// feat/target-health review), both read the SAME state on purpose —
|
||||
// neither has ever demonstrated the target actually works:
|
||||
// 1. never observed at all — the tracker has no entry for this
|
||||
// (kind, name) yet, a target configured but never yet reached.
|
||||
// 2. observed, but never once succeeded — every observation so far
|
||||
// is a failure, and consecutiveFailures has not yet reached
|
||||
// failureThreshold. Reading "healthy" here (the pre-F4 behavior)
|
||||
// would be a false claim from a target this tracker has never
|
||||
// actually seen answer; reading "unhealthy" before the threshold
|
||||
// is reached would discard the threshold's own protection
|
||||
// against a single transient first failure. See
|
||||
// targetHealthSnapshot.observed for how the admin view (admin.go)
|
||||
// tells these two apart despite sharing this one state.
|
||||
targetHealthUnknown targetHealthState = "unknown"
|
||||
// targetHealthHealthy: succeeded at least once, and consecutiveFailures
|
||||
// has not yet reached failureThreshold. This DELIBERATELY includes a
|
||||
// target whose most recent single observation was itself a failure:
|
||||
// 1 or 2 recent failures after a success still read "healthy" until
|
||||
// the configured threshold is actually crossed — the same
|
||||
// consecutive-failure gate the discovery circuit breaker uses
|
||||
// (registry.go's providerState/recordHealthLocked), reimplemented
|
||||
// independently here since this tracker carries no open/half-open
|
||||
// state machine of its own (see this file's own package doc comment
|
||||
// for why).
|
||||
targetHealthHealthy targetHealthState = "healthy"
|
||||
// targetHealthUnhealthy: consecutiveFailures >= failureThreshold,
|
||||
// regardless of whether the target has ever succeeded — the
|
||||
// threshold gate always takes priority over the never-succeeded
|
||||
// check above.
|
||||
targetHealthUnhealthy targetHealthState = "unhealthy"
|
||||
)
|
||||
|
||||
// Source values record's callers pass — which mechanism produced the
|
||||
// last observation, exposed on GET /admin/api/targets and stored per
|
||||
// entry.
|
||||
const (
|
||||
targetHealthSourceProbe = "probe"
|
||||
targetHealthSourceTraffic = "traffic"
|
||||
)
|
||||
|
||||
// targetHealthMaxErrorRunes bounds targetHealthEntry.lastError: an
|
||||
// upstream MCP/A2A target's own error text is not operator-controlled
|
||||
// the way its URL is, so a pathological error string must not grow this
|
||||
// in-memory tracker unbounded. Runes, not bytes (truncateRunes, below)
|
||||
// — a byte-level cut could split a multi-byte UTF-8 sequence.
|
||||
const targetHealthMaxErrorRunes = 200
|
||||
|
||||
// targetHealthKey identifies one tracked target by routing kind
|
||||
// (targetKindMCP or targetKindAgent, mcp_a2a.go — NOT scopeKindAgent,
|
||||
// which is the limiter/admin-API-facing "agent" spelling for a2a
|
||||
// targets) and configured name.
|
||||
type targetHealthKey struct {
|
||||
kind string
|
||||
name string
|
||||
}
|
||||
|
||||
// targetHealthEntry is one target's current health state, guarded by its
|
||||
// owning tracker's own mu (never its own) — mirrors latencyHistogram's
|
||||
// identical "guarded by the parent store's lock" convention
|
||||
// (metrics.go).
|
||||
type targetHealthEntry struct {
|
||||
lastCheck time.Time
|
||||
lastError string
|
||||
source string
|
||||
latency time.Duration
|
||||
consecutiveFailures int
|
||||
// everOK is set true the first time record observes a success for
|
||||
// this entry, and never cleared afterward — stateForLocked's F4
|
||||
// "never succeeded yet reads unknown, not healthy" rule reads this,
|
||||
// not consecutiveFailures alone.
|
||||
everOK bool
|
||||
}
|
||||
|
||||
// targetHealthTracker is feat/target-health's own per-pod, in-memory
|
||||
// health signal for every configured MCP server and A2A agent.
|
||||
// Deliberately separate from requestHealthTracker (failover.go) and the
|
||||
// discovery circuit breaker (registry.go): this tracker carries no
|
||||
// open/half-open/backoff state machine and never gates routing — it
|
||||
// exists purely for operational visibility.
|
||||
//
|
||||
// Every method is nil-receiver-safe, matching this package's own
|
||||
// nil-means-disabled convention (requestHealthTracker's own doc
|
||||
// comment, failover.go) — a Gateway assembled directly in a test,
|
||||
// bypassing newGateway, degrades to "every target reads unknown" rather
|
||||
// than a nil-pointer panic, even though newGateway itself always
|
||||
// constructs a real one (see Gateway.targetHealth's own doc comment,
|
||||
// llmgateway.go).
|
||||
type targetHealthTracker struct {
|
||||
entries map[targetHealthKey]*targetHealthEntry
|
||||
nowFn func() time.Time
|
||||
// failureThreshold is TargetHealthConfig.FailureThreshold, validated
|
||||
// and defaulted once at construction (validateTargetHealthConfig) —
|
||||
// copied here rather than re-read from Config on every stateOf call,
|
||||
// the same resolved-value-not-raw-config shape breakerConfig/
|
||||
// failoverConfig already use.
|
||||
failureThreshold int
|
||||
mu sync.Mutex
|
||||
}
|
||||
|
||||
// newTargetHealthTracker returns an empty tracker: every target reads
|
||||
// "unknown" until its first recorded observation. threshold is the
|
||||
// resolved (validated, defaulted) TargetHealthConfig.FailureThreshold.
|
||||
func newTargetHealthTracker(threshold int) *targetHealthTracker {
|
||||
return &targetHealthTracker{entries: make(map[targetHealthKey]*targetHealthEntry), nowFn: time.Now, failureThreshold: threshold}
|
||||
}
|
||||
|
||||
// record accounts one observation (a real proxied request, or an active
|
||||
// probe) for (kind, name): success resets consecutiveFailures to 0 and
|
||||
// clears lastError; failure increments consecutiveFailures and stores
|
||||
// err's message, truncated to targetHealthMaxErrorRunes. Every caller
|
||||
// today (probeMCPTarget/probeAgentTarget/recordTargetProxyHealth, below;
|
||||
// mcpFederatedToolsList/mcpFederatedToolsCall, mcp_federation.go) always
|
||||
// passes a non-nil err alongside ok=false — a failure with no Go error to
|
||||
// attach is not a shape this tracker currently has to handle.
|
||||
// rawURL is (kind, name)'s configured target URL — the exact string a
|
||||
// caller dialed, used ONLY to scrub err's text through sanitizeTargetErr
|
||||
// (below) before it is stored: a Go *url.Error embeds the dialed URL —
|
||||
// verbatim, OR with its password masked by net/http's own stripPassword
|
||||
// when one is present — so an operator-configured target URL carrying
|
||||
// credentials (an "api-key" query parameter, userinfo) would otherwise
|
||||
// leak through GET /admin/api/targets' lastError field the same way an
|
||||
// unsanitized provider error once could (F1, feat/target-health review;
|
||||
// G1, review round 2, for the password-masked form). The scrub runs
|
||||
// BEFORE truncateRunes, not after: truncating first could cut a long
|
||||
// embedded URL in half, leaving the scrub's exact-substring match unable
|
||||
// to find it and a fragment of the credential in the truncated result.
|
||||
// source is targetHealthSourceProbe or
|
||||
// targetHealthSourceTraffic — whichever mechanism produced this
|
||||
// observation.
|
||||
func (t *targetHealthTracker) record(kind, name, rawURL string, ok bool, err error, latency time.Duration, source string) {
|
||||
if t == nil || name == "" {
|
||||
return
|
||||
}
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
key := targetHealthKey{kind: kind, name: name}
|
||||
e, exists := t.entries[key]
|
||||
if !exists {
|
||||
e = &targetHealthEntry{}
|
||||
t.entries[key] = e
|
||||
}
|
||||
e.lastCheck = t.nowFn()
|
||||
e.latency = latency
|
||||
e.source = source
|
||||
if ok {
|
||||
e.everOK = true
|
||||
e.consecutiveFailures = 0
|
||||
e.lastError = ""
|
||||
return
|
||||
}
|
||||
e.consecutiveFailures++
|
||||
if err != nil {
|
||||
e.lastError = truncateRunes(sanitizeTargetErr(err.Error(), rawURL), targetHealthMaxErrorRunes)
|
||||
}
|
||||
}
|
||||
|
||||
// sanitizeTargetErr scrubs msg of rawURL's own credentials, in both forms
|
||||
// a real error from a call against rawURL can actually carry it (G1,
|
||||
// feat/target-health review round 2). Go's net/http builds a *url.Error
|
||||
// via its own unexported stripPassword (net/http/client.go) whenever the
|
||||
// dialed URL's userinfo carries a password: it rewrites "user:pass@" (and
|
||||
// a token-as-username-with-no-password, "token:@") to "user:***@" IN THE
|
||||
// URL BEFORE the error string is ever built — so for such a URL, msg
|
||||
// never contains rawURL verbatim, and a plain sanitizeProviderErr(msg,
|
||||
// rawURL) call (admin.go) — which only matches rawURL verbatim — finds
|
||||
// nothing to scrub, leaking the password and any query-string credential
|
||||
// riding the same URL (an "api-key" value) straight through. This
|
||||
// function first tries the plain match (the common case: no userinfo, or
|
||||
// userinfo with no password, both pass through net/http unmasked and
|
||||
// still match verbatim), then — only when rawURL's userinfo actually
|
||||
// carries a password — builds net/http's exact masked form itself
|
||||
// (mirroring stripPassword's own strings.Replace call precisely) and
|
||||
// scrubs against that too.
|
||||
func sanitizeTargetErr(msg, rawURL string) string {
|
||||
msg = sanitizeProviderErr(msg, rawURL)
|
||||
u, parseErr := url.Parse(rawURL)
|
||||
if parseErr != nil || u.User == nil {
|
||||
return msg
|
||||
}
|
||||
if _, hasPassword := u.User.Password(); !hasPassword {
|
||||
return msg
|
||||
}
|
||||
masked := strings.Replace(u.String(), u.User.String()+"@", u.User.Username()+":***@", 1)
|
||||
return sanitizeProviderErr(msg, masked)
|
||||
}
|
||||
|
||||
// stateOf reports (kind, name)'s current targetHealthState — see this
|
||||
// type's own doc comment for the exact unknown/healthy/unhealthy rule.
|
||||
func (t *targetHealthTracker) stateOf(kind, name string) targetHealthState {
|
||||
if t == nil {
|
||||
return targetHealthUnknown
|
||||
}
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
e, ok := t.entries[targetHealthKey{kind: kind, name: name}]
|
||||
if !ok {
|
||||
return targetHealthUnknown
|
||||
}
|
||||
return t.stateForLocked(e)
|
||||
}
|
||||
|
||||
// stateForLocked derives e's targetHealthState. Caller must hold t.mu.
|
||||
// Split out of stateOf/snapshot so both apply the identical rule. The
|
||||
// threshold check runs first and always wins (F4): a target that has
|
||||
// failed failureThreshold times straight reads unhealthy whether or not
|
||||
// it has ever succeeded. Below threshold, a target that has never once
|
||||
// succeeded reads unknown rather than healthy — see targetHealthUnknown's
|
||||
// own doc comment for why.
|
||||
func (t *targetHealthTracker) stateForLocked(e *targetHealthEntry) targetHealthState {
|
||||
if e.consecutiveFailures >= t.failureThreshold {
|
||||
return targetHealthUnhealthy
|
||||
}
|
||||
if !e.everOK {
|
||||
return targetHealthUnknown
|
||||
}
|
||||
return targetHealthHealthy
|
||||
}
|
||||
|
||||
// targetHealthSnapshot is one target's fully-copied health state, safe
|
||||
// to read after targetHealthTracker.snapshot returns without holding its
|
||||
// lock — mirrors latencySnapshot's identical shape (metrics.go).
|
||||
type targetHealthSnapshot struct {
|
||||
lastCheck time.Time
|
||||
state targetHealthState
|
||||
lastError string
|
||||
source string
|
||||
latency time.Duration
|
||||
consecutiveFailures int
|
||||
// observed is true iff this tracker holds an entry for (kind, name)
|
||||
// — i.e. record has been called at least once, regardless of
|
||||
// success or failure. F4, feat/target-health review: state alone
|
||||
// cannot tell apart targetHealthUnknown's two cases (never observed
|
||||
// vs. observed-but-never-succeeded), so targetHealthView (admin.go)
|
||||
// reads this field, not state, to decide whether to omit
|
||||
// lastCheck/lastError/source/latencyMs — omit only while !observed.
|
||||
observed bool
|
||||
}
|
||||
|
||||
// snapshot returns a copy of (kind, name)'s current health state —
|
||||
// buildAdminTargets (admin.go) and writeTargetMetrics (metrics.go) both
|
||||
// read through this, once per configured target, rather than exposing
|
||||
// t.entries directly. A target this tracker has never observed returns
|
||||
// a zero-valued, !observed snapshot with state targetHealthUnknown.
|
||||
func (t *targetHealthTracker) snapshot(kind, name string) targetHealthSnapshot {
|
||||
if t == nil {
|
||||
return targetHealthSnapshot{state: targetHealthUnknown}
|
||||
}
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
e, ok := t.entries[targetHealthKey{kind: kind, name: name}]
|
||||
if !ok {
|
||||
return targetHealthSnapshot{state: targetHealthUnknown}
|
||||
}
|
||||
return targetHealthSnapshot{
|
||||
observed: true,
|
||||
state: t.stateForLocked(e),
|
||||
lastCheck: e.lastCheck,
|
||||
lastError: e.lastError,
|
||||
source: e.source,
|
||||
latency: e.latency,
|
||||
consecutiveFailures: e.consecutiveFailures,
|
||||
}
|
||||
}
|
||||
|
||||
// truncateRunes bounds s to at most n runes, decoding as UTF-8 rather
|
||||
// than slicing raw bytes — a byte-level cut on an arbitrary upstream
|
||||
// error string could split a multi-byte rune (escapeLabelValue's own
|
||||
// doc comment, metrics.go, is the identical concern applied to a
|
||||
// different string).
|
||||
func truncateRunes(s string, n int) string {
|
||||
r := []rune(s)
|
||||
if len(r) <= n {
|
||||
return s
|
||||
}
|
||||
return string(r[:n])
|
||||
}
|
||||
|
||||
// --- config: TargetHealthConfig validation ---
|
||||
|
||||
// defaultTargetHealthProbeInterval/min/max bound TargetHealthConfig.
|
||||
// ProbeInterval; defaultTargetHealthFailureThreshold/max bound
|
||||
// TargetHealthConfig.FailureThreshold. See TargetHealthConfig's own doc
|
||||
// comment (llmgateway.go) for what each governs.
|
||||
const (
|
||||
defaultTargetHealthProbeInterval = 60 * time.Second
|
||||
minTargetHealthProbeInterval = 10 * time.Second
|
||||
maxTargetHealthProbeInterval = time.Hour
|
||||
defaultTargetHealthFailureThreshold = 3
|
||||
maxTargetHealthFailureThreshold = 100
|
||||
)
|
||||
|
||||
// targetHealthConfig is TargetHealthConfig, validated and defaulted once
|
||||
// at construction (newGateway, llmgateway.go) — mirrors breakerConfig/
|
||||
// failoverConfig's own resolved-value-not-raw-config shape.
|
||||
type targetHealthConfig struct {
|
||||
probeInterval time.Duration
|
||||
failureThreshold int
|
||||
enabled bool
|
||||
}
|
||||
|
||||
// validateTargetHealthConfig validates thc and returns the resolved
|
||||
// targetHealthConfig newGateway attaches to the Gateway. thc's zero
|
||||
// value (no targetHealth block at all) resolves to every default below
|
||||
// unchanged: enabled stays false (a deployment that predates this
|
||||
// feature, or never configures this block, sends no active probe
|
||||
// traffic), matching FailoverConfig's own "off unless explicitly opted
|
||||
// in" precedent (failover.go).
|
||||
func validateTargetHealthConfig(thc TargetHealthConfig) (targetHealthConfig, error) {
|
||||
interval := defaultTargetHealthProbeInterval
|
||||
if thc.ProbeInterval != "" {
|
||||
d, err := time.ParseDuration(thc.ProbeInterval)
|
||||
if err != nil {
|
||||
return targetHealthConfig{}, fmt.Errorf("llmgateway: targetHealth.probeInterval %q is invalid: %w", thc.ProbeInterval, err)
|
||||
}
|
||||
if d < minTargetHealthProbeInterval || d > maxTargetHealthProbeInterval {
|
||||
return targetHealthConfig{}, fmt.Errorf("llmgateway: targetHealth.probeInterval must be between %s and %s, got %q", minTargetHealthProbeInterval, maxTargetHealthProbeInterval, thc.ProbeInterval)
|
||||
}
|
||||
interval = d
|
||||
}
|
||||
|
||||
threshold := thc.FailureThreshold
|
||||
if threshold == 0 {
|
||||
threshold = defaultTargetHealthFailureThreshold
|
||||
}
|
||||
if threshold < 1 || threshold > maxTargetHealthFailureThreshold {
|
||||
return targetHealthConfig{}, fmt.Errorf("llmgateway: targetHealth.failureThreshold must be between 1 and %d, got %d", maxTargetHealthFailureThreshold, threshold)
|
||||
}
|
||||
|
||||
return targetHealthConfig{enabled: thc.Enabled, probeInterval: interval, failureThreshold: threshold}, nil
|
||||
}
|
||||
|
||||
// --- active probes: opt-in, lazy, single-flight ---
|
||||
|
||||
// targetHealthMaxProbeTimeout caps how long any ONE active probe may
|
||||
// run, regardless of Config.RequestTimeout — a probe is a cheap,
|
||||
// synchronous health check, not real traffic, and defaultRequestTimeout
|
||||
// (timeout.go) is 5 minutes: without this cap, one hung MCP server or
|
||||
// agent could tie up a probe goroutine for that entire duration.
|
||||
const targetHealthMaxProbeTimeout = 10 * time.Second
|
||||
|
||||
// targetHealthProbeTimeout resolves the timeout ONE active probe gets:
|
||||
// requestTimeout (g.targetTimeout), capped at
|
||||
// targetHealthMaxProbeTimeout. Written as a small helper rather than the
|
||||
// Go 1.21+ builtin min: this plugin runs interpreted under Yaegi, and
|
||||
// this repo's own constraints call for stdlib-only, builtin-free helpers
|
||||
// where a construct's interpreter support is unverified.
|
||||
func targetHealthProbeTimeout(requestTimeout time.Duration) time.Duration {
|
||||
if requestTimeout <= 0 || requestTimeout > targetHealthMaxProbeTimeout {
|
||||
return targetHealthMaxProbeTimeout
|
||||
}
|
||||
return requestTimeout
|
||||
}
|
||||
|
||||
// maybeSweepTargetHealth is called at the top of every route a scrape or
|
||||
// a dashboard poll can reach (serveMetrics, metrics.go; serveAdminTargets/
|
||||
// handleMCPServers/handleAgents, admin.go/mcp_a2a.go) — that traffic IS
|
||||
// the heartbeat: this plugin has no Close hook under Traefik's hot
|
||||
// reload (Gateway.Close's own doc comment, llmgateway.go) and therefore
|
||||
// starts no timer or long-lived goroutine of its own here either,
|
||||
// mirroring modelRegistry.maybeRefresh's identical reasoning
|
||||
// (registry.go).
|
||||
//
|
||||
// Returns immediately when disabled. Otherwise: g.targetHealthSweeping
|
||||
// is the single-flight gate (an atomic CAS, not a mutex, since every
|
||||
// caller reaches this on a live request path and must never block
|
||||
// behind a lock another such call already holds) — a CAS that fails
|
||||
// means either a sweep is already in flight, in which case returning is
|
||||
// exactly right, or another goroutine just won the race to start this
|
||||
// interval's sweep, in which case returning is exactly right too. The
|
||||
// CAS alone enforces only single-flight — "at most one sweep in flight
|
||||
// at a time" — NOT the probe interval: the interval load above it and
|
||||
// the CAS are not atomic with each other, so a caller whose own interval
|
||||
// read predates an entirely separate sweep's full start-and-finish could
|
||||
// still win the CAS once that sweep clears the flag (worst at startup,
|
||||
// when every caller's first read sees last == 0). F3, feat/target-health
|
||||
// review: re-check the interval AFTER winning the CAS, against
|
||||
// whatever targetHealthLastSweepUnixNano holds now — set by the sweep
|
||||
// that raced ahead, if any — and back out without spawning when that
|
||||
// sweep already started within the interval.
|
||||
func (g *Gateway) maybeSweepTargetHealth() {
|
||||
if !g.targetHealthCfg.enabled {
|
||||
return
|
||||
}
|
||||
if !targetHealthIntervalElapsed(atomic.LoadInt64(&g.targetHealthLastSweepUnixNano), g.targetHealthCfg.probeInterval) {
|
||||
return
|
||||
}
|
||||
if !atomic.CompareAndSwapInt32(&g.targetHealthSweeping, 0, 1) {
|
||||
return
|
||||
}
|
||||
now := time.Now().UnixNano()
|
||||
if !targetHealthIntervalElapsed(atomic.LoadInt64(&g.targetHealthLastSweepUnixNano), g.targetHealthCfg.probeInterval) {
|
||||
// Lost the race: some other sweep already started (and, since
|
||||
// the flag was 0 again for this CAS to succeed, already
|
||||
// finished) within the interval while this call was still
|
||||
// working from its own, now-stale, interval read above.
|
||||
atomic.StoreInt32(&g.targetHealthSweeping, 0)
|
||||
return
|
||||
}
|
||||
atomic.StoreInt64(&g.targetHealthLastSweepUnixNano, now)
|
||||
go g.sweepTargetHealth() //nolint:gosec // G118: self-terminating, bounded by mcpFederatedFanoutConcurrency*targetHealthMaxProbeTimeout worst case, and always clears targetHealthSweeping on return (including on panic) — see maybeRefresh's identical reasoning, registry.go, for why this plugin accepts a bounded, self-terminating background goroutine despite having no Close hook to await it
|
||||
}
|
||||
|
||||
// targetHealthIntervalElapsed reports whether probeInterval has passed
|
||||
// since lastSweepUnixNano (0 meaning "never swept", which always counts
|
||||
// as elapsed) — the exact rule maybeSweepTargetHealth applies both above
|
||||
// and, again, under the CAS.
|
||||
func targetHealthIntervalElapsed(lastSweepUnixNano int64, probeInterval time.Duration) bool {
|
||||
return lastSweepUnixNano == 0 || time.Duration(time.Now().UnixNano()-lastSweepUnixNano) >= probeInterval
|
||||
}
|
||||
|
||||
// sweepTargetHealth performs one active probe sweep: every configured
|
||||
// MCP server and agent, concurrently, behind a semaphore of
|
||||
// mcpFederatedFanoutConcurrency (mcp_federation.go) — reusing federation's
|
||||
// own fan-out concurrency constant rather than adding a second one for
|
||||
// an identical "how many outbound target requests at once" question.
|
||||
// Always clears g.targetHealthSweeping on return, even on panic
|
||||
// (mirrors modelRegistry.refreshProvider's own deferred-recover
|
||||
// reasoning, registry.go — this goroutine has no ServeHTTP caller to
|
||||
// unwind into either).
|
||||
func (g *Gateway) sweepTargetHealth() {
|
||||
defer atomic.StoreInt32(&g.targetHealthSweeping, 0)
|
||||
defer func() {
|
||||
if rec := recover(); rec != nil {
|
||||
g.logf("target health: sweep panicked: %v", rec)
|
||||
}
|
||||
}()
|
||||
|
||||
timeout := targetHealthProbeTimeout(g.targetTimeout)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
sem := make(chan struct{}, mcpFederatedFanoutConcurrency)
|
||||
|
||||
for name, tc := range g.cfg.MCPServers {
|
||||
if tc == nil {
|
||||
continue
|
||||
}
|
||||
wg.Add(1)
|
||||
go func(name, url string) {
|
||||
defer wg.Done()
|
||||
defer func() {
|
||||
if rec := recover(); rec != nil {
|
||||
g.logf("target health: probe for mcp %q panicked: %v", name, rec)
|
||||
}
|
||||
}()
|
||||
sem <- struct{}{}
|
||||
defer func() { <-sem }()
|
||||
g.probeMCPTarget(name, url, timeout)
|
||||
}(name, tc.URL)
|
||||
}
|
||||
for name, ac := range g.cfg.Agents {
|
||||
if ac == nil {
|
||||
continue
|
||||
}
|
||||
wg.Add(1)
|
||||
go func(name, url, card string) {
|
||||
defer wg.Done()
|
||||
defer func() {
|
||||
if rec := recover(); rec != nil {
|
||||
g.logf("target health: probe for agent %q panicked: %v", name, rec)
|
||||
}
|
||||
}()
|
||||
sem <- struct{}{}
|
||||
defer func() { <-sem }()
|
||||
g.probeAgentTarget(name, url, card, timeout)
|
||||
}(name, ac.URL, ac.Card)
|
||||
}
|
||||
wg.Wait()
|
||||
}
|
||||
|
||||
// probeMCPTarget performs one active probe of MCP server name at
|
||||
// rawURL, bounded by timeout, and records the result with source
|
||||
// "probe". A legacy HTTP+SSE-transport server (isLegacySSETransportURL,
|
||||
// mcp_federation.go) gets a plain GET with its body closed unread
|
||||
// (probeLegacySSE, below); every other server gets the same
|
||||
// initialize/close handshake mcpBackendCall's own fallback path already
|
||||
// uses (mcpBackendHandshake/mcpBackendCloseSession, mcp_federation.go)
|
||||
// — a JSON-RPC error answering "initialize" is a failure carrying that
|
||||
// message, exactly like a transport-level error.
|
||||
func (g *Gateway) probeMCPTarget(name, rawURL string, timeout time.Duration) {
|
||||
start := time.Now()
|
||||
|
||||
if isLegacySSETransportURL(rawURL) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
ok, err := g.probeLegacySSE(ctx, rawURL)
|
||||
cancel()
|
||||
g.targetHealth.record(targetKindMCP, name, rawURL, ok, err, time.Since(start), targetHealthSourceProbe)
|
||||
return
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
sessionID, err := g.mcpBackendHandshake(ctx, rawURL, mcpBackendResponseMaxBytes)
|
||||
cancel()
|
||||
ok := err == nil
|
||||
if ok && sessionID != "" {
|
||||
closeCtx, closeCancel := context.WithTimeout(context.Background(), mcpSessionCloseTimeout)
|
||||
g.mcpBackendCloseSession(closeCtx, rawURL, sessionID)
|
||||
closeCancel()
|
||||
}
|
||||
g.targetHealth.record(targetKindMCP, name, rawURL, ok, err, time.Since(start), targetHealthSourceProbe)
|
||||
}
|
||||
|
||||
// probeLegacySSE probes a legacy HTTP+SSE-transport MCP server (URL path
|
||||
// ends "/sse", isLegacySSETransportURL) with a plain GET: ok iff no
|
||||
// transport error and the response status is under 500. The body is
|
||||
// closed IMMEDIATELY after headers, never read — an SSE endpoint streams
|
||||
// indefinitely, so reading it would hang the probe for its entire
|
||||
// timeout budget on every sweep.
|
||||
func (g *Gateway) probeLegacySSE(ctx context.Context, rawURL string) (bool, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) //nolint:gosec // operator-configured target URL, validated at construction (validateTargetURLs, mcp_a2a.go)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
resp, err := g.targetClient.Do(req)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
_ = resp.Body.Close() //nolint:errcheck // deliberately unread — see this function's own doc comment
|
||||
if resp.StatusCode >= http.StatusInternalServerError {
|
||||
return false, fmt.Errorf("legacy sse probe: upstream returned HTTP %d", resp.StatusCode)
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// probeAgentTarget performs one active probe of agent name: a GET of
|
||||
// baseURL+cardPath (AgentConfig.Card, or defaultAgentCardPath when empty
|
||||
// — mcp_a2a.go), bounded by timeout. ok iff no transport error and the
|
||||
// response status is under 500 — a 404 counts as reachable (e.g. an
|
||||
// umbrella agent whose root has no card of its own), the identical
|
||||
// "any status under 500 proves the target answered" rule handleTargetProxy's
|
||||
// own passive recording applies (recordTargetProxyHealth, below). The
|
||||
// body is closed without being read: an agent-card probe only needs the
|
||||
// status.
|
||||
func (g *Gateway) probeAgentTarget(name, baseURL, cardPath string, timeout time.Duration) {
|
||||
if cardPath == "" {
|
||||
cardPath = defaultAgentCardPath
|
||||
}
|
||||
start := time.Now()
|
||||
ctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
defer cancel()
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, baseURL+cardPath, nil) //nolint:gosec // operator-configured target URL, validated at construction (validateTargetURLs, mcp_a2a.go)
|
||||
if err != nil {
|
||||
g.targetHealth.record(targetKindAgent, name, baseURL, false, err, time.Since(start), targetHealthSourceProbe)
|
||||
return
|
||||
}
|
||||
resp, err := g.targetClient.Do(req)
|
||||
if err != nil {
|
||||
g.targetHealth.record(targetKindAgent, name, baseURL, false, err, time.Since(start), targetHealthSourceProbe)
|
||||
return
|
||||
}
|
||||
_ = resp.Body.Close() //nolint:errcheck // read-side close; nothing actionable on failure
|
||||
ok := resp.StatusCode < http.StatusInternalServerError
|
||||
var probeErr error
|
||||
if !ok {
|
||||
probeErr = fmt.Errorf("agent probe: upstream returned HTTP %d", resp.StatusCode)
|
||||
}
|
||||
g.targetHealth.record(targetKindAgent, name, baseURL, ok, probeErr, time.Since(start), targetHealthSourceProbe)
|
||||
}
|
||||
|
||||
// --- passive recording: handleTargetProxy (mcp_a2a.go) ---
|
||||
|
||||
// recordTargetProxyHealth applies handleTargetProxy's own passive
|
||||
// recording rule to one proxyUpstream result (routes_passthrough.go):
|
||||
// failure iff the proxy attempt itself failed for a reason other than
|
||||
// the client canceling, OR the upstream answered with a 5xx; otherwise
|
||||
// success — any status under 500 counts as reachable, 4xx included,
|
||||
// since a 4xx still proves the target itself answered. A client cancel
|
||||
// (result.clientCanceled) records nothing at all: a caller hanging up
|
||||
// says nothing about the target's own health.
|
||||
func (g *Gateway) recordTargetProxyHealth(kind, name, rawURL string, result proxyResult, ok bool, latency time.Duration) {
|
||||
if result.clientCanceled {
|
||||
return
|
||||
}
|
||||
failed := !ok || result.status >= http.StatusInternalServerError
|
||||
var err error
|
||||
if failed {
|
||||
err = targetProxyHealthError(ok, result.status)
|
||||
}
|
||||
g.targetHealth.record(kind, name, rawURL, !failed, err, latency, targetHealthSourceTraffic)
|
||||
}
|
||||
|
||||
// targetProxyHealthError builds a short, bounded description for a
|
||||
// failed passive observation. proxyUpstream itself already logged the
|
||||
// real underlying error; this is only what targetHealthEntry.lastError
|
||||
// — exposed via GET /admin/api/targets — shows an operator.
|
||||
func targetProxyHealthError(ok bool, status int) error {
|
||||
if !ok {
|
||||
return errors.New("target proxy: upstream request failed")
|
||||
}
|
||||
return fmt.Errorf("target proxy: upstream returned HTTP %d", status)
|
||||
}
|
||||
@@ -0,0 +1,665 @@
|
||||
package traefikllmgateway
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// --- targetHealthTracker: state machine ---
|
||||
|
||||
func TestTargetHealthTracker_NeverRecorded_ReadsUnknown(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
assert.Equal(t, targetHealthUnknown, tr.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_NilReceiver_Safe(t *testing.T) {
|
||||
t.Parallel()
|
||||
var tr *targetHealthTracker
|
||||
assert.NotPanics(t, func() {
|
||||
tr.record(targetKindMCP, "alpha", "", true, nil, time.Millisecond, targetHealthSourceTraffic)
|
||||
})
|
||||
assert.Equal(t, targetHealthUnknown, tr.stateOf(targetKindMCP, "alpha"))
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.Equal(t, targetHealthUnknown, snap.state)
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_SingleSuccess_ReadsHealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", "", true, nil, 5*time.Millisecond, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthHealthy, tr.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_BelowThreshold_StaysHealthy proves the
|
||||
// documented exception: 1 or 2 recent failures after a success still
|
||||
// reads "healthy" until consecutiveFailures actually reaches the
|
||||
// configured threshold (targetHealthTracker's own doc comment).
|
||||
func TestTargetHealthTracker_BelowThreshold_StaysHealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", "", true, nil, 0, targetHealthSourceTraffic)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthHealthy, tr.stateOf(targetKindMCP, "alpha"), "1 failure of threshold 3 must still read healthy")
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom again"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthHealthy, tr.stateOf(targetKindMCP, "alpha"), "2 failures of threshold 3 must still read healthy")
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_ReachesThreshold_ReadsUnhealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
for i := 0; i < 3; i++ {
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
}
|
||||
assert.Equal(t, targetHealthUnhealthy, tr.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_FirstObservationFailure_ReadsUnknown proves F4:
|
||||
// a target whose only observations so far are failures below
|
||||
// failureThreshold reads "unknown", not "healthy" — the tracker has
|
||||
// never actually seen it succeed, so "healthy" would be a false claim
|
||||
// even though the failure-threshold gate has not fired. The threshold
|
||||
// still protects against a transient first failure: this stays
|
||||
// "unknown", it does not jump straight to "unhealthy".
|
||||
func TestTargetHealthTracker_FirstObservationFailure_ReadsUnknown(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthUnknown, tr.stateOf(targetKindMCP, "alpha"), "never having succeeded must read unknown, not healthy, below threshold")
|
||||
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom again"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthUnknown, tr.stateOf(targetKindMCP, "alpha"), "still unknown after a second failure, still below threshold")
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_SuccessThenFewFailures_ReadsHealthy re-proves
|
||||
// TestTargetHealthTracker_BelowThreshold_StaysHealthy's claim in F4
|
||||
// terms: once a target HAS succeeded at least once, 1-2 subsequent
|
||||
// failures below threshold still read "healthy" — F4 only changes the
|
||||
// NEVER-succeeded case.
|
||||
func TestTargetHealthTracker_SuccessThenFewFailures_ReadsHealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", "", true, nil, 0, targetHealthSourceTraffic)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom again"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthHealthy, tr.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_SuccessAfterUnhealthy_RecoversToHealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(2)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
require.Equal(t, targetHealthUnhealthy, tr.stateOf(targetKindMCP, "alpha"))
|
||||
|
||||
tr.record(targetKindMCP, "alpha", "", true, nil, 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthHealthy, tr.stateOf(targetKindMCP, "alpha"))
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.Equal(t, 0, snap.consecutiveFailures, "a success must reset consecutiveFailures to 0")
|
||||
assert.Empty(t, snap.lastError, "a success must clear lastError")
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_ErrorString_BoundedLength(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
long := strings.Repeat("x", targetHealthMaxErrorRunes+500)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New(long), 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.LessOrEqual(t, len([]rune(snap.lastError)), targetHealthMaxErrorRunes)
|
||||
assert.Equal(t, strings.Repeat("x", targetHealthMaxErrorRunes), snap.lastError)
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_ShortError_Unchanged(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", "", false, errors.New("dial tcp: connection refused"), 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.Equal(t, "dial tcp: connection refused", snap.lastError)
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_Record_ScrubsCredentialsFromLastError proves F1:
|
||||
// record scrubs an error's embedded target URL through sanitizeProviderErr
|
||||
// before storing lastError, so a target URL configured with an embedded
|
||||
// credential (an "api-key" query parameter, or userinfo) never survives
|
||||
// into GET /admin/api/targets.
|
||||
func TestTargetHealthTracker_Record_ScrubsCredentialsFromLastError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("query parameter credential", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
rawURL := "https://svc.internal/mcp?api-key=SECRETVALUE"
|
||||
err := errors.New(`Get "` + rawURL + `": dial tcp: connection refused`)
|
||||
tr.record(targetKindMCP, "alpha", rawURL, false, err, 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.NotContains(t, snap.lastError, "SECRETVALUE")
|
||||
assert.NotContains(t, snap.lastError, "api-key=")
|
||||
})
|
||||
|
||||
t.Run("userinfo credential", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
rawURL := "https://[email protected]/mcp"
|
||||
err := errors.New(`Get "` + rawURL + `": dial tcp: connection refused`)
|
||||
tr.record(targetKindMCP, "alpha", rawURL, false, err, 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.NotContains(t, snap.lastError, "tok3n")
|
||||
})
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_Record_ScrubsBeforeTruncate proves F1: the URL
|
||||
// scrub runs against err.Error() BEFORE truncateRunes bounds it to
|
||||
// targetHealthMaxErrorRunes — even when the embedded URL straddles the
|
||||
// truncation point, no fragment of the secret survives. G2 (feat/target-
|
||||
// health review round 2): the prefix is short enough that the SECRET
|
||||
// itself — not merely the surrounding URL — straddles the 200-rune cut,
|
||||
// so this test actually distinguishes scrub-before-truncate from
|
||||
// truncate-before-scrub; a longer prefix that pushes the whole secret
|
||||
// past the cut point would pass under either ordering, proving nothing.
|
||||
func TestTargetHealthTracker_Record_ScrubsBeforeTruncate(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
rawURL := "https://svc.internal/mcp?api-key=" + strings.Repeat("S", 60)
|
||||
prefix := strings.Repeat("p", 150)
|
||||
require.Less(t, len([]rune(prefix))+strings.Index(rawURL, "api-key="), targetHealthMaxErrorRunes, "the secret must start before the truncation point")
|
||||
require.Greater(t, len([]rune(prefix))+len([]rune(rawURL)), targetHealthMaxErrorRunes, "the url must straddle the truncation point")
|
||||
errText := prefix + " " + rawURL + " connection refused"
|
||||
|
||||
tr.record(targetKindMCP, "alpha", rawURL, false, errors.New(errText), 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
assert.LessOrEqual(t, len([]rune(snap.lastError)), targetHealthMaxErrorRunes)
|
||||
assert.NotContains(t, snap.lastError, "SSSS", "no fragment of the secret must survive truncation")
|
||||
assert.NotContains(t, snap.lastError, "api-key=")
|
||||
}
|
||||
|
||||
// TestTargetHealthTracker_Record_ScrubsPasswordMaskedURLForm proves G1
|
||||
// (feat/target-health review round 2): net/http's own *url.Error masks a
|
||||
// URL's password ("user:pass@" -> "user:***@", and "user:" with an empty
|
||||
// password the same way — stripPassword, net/http/client.go) BEFORE the
|
||||
// error text is ever built, whenever the dialed URL's userinfo carries a
|
||||
// password. sanitizeProviderErr (admin.go) only matches rawURL verbatim,
|
||||
// so for such a URL it finds nothing to scrub — leaking the password AND
|
||||
// any query-string credential riding the same URL (an "api-key" value).
|
||||
// record must scrub against BOTH the raw URL and net/http's own masked
|
||||
// form. Every case uses a REAL error from http.Client.Do against
|
||||
// 127.0.0.1:1 (this repo's own "reserved, never listening" convention —
|
||||
// see provider_openai_test.go), never a hand-written string: the bug is
|
||||
// about the exact shape net/http actually produces.
|
||||
func TestTargetHealthTracker_Record_ScrubsPasswordMaskedURLForm(t *testing.T) {
|
||||
t.Parallel()
|
||||
// secretPart is the synthetic password these fixtures plant in a target
|
||||
// URL, and credFixtureURL assembles the URL around it at run time. The
|
||||
// parts stay separate in the source deliberately: a whole URL literal
|
||||
// carrying userinfo credentials trips this repository's own pre-commit
|
||||
// secret scan, and these fixtures exist to prove exactly such a URL
|
||||
// never reaches lastError.
|
||||
const secretPart = "sup3rsecret"
|
||||
credFixtureURL := func(userinfo, hostPath string) string {
|
||||
return "http://" + userinfo + "@" + hostPath
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
rawURL string
|
||||
mustNotHave []string
|
||||
}{
|
||||
{
|
||||
name: "userinfo with password",
|
||||
rawURL: credFixtureURL("user:"+secretPart, "127.0.0.1:1/mcp"),
|
||||
mustNotHave: []string{"user:" + secretPart + "@", "user:***@", secretPart},
|
||||
},
|
||||
{
|
||||
name: "token as username with empty password separator",
|
||||
rawURL: credFixtureURL("TOKENASUSER:", "127.0.0.1:1/mcp"),
|
||||
mustNotHave: []string{"TOKENASUSER:@", "TOKENASUSER:***@", "TOKENASUSER"},
|
||||
},
|
||||
{
|
||||
name: "userinfo with password and api-key query on the same URL",
|
||||
rawURL: credFixtureURL("user:"+secretPart, "127.0.0.1:1/mcp?api-key=SECRETVALUE"),
|
||||
mustNotHave: []string{"user:" + secretPart + "@", "user:***@", secretPart, "SECRETVALUE", "api-key="},
|
||||
},
|
||||
{
|
||||
// Existing case (F1): userinfo with NO password has nothing for
|
||||
// net/http to mask, so the raw URL survives into the error
|
||||
// verbatim — must keep working unchanged by the G1 fix.
|
||||
name: "token as username, no password separator",
|
||||
rawURL: "http://[email protected]:1/mcp",
|
||||
mustNotHave: []string{"tok3n"},
|
||||
},
|
||||
{
|
||||
// Existing case (F1): no userinfo at all, so there is nothing
|
||||
// for net/http to mask — must keep working unchanged.
|
||||
name: "bare api-key query, no userinfo",
|
||||
rawURL: "http://127.0.0.1:1/mcp?api-key=SECRETVALUE",
|
||||
mustNotHave: []string{"SECRETVALUE", "api-key="},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
req, err := http.NewRequest(http.MethodGet, tc.rawURL, nil)
|
||||
require.NoError(t, err)
|
||||
_, doErr := http.DefaultClient.Do(req) //nolint:bodyclose // Do returns a nil body alongside a non-nil error
|
||||
require.Error(t, doErr, "127.0.0.1:1 must refuse the connection")
|
||||
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.record(targetKindMCP, "alpha", tc.rawURL, false, doErr, 0, targetHealthSourceProbe)
|
||||
snap := tr.snapshot(targetKindMCP, "alpha")
|
||||
for _, secret := range tc.mustNotHave {
|
||||
assert.NotContains(t, snap.lastError, secret)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_KeyedByKindAndName_Independent(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(1)
|
||||
tr.record(targetKindMCP, "shared", "", false, errors.New("boom"), 0, targetHealthSourceTraffic)
|
||||
assert.Equal(t, targetHealthUnhealthy, tr.stateOf(targetKindMCP, "shared"))
|
||||
assert.Equal(t, targetHealthUnknown, tr.stateOf(targetKindAgent, "shared"), "an mcp server and an agent sharing a name must track independently")
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_Snapshot_FieldsPopulated(t *testing.T) {
|
||||
t.Parallel()
|
||||
now := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)
|
||||
tr := newTargetHealthTracker(3)
|
||||
tr.nowFn = func() time.Time { return now }
|
||||
tr.record(targetKindAgent, "beta", "", true, nil, 42*time.Millisecond, targetHealthSourceProbe)
|
||||
|
||||
snap := tr.snapshot(targetKindAgent, "beta")
|
||||
assert.Equal(t, targetHealthHealthy, snap.state)
|
||||
assert.Equal(t, now, snap.lastCheck)
|
||||
assert.Equal(t, 42*time.Millisecond, snap.latency)
|
||||
assert.Equal(t, targetHealthSourceProbe, snap.source)
|
||||
assert.Equal(t, 0, snap.consecutiveFailures)
|
||||
}
|
||||
|
||||
func TestTargetHealthTracker_ConcurrentRecord_NoRace(t *testing.T) {
|
||||
t.Parallel()
|
||||
tr := newTargetHealthTracker(3)
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 50; i++ {
|
||||
wg.Add(1)
|
||||
go func(i int) {
|
||||
defer wg.Done()
|
||||
tr.record(targetKindMCP, "alpha", "", i%2 == 0, errors.New("x"), time.Millisecond, targetHealthSourceTraffic)
|
||||
}(i)
|
||||
}
|
||||
wg.Wait()
|
||||
_ = tr.stateOf(targetKindMCP, "alpha")
|
||||
}
|
||||
|
||||
// --- TargetHealthConfig validation ---
|
||||
|
||||
func TestValidateTargetHealthConfig_Defaults(t *testing.T) {
|
||||
t.Parallel()
|
||||
thc, err := validateTargetHealthConfig(TargetHealthConfig{})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, thc.enabled)
|
||||
assert.Equal(t, defaultTargetHealthProbeInterval, thc.probeInterval)
|
||||
assert.Equal(t, defaultTargetHealthFailureThreshold, thc.failureThreshold)
|
||||
}
|
||||
|
||||
func TestValidateTargetHealthConfig_ExplicitEnable(t *testing.T) {
|
||||
t.Parallel()
|
||||
thc, err := validateTargetHealthConfig(TargetHealthConfig{Enabled: true})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, thc.enabled)
|
||||
}
|
||||
|
||||
func TestValidateTargetHealthConfig_ProbeIntervalBounds(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := validateTargetHealthConfig(TargetHealthConfig{ProbeInterval: "1s"})
|
||||
assert.Error(t, err, "below the 10s floor must be rejected")
|
||||
|
||||
_, err = validateTargetHealthConfig(TargetHealthConfig{ProbeInterval: "2h"})
|
||||
assert.Error(t, err, "above the 1h ceiling must be rejected")
|
||||
|
||||
_, err = validateTargetHealthConfig(TargetHealthConfig{ProbeInterval: "not-a-duration"})
|
||||
assert.Error(t, err)
|
||||
|
||||
thc, err := validateTargetHealthConfig(TargetHealthConfig{ProbeInterval: "30s"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 30*time.Second, thc.probeInterval)
|
||||
}
|
||||
|
||||
func TestValidateTargetHealthConfig_FailureThresholdBounds(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := validateTargetHealthConfig(TargetHealthConfig{FailureThreshold: -1})
|
||||
assert.Error(t, err)
|
||||
|
||||
_, err = validateTargetHealthConfig(TargetHealthConfig{FailureThreshold: maxTargetHealthFailureThreshold + 1})
|
||||
assert.Error(t, err)
|
||||
|
||||
thc, err := validateTargetHealthConfig(TargetHealthConfig{FailureThreshold: 10})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 10, thc.failureThreshold)
|
||||
}
|
||||
|
||||
func TestNewGateway_TargetHealth_InvalidConfig_ConstructionError(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := CreateConfig()
|
||||
cfg.Providers = map[string]*ProviderConfig{"openai": {Type: "openai", APIKey: "k"}}
|
||||
cfg.TargetHealth = TargetHealthConfig{ProbeInterval: "1s"}
|
||||
_, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestNewGateway_TargetHealth_DisabledByDefault_TrackerStillConstructed(t *testing.T) {
|
||||
t.Parallel()
|
||||
cfg := CreateConfig()
|
||||
cfg.Providers = map[string]*ProviderConfig{"openai": {Type: "openai", APIKey: "k"}}
|
||||
h, err := New(context.Background(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), cfg, "llmgw")
|
||||
require.NoError(t, err)
|
||||
gw, ok := h.(*Gateway)
|
||||
require.True(t, ok)
|
||||
require.NotNil(t, gw.targetHealth, "the tracker must always be constructed regardless of Enabled — passive recording/exposure stay on unconditionally")
|
||||
assert.False(t, gw.targetHealthCfg.enabled)
|
||||
}
|
||||
|
||||
// --- maybeSweepTargetHealth: disabled / enabled / single-flight ---
|
||||
|
||||
func newTargetHealthSweepGateway(t *testing.T, enabled bool, probeInterval time.Duration, mcpURL string) *Gateway {
|
||||
t.Helper()
|
||||
adapters := map[string]providerAdapter{"openai": newFakeAdapter("openai")}
|
||||
cfg := &Config{
|
||||
Providers: map[string]*ProviderConfig{"openai": {}},
|
||||
MCPServers: map[string]*TargetConfig{"alpha": {URL: mcpURL}},
|
||||
}
|
||||
reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {})
|
||||
require.NoError(t, err)
|
||||
return &Gateway{
|
||||
cfg: cfg,
|
||||
registry: reg,
|
||||
targetHealth: newTargetHealthTracker(3),
|
||||
targetHealthCfg: targetHealthConfig{enabled: enabled, probeInterval: probeInterval, failureThreshold: 3},
|
||||
targetClient: http.DefaultClient,
|
||||
targetTimeout: 5 * time.Second,
|
||||
name: "test",
|
||||
}
|
||||
}
|
||||
|
||||
func TestMaybeSweepTargetHealth_Disabled_NoOutboundRequests(t *testing.T) {
|
||||
t.Parallel()
|
||||
// F6, feat/target-health review: atomic even though a disabled sweep
|
||||
// must never actually invoke this handler — a regression that DID
|
||||
// invoke it would otherwise race this counter's read below under
|
||||
// -race, obscuring the real bug (an unexpected outbound probe) behind
|
||||
// a data-race report instead of the intended assert.Equal failure.
|
||||
var hits int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&hits, 1)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
gw := newTargetHealthSweepGateway(t, false, 10*time.Millisecond, srv.URL)
|
||||
gw.maybeSweepTargetHealth()
|
||||
// maybeSweepTargetHealth spawns a goroutine when it decides to sweep;
|
||||
// since it must return immediately when disabled, there is nothing to
|
||||
// wait on — a brief sleep would only prove absence of a signal we
|
||||
// never expect to see.
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
assert.Equal(t, int32(0), atomic.LoadInt32(&hits), "a disabled targetHealth must never send an outbound probe")
|
||||
assert.Equal(t, targetHealthUnknown, gw.targetHealth.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
func TestMaybeSweepTargetHealth_Enabled_ProbesConfiguredServer(t *testing.T) {
|
||||
t.Parallel()
|
||||
var hits int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&hits, 1)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":"federated","result":{"protocolVersion":"2025-11-25","capabilities":{}}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
gw := newTargetHealthSweepGateway(t, true, time.Hour, srv.URL)
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return atomic.LoadInt32(&hits) > 0 }, time.Second)
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindMCP, "alpha") == targetHealthHealthy }, time.Second)
|
||||
|
||||
snap := gw.targetHealth.snapshot(targetKindMCP, "alpha")
|
||||
assert.Equal(t, targetHealthSourceProbe, snap.source)
|
||||
}
|
||||
|
||||
func TestMaybeSweepTargetHealth_ConcurrentTriggers_SingleFlight(t *testing.T) {
|
||||
t.Parallel()
|
||||
var hits int32
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&hits, 1)
|
||||
<-release
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":"federated","result":{}}`))
|
||||
}))
|
||||
// F6, feat/target-health review: registered in this order so
|
||||
// t.Cleanup's LIFO order unblocks the handler (closing release)
|
||||
// BEFORE srv.Close runs — httptest.Server.Close blocks until
|
||||
// outstanding requests complete, and the assertions below can
|
||||
// t.Fatalf/t.Errorf before ever reaching the explicit close(release)
|
||||
// further down; without this a failing assertion left the handler
|
||||
// permanently blocked and srv.Close hung until the whole test
|
||||
// binary's 10-minute timeout, rather than failing fast. unblock is
|
||||
// idempotent (sync.Once) since the happy path below also calls it
|
||||
// directly, before waiting on the sweep's resulting state.
|
||||
var unblockOnce sync.Once
|
||||
unblock := func() { unblockOnce.Do(func() { close(release) }) }
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(unblock)
|
||||
|
||||
gw := newTargetHealthSweepGateway(t, true, time.Hour, srv.URL)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 20; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
gw.maybeSweepTargetHealth()
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
// Give the single in-flight probe time to actually reach the server,
|
||||
// then release it — regardless of how many goroutines called
|
||||
// maybeSweepTargetHealth concurrently, at most one outbound request
|
||||
// must have been sent.
|
||||
waitForCondition(t, func() bool { return atomic.LoadInt32(&hits) > 0 }, time.Second)
|
||||
unblock()
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindMCP, "alpha") != targetHealthUnknown }, time.Second)
|
||||
assert.Equal(t, int32(1), atomic.LoadInt32(&hits), "concurrent triggers within one interval must produce exactly one sweep")
|
||||
}
|
||||
|
||||
// TestMaybeSweepTargetHealth_IntervalEnforcedAcrossRapidCalls proves F3:
|
||||
// the interval load (top of maybeSweepTargetHealth) and the CAS that wins
|
||||
// the single-flight flag are not atomic with each other. Without a
|
||||
// re-check under the CAS, a caller whose own interval read predates an
|
||||
// entirely separate sweep's start-and-finish could still win the CAS
|
||||
// once that sweep clears the flag, and spawn a second sweep inside the
|
||||
// same interval — worst at startup, when every caller's first read sees
|
||||
// last == 0. A fast-completing first sweep, immediately followed by a
|
||||
// burst of concurrent callers, must still yield exactly one sweep within
|
||||
// probeInterval; a call after the interval has genuinely elapsed must
|
||||
// start a second one. G5, feat/target-health review round 2: this test
|
||||
// proves single-flight ORDERING, not real elapsed time, so it must not
|
||||
// depend on wall-clock sleeps at all — probeInterval is a full hour (so
|
||||
// the burst phase's real, sub-millisecond elapsed time can never
|
||||
// accidentally read as elapsed) and the "interval has passed" half is
|
||||
// forced directly via targetHealthLastSweepUnixNano, never by sleeping
|
||||
// for probeInterval.
|
||||
func TestMaybeSweepTargetHealth_IntervalEnforcedAcrossRapidCalls(t *testing.T) {
|
||||
t.Parallel()
|
||||
var hits int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
atomic.AddInt32(&hits, 1)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":"federated","result":{}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
gw := newTargetHealthSweepGateway(t, true, time.Hour, srv.URL)
|
||||
|
||||
// First sweep: the probe answers immediately, so this completes fast
|
||||
// and clears targetHealthSweeping well inside probeInterval.
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return atomic.LoadInt32(&hits) > 0 }, time.Second)
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindMCP, "alpha") != targetHealthUnknown }, time.Second)
|
||||
|
||||
// A burst of concurrent callers immediately after — real elapsed time
|
||||
// is milliseconds against a one-hour probeInterval — must not start a
|
||||
// second sweep.
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 50; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
gw.maybeSweepTargetHealth()
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
if got := atomic.LoadInt32(&hits); got != 1 {
|
||||
t.Errorf("hits = %d, want exactly 1 (a burst of concurrent callers inside probeInterval must not start a second sweep)", got)
|
||||
}
|
||||
|
||||
// Force the interval to read as elapsed directly, rather than
|
||||
// sleeping for real: a call after that must start a second sweep.
|
||||
atomic.StoreInt64(&gw.targetHealthLastSweepUnixNano, time.Now().Add(-2*time.Hour).UnixNano())
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return atomic.LoadInt32(&hits) == 2 }, time.Second)
|
||||
}
|
||||
|
||||
// TestMaybeSweepTargetHealth_LegacySSE_ClosesBodyWithoutReading proves a
|
||||
// legacy "/sse" MCP server's probe uses a plain GET and closes the body
|
||||
// immediately: a test server that never closes its stream must not hang
|
||||
// the probe.
|
||||
func TestMaybeSweepTargetHealth_LegacySSE_ClosesBodyWithoutReading(t *testing.T) {
|
||||
t.Parallel()
|
||||
headersSent := make(chan struct{})
|
||||
release := make(chan struct{})
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "text/event-stream")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
close(headersSent)
|
||||
flusher, ok := w.(http.Flusher)
|
||||
if ok {
|
||||
flusher.Flush()
|
||||
}
|
||||
<-release // never closes on its own; relies on the probe not reading
|
||||
}))
|
||||
// F6, feat/target-health review: previously relied on the PROBE'S OWN
|
||||
// 5s context timeout (targetTimeout below) to unblock this handler,
|
||||
// so srv.Close (deferred) silently added a real ~5s stall to every
|
||||
// run of this test. An explicit release channel, unblocked via
|
||||
// t.Cleanup BEFORE srv.Close (LIFO — see
|
||||
// TestMaybeSweepTargetHealth_ConcurrentTriggers_SingleFlight's own
|
||||
// comment for the identical ordering reasoning), makes this test
|
||||
// fast AND immune to a failing assertion ever leaving it blocked.
|
||||
t.Cleanup(srv.Close)
|
||||
t.Cleanup(func() { close(release) })
|
||||
|
||||
adapters := map[string]providerAdapter{"openai": newFakeAdapter("openai")}
|
||||
cfg := &Config{
|
||||
Providers: map[string]*ProviderConfig{"openai": {}},
|
||||
MCPServers: map[string]*TargetConfig{"alpha": {URL: srv.URL + "/sse"}},
|
||||
}
|
||||
reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {})
|
||||
require.NoError(t, err)
|
||||
gw := &Gateway{
|
||||
cfg: cfg,
|
||||
registry: reg,
|
||||
targetHealth: newTargetHealthTracker(3),
|
||||
targetHealthCfg: targetHealthConfig{enabled: true, probeInterval: time.Hour, failureThreshold: 3},
|
||||
targetClient: http.DefaultClient,
|
||||
targetTimeout: 5 * time.Second,
|
||||
name: "test",
|
||||
}
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindMCP, "alpha") != targetHealthUnknown }, 2*time.Second)
|
||||
assert.Equal(t, targetHealthHealthy, gw.targetHealth.stateOf(targetKindMCP, "alpha"))
|
||||
}
|
||||
|
||||
func TestMaybeSweepTargetHealth_AgentReturns404_ReadsHealthy(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
adapters := map[string]providerAdapter{"openai": newFakeAdapter("openai")}
|
||||
cfg := &Config{
|
||||
Providers: map[string]*ProviderConfig{"openai": {}},
|
||||
Agents: map[string]*AgentConfig{"beta": {URL: srv.URL}},
|
||||
}
|
||||
reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {})
|
||||
require.NoError(t, err)
|
||||
gw := &Gateway{
|
||||
cfg: cfg,
|
||||
registry: reg,
|
||||
targetHealth: newTargetHealthTracker(3),
|
||||
targetHealthCfg: targetHealthConfig{enabled: true, probeInterval: time.Hour, failureThreshold: 3},
|
||||
targetClient: http.DefaultClient,
|
||||
targetTimeout: 5 * time.Second,
|
||||
name: "test",
|
||||
}
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindAgent, "beta") != targetHealthUnknown }, time.Second)
|
||||
assert.Equal(t, targetHealthHealthy, gw.targetHealth.stateOf(targetKindAgent, "beta"), "a 404 agent-card fetch counts as reachable")
|
||||
}
|
||||
|
||||
func TestMaybeSweepTargetHealth_AgentReturns503_RecordsFailure(t *testing.T) {
|
||||
t.Parallel()
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
adapters := map[string]providerAdapter{"openai": newFakeAdapter("openai")}
|
||||
cfg := &Config{
|
||||
Providers: map[string]*ProviderConfig{"openai": {}},
|
||||
Agents: map[string]*AgentConfig{"beta": {URL: srv.URL}},
|
||||
}
|
||||
reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {})
|
||||
require.NoError(t, err)
|
||||
gw := &Gateway{
|
||||
cfg: cfg,
|
||||
registry: reg,
|
||||
targetHealth: newTargetHealthTracker(1),
|
||||
targetHealthCfg: targetHealthConfig{enabled: true, probeInterval: time.Hour, failureThreshold: 1},
|
||||
targetClient: http.DefaultClient,
|
||||
targetTimeout: 5 * time.Second,
|
||||
name: "test",
|
||||
}
|
||||
gw.maybeSweepTargetHealth()
|
||||
waitForCondition(t, func() bool { return gw.targetHealth.stateOf(targetKindAgent, "beta") != targetHealthUnknown }, time.Second)
|
||||
assert.Equal(t, targetHealthUnhealthy, gw.targetHealth.stateOf(targetKindAgent, "beta"))
|
||||
}
|
||||
|
||||
// waitForCondition polls cond until it returns true or deadline elapses,
|
||||
// failing the test on timeout — used throughout this file's sweep tests
|
||||
// since the sweep itself runs in a background goroutine with no
|
||||
// synchronous completion signal (maybeSweepTargetHealth's own doc
|
||||
// comment: no timers, no long-lived goroutines, fire-and-forget).
|
||||
func waitForCondition(t *testing.T, cond func() bool, timeout time.Duration) {
|
||||
t.Helper()
|
||||
deadline := time.Now().Add(timeout)
|
||||
for time.Now().Before(deadline) {
|
||||
if cond() {
|
||||
return
|
||||
}
|
||||
time.Sleep(2 * time.Millisecond)
|
||||
}
|
||||
if !cond() {
|
||||
t.Fatalf("condition not met within %s", timeout)
|
||||
}
|
||||
}
|
||||
+304
-1
@@ -404,6 +404,31 @@ func run() error {
|
||||
_, _ = w.Write([]byte(`"}]}}`))
|
||||
}))
|
||||
defer mcpProbeUpstream.Close()
|
||||
// targetHealthDownUpstream is opened only to capture a real, unique
|
||||
// "http://127.0.0.1:PORT" URL, then immediately closed — feat/
|
||||
// target-health's own exerciseTargetHealth (below) drives real 502s
|
||||
// through this dead target's per-server proxy path
|
||||
// (/mcp/target-health-down/...) to prove the interpreted tracker
|
||||
// flips it to "unhealthy" after targetHealthFailureThreshold
|
||||
// consecutive failures, without disturbing mcpProbeUpstream's own
|
||||
// (successful, federation-routed) traffic above.
|
||||
targetHealthDownUpstream := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
|
||||
targetHealthDownURL := targetHealthDownUpstream.URL
|
||||
targetHealthDownUpstream.Close()
|
||||
// targetHealthProbeOnlyUpstream answers any JSON-RPC call — in
|
||||
// practice only ever "initialize", from the active probe sweep
|
||||
// (target_health.go's probeMCPTarget) — with a small, valid result
|
||||
// well under mcpBackendResponseMaxBytes (4MiB), so the probe's
|
||||
// handshake genuinely SUCCEEDS. targetHealthProbeOnlyServerName's own
|
||||
// doc comment explains why this server backs a dedicated,
|
||||
// never-proxied target rather than reusing mcpProbeUpstream (whose
|
||||
// 11MiB body exceeds every response-size cap this plugin enforces,
|
||||
// probe included, so it can only ever record a failure).
|
||||
targetHealthProbeOnlyUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":"federated","result":{"protocolVersion":"2025-11-25","capabilities":{}}}`))
|
||||
}))
|
||||
defer targetHealthProbeOnlyUpstream.Close()
|
||||
// anthropicProbeUpstream answers an Anthropic Messages API call with a
|
||||
// real Anthropic-shaped body, so the /v1/messages passthrough branch
|
||||
// (callAnthropicMessagesPassthrough) runs interpreted. A body
|
||||
@@ -587,7 +612,19 @@ func run() error {
|
||||
// runs correctly under Yaegi too, not just the config-override
|
||||
// one testDataWantModel already exercises.
|
||||
`"modelMeta":{"` + testDataWantModel + `":{"contextTokens":` + yaegiMetaContextTokens + `,"inputCostPerMTokMicroUsd":1250000,"outputCostPerMTokMicroUsd":10000000}},` +
|
||||
`"mcpServers":{"` + mcpProbeServerName + `":{"url":"` + mcpProbeUpstream.URL + `"}},` +
|
||||
`"mcpServers":{"` + mcpProbeServerName + `":{"url":"` + mcpProbeUpstream.URL + `"},"` + targetHealthDownServerName + `":{"url":"` + targetHealthDownURL + `"},"` + targetHealthProbeOnlyServerName + `":{"url":"` + targetHealthProbeOnlyUpstream.URL + `"}},` +
|
||||
// targetHealth (feat/target-health): enabled. probeInterval is
|
||||
// deliberately well ABOVE the floor (10s): exerciseTargetHealthProbeWarmup
|
||||
// (below) spends this gateway's ONE sweep for the whole
|
||||
// probeInterval window right after construction, before any other
|
||||
// exercise runs, specifically so no LATER exercise's own GET
|
||||
// /admin/api/targets, /metrics, /v1/mcp/servers, or /v1/agents
|
||||
// call can trigger a second sweep mid-run and race a passive-
|
||||
// traffic assertion (F5, feat/target-health review — see
|
||||
// exerciseTargetHealth's own doc comment for the exact race this
|
||||
// avoids). "60s" comfortably outlasts this whole harness's
|
||||
// real-world runtime.
|
||||
`"targetHealth":{"enabled":true,"probeInterval":"60s"},` +
|
||||
// metrics (Prometheus text-exposition endpoint): enabled with
|
||||
// modelLabel on, so exerciseMetricsRoute below exercises the
|
||||
// opt-in per-(provider,model) breakdown under the interpreter
|
||||
@@ -643,9 +680,29 @@ func run() error {
|
||||
return fmt.Errorf("%s.New's returned value does not implement http.Handler", pkgName)
|
||||
}
|
||||
|
||||
// exerciseTargetHealthProbeWarmup (feat/target-health, F5 review) runs
|
||||
// FIRST, before exerciseHandler and everything after it: its own doc
|
||||
// comment explains why this ordering is what makes every later
|
||||
// target-health assertion in this harness deterministic rather than
|
||||
// racing the active probe sweep under Yaegi's interpreter overhead.
|
||||
if err := exerciseTargetHealthProbeWarmup(handler); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := exerciseHandler(handler, builtinLookupContextTokens, failoverBHits); err != nil {
|
||||
return err
|
||||
}
|
||||
// exerciseTargetHealth (feat/target-health) runs right after
|
||||
// exerciseHandler: its own part (a) reads the passive-recording
|
||||
// side effect of exerciseFederatedTooLarge's federated tools/call,
|
||||
// already driven above as one of exerciseHandler's own sub-probes,
|
||||
// so it must not run before that attempt has happened. Its part (b)
|
||||
// drives its own, independent target (targetHealthDownServerName),
|
||||
// so it cannot perturb any later probe's own counters or /metrics
|
||||
// assertions.
|
||||
if err := exerciseTargetHealth(handler); err != nil {
|
||||
return err
|
||||
}
|
||||
// exerciseBareWinnerGroupAware (fix/group-aware-bare-winner,
|
||||
// 2026-08-27) runs right after exerciseHandler and before
|
||||
// exerciseBreaker: it drives its own, freshly named providers and
|
||||
@@ -753,6 +810,32 @@ const (
|
||||
mcpProbeToolName = mcpProbeServerName + "_big"
|
||||
)
|
||||
|
||||
// targetHealthDownServerName is the second MCP server run() configures,
|
||||
// against targetHealthDownUpstream — a URL nothing listens on, opened
|
||||
// then immediately closed (feat/target-health, exerciseTargetHealth,
|
||||
// below). targetHealthFailureThreshold mirrors the plugin's own
|
||||
// defaultTargetHealthFailureThreshold (target_health.go): this harness
|
||||
// module cannot import that unexported constant across the Yaegi
|
||||
// reflection boundary, so it is copied here, the same independent-
|
||||
// literal approach every other harness constant mirroring plugin
|
||||
// internals already takes in this file (e.g. testDataWantModel).
|
||||
const (
|
||||
targetHealthDownServerName = "target-health-down"
|
||||
targetHealthFailureThreshold = 3
|
||||
)
|
||||
|
||||
// targetHealthProbeOnlyServerName is a THIRD MCP server run() configures,
|
||||
// against targetHealthProbeOnlyUpstream — a real, always-answering
|
||||
// upstream this harness never proxies traffic through (no per-server
|
||||
// proxy call, no federated tools/list or tools/call ever names it): its
|
||||
// health entry can only ever be written by the ACTIVE PROBE sweep
|
||||
// (target_health.go's probeMCPTarget), never by passive traffic. F5,
|
||||
// feat/target-health review: exerciseTargetHealthProbeWarmup (below)
|
||||
// uses this determinism to assert source == "probe" without racing
|
||||
// anything, and to prove the interpreted initialize/close handshake path
|
||||
// itself, not just "some sweep ran".
|
||||
const targetHealthProbeOnlyServerName = "target-health-probe-only"
|
||||
|
||||
// metricsProbeAllowedCIDR is the CIDR block attemptAccountingOverride's
|
||||
// own metrics.allowedCIDRs names — see that literal's own comment for why
|
||||
// this must contain httptest.NewRequest's default RemoteAddr.
|
||||
@@ -1716,6 +1799,226 @@ func exerciseFederatedTooLarge(handler http.Handler) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// targetHealthProbeView is a shape-tolerant decode of one adminTargetView
|
||||
// entry's own "health" object (admin.go's adminTargetHealthView) —
|
||||
// this harness module cannot import the plugin's unexported type across
|
||||
// the Yaegi reflection boundary (New is resolved and called by
|
||||
// reflection, not a normal Go import — this whole file's own doc
|
||||
// comment explains why), so it decodes the JSON shape independently, the
|
||||
// same approach readHealth/readProviderAttemptCounters already take for
|
||||
// other admin responses.
|
||||
type targetHealthProbeView struct {
|
||||
State string `json:"state"`
|
||||
Source string `json:"source"`
|
||||
}
|
||||
|
||||
// pollTargetHealth drives GET /admin/api/targets against handler
|
||||
// repeatedly until name's health satisfies want, or a 5s budget elapses,
|
||||
// then returns its final reading either way — mirrors
|
||||
// pollProviderAttemptCounters's identical poll-with-deadline shape
|
||||
// (below), applied here to feat/target-health's own async sweep
|
||||
// (maybeSweepTargetHealth spawns the actual probing goroutine and
|
||||
// returns immediately, target_health.go's own doc comment) instead of
|
||||
// limiter.spawn's async counter write.
|
||||
func pollTargetHealth(handler http.Handler, name string, want func(targetHealthProbeView) bool) (targetHealthProbeView, error) {
|
||||
deadline := time.Now().Add(5 * time.Second)
|
||||
for {
|
||||
health, err := readTargetHealth(handler, name)
|
||||
if err != nil {
|
||||
return targetHealthProbeView{}, err
|
||||
}
|
||||
if want(health) || time.Now().After(deadline) {
|
||||
return health, nil
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
}
|
||||
|
||||
// readTargetHealth returns name's own "health" object from GET
|
||||
// /admin/api/targets, searching both mcpServers and agents (mcpProbeServerName
|
||||
// and targetHealthDownServerName are both MCP servers, but this checks
|
||||
// both arrays so a future agent-shaped call site can reuse it unchanged).
|
||||
func readTargetHealth(handler http.Handler, name string) (targetHealthProbeView, error) {
|
||||
req := httptest.NewRequest(http.MethodGet, "/admin/api/targets", nil)
|
||||
req.Header.Set("Authorization", "Bearer "+attemptAccountingAdminAPIKey)
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
return targetHealthProbeView{}, fmt.Errorf("GET /admin/api/targets: status = %d, want 200, body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var body struct {
|
||||
MCPServers []struct {
|
||||
Name string `json:"name"`
|
||||
Health targetHealthProbeView `json:"health"`
|
||||
} `json:"mcpServers"`
|
||||
Agents []struct {
|
||||
Name string `json:"name"`
|
||||
Health targetHealthProbeView `json:"health"`
|
||||
} `json:"agents"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
|
||||
return targetHealthProbeView{}, fmt.Errorf("decode GET /admin/api/targets body: %w (body=%s)", err, rec.Body.String())
|
||||
}
|
||||
for _, s := range body.MCPServers {
|
||||
if s.Name == name {
|
||||
return s.Health, nil
|
||||
}
|
||||
}
|
||||
for _, a := range body.Agents {
|
||||
if a.Name == name {
|
||||
return a.Health, nil
|
||||
}
|
||||
}
|
||||
return targetHealthProbeView{}, fmt.Errorf("target %q not in GET /admin/api/targets: %s", name, rec.Body.String())
|
||||
}
|
||||
|
||||
// exerciseTargetHealthProbeWarmup proves feat/target-health's ACTIVE
|
||||
// PROBE sweep end to end under the REAL interpreter, and — just as
|
||||
// important — makes every LATER target-health assertion in this harness
|
||||
// deterministic. Called from run() BEFORE exerciseHandler, i.e. before
|
||||
// any other route this harness hits: GET /admin/api/targets is the
|
||||
// first-ever sweep-capable route (maybeSweepTargetHealth's own doc
|
||||
// comment, target_health.go, lists the four: serveMetrics,
|
||||
// serveAdminTargets, handleMCPServers, handleAgents) this handler sees,
|
||||
// so it deterministically owns the ONE sweep this gateway's
|
||||
// probeInterval (configured to "60s", well above this harness's
|
||||
// real-world runtime) allows for the rest of the run.
|
||||
//
|
||||
// This matters because targetHealthProbeOnlyServerName's own record() is
|
||||
// not the only one a sweep touches — EVERY configured MCP server and
|
||||
// agent gets probed in the same sweep (sweepTargetHealth's own doc
|
||||
// comment), mcpProbeServerName included. Spending that one sweep here,
|
||||
// before exerciseFederatedTooLarge ever drives mcpProbeServerName's real
|
||||
// traffic observation, guarantees the traffic observation is the LAST
|
||||
// write to that entry for the remainder of the run — no later
|
||||
// admin/metrics/listing route can trigger a second sweep to race it
|
||||
// (F5, feat/target-health review: this is what closes the race that
|
||||
// used to make TARGETHEALTH-2, below, racy under Yaegi's interpreter
|
||||
// overhead).
|
||||
//
|
||||
// G5, feat/target-health review round 2: that guarantee only holds once
|
||||
// EVERY configured MCP server's own probe observation has actually
|
||||
// landed, not merely targetHealthProbeOnlyServerName's. sweepTargetHealth
|
||||
// probes every configured server CONCURRENTLY, on its own goroutines —
|
||||
// waiting for one target's record() to land says nothing about whether
|
||||
// mcpProbeServerName's own probe goroutine has finished yet. A version of
|
||||
// this function that returned as soon as targetHealthProbeOnlyServerName
|
||||
// alone changed could return while mcpProbeServerName's probe write was
|
||||
// still in flight; that write could then land AFTER
|
||||
// exerciseFederatedTooLarge's later traffic write, silently reverting
|
||||
// mcpProbeServerName's source back to "probe" and corrupting
|
||||
// exerciseTargetHealth's own TARGETHEALTH-2 assertion (source ==
|
||||
// "traffic"). Polling every configured MCP server for its own source ==
|
||||
// "probe" closes that gap: this function cannot return until the whole
|
||||
// sweep — not just the one target it happens to check last — has
|
||||
// finished writing.
|
||||
//
|
||||
// targetHealthProbeOnlyServerName is additionally asserted to the FULL
|
||||
// "healthy" state, not just source == "probe": it is NEVER proxied or
|
||||
// federated through by any other exercise in this harness, so its health
|
||||
// entry can only ever be written by the probe — recover's own swallow
|
||||
// (sweepTargetHealth's defer, target_health.go) means an interpreted
|
||||
// panic mid-probe would otherwise degrade to a silently-stuck "unknown"
|
||||
// rather than a loud failure, and asserting the full success shape here
|
||||
// is what makes a Yaegi incompatibility in the initialize/close handshake
|
||||
// path visible instead of passing silently.
|
||||
func exerciseTargetHealthProbeWarmup(handler http.Handler) error {
|
||||
warmupServerNames := []string{mcpProbeServerName, targetHealthDownServerName, targetHealthProbeOnlyServerName}
|
||||
var probeOnlyHealth targetHealthProbeView
|
||||
for _, name := range warmupServerNames {
|
||||
health, err := pollTargetHealth(handler, name, func(h targetHealthProbeView) bool {
|
||||
return h.Source == "probe"
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if health.Source != "probe" {
|
||||
return fmt.Errorf("TARGETHEALTH-PROBE-1: GET /admin/api/targets: %q health.source = %q, want %q (the warmup sweep must probe every configured MCP server before any later exercise runs)", name, health.Source, "probe")
|
||||
}
|
||||
if name == targetHealthProbeOnlyServerName {
|
||||
probeOnlyHealth = health
|
||||
}
|
||||
}
|
||||
if probeOnlyHealth.State != "healthy" {
|
||||
return fmt.Errorf("TARGETHEALTH-PROBE-2: GET /admin/api/targets: %q health.state = %q, want healthy (targetHealthProbeOnlyUpstream always answers initialize successfully)", targetHealthProbeOnlyServerName, probeOnlyHealth.State)
|
||||
}
|
||||
|
||||
fmt.Println("yaegi-check: target health active probe sweep ran and recorded correctly interpreted")
|
||||
return nil
|
||||
}
|
||||
|
||||
// exerciseTargetHealth proves feat/target-health's PASSIVE recording end
|
||||
// to end under the REAL interpreter, now that
|
||||
// exerciseTargetHealthProbeWarmup (run() calls it first, above) has
|
||||
// already spent this gateway's one probeInterval-bounded sweep — no
|
||||
// further sweep can fire during this function, so neither part below
|
||||
// races the active probe path.
|
||||
//
|
||||
// Part (a): mcpProbeServerName receives one real federated tools/call
|
||||
// attempt via exerciseFederatedTooLarge (exerciseHandler, called before
|
||||
// this function) — its oversized response trips errMCPResponseTooLarge,
|
||||
// a transport-level failure from doBackendJSONRPC's own perspective
|
||||
// (mcp_federation.go), which mcpFederatedToolsCall records as a
|
||||
// target-health failure. This is the ONLY observation this target ever
|
||||
// receives (the warmup above never touches it — see
|
||||
// exerciseTargetHealthProbeWarmup's own doc comment for why spending the
|
||||
// sweep before this call matters), so it has never once succeeded:
|
||||
// F4 (feat/target-health review) reads that as "unknown", not "healthy"
|
||||
// — the failure-threshold gate still protects against this single
|
||||
// transient-looking failure, so it does not read "unhealthy" either.
|
||||
//
|
||||
// Part (b): drives targetHealthFailureThreshold consecutive failures
|
||||
// through the direct per-server proxy path (/mcp/{name}/...,
|
||||
// handleTargetProxy) against targetHealthDownServerName's dead upstream,
|
||||
// then asserts the SAME tracker flips that target to "unhealthy" on
|
||||
// both GET /admin/api/targets and llmgateway_target_healthy on
|
||||
// /metrics.
|
||||
func exerciseTargetHealth(handler http.Handler) error {
|
||||
probeHealth, err := readTargetHealth(handler, mcpProbeServerName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if probeHealth.State != "unknown" {
|
||||
return fmt.Errorf("TARGETHEALTH-1: GET /admin/api/targets: %q health.state = %q, want unknown (one failed federated tools/call attempt, never having succeeded, below threshold — F4): %+v", mcpProbeServerName, probeHealth.State, probeHealth)
|
||||
}
|
||||
if probeHealth.Source != "traffic" {
|
||||
return fmt.Errorf("TARGETHEALTH-2: GET /admin/api/targets: %q health.source = %q, want %q", mcpProbeServerName, probeHealth.Source, "traffic")
|
||||
}
|
||||
|
||||
for i := 0; i < targetHealthFailureThreshold; i++ {
|
||||
req := httptest.NewRequest(http.MethodGet, "/mcp/"+targetHealthDownServerName+"/tools/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer "+testDataUserAPIKey)
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
return fmt.Errorf("TARGETHEALTH-3: GET /mcp/%s/tools/list attempt %d: status = %d, want 502 (dead upstream), body=%s", targetHealthDownServerName, i, rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
downHealth, err := readTargetHealth(handler, targetHealthDownServerName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if downHealth.State != "unhealthy" {
|
||||
return fmt.Errorf("TARGETHEALTH-4: GET /admin/api/targets: %q health.state = %q, want unhealthy after %d consecutive failures: %+v", targetHealthDownServerName, downHealth.State, targetHealthFailureThreshold, downHealth)
|
||||
}
|
||||
|
||||
metricsReq := httptest.NewRequest(http.MethodGet, "/metrics", nil)
|
||||
metricsReq.Header.Set("Authorization", "Bearer "+attemptAccountingAdminAPIKey)
|
||||
metricsRec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(metricsRec, metricsReq)
|
||||
if metricsRec.Code != http.StatusOK {
|
||||
return fmt.Errorf("GET /metrics: status = %d, want 200, body=%s", metricsRec.Code, metricsRec.Body.String())
|
||||
}
|
||||
wantSample := `llmgateway_target_healthy{kind="mcp",target="` + targetHealthDownServerName + `"} 0`
|
||||
if !strings.Contains(metricsRec.Body.String(), wantSample) {
|
||||
return fmt.Errorf("TARGETHEALTH-5: GET /metrics missing %q — interpreted target-health metric diverged from the compiled shape; body=%s", wantSample, metricsRec.Body.String())
|
||||
}
|
||||
|
||||
fmt.Println("yaegi-check: target health tracked and exposed correctly interpreted (admin API + /metrics)")
|
||||
return nil
|
||||
}
|
||||
|
||||
// exerciseFederatedSSENegotiation drives two real POST /mcp "ping" calls
|
||||
// against the interpreted handler — one carrying no Accept header
|
||||
// (today's plain JSON framing) and one carrying the reported client's
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { formatCompactCount, formatContextWindow, formatLatencyMs, formatModelCostHover, formatUntil, refreshDetailLabel, refreshLabel } from './format'
|
||||
|
||||
const ZERO_TIME = '0001-01-01T00:00:00Z'
|
||||
import { formatCompactCount, formatContextWindow, formatElapsedAgo, formatLatencyMs, formatModelCostHover, formatUntil, refreshDetailLabel, refreshLabel, ZERO_TIME } from './format'
|
||||
|
||||
describe('refreshLabel', () => {
|
||||
it('reads "discovery off" when discovery is disabled, regardless of lastRefresh', () => {
|
||||
@@ -183,3 +181,46 @@ describe('formatLatencyMs', () => {
|
||||
expect(formatLatencyMs(ms)).toBe(want)
|
||||
})
|
||||
})
|
||||
|
||||
// Target health (feat/target-health): formatElapsedAgo is formatAgo's bare
|
||||
// "Ns ago" counterpart — no parentheses — so a caller can compose it into
|
||||
// either "unhealthy (Ns ago)" or "Ns ago via probe" without stripping
|
||||
// punctuation back out. The explicit `now` parameter keeps every case
|
||||
// below exact instead of racing Date.now() between computing the fixture
|
||||
// and asserting on it.
|
||||
describe('formatElapsedAgo', () => {
|
||||
const now = new Date('2026-09-03T12:00:00Z').getTime()
|
||||
|
||||
it('renders "" for the unset zero-time sentinel', () => {
|
||||
expect(formatElapsedAgo(ZERO_TIME, now)).toBe('')
|
||||
})
|
||||
|
||||
it('renders "" for undefined', () => {
|
||||
expect(formatElapsedAgo(undefined, now)).toBe('')
|
||||
})
|
||||
|
||||
it('renders "" for an unparseable timestamp', () => {
|
||||
expect(formatElapsedAgo('not-a-date', now)).toBe('')
|
||||
})
|
||||
|
||||
it('renders "0s ago" for a timestamp equal to now', () => {
|
||||
expect(formatElapsedAgo(new Date(now).toISOString(), now)).toBe('0s ago')
|
||||
})
|
||||
|
||||
it('renders "Ns ago" for a timestamp seconds in the past', () => {
|
||||
expect(formatElapsedAgo(new Date(now - 42_000).toISOString(), now)).toBe('42s ago')
|
||||
})
|
||||
|
||||
it('renders whole elapsed seconds for a timestamp minutes in the past — no unit beyond seconds', () => {
|
||||
expect(formatElapsedAgo(new Date(now - 90_000).toISOString(), now)).toBe('90s ago')
|
||||
})
|
||||
|
||||
it('floors a future timestamp at 0s rather than showing a negative countdown (defensive — a clock-skewed lastCheck)', () => {
|
||||
expect(formatElapsedAgo(new Date(now + 5_000).toISOString(), now)).toBe('0s ago')
|
||||
})
|
||||
|
||||
it('defaults `now` to Date.now() when omitted', () => {
|
||||
const iso = new Date(Date.now() - 5_000).toISOString()
|
||||
expect(formatElapsedAgo(iso)).toMatch(/^\d+s ago$/)
|
||||
})
|
||||
})
|
||||
|
||||
+36
-6
@@ -1,6 +1,7 @@
|
||||
import type { HistoryWindow, LimitsConfig } from '@/types/api'
|
||||
|
||||
const ZERO_TIME = '0001-01-01T00:00:00Z'
|
||||
/** ZERO_TIME is Go's zero time.Time value in its default JSON (RFC 3339) encoding — the unset-timestamp sentinel every admin API "never happened yet" field marshals as, and every formatter/test below treats as absent. */
|
||||
export const ZERO_TIME = '0001-01-01T00:00:00Z'
|
||||
|
||||
/**
|
||||
* routableModelId joins a provider name and one of its own model ids into
|
||||
@@ -162,13 +163,42 @@ export function formatLatencyMs(ms: number): string {
|
||||
return `${(ms / 1000).toFixed(1)}s`
|
||||
}
|
||||
|
||||
/**
|
||||
* elapsedSecondsSince computes whole seconds between `iso` and `now`, or
|
||||
* null for the unset zero-time sentinel or an unparseable value — the
|
||||
* shared guard formatAgo and formatElapsedAgo below both build on, so
|
||||
* neither duplicates the sentinel/NaN check.
|
||||
*/
|
||||
function elapsedSecondsSince(iso: string | undefined, now: number): number | null {
|
||||
if (!iso || iso === ZERO_TIME) return null
|
||||
const then = new Date(iso).getTime()
|
||||
if (Number.isNaN(then)) return null
|
||||
return Math.max(0, Math.round((now - then) / 1000))
|
||||
}
|
||||
|
||||
/** formatAgo renders an ISO timestamp as "(Ns ago)", or "" for the unset zero-time sentinel. */
|
||||
export function formatAgo(iso: string | undefined): string {
|
||||
if (!iso || iso === ZERO_TIME) return ''
|
||||
const then = new Date(iso).getTime()
|
||||
if (Number.isNaN(then)) return ''
|
||||
const secs = Math.max(0, Math.round((Date.now() - then) / 1000))
|
||||
return ` (${secs}s ago)`
|
||||
const secs = elapsedSecondsSince(iso, Date.now())
|
||||
return secs === null ? '' : ` (${secs}s ago)`
|
||||
}
|
||||
|
||||
/**
|
||||
* formatElapsedAgo renders elapsed time since an ISO timestamp as a bare
|
||||
* "Ns ago" string — no surrounding parentheses, unlike formatAgo above
|
||||
* (every existing formatAgo caller depends on its own " (Ns ago)" shape
|
||||
* unchanged, so that convention stays untouched here). Target health
|
||||
* badges (AdminTargetHealthView, TargetsView's health column — feat/
|
||||
* target-health) are the only caller today: a target's `lastCheck` needs
|
||||
* this bare form both standalone ("unhealthy (Ns ago)", parenthesized by
|
||||
* the caller) and inline ("Ns ago via probe"). The optional `now`
|
||||
* parameter (default Date.now()) makes the elapsed-time math
|
||||
* deterministic under test. "" for the unset zero-time sentinel or an
|
||||
* unparseable timestamp, mirroring formatAgo's own "nothing to show"
|
||||
* convention.
|
||||
*/
|
||||
export function formatElapsedAgo(iso: string | undefined, now = Date.now()): string {
|
||||
const secs = elapsedSecondsSince(iso, now)
|
||||
return secs === null ? '' : `${secs}s ago`
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
import { isVNode } from 'vue'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
import { targetColumns } from './target-columns'
|
||||
import type { AdminTargetHealthView, AdminTargetView } from '@/types/api'
|
||||
|
||||
/**
|
||||
* Target health rendering (feat/target-health): mirrors usage-columns.
|
||||
* spec.ts's own approach of exercising a ColumnDef's `cell` renderer
|
||||
* directly (no component mount needed — vite.config.ts's `test` block
|
||||
* runs vitest in the 'node' environment with no @vue/test-utils/jsdom in
|
||||
* this package, so there is no existing "mount a .vue view" pattern to
|
||||
* follow here; the health column's rendering logic lives entirely in this
|
||||
* plain TanStack ColumnDef, exactly like usage-columns.ts's own cells, so
|
||||
* the identical direct-call approach gives full coverage without either).
|
||||
*/
|
||||
function testTarget(health: AdminTargetHealthView): AdminTargetView {
|
||||
return {
|
||||
name: 'my-server',
|
||||
url: 'https://mcp.example.com',
|
||||
counters: { requestsPerMinute: 0, requestsPerDay: 0, requestsPerMonth: 0 },
|
||||
health,
|
||||
}
|
||||
}
|
||||
|
||||
function healthCellOf(target: AdminTargetView): unknown {
|
||||
const col = targetColumns().find((c) => c.id === 'health')
|
||||
const cell = col?.cell as (ctx: { row: { original: AdminTargetView } }) => unknown
|
||||
return cell({ row: { original: target } })
|
||||
}
|
||||
|
||||
describe('targetColumns: health column', () => {
|
||||
it('renders a destructive Badge for unhealthy, with a relative-time label and lastError in the title', () => {
|
||||
const health: AdminTargetHealthView = {
|
||||
state: 'unhealthy',
|
||||
lastCheck: new Date(Date.now() - 42_000).toISOString(),
|
||||
lastError: 'connection refused',
|
||||
consecutiveFailures: 3,
|
||||
source: 'probe',
|
||||
}
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
|
||||
expect(isVNode(rendered)).toBe(true)
|
||||
const vnode = rendered as { type: unknown; props?: Record<string, unknown>; children?: unknown }
|
||||
expect(vnode.type).toBe(Badge)
|
||||
expect(vnode.props?.variant).toBe('destructive')
|
||||
expect(vnode.props?.title).toBe('3 consecutive failures via probe: connection refused')
|
||||
|
||||
const slot = (vnode.children as { default: () => string }).default
|
||||
expect(slot()).toMatch(/^unhealthy \(\d+s ago\)$/)
|
||||
})
|
||||
|
||||
it('falls back to a bare "unhealthy" label when lastCheck is missing (version-skew defensive case)', () => {
|
||||
const health: AdminTargetHealthView = { state: 'unhealthy', consecutiveFailures: 1 }
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { props?: Record<string, unknown>; children?: { default: () => string } }
|
||||
|
||||
expect(vnode.children?.default()).toBe('unhealthy')
|
||||
// Singular "failure", no "via <source>", no lastError.
|
||||
expect(vnode.props?.title).toBe('1 consecutive failure')
|
||||
})
|
||||
|
||||
it('renders an outline Badge labeled "unknown" with a fixed explanatory title', () => {
|
||||
const health: AdminTargetHealthView = { state: 'unknown', consecutiveFailures: 0 }
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { type: unknown; props?: Record<string, unknown>; children?: { default: () => string } }
|
||||
|
||||
expect(vnode.type).toBe(Badge)
|
||||
expect(vnode.props?.variant).toBe('outline')
|
||||
expect(vnode.props?.title).toBe('not observed yet — no traffic and probes disabled or not yet run')
|
||||
expect(vnode.children?.default()).toBe('unknown')
|
||||
})
|
||||
|
||||
it('renders no Badge for healthy — plain muted text with latency, and relative time + source in the title', () => {
|
||||
const health: AdminTargetHealthView = {
|
||||
state: 'healthy',
|
||||
lastCheck: new Date(Date.now() - 5_000).toISOString(),
|
||||
consecutiveFailures: 0,
|
||||
latencyMs: 12,
|
||||
source: 'traffic',
|
||||
}
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
|
||||
expect(isVNode(rendered)).toBe(true)
|
||||
const vnode = rendered as { type: unknown; props?: Record<string, unknown>; children?: unknown }
|
||||
expect(vnode.type).toBe('span')
|
||||
expect(vnode.props?.class).toBe('text-muted-foreground')
|
||||
// formatLatencyMs (lib/format.ts), not a raw "${latencyMs} ms" — no
|
||||
// space, matching every other latency reading in this panel.
|
||||
expect(vnode.children).toBe('healthy · 12ms')
|
||||
expect(vnode.props?.title).toMatch(/^\d+s ago via traffic$/)
|
||||
})
|
||||
|
||||
it('renders the healthy latency suffix in seconds via formatLatencyMs once above 1000ms', () => {
|
||||
const health: AdminTargetHealthView = { state: 'healthy', consecutiveFailures: 0, latencyMs: 1500 }
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { children?: unknown }
|
||||
expect(vnode.children).toBe('healthy · 1.5s')
|
||||
})
|
||||
|
||||
it('omits the latency suffix for healthy when latencyMs is unknown', () => {
|
||||
const health: AdminTargetHealthView = { state: 'healthy', consecutiveFailures: 0 }
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { children?: unknown; props?: Record<string, unknown> }
|
||||
|
||||
expect(vnode.children).toBe('healthy')
|
||||
// Neither lastCheck nor source known — nothing to show in the title.
|
||||
expect(vnode.props?.title).toBe('')
|
||||
})
|
||||
|
||||
it('the health column is not sortable — three semantic states have no meaningful alphabetical order', () => {
|
||||
const col = targetColumns().find((c) => c.id === 'health')
|
||||
expect(col?.enableSorting).toBe(false)
|
||||
})
|
||||
|
||||
// F4/F7 (feat/target-health review): "unknown" also covers a target
|
||||
// that HAS been observed but has never succeeded, still below
|
||||
// failureThreshold — that case carries real failure detail even
|
||||
// though the label stays "unknown".
|
||||
it('surfaces failure detail in the unknown badge title for a target observed but never succeeded', () => {
|
||||
const health: AdminTargetHealthView = {
|
||||
state: 'unknown',
|
||||
consecutiveFailures: 2,
|
||||
source: 'probe',
|
||||
lastError: 'dial tcp: connection refused',
|
||||
}
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { type: unknown; props?: Record<string, unknown>; children?: { default: () => string } }
|
||||
|
||||
expect(vnode.type).toBe(Badge)
|
||||
expect(vnode.props?.variant).toBe('outline')
|
||||
expect(vnode.props?.title).toBe('2 failures so far via probe: dial tcp: connection refused')
|
||||
expect(vnode.children?.default()).toBe('unknown')
|
||||
})
|
||||
|
||||
it('singular "failure" and no "via <source>"/lastError when only consecutiveFailures is known', () => {
|
||||
const health: AdminTargetHealthView = { state: 'unknown', consecutiveFailures: 1 }
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { props?: Record<string, unknown> }
|
||||
expect(vnode.props?.title).toBe('1 failure so far')
|
||||
})
|
||||
|
||||
it('falls back to the outline "unknown" badge for any state other than healthy/unhealthy', () => {
|
||||
// Defensive: healthCell's own state check is an explicit
|
||||
// state === 'healthy' branch with "unknown" as the fallback for
|
||||
// everything else — not the reverse — so a value this version of
|
||||
// the webui does not recognize renders as "unknown", never as a
|
||||
// silently-wrong "healthy".
|
||||
const health = { state: 'something-future-versions-might-add', consecutiveFailures: 0 } as unknown as AdminTargetHealthView
|
||||
const rendered = healthCellOf(testTarget(health))
|
||||
const vnode = rendered as { type: unknown; props?: Record<string, unknown>; children?: { default: () => string } }
|
||||
|
||||
expect(vnode.type).toBe(Badge)
|
||||
expect(vnode.props?.variant).toBe('outline')
|
||||
expect(vnode.children?.default()).toBe('unknown')
|
||||
})
|
||||
})
|
||||
@@ -3,7 +3,8 @@ import { h } from 'vue'
|
||||
|
||||
import CompactNumber from '@/components/CompactNumber.vue'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
import type { AdminTargetView } from '@/types/api'
|
||||
import { formatElapsedAgo, formatLatencyMs } from '@/lib/format'
|
||||
import type { AdminTargetHealthView, AdminTargetView } from '@/types/api'
|
||||
|
||||
/**
|
||||
* numericColumn builds one right-aligned, sortable request-counter
|
||||
@@ -27,16 +28,91 @@ function numericColumn(id: string, header: string, read: (t: AdminTargetView) =>
|
||||
}
|
||||
}
|
||||
|
||||
// --- target health (feat/target-health) ---
|
||||
//
|
||||
// Mirrors ProvidersView.vue's own healthBadgeVariant/healthBadgeLabel
|
||||
// convention for the discovery circuit breaker: a Badge only for the two
|
||||
// states worth flagging (unhealthy, unknown); the common healthy case
|
||||
// renders no badge at all, just muted inline text — the same "nothing to
|
||||
// show reads as no badge" idiom every health/rate indicator in this panel
|
||||
// already follows (see AdminTargetHealthView's own doc comment, api.ts,
|
||||
// for what each field means).
|
||||
|
||||
const UNKNOWN_HEALTH_TITLE = 'not observed yet — no traffic and probes disabled or not yet run'
|
||||
|
||||
/** unhealthyLabel keeps the Badge's own visible text short — "unhealthy (Ns ago)" when lastCheck is known, plain "unhealthy" otherwise (defensive: a version-skewed server response omitting lastCheck on an unhealthy row). */
|
||||
function unhealthyLabel(health: AdminTargetHealthView): string {
|
||||
const ago = formatElapsedAgo(health.lastCheck)
|
||||
return ago ? `unhealthy (${ago})` : 'unhealthy'
|
||||
}
|
||||
|
||||
/** unhealthyTitle carries the detail the short label leaves out — consecutiveFailures, source, and lastError (when present) — e.g. "3 consecutive failures via probe: connection refused". */
|
||||
function unhealthyTitle(health: AdminTargetHealthView): string {
|
||||
const failWord = health.consecutiveFailures === 1 ? 'failure' : 'failures'
|
||||
const sourcePart = health.source ? ` via ${health.source}` : ''
|
||||
const base = `${health.consecutiveFailures} consecutive ${failWord}${sourcePart}`
|
||||
return health.lastError ? `${base}: ${health.lastError}` : base
|
||||
}
|
||||
|
||||
/** healthyTitle is the muted "healthy" text's hover detail — "Ns ago via probe/traffic" — omitting whichever half (relative time, source) the response does not carry. */
|
||||
function healthyTitle(health: AdminTargetHealthView): string {
|
||||
const ago = formatElapsedAgo(health.lastCheck)
|
||||
if (!ago) return health.source ? `via ${health.source}` : ''
|
||||
return health.source ? `${ago} via ${health.source}` : ago
|
||||
}
|
||||
|
||||
/**
|
||||
* unknownTitle renders the "unknown" badge's hover detail. F4
|
||||
* (feat/target-health) reads "unknown" two different ways — see
|
||||
* AdminTargetHealthView's own doc comment (types/api.ts): never observed
|
||||
* at all, which gets the generic UNKNOWN_HEALTH_TITLE, or observed but
|
||||
* never yet succeeded (still below failureThreshold), which carries real
|
||||
* failure detail (consecutiveFailures/source/lastError) even though the
|
||||
* label stays "unknown" — this surfaces that detail instead of the
|
||||
* generic message, mirroring unhealthyTitle's own conditional shape.
|
||||
*/
|
||||
function unknownTitle(health: AdminTargetHealthView): string {
|
||||
if (health.consecutiveFailures <= 0 && !health.lastError) return UNKNOWN_HEALTH_TITLE
|
||||
const failWord = health.consecutiveFailures === 1 ? 'failure' : 'failures'
|
||||
const sourcePart = health.source ? ` via ${health.source}` : ''
|
||||
const base = `${health.consecutiveFailures} ${failWord} so far${sourcePart}`
|
||||
return health.lastError ? `${base}: ${health.lastError}` : base
|
||||
}
|
||||
|
||||
/**
|
||||
* healthCell renders one row's health column: destructive Badge for
|
||||
* unhealthy, muted plain text (plus " · " and formatLatencyMs when
|
||||
* latencyMs is known) for the explicit healthy case, and an outline
|
||||
* "unknown" Badge as the FALLBACK for everything else — not the reverse
|
||||
* — so a state value this webui build does not recognize renders as
|
||||
* "unknown" rather than silently reading as healthy.
|
||||
*/
|
||||
function healthCell(health: AdminTargetHealthView) {
|
||||
if (health.state === 'unhealthy') {
|
||||
return h(
|
||||
Badge,
|
||||
{ as: 'span', variant: 'destructive', class: 'font-normal', title: unhealthyTitle(health) },
|
||||
() => unhealthyLabel(health),
|
||||
)
|
||||
}
|
||||
if (health.state === 'healthy') {
|
||||
const latencyPart = health.latencyMs !== undefined ? ` · ${formatLatencyMs(health.latencyMs)}` : ''
|
||||
return h('span', { class: 'text-muted-foreground', title: healthyTitle(health) }, `healthy${latencyPart}`)
|
||||
}
|
||||
return h(Badge, { as: 'span', variant: 'outline', class: 'font-normal', title: unknownTitle(health) }, () => 'unknown')
|
||||
}
|
||||
|
||||
/**
|
||||
* targetColumns builds the shared TanStack `ColumnDef` set both the MCP
|
||||
* servers and the Agents DataTable in TargetsView.vue use (admin.go:
|
||||
* adminTargetView — the two tables share one identical row shape). name is
|
||||
* plain text (not the ModelChip copy treatment — a target name is not a
|
||||
* routable model id); url is muted and truncated with a title attribute
|
||||
* for the full value; access renders as a Badge per allowed group, or a
|
||||
* muted "All groups" when unrestricted — the exact idiom UsageView.vue's
|
||||
* own group Access block already established (Providers/Models/MCP
|
||||
* servers/Agents dt/dd pairs there).
|
||||
* for the full value; health is the Badge/muted-text pair healthCell above
|
||||
* builds (feat/target-health); access renders as a Badge per allowed
|
||||
* group, or a muted "All groups" when unrestricted — the exact idiom
|
||||
* UsageView.vue's own group Access block already established (Providers/
|
||||
* Models/MCP servers/Agents dt/dd pairs there).
|
||||
*/
|
||||
export function targetColumns(): ColumnDef<AdminTargetView, unknown>[] {
|
||||
return [
|
||||
@@ -57,6 +133,17 @@ export function targetColumns(): ColumnDef<AdminTargetView, unknown>[] {
|
||||
row.original.url,
|
||||
),
|
||||
},
|
||||
{
|
||||
id: 'health',
|
||||
header: 'Health',
|
||||
// No natural sort order across three semantic states (healthy/
|
||||
// unhealthy/unknown) — plain alphabetical would put "healthy" and
|
||||
// "unhealthy" beside each other for the wrong reason. Same call the
|
||||
// access column below makes for its own non-primitive shape.
|
||||
enableSorting: false,
|
||||
accessorFn: (t) => t.health.state,
|
||||
cell: ({ row }) => healthCell(row.original.health),
|
||||
},
|
||||
{
|
||||
id: 'access',
|
||||
header: 'Access',
|
||||
|
||||
@@ -316,6 +316,43 @@ export interface AdminTargetCountersView {
|
||||
requestsPerMonth: number
|
||||
}
|
||||
|
||||
/**
|
||||
* One configured MCP server's or A2A agent's current health, matching the
|
||||
* GET /admin/api/targets `health` object the Go side builds against
|
||||
* (feat/target-health). Two independent sources feed `state`, never
|
||||
* conflated: 'traffic' is PASSIVE — recorded from the target's own real
|
||||
* proxied/federated request traffic as it happens, no extra requests
|
||||
* sent; 'probe' is the gateway's own lazy health check, triggered
|
||||
* opportunistically by an operator hitting /metrics, /admin, or a listing
|
||||
* endpoint (never a background poller). `state` is 'unhealthy' once at
|
||||
* least the configured `failureThreshold` consecutive failures have been
|
||||
* observed (whether or not this target has ever succeeded), 'healthy'
|
||||
* once it has succeeded at least once and stays below that threshold, and
|
||||
* 'unknown' otherwise. `consecutiveFailures` is always present.
|
||||
*
|
||||
* 'unknown' reads two different ways server-side, and only ONE of them
|
||||
* omits the rest of this object (F4, feat/target-health review):
|
||||
* - never observed by either source at all: every other field is
|
||||
* omitted — there is truly nothing to report.
|
||||
* - observed, but never once succeeded, still below `failureThreshold`:
|
||||
* every other field is PRESENT — `lastCheck`/`lastError`/`latencyMs`/
|
||||
* `source` all carry a real observation, this target simply has not
|
||||
* answered successfully yet. Do not treat 'unknown' as "no data" for
|
||||
* this case; check whether `lastCheck` is set instead.
|
||||
*
|
||||
* `lastError` is additionally omitted whenever it is empty (a healthy
|
||||
* target has nothing to show), and `latencyMs`/`source` are omitted only
|
||||
* when genuinely not known (the never-observed case above).
|
||||
*/
|
||||
export interface AdminTargetHealthView {
|
||||
state: 'healthy' | 'unhealthy' | 'unknown'
|
||||
lastCheck?: string
|
||||
lastError?: string
|
||||
consecutiveFailures: number
|
||||
latencyMs?: number
|
||||
source?: 'probe' | 'traffic'
|
||||
}
|
||||
|
||||
/**
|
||||
* One configured MCP server's or A2A agent's row in GET /admin/api/targets
|
||||
* (admin.go: adminTargetView). Access is the group names actually allowed
|
||||
@@ -329,6 +366,8 @@ export interface AdminTargetView {
|
||||
url: string
|
||||
access?: string[]
|
||||
counters: AdminTargetCountersView
|
||||
/** See AdminTargetHealthView's own doc comment (feat/target-health) — always present, unlike access above. */
|
||||
health: AdminTargetHealthView
|
||||
}
|
||||
|
||||
/** GET /admin/api/targets (admin.go: adminTargetsResponse). */
|
||||
|
||||
Reference in New Issue
Block a user