From 144a3c1f88c5e87a8ed02444c689eae91b766a15 Mon Sep 17 00:00:00 2001 From: Zeying Zhu Date: Thu, 7 May 2026 00:18:01 -0400 Subject: [PATCH 1/2] fix(gorilla-s3): align prefix-template placeholder vocabulary with agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-v7 the backend's `bucket_prefix` accepted only the long-form `{year}`/`{month}`/`{day}`/`{hour}` placeholders, but the agent-side `gorillas3processor` (and the v6 demo deploy's `ASAP_GORILLA_S3_PREFIX_TEMPLATE`) writes objects under the short-form `{YYYY}`/`{MM}`/`{DD}`/`{HH}` spelling. So when a deploy used the short-form template, `{tenant}` and `{metric}` substituted but the timestamp placeholders stayed literal — every backend `index.json` fetch issued a path like `default//{YYYY}/{MM}/{DD}/{HH}/index.json` that missed the real chunks on disk. Issue #46 criterion ⑥ (freshness probes) surfaced as 0 samples on every path because of this — the chunks were on MinIO, the routing table directed the query through the Gorilla engine, but the cold store could not find them. v7 dual-routing closed the routing-side gap; this aligns the placeholder vocabulary so the chunks are actually located. The fix accepts BOTH spellings; existing deploys using the long-form keep working unchanged. 3 new tests: long-form preserved, agent-side aliases work, mixed-form works. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../query/fallback/cold_store/gorilla_s3.rs | 85 ++++++++++++++++++- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs b/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs index cae16163..7e3d878c 100644 --- a/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs +++ b/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs @@ -396,18 +396,53 @@ impl GorillaS3ColdStore { /// Shared prefix-rendering helper used by [`Self::index_key`] / /// [`Self::postings_key`]. Always ends with `/`. + /// + /// Accepts BOTH placeholder vocabularies: + /// + /// * `{year}`/`{month}`/`{day}`/`{hour}` — the backend's + /// long-standing names. + /// * `{YYYY}`/`{MM}`/`{DD}`/`{HH}` — the agent + /// `gorillas3processor`'s naming, documented in + /// `opentelemetry-collector-contrib-patch/processor/ + /// gorillas3processor/config.go`. + /// + /// Pre-v7 the two sides used different placeholders, so when a + /// deploy set `ASAP_GORILLA_S3_PREFIX_TEMPLATE` to the + /// agent-side spelling (the v6 demo does — see + /// `deploy/docker-compose/mvp-v6-multi-stage.yml`), the backend + /// substituted `{tenant}` and `{metric}` but left the + /// timestamp placeholders un-replaced, so every `index.json` + /// fetch issued a literal `{YYYY}/{MM}/{DD}/{HH}` path that + /// missed the actual chunk objects on disk. Issue #46 + /// criterion ⑥ (freshness probes) surfaced as 0 samples on + /// every path because of this. Accepting both spellings keeps + /// pre-v7 deploys working AND the v6/v7 demo deploy aligned. fn bucket_prefix(&self, metric: &str, ts_ms: i64) -> String { let dt: DateTime = DateTime::::from_timestamp_millis(ts_ms) .unwrap_or_else(|| DateTime::::from_timestamp(0, 0).unwrap()); + let year = format!("{:04}", dt.year()); + let month = format!("{:02}", dt.month()); + let day = format!("{:02}", dt.day()); + let hour = format!("{:02}", dt.hour()); let prefix = self .config .prefix_template .replace("{tenant}", &self.config.tenant) .replace("{metric}", metric) - .replace("{year}", &format!("{:04}", dt.year())) - .replace("{month}", &format!("{:02}", dt.month())) - .replace("{day}", &format!("{:02}", dt.day())) - .replace("{hour}", &format!("{:02}", dt.hour())); + // Long-form placeholders (the backend's historical + // spelling — preserved for backwards compatibility). + .replace("{year}", &year) + .replace("{month}", &month) + .replace("{day}", &day) + .replace("{hour}", &hour) + // Agent-side `{YYYY}`/`{MM}`/`{DD}`/`{HH}` aliases — + // matches the spelling in the agent's + // `gorillas3processor/config.go` and + // `s3_sink.go::renderPrefix`. + .replace("{YYYY}", &year) + .replace("{MM}", &month) + .replace("{DD}", &day) + .replace("{HH}", &hour); let mut key = prefix; if !key.ends_with('/') { key.push('/'); @@ -1153,6 +1188,48 @@ mod tests { assert_eq!(chunks[1].key, key13); } + #[test] + fn bucket_prefix_supports_long_form_placeholders() { + // Backend's historical spelling — preserved. + let mut config = cfg(); + config.prefix_template = "{tenant}/{metric}/{year}/{month}/{day}/{hour}/".to_string(); + let store = InMemoryObjectStore::new(); + let cs = GorillaS3ColdStore::new(Arc::new(store), config); + let key = cs.bucket_prefix("foo", ms(2026, 5, 6, 12, 0, 0)); + assert_eq!(key, "tenant1/foo/2026/05/06/12/"); + } + + #[test] + fn bucket_prefix_supports_agent_side_yyyy_mm_dd_hh_placeholders() { + // v7 fix: the agent's gorillas3processor uses + // `{YYYY}`/`{MM}`/`{DD}`/`{HH}`. Pre-v7 the backend left + // these literal; v7 substitutes them so a deploy that + // configures the routing yaml with the agent-side + // spelling gets matching index.json keys on both sides. + let mut config = cfg(); + config.prefix_template = "{tenant}/{metric}/{YYYY}/{MM}/{DD}/{HH}/".to_string(); + let store = InMemoryObjectStore::new(); + let cs = GorillaS3ColdStore::new(Arc::new(store), config); + let key = cs.bucket_prefix("http_freshness_probe_archive", ms(2026, 5, 7, 4, 0, 0)); + assert_eq!( + key, + "tenant1/http_freshness_probe_archive/2026/05/07/04/", + "v7 must substitute {{YYYY}}/{{MM}}/{{DD}}/{{HH}} the same as the long-form names", + ); + } + + #[test] + fn bucket_prefix_handles_mixed_long_and_short_placeholders() { + // Defensive — accept a mix in case some operator templates + // it that way. + let mut config = cfg(); + config.prefix_template = "{tenant}/{metric}/{year}/{MM}/{DD}/{hour}/".to_string(); + let store = InMemoryObjectStore::new(); + let cs = GorillaS3ColdStore::new(Arc::new(store), config); + let key = cs.bucket_prefix("m", ms(2026, 5, 7, 4, 0, 0)); + assert_eq!(key, "tenant1/m/2026/05/07/04/"); + } + #[test] fn from_env_requires_bucket() { // Don't pollute global env in a unit test; just exercise the From 5374fa3cea4abc06da51af357792ab1a8f6fa0bc Mon Sep 17 00:00:00 2001 From: Zeying Zhu Date: Thu, 7 May 2026 00:40:14 -0400 Subject: [PATCH 2/2] fix(gorilla-s3): prefix bare-basename keys from agent-produced index entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the agent's gorillas3processor writes index.json, each entry's `object` field carries just the chunk basename (e.g. `part-1778128694-000000.gor`). Backend-produced entries carry the full S3 key (e.g. `tenant/metric/2026/05/07/04/part-1778128694-000000.gor`). `list_chunks` was passing entry.key through to ChunkRef.key unchanged, so subsequent `read_chunk` issued GET against the bare basename and hit "not found". The agent-side index.json deserialize landed in the v7 follow-ups, but the keys-need-prefixing tail is only visible end-to-end. Fix: in `list_chunks`, detect a bare basename (no `/` in entry.key) and prepend the per-hour `bucket_prefix(metric, hour_ms)`. Preserves backend-produced entry handling exactly (key already contains `/`). Required for issue #46 criterion ⑥ archive freshness path. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../query/fallback/cold_store/gorilla_s3.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs b/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs index 7e3d878c..e2fe8a8a 100644 --- a/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs +++ b/asap-query-engine/src/drivers/query/fallback/cold_store/gorilla_s3.rs @@ -531,13 +531,26 @@ impl ColdStore for GorillaS3ColdStore { let mut out = Vec::new(); for hour_ms in Self::hour_starts(start_ms, end_ms) { let idx = self.fetch_index(metric, hour_ms).await?; + let bucket_prefix = self.bucket_prefix(metric, hour_ms); for entry in idx.prune_by_time((start_ns, end_ns)) { let (entry_start_ms, entry_end_ms) = ( (entry.time_range.0 / 1_000_000) as i64, (entry.time_range.1 / 1_000_000) as i64, ); + // v7: agent-produced index entries carry just the + // chunk's basename (`part-NNNN-MMMM.gor`), not the + // full S3 key. Detect a bare basename (no `/`) and + // prepend the bucket prefix so the subsequent + // `read_chunk` GET hits the right object. + // Backend-produced entries carry the full key; we + // leave those unchanged. + let key = if entry.key.contains('/') { + entry.key.clone() + } else { + format!("{}{}", bucket_prefix, entry.key) + }; out.push(ChunkRef { - key: entry.key.clone(), + key, metric: metric.to_string(), time_range_ms: (entry_start_ms, entry_end_ms), label_hash: entry.label_hash,