[observer] Remove trace processing, add live-scheduled replay for testbench parity - #49276
Merged
Conversation
…testbench - Add `lastDropped` atomic flag to observer handle, set on each channel send to track whether the observation was accepted or dropped - Add `Dropped` boolean column to metric parquet schema; recording handle reads the flag and passes it through to the parquet writer - Testbench skips `Dropped=true` metrics during parquet load so replay only processes data the live observer actually ingested - Skip trace stats loading in testbench — trace stats are deprioritized and the ~21s structural delay is the dominant source of live/testbench divergence. Data is still recorded for future analysis. - Remove dead code: storageHandle, parquetTraceStatsView, and related types that were only used by the removed trace stats processing - Enable scanmw/scanwelch and disable rrcf in gensim-eks live-and-record mode for parity data collection - Backwards compatible: old parquet files without the Dropped column default to false (all accepted) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Go Package Import DifferencesBaseline: e5b320d
|
Contributor
Gitlab CI Configuration Changes
|
| Removed | Modified | Added | Renamed |
|---|---|---|---|
| 0 | 361 | 0 | 0 |
Updated: .gitlab/distribution.yml
Changes Summary
| Removed | Modified | Added | Renamed |
|---|---|---|---|
| 0 | 0 | 2 | 0 |
ℹ️ Diff available in the job log.
Contributor
Static quality checks❌ Please find below the results from static quality gates Error
Gate failure full details
Static quality gates prevent the PR to merge! Successful checksInfo
On-wire sizes (compressed)
|
- Remove all trace processing from the observer: ObserveTrace and ObserveTraceStats are no-ops, fetcher no longer fetches or deserializes traces/stats, stats_metrics.go deleted, traceObs/spanObs/processTrace and all trace adapter types removed - Add ReplayWithLiveSchedule to engine: when advances.jsonl is present, testbench replays at the exact timestamps the live observer advanced instead of advancing at every stored data timestamp. Eliminates scheduling-caused divergences (5 of 11 in A/B testing) - Add SkipDroppedMetrics config flag on TestBenchConfig (off by default) for filtering parquet metrics marked as dropped by the live channel - Update docs: AGENTS.md, README.md, observer-engine.allium - Add tests for ReplayWithLiveSchedule (advance filtering + onReplayEnd flush + no-match graceful handling) - Enable scanmw/scanwelch, disable rrcf in gensim-eks live-and-record Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ellataira
changed the title
…recording stop - Fix ReplayWithLiveSchedule: advance times from the live log are upToSec values (typically dataTimeSec-1), which may not exist in DataTimestamps(). Replace exact-match set lookup with sorted iteration that triggers when the data stream reaches or passes each live advance time. - Add sparse data test (data at 100, 105 with advance times 99, 104). - Document that ObserveTrace no-op intentionally stops trace parquet population since traces are abandoned for the observer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aaf57636e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Remove trace fetching entirely from fetcher: runTraceFetcher, fetchTraces, all trace/stats adapter types (tracerPayloadView, spanView, statsPayloadView, statsRowIterator, statsRowView), and the pb trace proto import. Only profile fetching remains. - Fix live-scheduled replay: use len() > 0 instead of != nil to guard against empty-but-non-nil liveAdvanceTimes from advances.jsonl with zero entries (analysis-disabled recordings). - Rename fetcher handle from "trace-agent" to "profile-agent". - Update docs: README.md, AGENTS.md, observer-engine.allium. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lukesteensen
approved these changes
Apr 15, 2026
lukesteensen
left a comment
lukesteensen
left a comment
Contributor
There was a problem hiding this comment.
Nice! Neat idea to use the live advance times
2 tasks
ellataira
added a commit
that referenced
this pull request
Apr 17, 2026
## Summary Lowers the `parquet_count` filter threshold in `_resolve_zip_from_runs_jsonl` at `tasks/libs/q/eval.py:446` from `>= 100` to `>= 50`. ## Why Post trace-removal (#49276), legitimate recordings on short scenarios produce substantially fewer parquets than before. The old `>= 100` threshold was filtering out valid runs and causing `q.download-scenarios` to silently fall back to stale pre-trace-removal recordings. Observed `parquet_count` values in `runs.jsonl` for `213_PagerDuty_June_2014_Outage` alone: - `0`, `2` — known failed runs - `36` — truncated recording (~18 min of 25) - `60`, `86` — legitimate complete short-scenario recordings (post trace-removal) - `194` — pre-trace-removal full recording With threshold `100`, all recent April 2026 runs were skipped and the downloader kept returning the March 2026 zip. ## Choice of 50 - Failure modes are empirically `0` or `2` parquets - Legitimate short-scenario recordings land around `60–90` - `50` is safely above failures, below the lowest valid recording, and excludes truncated runs (the `36`-parquet April 16 run) ## Test plan - [x] Verified `q.download-scenarios --scenario=213_pagerduty` now resolves the `20260417-e066c10048.zip` (60 parquets) instead of `20260319-af94cb8f2c.zip` - [ ] Optional: re-run `q.download-scenarios` for other scenarios to confirm no regressions 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four changes to achieve live/testbench parity for observer detection:
Mark dropped observations in parquet —
Droppedboolean column on metric parquet schema. The live observer reports whether eachObserveMetriccall was dropped viaObserveMetricAndReportDrop, and the recording handle propagates this to parquet. Testbench can filter via--skip-droppedCLI flag (off by default).Remove trace stats metric derivation —
stats_metrics.godeleted. The observer no longer derivestrace.*metrics from trace stats.ObserveTraceandObserveTraceStatsare no-ops on the analysis handle. The fetcher still runs and the recorder still writes traces/stats to parquet for archival, but no metrics are produced from them.Live-scheduled replay — when
advances.jsonlis present, testbench replays at the exact timestamps the live observer advanced instead of advancing at every stored data timestamp. Handles sparse data correctly (advance times that don't match data timestamps are triggered when data passes them).Skip trace stats in testbench — testbench no longer loads trace stats from parquet during replay, since they are not processed by the live observer.
Motivation
Goal: When we tune detectors on testbench, the results must match what the live observer produces. If testbench and live diverge, we can't trust eval scores.
Method: We ran a gensim-eks episode on food_delivery_redis with
--mode=live-and-record(ScanMW, ScanWelch, BOCPD enabled). The parity instrumentation (#48706, #49140) recorded what every detector returned on every Detect() call during the live run (detect_digests.jsonl) and when the engine advanced (advances.jsonl).We then replayed the same parquet data through testbench and compared detector outputs against live — same detector, same timestamp: did it produce the same anomalies?
How we identified root causes
We tested three independent factors that could cause testbench to diverge from live. For each, we toggled it on/off and measured how many of the 183 live Detect() calls produced identical results in replay.
Factor 1 — Drop filtering: Does excluding observations that the live channel dropped change detection results?
Factor 2 — Trace stats: Do
trace.*metrics derived from trace stats cause divergence?Factor 3 — Scheduling: Does replay advancing at every data timestamp (vs live's sparser schedule) cause divergence?
Remaining 3 divergences — All at the very first Detect() call. Live detectors had accumulated state before recording began; replay starts fresh. This is a cold start effect, unavoidable without recording detector state. After cold start: 60/60 matched, zero divergences.
Current state: testbench parity with this PR
With all changes applied (live-scheduled replay + trace stats derivation removed + drop labels):
Bottom line: After cold start, the testbench produces identical detector results to live. Detector tuning on testbench will match live behavior.
Changes
Drop marking:
observer.go:ObserveMetricAndReportDropmethod onhandle,hfFilteredHandle,noopObserveHandle— returns whether the channel dropped the observation.recorder.go:metricDropObserverinterface, recording handle type-asserts inner for drop reporting, passesdroppedtoWriteMetric.parquet_metric_writer.go:Droppedboolean column in Arrow schema.parquet_metric_reader.go: ReadsDroppedcolumn (tolerates missing column for old recordings).component.go:Dropped boolonMetricData.testbench.go:SkipDroppedMetricsconfig flag (off by default).main.go:--skip-droppedCLI flag.Trace stats metric derivation removal:
stats_metrics.go: Deleted (processStatsView, DDSketch percentile extraction, metric derivation).observer.go:ObserveTraceandObserveTraceStatsare no-ops on the analysis handle. RemovedtraceObs/spanObstypes,processTrace,copyFloat64Map, trace field fromobservationstruct.testbench.go: Removed trace stats loading fromloadParquetDir.AGENTS.md,README.md,observer-engine.allium: Updated.Fetcher (recording-only path):
fetcher.go: Still fetches traces and stats from remote agents, but only when recording is enabled.ObserveTrace/ObserveTraceStatscalls hit no-ops on the inner handle; the recording handle wrapper still writes to parquet for archival.statsPayloadView/statsRowIterator/statsRowViewtypes moved here from deletedstats_metrics.go.observer.go: Fetcher gated behindrecordingEnabled && recorderAvailable.Live-scheduled replay:
engine.go:ReplayWithLiveSchedule()— iterates data timestamps but only advances when data reaches a live advance time (handles sparse data where advance times don't match data timestamps). IncludesonReplayEndflush.advance_log.go:liveAdvanceTimes()method.testbench.go: Uses live-scheduled replay whenadvances.jsonlpresent.liveAdvanceTimesfield, reset on scenario load.Config:
agent-values.yaml.tmpl: Enable scanmw/scanwelch, disable rrcf forlive-and-recordmode.Backwards compatibility
Droppedcolumn: defaults tofalse(all accepted).advances.jsonl: falls back toReplayStoredData(normal replay).Test plan
go test ./comp/observer/impl/...passesgo test ./comp/anomalydetection/recorder/impl/...passesgo vetpassesTestParquetDroppedColumnRoundTrip— dropped flag write/read round-tripTestReplayWithLiveScheduleOnlyAdvancesAtLiveTimestamps— dense data, exact matchesTestReplayWithLiveScheduleSparseData— sparse data with offset advance timesTestReplayWithLiveScheduleNoMatchingTimestamps— graceful handling when no timestamps match🤖 Generated with Claude Code