remotewrite/receiver: propose programmatic Receiver harness (mirrors sender) - #260
Open
om7057 wants to merge 2 commits into
Open
remotewrite/receiver: propose programmatic Receiver harness (mirrors sender)#260om7057 wants to merge 2 commits into
om7057 wants to merge 2 commits into
Conversation
om7057
marked this pull request as ready for review
August 11, 2026 18:13
bwplotka
force-pushed
the
feature/rw2senderfix
branch
2 times, most recently
from
August 12, 2026 10:05
ebdbd1f to
0380477
Compare
Member
|
Nice! I just updated the rw2sender branch, so you might want to update. But also we probably have to finish and get #256 merged so we can agree on the framework first. Will try to progress there |
…etheus#208) Ports the sender suite's programmatic pattern (Sender/RunTests/ComplianceTests, see remotewrite/sender) to the receiver side, so receiver compliance tests can be imported and run against a target Receiver implementation instead of being driven by an external config file + env vars. This converts metric_test.go's cases as a proof of concept (84 generated sub-cases) and adds a self-test that runs them against a downloaded Prometheus release binary. It intentionally lives at remotewrite/receiver/next rather than replacing remotewrite/receiver outright, to avoid breaking the existing suite while the design gets reviewed; if accepted, the remaining 9 files (combined, exemplar, histograms, metadata, request, rw1_compat) would be converted the same way and this would move up to replace remotewrite/receiver. One notable design deviation from sender: RunTests starts the receiver once per whole run rather than once per test case, since real receivers are too costly to restart per case and these tests only assert on the synchronous HTTP response to each write. Also note: remotewrite/receiver currently fails `go vet` on this branch (ts.CreatedTimestamp undefined) because the pinned prometheus/prometheus version predates writev2 start-timestamp support; the pending dependabot bump to v0.312.0 is a prerequisite for CreatedTimestamp-based cases. Signed-off-by: om7057 <kulkarniom7057@gmail.com>
om7057
force-pushed
the
receiver-programmatic-poc
branch
from
August 12, 2026 20:09
e3aa09b to
6ba4486
Compare
Author
Member
|
Can you change in-place? It will easier to review and effectively use |
Converts the remaining test files (combined, exemplar, histograms, metadata, request, rw1_compat) to the programmatic Test/RunTests/ ComplianceTests pattern, retires the old config-file + env-var driven package main scaffolding (main_test.go, helpers_test.go, config_example.yml), and moves the whole package from remotewrite/receiver/next up to remotewrite/receiver in place, as requested in review. Adds two Test fields needed by the remaining files that the metric.go proof of concept didn't exercise: - BuildRequest, for cases that can't be expressed via RequestOpts alone (RW1-format requests, corrupted bodies/headers, orphan metadata/exemplars via UnsafeRequest). - Raw, to run a case once instead of the default MUST/SHOULD dual-variant generation, for cases that were only ever asserted once pre-conversion. Also fixes two bugs found while running the full suite against a real Prometheus binary for the first time: - StartTimestamp (previously named CreatedTimestamp) was never wired into the request; renamed to match the actual field name and wired into writev2.Sample/Histogram.StartTimestamp. - Dropped a 10ms pacing ticker between generateRequest calls during the earlier PoC; without it, back-to-back requests hitting the same series (several table-driven cases share metric names) could land on the same millisecond timestamp, which the receiver treats inconsistently. Restored it. Verified: go build/vet/golangci-lint clean across the whole remotewrite module, and the full suite (199 sub-tests) passes against both a locally built and a downloaded Prometheus release binary, except two SHOULD-level BadContentTypes cases expecting 415 Unsupported Media Type. Checked storage/remote/write_handler.go in prometheus/prometheus: it never emits 415, only 400, for any content type rejection, so those two are a genuine, pre-existing compliance gap in Prometheus itself, not a bug in this conversion. Left them as faithfully-ported failing assertions rather than weakening the test, since that's the point of the suite. Signed-off-by: om7057 <kulkarniom7057@gmail.com>
Author
|
Done, moved everything in-place. Converted the remaining 6 files too, so this now replaces remotewrite/receiver directly rather than sitting in next/. Ran the full suite (199 sub-tests) against a real Prometheus binary for the first time and it caught two real bugs in my draft (both described in the updated PR description). Fixed both. Two design questions in the description could use your input when you get a chance, especially the lifecycle one (starting the receiver once per run instead of once per case). |
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.
What
Progresses #208 for the receiver side: converts
remotewrite/receiverfrom the config-file + env-var drivenpackage maintest binary into an importable, programmatic package, mirroring theSender/RunTests/ComplianceTestspattern already landed forremotewrite/sender(#212).All 9 original test files (metric, combined, exemplar, histograms, metadata, request, rw1_compat, plus the main_test.go/helpers_test.go scaffolding) are now converted into the new declarative
[]Test{...}shape and moved in-place, replacing the old package directly rather than staying in a parallelnext/directory.Verified:
go build/go vet/golangci-lintclean across the wholeremotewritemodule. The full suite (199 sub-tests) passes against both a locally built and a downloaded Prometheus release binary, except two SHOULD-levelBadContentTypescases expecting415 Unsupported Media Type. I checkedstorage/remote/write_handler.goin prometheus/prometheus: it never emits 415, only 400, for any content-type rejection, so this looks like a genuine, pre-existing compliance gap in Prometheus itself rather than a bug in this conversion. Left the assertion as faithfully ported rather than weakening it.Also fixed two things found while actually running the full suite for the first time (the original suite was never run end-to-end against a real receiver in this shape before):
StartTimestamp(previously misnamedCreatedTimestampin my draft) wasn't wired into the request; now set onwritev2.Sample/Histogram.StartTimestamp.Design questions for reviewers
sender.sender.RunTestsrestarts a fresh sender subprocess per test case. A receiver under test is too costly to restart per case, and these tests only assert on the synchronous HTTP response to each write, soRunTestsstarts the receiver once for the whole run and sends all cases at it sequentially, matching the pre-conversion suite'sTestMain-once behaviour. Open to alternatives if this is wrong.ReceiverperRunTestscall, matchingsender's shape. Should callers just callRunTestsonce per receiver instead?PROMETHEUS_RW2_COMPLIANCE_READY_TIMEOUT(default 3m), since a receiver may need to download/build a binary first. Happy to align naming withsender'sPROMETHEUS_RW2_COMPLIANCE_TEST_TIMEOUT.Testfields needed for casesRequestOptsalone couldn't express:BuildRequest(RW1-format requests, corrupted bodies/headers, orphan metadata/exemplars viaUnsafeRequest) andRaw(run a case once instead of the default MUST/SHOULD dual-variant, for cases that were only ever asserted once pre-conversion).