feat: improve trace correctness and diagnostics - #44
Conversation
Entire-Checkpoint: 01KZRGJ3E1SZJ0HNFEQWZ40RZ8
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1afd8c67ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| WITH execution_rollup AS ( | ||
| SELECT trace_id, | ||
| MIN(julianday(replace(started_at, ' +0000 UTC', 'Z'))) AS start_jd, | ||
| MAX(julianday(replace(started_at, ' +0000 UTC', 'Z')) + COALESCE(duration_ms, 0) / 86400000.0) AS end_jd, | ||
| COUNT(*) AS execution_count, | ||
| SUM(CASE WHEN status = 'error' THEN 1 ELSE 0 END) AS error_count, | ||
| SUM(CASE WHEN cold_start = 1 THEN 1 ELSE 0 END) AS cold_start_count, | ||
| MAX(is_outlier) AS has_outlier | ||
| FROM executions | ||
| WHERE trace_id IS NOT NULL |
There was a problem hiding this comment.
Bound trace aggregation before grouping retained history
On instances with substantial execution history, every trace-list request—including a small limit or recent since filter—must aggregate all retained executions and user_spans first, because those predicates are only applied outside these rollup CTEs. The query also scans executions again for local_roots, and wrapping started_at in julianday(replace(...)) prevents the new raw timestamp index from bounding that work. This makes /api/v1/traces increasingly expensive as invocation volume grows; select candidate trace IDs before the rollups where semantics permit, or maintain indexed per-trace summaries.
Useful? React with 👍 / 👎.
| return (logs || []).filter((entry) => | ||
| entry.span_id === row.span_id || | ||
| (row.type === 'system' && entry.execution_id === row.execution_id), |
There was a problem hiding this comment.
Associate structured logs with selected user spans
When a user selects a code span and switches to “Selected span,” this predicate always hides its logs: runtime log entries carry the execution's system span ID, while the user-span ID is generated server-side only after orva.trace.span() finishes, and the execution-ID fallback is explicitly limited to system rows. Consequently, logs emitted inside any user-defined span produce an empty selected view; propagate the active user-span ID into logging or associate these entries by execution and the user span's time interval.
Useful? React with 👍 / 👎.
Summary
Validation
Notes