feat(observability): add bounded structured runtime events - #17
feat(observability): add bounded structured runtime events#17integrate-your-mind wants to merge 19 commits into
Conversation
…to codex/observability # Conflicts: # .buildkite/scripts/ci-internal.sh # scripts/verify-buildkite.test.mjs
…to codex/observability
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
integrate-your-mind
left a comment
integrate-your-mind
left a comment
There was a problem hiding this comment.
The structured-event shape, privacy boundary, bounded channel, and socket delivery split look coherent. I found three classification paths that can produce misleading terminal events; details are inline.
| | BuildError::Router(_) | ||
| | BuildError::Json(_) => (Outcome::Error, ErrorClass::InvalidInput), | ||
| BuildError::Artifact(_) => (Outcome::Error, ErrorClass::Protocol), |
There was a problem hiding this comment.
These wrapper arms discard the nested error kind. scan_app_dir and write_route_types can return RouterError::Io, so permission or disk failures are emitted as invalid_input; likewise, ProductionArtifactError::Io would be emitted as protocol. Please match the nested variants and pass I/O errors through classify_io_error, while reserving the current classes for actual route/artifact validation failures. Otherwise operators get the wrong cause from the structured event.
| std::io::ErrorKind::TimedOut | std::io::ErrorKind::WouldBlock => { | ||
| (Outcome::Timeout, ErrorClass::Timeout) | ||
| } | ||
| std::io::ErrorKind::InvalidInput | std::io::ErrorKind::InvalidData => { | ||
| (Outcome::Error, ErrorClass::InvalidInput) | ||
| } | ||
| _ => (Outcome::Error, ErrorClass::Io), |
There was a problem hiding this comment.
run_build_input_verifier and install_verified_outputs_unless_cancelled both represent cancellation with io::ErrorKind::Interrupted. This fallback turns those cases into outcome=error / error_class=io, so a signal during preflight or activation is not reported as cancelled. Please map Interrupted to (Outcome::Cancelled, ErrorClass::Cancelled) here, or add a dedicated build-cancellation variant if generic interrupted I/O must stay classified as I/O. A test through run_observed_build would lock this down.
| fn render_failure_response( | ||
| &self, | ||
| path: &str, | ||
| match_result: &RouteMatch, | ||
| error: &PageRenderError, | ||
| ) -> DevResponse { | ||
| if let Some(diagnostic) = | ||
| self.legacy_request_failure_diagnostic("render", path, &match_result.route.path, error) | ||
| { | ||
| eprintln!("{diagnostic}"); | ||
| } | ||
| build_production_render_error_response(path, match_result, error) | ||
| } | ||
|
|
||
| fn bundle_failure_response( | ||
| &self, | ||
| path: &str, | ||
| match_result: &RouteMatch, | ||
| error: &ClientBundleError, | ||
| ) -> DevResponse { | ||
| if let Some(diagnostic) = | ||
| self.legacy_request_failure_diagnostic("bundle", path, &match_result.route.path, error) | ||
| { | ||
| eprintln!("{diagnostic}"); | ||
| } | ||
| build_production_bundle_error_response(path, match_result, error) | ||
| } | ||
|
|
There was a problem hiding this comment.
These helpers erase the typed renderer/bundler error and return only an HTTP response. By the time emit_request_completed runs, the status-only classifier can distinguish 504 from 500, but Cancelled, OutputLimitExceeded, NodeFailed, Protocol, and non-timeout Io failures all collapse to internal. Please carry the bounded outcome/error class/failure phase alongside the response (for example in HandledProductionResponse) and prefer it over status-only classification; the existing classify_page_render_error already has most of these distinctions. Otherwise most normalized runtime classes are unreachable on the normal production render path.
2 participants
Summary
Adds an opt-in structured observability contract for Ferrite native build, server, render, navigation, stream, and transport boundaries without claiming vendor tracing.
ferrite.observabilityv1 JSON event schema--event-log json; default behavior remains offExact state
908bbc94c269fac56ea1e3079c42e47c157323b139ded8f651d769a4688f046970f3da00cb903b0d74e3fa1269f8ec3c77d86cc5f9578470eeae262fValidation
Observability source is byte-identical from fully tested
0c4545athrough the exact PR head; later commits only reconciled the moving parent and removed an unrelated site delta.9d530fb; final parent-only delta targeted test passed 23/239d530fb9d530fb: 5 tarballs, starter install/check, missing-artifact failure, consumer build/serve, source stability9d530fb: all 11 workspace packages verified with--lockedENOSPCwrite after the same case passed in the exact-head non-coverage runNormal / failure / odd proof
Risk and limitations
503remains outside this PR.9d530fb, whose package/observability tree is unchanged at this head; this PR does not claim release readiness.Rollback
Revert this PR. Structured logging is opt-in, adds no migration, and leaves default runtime output unchanged.