[chore] repo: Sync fork with current upstream main#26
Conversation
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5 Codex
Assisted-by: ChatGPT 5.4
There was a problem hiding this comment.
Telemetry Review
Advisory: 2 medium findings in changed files.
2 findings appear pre-existing in code touched by this diff.
Key findings:
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:204 — hp.Logger.Info("Failed to apply config, trying next one") in iterateDayLevel fires when the newest S3 config object fails to apply and the processor falls back to an older one. This is a degraded-state signal: the intended config was rejected and the processor is silently rolling back. Info severity hides this from Warn-level log filters and error-rate dashboards.
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:250 — hp.Logger.Info("Failed to iterate day level") in iterateAllDays fires when an entire day-prefix fails to iterate. The processor skips the whole day and tries the next one. This is a broader degraded-state signal than the per-object fallback: a full day's worth of configs is being bypassed. Info severity makes this invisible to Warn-level filters and alert rules.
These findings are advisory; address them before re-requesting review if applicable.
There was a problem hiding this comment.
APPROVE — threads resolved, code is ready.
Three threads from coderabbitai/cursor on the inmemorystorage extension:
- coderabbitai (test coverage gaps for Get/Set/Delete/Batch): Pre-existing gaps in the upstream extension, not regressions from this PR. Out of scope.
- cursor (stability mismatch —
metadata.yamlsaysalpha,factory.gousesStabilityLevelDevelopment): Valid — should beStabilityLevelAlpha. Log as a follow-up; doesn't block the SAW-6831 upgrade goal. - cursor (shared
sync.MapinGetClientignores kind/ID/name): Intentional for this in-memory test extension. All clients sharing one map is by design — the purpose is lightweight cross-component coordination in tests, not isolation.
All three resolved. Ready to merge.
Assisted-by: ChatGPT 5.2
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR involves a major upstream sync, the addition of a new internal storage extension, and significant logic changes to the loadbalancingexporter's queuing and compression mechanisms.
There was a problem hiding this comment.
[ARCH-REVIEW] Re-review: ✅ APPROVE
Prior blocking issue resolved: client.go:Batch() now returns an error instead of panicking; TestClientBatchAfterCloseReturnsError confirms it. Prior concern resolved: test is end-to-end again (real testStorageHost + inmemorystorage extension, queue+codec path exercised with sink assertion). No unresolved threads.
Assisted-by: ChatGPT 5.2
There was a problem hiding this comment.
[ARCH-REVIEW] Re-review: APPROVE
Prior blocking issue resolved: client.go:41 panic("client already closed") → return errors.New("client already closed"). Test TestClientBatchAfterCloseReturnsError validates the fix. All 7 threads resolved. Zero unresolved, zero new findings.
There was a problem hiding this comment.
Telemetry Review
Advisory: 1 high finding and 3 medium findings in changed files.
4 findings appear pre-existing in code touched by this diff.
Key findings:
- [HIGH] [EXISTING] @ processor/hotreloadprocessor/processor.go:104 — In
processor.go,applyConfigWithTelemetryemits three timestamp metrics (triggerNewestFileSuccessTimestamp,triggerNewestFileFailedTimestamp,triggerRollbackFileSuccessTimestamp) with per-recording labelskey(an S3 object path containing a Unix timestamp, e.g.test-org-id/prefix-1/3000000000000/1234567890.yaml) andconfig_hash(a SHA-256 hex digest of the config YAML). Both values are effectively unbounded:keygrows monotonically with each new config file written to S3, andconfig_hashchanges with every config content change. Additionally,triggerReloadDurationalso receiveskeyas a label.triggerNewestFileFailedTimestampfurther addsreason=err.Error(), which is a free-form error string. These labels are appended on top of the four fixedprocessorAttrlabels, making the total label set: processor × configuration_file × destination × collector_version × key × config_hash × result × reload — a combinatorial explosion for any metric backend that indexes all label combinations. - [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:192 — iterateDayLevel logs at Info level for every S3 object whose applyConfig call fails, inside a per-object loop. On each refresh cycle, if K objects in the day-level prefix all fail validation, K Info lines are emitted. Because seenConfigs only suppresses re-fetching (not re-logging) for objects added in future cycles, and because the refresh ticker fires repeatedly, a sustained misconfiguration produces a continuous stream of per-object Info logs. Info is also the wrong severity for a degraded path where a config candidate was rejected — operators cannot distinguish this from a healthy informational event.
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:237 — iterateAllDays logs at Info level for every day-prefix whose iterateDayLevel call returns an error, inside a per-day-prefix loop. The 'no valid config found' terminal error from iterateDayLevel is a normal outcome when a day-bucket has been exhausted, not an operator-actionable event. Combined with the inner loop in iterateDayLevel, a worst-case refresh cycle with D day-prefixes each containing N objects produces up to D×N Info log lines per tick of the refresh interval. The outer loop's Info log adds no triage value beyond what the inner loop already emits.
- 1 additional finding in changed files.
These findings are advisory; address them before re-requesting review if applicable.
Assisted-by: ChatGPT 5.4
There was a problem hiding this comment.
[ARCH-REVIEW] Re-review: ✅ APPROVE
Prior blocking issue resolved: client.go:41 now returns errors.New("client already closed") instead of panicking. TestClientBatchAfterCloseReturnsError added to cover it. TestConsumeLogsWithQueueCompressionAndInMemoryStorage restored as a proper integration test using the new extension. 0 unresolved threads.
There was a problem hiding this comment.
Telemetry Review
Advisory: 4 medium findings in changed files.
4 findings appear pre-existing in code touched by this diff.
Key findings:
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:204 — iterateDayLevel emits hp.Logger.Info("Failed to apply config, trying next one") for every S3 object whose applyConfig call fails, inside a loop that is re-executed on every refresh-interval tick. If a set of configs persistently fails (e.g. schema mismatch, validation error), each new unseen key will produce an Info log on every scan cycle. The seenConfigs map prevents re-fetching already-fetched keys but does not prevent re-logging for newly discovered failing keys. At Info level this floods the log stream with repetitive, low-triage-value entries.
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:204 — In iterateDayLevel, a failed applyConfig call is logged at Info ('Failed to apply config, trying next one'). applyConfig failure is a service-owned event: the processor attempted to activate a config object from S3 and the activation failed. Logging this at Info means it is indistinguishable from normal scan progress in log-level filters and will not trigger Warn-level alerts. Operators cannot tell from dashboards or alert rules whether any config application failures occurred during a scan cycle.
- [MEDIUM] [EXISTING] @ processor/hotreloadprocessor/s3.go:250 — In iterateAllDays, a failed iterateDayLevel call is logged at Info ('Failed to iterate day level'). This represents an entire day-bucket of configs being unusable — a broader failure than a single config object. Logging at Info hides this from Warn-level alert rules and log filters. Operators scanning for degraded config-reload behavior will miss it.
- 1 additional finding in changed files.
These findings are advisory; address them before re-requesting review if applicable.
Assisted-by: ChatGPT 5.4
Assisted-by: ChatGPT 5.4
Assisted-by: ChatGPT 5.4
Assisted-by: ChatGPT 5.4
Assisted-by: ChatGPT 5.4
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR introduces a new storage extension, modifies core business logic in the loadbalancing exporter and hotreload processor, and updates CI workflows. These require human review.
repo: Sync fork with current upstream main
Merged the latest upstream main into the fork and kept the hotreloadprocessor lint surface green.
Description
Note
Medium Risk
Touches exporter resilience/queue configuration and introduces a new storage extension that is auto-used when
compress_in_memoryis enabled, which can affect runtime behavior and buffering. Hot reload and CI changes are mostly safety/robustness improvements with limited blast radius.Overview
Restores
loadbalancingexportersupport forsending_queue.compress_in_memoryby introducing an internal in-process storage backend and updating queue parsing/validation to treatsending_queueas opt-in (including explicitsending_queue.enabled) while auto-injecting an in-memoryStorageIDwhen compression-in-memory is requested.Adds a new alpha
extension/storage/inmemorystoragemodule (ephemeral shared storage) and registers it across repo metadata/config generators (builder config, distributions, versions, codecov components, issue templates, tidylist). Updates exporter docs/tests to reflect the new compatibility behavior and tightens goleak ignores for newer Windows syscalls.Hardens
hotreloadprocessorby rejecting multiple pipelines per signal, simplifying file-watcher construction, improving S3 iteration to continue past invalid configs, and ensuring decryptor resources are closed when supported; tests are updated to uset.Context().Includes small CI/automation fixes: better codegen drift diagnostics in
build-and-test.yml, fixes/workflow-approvecommand detection, stabilizes disk-space baseline reporting, and adjusts codeowner activity report generation to count owners via label mappings.Written by Cursor Bugbot for commit 79c21e2. This will update automatically on new commits. Configure here.
Summary by cubic
Syncs our
opentelemetry-collector-contribfork to upstream v0.148.0, preserves Sawmills patches, restoressending_queue.compress_in_memoryvia a newinmemorystorageextension (auto‑used when enabled), and hardens hot‑reload and CI. Adds a small test flake guard for Windows DNS resolver leaks.Dependencies
exporter/loadbalancing: queue disabled by default; parsesending_queue.enabled; require non‑nonepayload_compressionwhen compression is set; auto‑inject in‑memoryStorageIDwhencompress_in_memory: true; tests/docs updated (ignore Windows DNS resolver goroutines in goleak); module deps tidied (add directgo.opentelemetry.io/collector/extensionand local replace forextension/storage/inmemorystorage).extension/storage/inmemorystorage(alpha, process‑local); implementation unexported; used automatically by the exporter;go.modadded.processor/hotreloadprocessor: reject multiple pipelines per signal; close S3 decryptor when closable; continue past invalid S3 objects; simpler file watcher; tests uset.Context().CI and Repo Automation
/workflow-approvecommand guard and set disk‑space baseline from the initial reading.git status/git diff) when generated code is stale.extension/storage/inmemorystorageto component dropdowns; changelog config: registeredextension/storage/inmemorystoragein.chloggencomponents.Linear: SAW-6831 upgrades the fork to v0.148.0 and preserves Sawmills patches; it unblocks downstream builder/config work in SAW-6833.
Written for commit 79c21e2. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation