From 730eb5ea9d2e84ebcd108b96845cee7397343f1f Mon Sep 17 00:00:00 2001 From: Evanfeenstra Date: Tue, 15 Sep 2026 16:00:27 -0700 Subject: [PATCH] gateway: tolerate non-string values in Bifrost log metadata (fixes 502'd rollups) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bifrost's logging plugin stamps bool markers into a row's metadata: `realtime: true` on realtime turns and `isAsyncRequest: true` on x-bf-async jobs (plugins/logging/main.go). The plugin decoded metadata straight into map[string]string, so a single such row in the window failed the whole /api/logs page and every rollup built on it (spend.by_agent, by_user, by_agent_user, histogram.cost) returned 502 — the dashboard went blank on 2026-09-15 after a realtime turn landed in the 24h window. Decode metadata through a metadataMap type instead: strings verbatim, bool/number values as their JSON text, nested and null values dropped. Applied to both the list row and the detail row. --- gateway/internal/adminapi/logstore_client.go | 75 +++++++++++++++---- .../internal/adminapi/logstore_client_test.go | 70 +++++++++++++++++ 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/gateway/internal/adminapi/logstore_client.go b/gateway/internal/adminapi/logstore_client.go index 88deb645a..1e2187b87 100644 --- a/gateway/internal/adminapi/logstore_client.go +++ b/gateway/internal/adminapi/logstore_client.go @@ -1,6 +1,7 @@ package adminapi import ( + "bytes" "context" "encoding/base64" "encoding/json" @@ -77,6 +78,53 @@ func basicAuth(user, pass string) string { // ─── search ────────────────────────────────────────────────────────── +// metadataMap is Bifrost's log `metadata` decoded to strings. +// +// On the wire it is a map[string]interface{} (framework/logstore/ +// tables.go: MetadataParsed). The dim headers the plugin stamps are +// all strings, but Bifrost's own logging plugin adds bool markers to +// some rows — `realtime: true` on realtime turns and `isAsyncRequest: +// true` on x-bf-async jobs (plugins/logging/main.go). Decoding the +// map straight into map[string]string made one such row fail the +// whole /api/logs page it sat on, which took every rollup for the +// window down with it (2026-09-15: a single realtime turn in the +// 24h window 502'd spend.by_agent, by_user, histogram.cost, ...). +// +// String values are kept verbatim. Bool and number values are kept +// as their JSON text ("true", "42") so they stay visible in detail +// views and usable as filters. Nested objects/arrays and nulls are +// dropped — no dim is ever nested, and a rollup has no use for them. +type metadataMap map[string]string + +func (m *metadataMap) UnmarshalJSON(b []byte) error { + b = bytes.TrimSpace(b) + if len(b) == 0 || bytes.Equal(b, []byte("null")) { + *m = nil + return nil + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(b, &raw); err != nil { + return err + } + out := make(metadataMap, len(raw)) + for k, v := range raw { + v = bytes.TrimSpace(v) + // null must be checked first: json.Unmarshal(null, &string) is + // a silent no-op, which would keep the key as "". + if len(v) == 0 || v[0] == '{' || v[0] == '[' || bytes.Equal(v, []byte("null")) { + continue + } + var s string + if err := json.Unmarshal(v, &s); err == nil { + out[k] = s + continue + } + out[k] = string(v) + } + *m = out + return nil +} + // logstoreLog is the subset of Bifrost's Log columns phase 8 reads. // Defined inline — tygo doesn't emit this; the SPA only sees the // shapes the plugin's own handlers return. @@ -90,11 +138,10 @@ type logstoreLog struct { Latency float64 `json:"latency"` CustomerID string `json:"customer_id"` - // Bifrost emits metadata as a JSON string on the wire (the gorm - // model marks it `json:"-"` then a separate hook re-attaches as - // "metadata"). Decoding as map[string]string covers every dim - // header the plugin canonicalises. - Metadata map[string]string `json:"metadata"` + // Metadata is Bifrost's per-row label map (dim headers, x-bf-lh-* + // labels, and Bifrost's own markers). See metadataMap for why it + // is not decoded straight into map[string]string. + Metadata metadataMap `json:"metadata"` // TokenUsage is Bifrost's provider-reported usage. The list // endpoint selects the denormalised prompt/completion/total @@ -249,15 +296,15 @@ func (c *logstoreClient) searchAll( // Bifrost minimal: a new field in `schemas.ChatMessage` upstream is // invisible to us. type logstoreLogDetail struct { - ID string `json:"id"` - Timestamp string `json:"timestamp"` - Provider string `json:"provider"` - Model string `json:"model"` - Status string `json:"status"` - Cost float64 `json:"cost"` - Latency float64 `json:"latency"` - CustomerID string `json:"customer_id"` - Metadata map[string]string `json:"metadata"` + ID string `json:"id"` + Timestamp string `json:"timestamp"` + Provider string `json:"provider"` + Model string `json:"model"` + Status string `json:"status"` + Cost float64 `json:"cost"` + Latency float64 `json:"latency"` + CustomerID string `json:"customer_id"` + Metadata metadataMap `json:"metadata"` // Heavy body fields — present on /api/logs/{id}, absent on // /api/logs. Optional because some rows (errors, realtime diff --git a/gateway/internal/adminapi/logstore_client_test.go b/gateway/internal/adminapi/logstore_client_test.go index ecf268f8d..d63fa7570 100644 --- a/gateway/internal/adminapi/logstore_client_test.go +++ b/gateway/internal/adminapi/logstore_client_test.go @@ -259,6 +259,76 @@ func TestLogstoreLog_Tokens(t *testing.T) { } } +// Bifrost's logging plugin stamps non-string values into metadata +// (`realtime: true` on realtime turns, `isAsyncRequest: true` on +// x-bf-async jobs). One such row on a page used to fail the whole +// decode and 502 every rollup for the window. Strings must survive +// verbatim, scalars as their JSON text, and nested/null values must +// simply be dropped — never an error. +func TestMetadataMap_ToleratesNonStringValues(t *testing.T) { + var l logstoreLog + err := json.Unmarshal([]byte(`{ + "id": "rt1", + "metadata": { + "run-id": "r1", + "agent-name": "canvas-agent", + "realtime": true, + "isAsyncRequest": true, + "retries": 3, + "ratio": 0.5, + "nested": {"a": "b"}, + "list": [1, 2], + "gone": null + } + }`), &l) + if err != nil { + t.Fatalf("bool/number metadata must decode, got: %v", err) + } + want := map[string]string{ + "run-id": "r1", + "agent-name": "canvas-agent", + "realtime": "true", + "isAsyncRequest": "true", + "retries": "3", + "ratio": "0.5", + } + if len(l.Metadata) != len(want) { + t.Errorf("metadata = %v, want %v", l.Metadata, want) + } + for k, v := range want { + if l.Metadata[k] != v { + t.Errorf("metadata[%q] = %q, want %q", k, l.Metadata[k], v) + } + } + if dimensionValue(l, "run-id") != "r1" { + t.Errorf("dimensionValue must still index the map: %q", dimensionValue(l, "run-id")) + } + + // Detail rows decode through the same type. + var d logstoreLogDetail + if err := json.Unmarshal([]byte(`{"id":"rt1","metadata":{"realtime":true,"user-id":"u1"}}`), &d); err != nil { + t.Fatalf("detail row: %v", err) + } + if d.Metadata["realtime"] != "true" || d.Metadata["user-id"] != "u1" { + t.Errorf("detail metadata = %v", d.Metadata) + } + + // null / absent metadata stay nil so callers can index freely. + var n logstoreLog + if err := json.Unmarshal([]byte(`{"id":"x","metadata":null}`), &n); err != nil || n.Metadata != nil { + t.Errorf("null metadata: err=%v map=%v", err, n.Metadata) + } + var a logstoreLog + if err := json.Unmarshal([]byte(`{"id":"x"}`), &a); err != nil || a.Metadata["run-id"] != "" { + t.Errorf("absent metadata: err=%v map=%v", err, a.Metadata) + } + // A non-object metadata value is still a decode error, not silent data loss. + var bad logstoreLog + if err := json.Unmarshal([]byte(`{"id":"x","metadata":"oops"}`), &bad); err == nil { + t.Error("string-typed metadata must be rejected") + } +} + func TestNewLogstoreClient_RequiresCreds(t *testing.T) { if newLogstoreClient("", "x") != nil || newLogstoreClient("x", "") != nil { t.Error("missing creds must yield nil (routes skipped), not a client that 401s")