Skip to content

Commit 72ad5fe

Browse files
authored
Expose proxy errors as first-class browser telemetry events (#331)
## Summary The metro egress host-proxy serves branded 5xx error pages carrying an `X-Kernel-Proxy-Error` header with a typed code when a proxy-layer failure occurs. This change makes those failures **first-class in browser telemetry** from the image side: the CDP collector now emits a dedicated low-volume `proxy_error` event carrying the code, instead of burying them in the raw `network_response` stream. ## What changed - **`openapi.yaml` + regenerated `oapi.go` / `category_gen.go`**: new `BrowserProxyErrorEvent`/`BrowserProxyErrorEventData` schema (`proxy_error`, category `network`) registered in the `KnownBrowserTelemetryEvent` discriminated union. `code` is a typed enum mirroring the proxy's wire values (`destination_blocked`, `provider_blacklisted`, `provider_unreachable`, `proxy_unavailable`, `upstream_timeout`, `upstream_dns_failure`, `upstream_connect_failed`); `status` is a required `int` (502). - **`cdpmonitor/handlers.go`**: on `Network.responseReceived`, when a 5xx response carries `X-Kernel-Proxy-Error`, emit `proxy_error` with the header value as `code`, plus `status`, `url`, `method`, `request_id`, `nav_seq`, and target/frame context. Header lookup is case-insensitive; origin 5xx pass-through is untouched (no masking). Requests in flight at CDP attach get their context from the CDP params. - **Enum validation + rate limit**: `code` is validated against the generated enum before use, so unknown header values are dropped and the rate-limit map stays bounded; emission is deduplicated per (session, code) with a 1s min interval so volume tracks an outage without flooding the ring. - **`lib/events/otlpconvert.go`**: `proxy_error` maps to ERROR severity only for the top-level Document and WARN for subresources; `status` promotes to `http.response.status_code`. - **Tests**: classifier/rate-limit unit tests, response-path unit tests (branded, untracked), and a real-Chromium e2e that serves a branded 502 and asserts the emitted event. ## Notes / trade-offs - The event is header-driven: it is emitted only when the proxy's branded page is observed, so the `code` is always the real header value. No derived/synthetic codes. - It rides the `network` telemetry category (CDP-derived and opt-in). Its value is **per-session/per-URL attribution** for sessions already capturing the network stream — not a default-on alerting signal (proxy failures are only observable while the CDP collector runs). - No Chromium patch and no change to the browser fork: the image-side observation point is the CDP collector. The `oapi.go` diff is large because it is regenerated (embedded spec + union accessors). - Deliberately not in scope: making the event default-on (would require the CDP collector always running), server-side emission from metro, and a provenance gate (remote addresses can't distinguish proxy-generated from origin responses; the header is signal-integrity advisory). ## Related - kernel/kernel #3283 mirrors the schema into the API's `BrowserTelemetryEvent` union + Stainless SDK models so consumers can type against `proxy_error`. ## Tests - `go test ./lib/cdpmonitor/ ./lib/events/` — green (unit + real-Chromium e2e). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new telemetry on the hot CDP response path (gated to 502 + header) and changes OTLP alerting semantics for document vs subresource proxy failures; scope is bounded by enum validation and sampling. > > **Overview** > Introduces **`proxy_error`** as a first-class network telemetry event when the CDP monitor classifies a **502** response that carries **`X-Kernel-Proxy-Error`**, surfacing metro egress proxy failures with a typed **`code`** instead of only generic `network_response` traffic. > > **Schema & pipeline:** OpenAPI adds `BrowserProxyErrorEvent` (enum codes aligned with metro), regenerated `oapi.go` / `category_gen.go`, and union wiring. **`cdpmonitor`** detects the header on `Network.responseReceived` (502-only gate), fills request/nav context from pending state or CDP params, validates codes against the enum, and rate-limits to at most one emit per session+code+resource type per second. > > **OTLP:** Promotes `code` to `kernel.proxy_error_code`; **`proxy_error`** severity is **ERROR** for `Document` and **WARN** for subresources. > > Unit tests, a Chromium e2e, and README taxonomy updates cover the new path. Events remain opt-in via the network CDP collector category. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ed5697e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: chruffins <23645059+chruffins@users.noreply.github.com>
1 parent 30573de commit 72ad5fe

12 files changed

Lines changed: 1065 additions & 404 deletions

File tree

server/lib/cdpmonitor/README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Chrome can restart independently of the monitor. When that happens, `UpstreamPro
1010

1111
## Event taxonomy
1212

13-
**CDP-derived** (1-to-1 with a CDP notification): `console_log`, `console_error`, `network_request`, `network_response`, `network_loading_failed`, `page_tab_opened`, `page_navigation`, `page_dom_content_loaded`, `page_load`, `page_layout_shift`, `page_lcp`
13+
**CDP-derived** (1-to-1 with a CDP notification): `console_log`, `console_error`, `network_request`, `network_response`, `network_loading_failed`, `proxy_error` (classified from a branded 5xx response carrying the `X-Kernel-Proxy-Error` header), `page_tab_opened`, `page_navigation`, `page_dom_content_loaded`, `page_load`, `page_layout_shift`, `page_lcp`. `proxy_error` is an opt-in per-session/per-URL refinement of the raw `network` events: it is only observable while the network category (CDP collector) is running, so it is not a default-on alerting signal.
1414

1515
**Computed** (inferred from sequences of CDP events): `network_idle` (fires when in-flight requests drop to zero), `page_layout_settled` (1 s after `page_load` with no intervening layout shifts), `page_navigation_settled` (fires once `page_dom_content_loaded` and `page_layout_settled` have both fired for the same navigation; intentionally independent of `network_idle` so that a single hung request cannot stall the event).
1616

@@ -156,10 +156,10 @@ target_id <- one per tab/window; stable across navigations
156156
| --- | --- | --- |
157157
| `target_id` | `source.metadata`, most `data` objects | The browser tab. Use this to group all events from one tab session. |
158158
| `cdp_session_id` | `source.metadata` | The WebSocket sub-channel. Not stable across reconnects. |
159-
| `frame_id` | `page_navigation`, `network_request`, `network_response`, `network_loading_failed` | The frame the request or navigation belongs to. Top-level frame has no `parent_frame_id`. |
159+
| `frame_id` | `page_navigation`, `network_request`, `network_response`, `network_loading_failed`, `proxy_error` | The frame the request or navigation belongs to. Top-level frame has no `parent_frame_id`. |
160160
| `source_frame_id` | `page_layout_shift`, `page_lcp` | The frame where the layout shift or LCP element occurred. Distinct from the nav context `frame_id`, which is always the top-level navigated frame. |
161-
| `loader_id` | `page_navigation`, `network_request`, `network_response` | The document load that owns a request. Join `network_request.loader_id` to `page_navigation.loader_id` to correlate requests with the navigation that triggered them. |
162-
| `request_id` | `network_request`, `network_response`, `network_loading_failed` | A single request chain (including redirects). Links request to its eventual response or failure. |
161+
| `loader_id` | `page_navigation`, `network_request`, `network_response`, `proxy_error` | The document load that owns a request. Join `network_request.loader_id` to `page_navigation.loader_id` to correlate requests with the navigation that triggered them. |
162+
| `request_id` | `network_request`, `network_response`, `network_loading_failed`, `proxy_error` | A single request chain (including redirects). Links request to its eventual response or failure. |
163163

164164
### Navigation context fields
165165

@@ -171,7 +171,7 @@ Most event `data` objects include a nav context block stamped at the last `page_
171171
| `frame_id` | Frame ID of the navigated top-level frame. |
172172
| `loader_id` | Loader ID of the current document. |
173173
| `url` | URL of the current page at the time of the last navigation. |
174-
| `nav_seq` | Monotonically increasing counter, incremented on each `page_navigation`. Use it to detect that the page has navigated between two events in the same session. For `network_request`/`network_response`/`network_loading_failed`, the `nav_seq` is captured at request-send time and carried forward to the response so a request/response pair always shares an epoch. |
174+
| `nav_seq` | Monotonically increasing counter, incremented on each `page_navigation`. Use it to detect that the page has navigated between two events in the same session. For `network_request`/`network_response`/`network_loading_failed`/`proxy_error`, the `nav_seq` is captured at request-send time and carried forward to the response so a request/response pair always shares an epoch. |
175175

176176
### Events that do not compose `BrowserEventContext`
177177

@@ -202,6 +202,7 @@ Unless otherwise noted, events also include the nav context fields described abo
202202
| `network_request` | `request_id`, `loader_id`, `frame_id`, `document_url`, `method`, `url`, `headers`, `initiator_type`. Optional: `post_data`, `resource_type`, `is_redirect` + `redirect_url`. |
203203
| `network_response` | `request_id`, `loader_id`, `frame_id`, `method`, `url`, `status`, `headers`. Optional: `status_text`, `mime_type`, `resource_type`, `body` (truncated text body for textual MIME types). |
204204
| `network_loading_failed` | `request_id`, `error_text`, `canceled`. Optional (absent when the request record was not found): `url`, `loader_id`, `frame_id`, `resource_type`. |
205+
| `proxy_error` | `request_id`, `code` (typed enum matching the metro header values), `status` (502). Optional: `url`, `loader_id`, `frame_id`, `method`, `resource_type`. Emitted when a 502 response carries the `X-Kernel-Proxy-Error` header; unknown codes are dropped and emission is sampled to at most one per session+code+resource_type per second. WebSocket handshakes are not classified (documented non-goal). |
205206

206207
#### Page events
207208

server/lib/cdpmonitor/chrome_e2e_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"net/http"
8+
"net/http/httptest"
79
"os"
810
"os/exec"
911
"strings"
@@ -14,6 +16,7 @@ import (
1416

1517
"github.com/coder/websocket"
1618
"github.com/coder/websocket/wsjson"
19+
"github.com/kernel/kernel-images/server/lib/events"
1720
"github.com/stretchr/testify/require"
1821
)
1922

@@ -307,3 +310,51 @@ func (c *cdpConn) evalRect(t *testing.T, ctx context.Context, sessionID, selecto
307310
require.NoError(t, json.Unmarshal([]byte(resp.Result.Result.Value), &rect))
308311
return rect
309312
}
313+
314+
// TestProxyErrorE2E drives a real browser to a stub origin that serves a branded
315+
// 502 with the X-Kernel-Proxy-Error header and asserts the CDP collector emits a
316+
// proxy_error telemetry event. It exercises the image-side detection
317+
// (Network.responseReceived header classification) end to end through a real
318+
// browser, without needing the metro host-proxy.
319+
func TestProxyErrorE2E(t *testing.T) {
320+
if os.Getenv("KERNEL_CDPMONITOR_CHROME_E2E") == "" {
321+
t.Skip("set KERNEL_CDPMONITOR_CHROME_E2E=1 to run the real-Chromium proxy error test")
322+
}
323+
chrome := findChromium(t)
324+
325+
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
326+
defer cancel()
327+
328+
stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
329+
w.Header().Set("X-Kernel-Proxy-Error", "provider_blacklisted")
330+
w.WriteHeader(http.StatusBadGateway)
331+
fmt.Fprintln(w, "<html><body>proxy error</body></html>")
332+
}))
333+
defer stub.Close()
334+
335+
browserWS := launchChromium(t, ctx, chrome)
336+
cdp := dialCDP(t, ctx, browserWS)
337+
defer cdp.close()
338+
339+
ec := newEventCollector()
340+
m := New(&staticUpstream{url: browserWS}, ec.publishFn(), 99, discardLogger, nil)
341+
require.NoError(t, m.Start(ctx))
342+
defer m.Stop()
343+
344+
// A fresh page target the monitor auto-attaches to and enables Network on.
345+
targetID := cdp.call(t, ctx, "", "Target.createTarget", map[string]any{"url": "about:blank"}).targetID(t)
346+
ec.waitFor(t, EventTabOpened, 5*time.Second)
347+
348+
// Drive navigation from a separate flat session on the same target.
349+
sessionID := cdp.call(t, ctx, "", "Target.attachToTarget", map[string]any{"targetId": targetID, "flatten": true}).sessionID(t)
350+
cdp.call(t, ctx, sessionID, "Page.enable", nil)
351+
cdp.call(t, ctx, sessionID, "Page.navigate", map[string]any{"url": stub.URL})
352+
353+
ev := ec.waitFor(t, EventProxyError, 10*time.Second)
354+
require.Equal(t, events.Network, ev.Category)
355+
require.Equal(t, "Network.responseReceived", *ev.Source.Event)
356+
var data map[string]any
357+
require.NoError(t, json.Unmarshal(ev.Data, &data))
358+
require.Equal(t, "provider_blacklisted", data["code"])
359+
require.Equal(t, float64(502), data["status"])
360+
}

server/lib/cdpmonitor/handlers.go

Lines changed: 131 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -491,16 +491,57 @@ func (m *Monitor) handleNetworkRequest(p cdpNetworkRequestWillBeSentParams, sess
491491
}
492492
}
493493

494-
func (m *Monitor) handleResponseReceived(p cdpNetworkResponseReceivedParams, _ string) {
494+
func (m *Monitor) handleResponseReceived(p cdpNetworkResponseReceivedParams, sessionID string) {
495495
m.pendReqMu.Lock()
496-
if state, ok := m.pendingRequests[p.RequestID]; ok {
497-
state.status = p.Response.Status
498-
state.statusText = p.Response.StatusText
499-
state.resHeaders = p.Response.Headers
500-
state.mimeType = p.Response.MimeType
501-
m.pendingRequests[p.RequestID] = state
496+
var (
497+
state networkReqState
498+
ok bool
499+
)
500+
if st, present := m.pendingRequests[p.RequestID]; present {
501+
st.status = p.Response.Status
502+
st.statusText = p.Response.StatusText
503+
st.resHeaders = p.Response.Headers
504+
st.mimeType = p.Response.MimeType
505+
m.pendingRequests[p.RequestID] = st
506+
state, ok = st, true
502507
}
503508
m.pendReqMu.Unlock()
509+
510+
// Branded proxy error pages are always served as 502 (the producer
511+
// hardcodes that status), so gate detection on it exactly and leave every
512+
// other response paying nothing extra beyond the status compare.
513+
code, isProxyErr := "", false
514+
if p.Response.Status == 502 {
515+
code, isProxyErr = proxyErrorCode(p.Response.Headers)
516+
}
517+
if !isProxyErr {
518+
return
519+
}
520+
521+
var (
522+
navSeq int64
523+
method string
524+
url *string
525+
frame *string
526+
loader *string
527+
resType string
528+
)
529+
if ok {
530+
navSeq, method = state.navSeq, state.method
531+
url, frame, loader = optPtr(state.url), optPtr(state.frameID), optPtr(state.loaderID)
532+
resType = state.resourceType
533+
} else {
534+
// A branded response with no tracked pending request (e.g. in flight
535+
// when CDP attached) still carries its context on the params; fall back
536+
// to the current nav sequence like Network.loadingFailed so the event
537+
// stays correlateable by nav_seq.
538+
url, frame, loader = optPtr(p.Response.URL), optPtr(p.FrameID), optPtr(p.LoaderID)
539+
resType = p.Type
540+
if cs := m.computedFor(sessionID); cs != nil {
541+
navSeq = int64(cs.currentNavSeq())
542+
}
543+
}
544+
m.publishProxyError(sessionID, p.RequestID, code, p.Response.Status, navSeq, method, resType, url, frame, loader)
504545
}
505546

506547
func (m *Monitor) handleLoadingFinished(ctx context.Context, p cdpNetworkLoadingFinishedParams, sessionID string) {
@@ -622,13 +663,96 @@ func (m *Monitor) handleLoadingFailed(p cdpNetworkLoadingFailedParams, sessionID
622663
}
623664
data, _ := json.Marshal(failPayload)
624665
m.publishEvent(EventNetworkLoadingFailed, events.Network, oapi.BrowserEventSource{Kind: oapi.Cdp}, "Network.loadingFailed", data, sessionID)
666+
625667
if ok {
626668
if cs := m.computedFor(state.sessionID); cs != nil {
627669
cs.onLoadingFinished()
628670
}
629671
}
630672
}
631673

674+
// proxyErrorHeader is the response header the metro egress host-proxy sets on
675+
// branded 502 error pages to signal a proxy-layer failure to automation clients.
676+
const proxyErrorHeader = "x-kernel-proxy-error"
677+
678+
// proxyErrorCode returns the X-Kernel-Proxy-Error header value from a CDP
679+
// response header map, if present. Callers gate on 5xx status before reaching
680+
// here, so the full header map decode is already off the common path.
681+
func proxyErrorCode(resHeaders json.RawMessage) (string, bool) {
682+
if len(resHeaders) == 0 {
683+
return "", false
684+
}
685+
var hdrs map[string]any
686+
if err := json.Unmarshal(resHeaders, &hdrs); err != nil {
687+
return "", false
688+
}
689+
for k, v := range hdrs {
690+
if strings.EqualFold(k, proxyErrorHeader) {
691+
if s, ok := v.(string); ok && s != "" {
692+
return s, true
693+
}
694+
}
695+
}
696+
return "", false
697+
}
698+
699+
// proxyErrorMinInterval bounds proxy_error volume: proxy failures repeat per
700+
// request during an outage, and a single typed signal per code is enough to
701+
// alert on while keeping the fixed-capacity ring usable at peak.
702+
const proxyErrorMinInterval = time.Second
703+
704+
// proxyErrorRateLimited reports whether a proxy_error for the given session,
705+
// code, and resource type should be dropped. Codes are validated against the
706+
// published enum (kept in lockstep with the metro egressproxy header codes in
707+
// kernel/kernel packages/metro-api/lib/egressproxy/proxy_error.go), so unknown
708+
// wire values are dropped and never occupy a rate-limit slot. At most one event
709+
// per session+code+resource_type per interval is emitted; the interval is a
710+
// sampling bound, not a per-URL dedup.
711+
func (m *Monitor) proxyErrorRateLimited(sessionID, code, resourceType string) bool {
712+
if !oapi.BrowserProxyErrorEventDataCode(code).Valid() {
713+
m.log.Warn("cdpmonitor: dropping proxy_error with unknown code", "code", code)
714+
return true
715+
}
716+
now := time.Now()
717+
key := sessionID + ":" + code + ":" + resourceType
718+
m.proxyRateMu.Lock()
719+
defer m.proxyRateMu.Unlock()
720+
last, ok := m.proxyLastEmit[key]
721+
if ok && now.Sub(last) < proxyErrorMinInterval {
722+
return true
723+
}
724+
m.proxyLastEmit[key] = now
725+
return false
726+
}
727+
728+
// publishProxyError emits a typed proxy_error event for a branded proxy-layer
729+
// failure observed on the browser's network path (a 5xx response carrying the
730+
// X-Kernel-Proxy-Error header). The code is the header value, validated against
731+
// the published enum.
732+
func (m *Monitor) publishProxyError(sessionID, requestID, code string, status int, navSeq int64, method, resourceType string, url, frameID, loaderID *string) {
733+
if m.proxyErrorRateLimited(sessionID, code, resourceType) {
734+
return
735+
}
736+
m.sessionsMu.RLock()
737+
info := m.sessions[sessionID]
738+
m.sessionsMu.RUnlock()
739+
data, _ := json.Marshal(oapi.BrowserProxyErrorEventData{
740+
SessionId: sessionID,
741+
TargetId: info.targetID,
742+
TargetType: oapi.BrowserTargetType(info.targetType),
743+
NavSeq: navSeq,
744+
RequestId: requestID,
745+
Url: url,
746+
FrameId: frameID,
747+
LoaderId: loaderID,
748+
Method: optPtr(method),
749+
Status: status,
750+
Code: oapi.BrowserProxyErrorEventDataCode(code),
751+
ResourceType: optPtr(resourceType),
752+
})
753+
m.publishEvent(EventProxyError, events.Network, oapi.BrowserEventSource{Kind: oapi.Cdp}, "Network.responseReceived", data, sessionID)
754+
}
755+
632756
func (m *Monitor) handleFrameNavigated(p cdpPageFrameNavigatedParams, sessionID string) {
633757
// Pre-fetch target info and computedState before acquiring pendReqMu to
634758
// avoid a pendReqMu → sessionsMu ordering cycle.

0 commit comments

Comments
 (0)