fix: gorillas3processor encoder timestamp header offset - #307
Merged
Merged
Conversation
The v7 fix in commit cd6914f corrected the writer side of the GORILLA1 chunk header (`buf[5:9]` → `buf[9:13]` for the seriesCount slot, so it no longer clobbers the magic suffix + version byte). The Go test-only decoder helper in `encoder_test.go::decodeChunk` was left at the pre-v7 offsets — magic read as 4 bytes, version at byte 4, seriesCount at [5:9] — so a regression in the writer would slip past the round-trip assertions today. This change: 1. Aligns `decodeChunk` with the on-wire layout the asap-gorilla Rust decoder uses (`asap-gorilla/src/decoder.rs::from_reader` reads magic at [..8], version at [8], seriesCount at [9..13]; constants in `block.rs::HEADER_LEN = 13`). The four existing round-trip tests now exercise the same offsets the production consumer does. 2. Adds `TestChunkHeaderByteLayoutMatchesGorillaDecoder` as an explicit regression guard. It asserts magic is at [0:8], version at [8], seriesCount at [9:13], **and** that bytes [5:9] are NOT the seriesCount (negative assertion catches the v6.x style bug exactly). Verified locally: with the encoder reverted to the bad offset, this test fails with the expected "expected GORILLA1, got GORIL\x02\x00\x00" diff. Test count: 27 pass (`go test -count=1` in `opentelemetry-collector-contrib-patch/processor/gorillas3processor`). Image: `asap/sketchcol:dev` rebuilt against this branch (sha256:0cf970659ef8352285f859aaa233d007e210a3c6c29524fe57f52aa9a2871f32). The image embeds gorillas3processor + the GORILLA1 magic + the chunk write log lines, confirmed via `grep -aoE` inside the container. Closes the freshness-probe consumer leg of MVP Fix 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 7, 2026
zzylol
added a commit
that referenced
this pull request
May 7, 2026
) Phase 3.2 audit confirms `gorillas3processor/encoder.go` writes seriesCount at the canonical [9..13] offset (line 311), the test decoder helper reads at [9..13] (encoder_test.go:210), and the canonical Rust decoder reads at header[9..13] (decoder.rs:84). All three implementations agree. Conflicting v7 / Step 2.5 reports of a `[5..9]` offset bug in the Go writer did not reproduce on origin/main; those reports were based on stale source. The Fix 2 agent's PR #307 claim (writer was already correct, only the test decoder was at the buggy offset) was correct as of today's main. Add two negative tests in asap-gorilla/tests/byte_compat.rs that pin the contract from the Rust side: 1. decoder_rejects_pre_v7_seriescount_at_offset_5_9 — synthesizes a block with seriesCount at the buggy [5..9] offset, asserts the canonical decoder rejects it with BadMagic. Documents the original failure mode in code. 2. decoder_accepts_canonical_seriescount_at_offset_9_13 — positive twin: same shape, seriesCount at [9..13], must decode cleanly. Companion to the Go-side regression guard TestChunkHeaderByteLayoutMatchesGorillaDecoder added in PR #307. The Go test pins the writer; the Rust tests pin the decoder. Either direction of future drift now fails CI. Phase 3.2 conclusion: no encoder fix is required. The freshness probe failure (all three CSV paths empty in the latest run) has upstream root causes — the controller-emitted bootstrap agent.yaml drops the gorillas3 processor entirely (only `processors: [ddsketch]`), so freshness counters never reach MinIO; the warm-tier ddsketch transform also strips the original metric name. Those are out of scope for the encoder-offset audit; they belong to a separate "controller config emission for freshness probes" task. No image rebuild needed: encoder.go is unchanged, so asap/sketchcol:dev byte content is identical. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
gorillas3processorGo test-only decoder with the v7 chunk-header layout already in production: magic at[0:8], version at[8], seriesCount at[9:13]— matchingasap-gorilla::decoder(block.rs::HEADER_LEN = 13).TestChunkHeaderByteLayoutMatchesGorillaDecoderas a regression guard. Includes a negative assertion that bytes[5:9]are NOT the seriesCount, so the pre-v7 bug shape (buf[5:9]write) fails this test loudly.last_over_time(http_freshness_probe_*[10s])consumer exercises.Context
PR #302 (commit
cd6914f) already moved the writer frombuf[5:9]tobuf[9:13]. The test helper inencoder_test.go::decodeChunkwas left at the buggy offsets, which (a) silently miscompared the 8-byte magic against a 4-byte slice and (b) would have hidden a writer-side regression. This PR finishes the alignment.Test plan
go test -count=1 ./opentelemetry-collector-contrib-patch/processor/gorillas3processorpasses (27/27)TestChunkHeaderByteLayoutMatchesGorillaDecoderfails as expected when the encoder is reverted tobuf[5:9](verified locally; restored)bash build_sketchcollector.sh --skip-patchesbuilds cleanlydocker build -t asap/sketchcol:dev -f deploy/docker/Dockerfile.sketchcol .produces imagesha256:0cf970659ef8352285f859aaa233d007e210a3c6c29524fe57f52aa9a2871f32GORILLA1,gorillas3: chunk written,gorillas3: postings written,gorillas3processor(verified viagrep -aoEinside the container)🤖 Generated with Claude Code