fix(gorillas3): retire Bucket field validation + log + TSDBBucket fallback (B1 downstream) - #387
Merged
Merged
Conversation
…lback (B1 downstream Phase 1)
The gorillas3 processor's `Bucket` field is no longer needed at
runtime in the current `BlockFormat: prometheus_tsdb`-only build —
the actual S3 write path (`PutTSDBBlock` → `putWithRetryToBucket`)
takes the bucket name as an arg from `TSDBBucket`. The legacy
`Bucket`-based S3 calls (`PutChunk`, `PutPostings` via
`putWithRetry` → `putOnce`) have ZERO non-test callers in the
current code; confirmed by:
grep -rn '\.PutChunk\b\|\.PutPostings\b' \
opentelemetry-collector-contrib-patch/processor/gorillas3processor/ \
| grep -v _test.go
# (no output)
This PR is Phase 1 of a three-phase retire (the cross-repo
coordination is in the commit body, not the title):
Phase 1 (this PR, ASAPCollector):
* `Config::Validate()` no longer requires `Bucket` non-empty.
* `TSDBBucket == "" → fallback to Bucket` shim retired;
validation now requires `TSDBBucket` directly for non-agent
roles.
* Startup log line `zap.String("bucket", p.cfg.Bucket)` removed.
* `s3_sink.go`'s TSDB upload path no longer falls back to
`cfg.Bucket` when `TSDBBucket` is empty.
* `Bucket` field stays in the Config struct (deprecated, doc
comment marks it so) — mapstructure-strict will reject yaml
with unknown keys, and the controller still emits
`bucket: asap-gorilla`, so the field must continue to parse.
Tests for the field's existence (`TestConfig_BucketRequired`)
get reworded to `TestConfig_TSDBBucketRequired`.
* `processor_test.go`: bucket-only test setups rekeyed to
TSDBBucket.
Phase 2 (ASAPQuery-backend, queued behind in-flight PR):
* Controller's `build_gorillas3_yaml` stops emitting the
`bucket:` line.
Phase 3 (ASAPCollector, after Phase 2 lands):
* Delete the `Bucket` field entirely.
* Delete the now-dead `PutChunk` / `PutPostings` /
`putWithRetry` / `putOnce` / `updateIndex` / `fetchIndex`
methods in `s3_sink.go`. Companion deletion of `chunkSink`
interface members + the legacy `.gor`-chunk indexCache
machinery.
Test plan:
* `go test ./...` in `processor/gorillas3processor/` — green.
* `bash build_asap_otel.sh --skip-patches` — builds clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
4 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…methods (B1 downstream Phase 3) (#388) Final phase of the gorillas3 Bucket-field retire. After: * PR #387 — Phase 1: removed validation + log + TSDBBucket-fallback; field kept in Config for mapstructure compat * ASAPQuery-backend PR #279 — Phase 2: controller stops emitting `bucket:` in its YAML Phase 3 (this PR) removes the field entirely + all the dead write-path machinery that referenced it: Deleted: * `Config.Bucket` field * `s3Sink.PutChunk` / `PutPostings` methods (zero non-test callers) * `s3Sink.putWithRetry` / `putOnce` (callers were PutChunk/PutPostings) * `s3Sink.updateIndex` / `fetchIndex` (callers were PutChunk path) * `s3Sink.indexCache` + `indexMu` fields + initialization * `chunkHints` / `indexEntry` / `indexFile` types (only the dead chunk write-path consumed them) * `chunkSink` interface methods for PutChunk / PutPostings * `mockSink.PutChunk` / `PutPostings` + chunks/postings test tracking + `TestFlushWindow_DoesNotEmitLegacyChunkOrPostings` (now redundant — the code path is gone) * stale `Bucket: "asap-gorilla"` literal in TestGatewayFragmentRole_FinalizesFragmentsToTSDBBlock Build verification: * `go test ./...` in processor/gorillas3processor: green (19 tests, down from 20 — the deleted redundant negative test) * `bash build_asap_otel.sh --skip-patches`: clean * empirical confmap-strict check: a yaml carrying a stale `bucket: legacy-bucket` is now rejected with `invalid keys: bucket` (OTel confmap's `ErrorUnused` is on by default since v0.79). Post Phase 2 the controller no longer emits it, so this only affects hand-rolled operator configs — those should be cleaned up too. Closes the gorillas3 Bucket-field retire thread. 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
The gorillas3 processor's `Bucket` field is no longer needed at runtime in the current `BlockFormat: prometheus_tsdb`-only build — the actual S3 write path (`PutTSDBBlock` → `putWithRetryToBucket`) takes the bucket from `TSDBBucket`. The legacy `Bucket`-based calls (`PutChunk`, `PutPostings` via `putWithRetry`) have zero non-test callers.
Phase 1 of a three-phase retire (full plan in the commit body). Highlights:
Test plan
Phases 2 + 3 (follow-ups)
🤖 Generated with Claude Code