[wip] adding more mo telemetry events#247
Conversation
|
Firetiger deploy monitoring skipped This PR didn't match the auto-monitor filter configured on your GitHub connection:
Reason: PR title and empty body suggest telemetry event additions rather than kernel API or Temporal workflow changes; please confirm if this affects packages/api/cmd/api/ or packages/api/lib/temporal to opt into deploy monitoring. To monitor this PR anyway, reply with |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Schemas: - New event types: api_call (HTTP middleware), cdp_connect / cdp_disconnect (CDP proxy), live_view_connect / live_view_disconnect (Neko, producer in a follow-up). - captcha_solve_result enum trimmed to the producer-normalized vendor set (hcaptcha, recaptcha_v2, recaptcha_v3, turnstile, geetest, other). - New `api` user-toggleable category for api_call events. Producers: - chi middleware emits api_call with operationId / status / request_id / duration_ms. Disabled by default; the telemetry handler flips the package-level toggle on/off when the api category is enabled. - WebSocketProxyHandler emits cdp_connect on accept and cdp_disconnect on teardown with message_count and reason (client_close / upstream_changed / upstream_error / context_cancelled). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e22845. Configure here.
| "operation_id": tc.operationID, | ||
| "status": ww.Status(), | ||
| "duration_ms": float64(time.Since(start).Microseconds()) / 1000.0, | ||
| }) |
There was a problem hiding this comment.
Ad-hoc map literals instead of generated oapi types
Medium Severity
Event data payloads in TelemetryHTTPMiddleware and publishCdpDisconnect are constructed using map[string]any literals instead of the generated oapi.BrowserApiCallEventData and oapi.BrowserCdpDisconnectEventData types. The existing cdpmonitor producers already use generated types (e.g., oapi.BrowserConsoleLogEventData). Using ad-hoc maps bypasses compile-time checking and risks drift between the documented API contract and actual event shapes — for instance, the generated DurationMs field is float32 but the map uses float64, and MessageCount is int in the schema but int64 in the map.
Additional Locations (1)
Triggered by learned rule: Producer repo must own typed event schemas in OpenAPI and use generated oapi types
Reviewed by Cursor Bugbot for commit 0e22845. Configure here.
| cdpReasonUpstreamChanged = "upstream_changed" | ||
| cdpReasonUpstreamError = "upstream_error" | ||
| cdpReasonContextCanceled = "context_cancelled" | ||
| ) |
There was a problem hiding this comment.
CDP disconnect reasons use raw strings not generated enum
Low Severity
The CDP disconnect reason constants (cdpReasonClientClose, cdpReasonUpstreamChanged, etc.) are declared as plain string values. The generated code already provides oapi.BrowserCdpDisconnectEventDataReason as a typed enum with constants like oapi.ClientClose, oapi.UpstreamChanged, etc. The hand-rolled string constants duplicate the generated ones and bypass the type system, allowing silent drift if the OpenAPI enum values ever change.
Triggered by learned rule: Producer repo must own typed event schemas in OpenAPI and use generated oapi types
Reviewed by Cursor Bugbot for commit 0e22845. Configure here.
1 participant
Note
Medium Risk
Adds new request/WS middleware and expands telemetry configuration/schema, which can affect API routing performance and emitted event volume/shape. Behavior is gated behind config, but incorrect toggling could lead to missing or unexpected telemetry.
Overview
Adds an
apitelemetry category and wires it throughPUT/PATCH /telemetryso enabling/disabling that category dynamically turns API-call event emission on/off.Introduces
TelemetryHTTPMiddleware+TelemetryStrictMiddlewareto emitapi_callevents for documented OpenAPI operations (operationId, status, duration, request_id), and updatesmain.goto installRequestIDplus these middleware.Extends the DevTools WebSocket proxy to optionally publish
cdp_connect/cdp_disconnectsystem events (including disconnect reason, duration, and relayed message count), updates call sites/tests, and updates the OpenAPI spec/generatedoapitypes to include the new events andapicategory.Reviewed by Cursor Bugbot for commit 0e22845. Bugbot is set up for automated code reviews on this repo. Configure here.