feat: forward beta node logs from the daemon - #175
Conversation
Beta participants need their node logs in the beta-channel Elasticsearch so the release gates have evidence to judge a build on, and the only way to collect them today is for each user to hand-roll a Vector config against their node log directory. That does not scale past the first few technical users and it is not something we can ask a wider beta cohort to do. The daemon already supervises every node on the machine and already knows each one's log directory, so it is the natural place to do this: no Windows service, no MSI, no admin prompt, no systemd unit, no separate install, and cross-platform for free. `ant node logs forward enable --token <token>` is the consent act; `disable` stops the flow and changes nothing else about any node. Two properties drive the design. Forwarding must never slow a node down, so it only ever reads log files, on its own task, with a bounded drop-oldest queue and bounded retries — a lost batch is acceptable, a stalled or memory-hungry daemon is not. And a daemon restart must neither duplicate nor lose events, which is why tail offsets are persisted and every document carries a deterministic `_id` derived from node, file and byte offset. That `_id` is coupled to the index name, not independent of it: `_id` uniqueness is per index, so each document is filed under an index derived from its own `@timestamp` rather than the wall clock. Deriving the index from the wall clock would send a batch replayed after midnight to a different daily index, where the duplicate would be silently accepted instead of rejected with a 409. Notable contract details from V2-1016, all covered by tests: - the bulk action must be `create`, never `index` — the write key grants `create_doc`, and `index` comes back as a per-item 403 - a `_bulk` response is HTTP 200 even when documents failed; success is per position in `items[].status`, so trusting the HTTP status alone silently discards failures - at a position, 201 is created, 200 is dropped by the server-side level filter, and 409 is a document our own earlier attempt already landed — all three are successes and none is retried - `host` and `beta_user` are never sent: the ingest pipeline strips the first (hostnames routinely contain personal names) and stamps the second from the authenticated API key Node file logging remains off by default. `enable` forwards only nodes that already have a log directory and reports the ones it is skipping, pointing at `--log-dir-path`, rather than appearing to succeed while shipping nothing. - Add ant-core/src/node/daemon/forward/: config (0600 token file), line parsing for both the text and JSON log layouts, rotation-aware tailing with persisted offsets, document tagging, a bounded batching sink with per-position retry, the Elasticsearch bulk sink, and the background task - Add GET /api/v1/logs/forward and POST .../enable|disable, with OpenAPI paths and schemas - Add `ant node logs forward enable|disable|status`, dual-path so the opt-in is still recorded when the daemon is down - Update CLAUDE.md and the e2e node management skill The status API returns a token fingerprint, never the token itself. Test results: - cargo test -p ant-core --lib: 562 passed (105 new) - cargo test -p ant-core --test log_forward_integration: 8 passed, driven against a real HTTP endpoint speaking the bulk contract, covering restart resume, replay idempotency, daily rotation, multi-line events and index-by-event-date - daemon_integration 6, node_add_integration 3, datamap_file 20, merkle_unit 8, unit_self_encrypt 16, ant-cli 18 — all passed - cargo clippy --all-targets --all-features -- -D warnings: clean - cargo fmt --all -- --check: clean `data::client::adaptive::tests::controller_perf_overhead_is_bounded` fails under a fully parallel run on a loaded machine. It is pre-existing and unrelated: it fails identically with these tests excluded, passes in isolation, and adaptive.rs is untouched here. The beta endpoint is still being provisioned, so a smoke test against the real logs.autonomi.com is outstanding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whether forwarding is enabled before or after a node starts changes what gets shipped, and the difference is not obvious from the code: the join-at-end rule applies only to files that already existed when forwarding was switched on, so a node that has never run is read from its first byte while one already running is picked up from wherever it had got to. That matters more than it looks. ant-node reports its version, commit and peer id on its startup line, so enabling after the node is up costs those fields on every document in the batch, along with the bootstrap and listen-address lines that show whether the node actually joined. Both directions were verified by hand before being written down here; these tests stop a later change to the priming logic silently reversing either one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Beta participants have nowhere to learn two things they need to know, and both are the kind of detail that only announces itself once it has already cost someone a run. The first is that log forwarding is optional. It is opt-in telemetry from people's own machines, so the docs need to say plainly that running beta builds is the whole requirement and that declining to forward costs nothing — if it reads as expected, the consent that `enable` is supposed to represent stops meaning very much. The section is marked optional in its heading and its walkthrough is conditional on having chosen to turn it on. It still makes the case for saying yes, because an informed choice needs one, but leaves it a choice. The second is ordering. Enabling forwarding is forward-looking consent, so for a log file that already exists the daemon starts reading at the end of it. A node that has not started yet has no file, so the one it creates is read from the first line — meaning enable-then-start captures the node's startup line and start-then-enable silently skips it, losing the version, commit and peer id that say which build produced everything that follows. The same applies to --log-dir-path, which has to be set when a node is added: node file logging is off by default, and a node added without it writes nothing to forward. `enable` reports those nodes rather than failing quietly, but the fix is to re-add the node, so it is much better to get it right first time. Adds a Beta Programme section covering both, with the working order as a single copyable block and the reasoning below it, plus what is and is not sent, how to tell if delivery is failing, and how to stop — keeping turning forwarding off separate from leaving the beta channel, since they are unrelated actions. The existing beta channel content was filed under `ant update` although it is not an `ant update` subcommand; it moves here rather than being duplicated. Also adds the `ant node logs forward` command reference alongside the other subcommands, the three new REST endpoints to the API table, and the new modules to the project structure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Installing the beta client meant downloading an archive from the releases page by hand, because the
installers could only ever fetch the newest stable build. That is a poor first step for a programme
whose whole point is getting more people onto the pre-release build, and it is the one part of beta
onboarding with no way around it.
Both installers now take ANT_CHANNEL=stable|beta, defaulting to stable so existing invocations are
unaffected. ANT_VERSION still overrides it.
The version resolution is the substance of this. /releases/latest cannot serve the beta channel at
all: GitHub excludes pre-releases from that endpoint, so it would always return the newest stable
build. Beta scans the release list instead, while stable keeps using /releases/latest, which already
means exactly the right thing.
Picking the highest pre-release from that list would be worse than not implementing this. Semver
ranks -rc above -beta, and both 0.3.4-beta.1 and 0.3.4-rc.1 exist right now, so the naive choice
installs a release candidate — code published before the release gates have reported. Both scripts
therefore mirror version_matches_channel from ant-core/src/channel.rs: final releases on either
channel, -beta.N additionally on beta, everything else rejected, matching the whole first identifier
so `betamax.1` is not caught by a prefix test.
Semver comparison is hand-rolled in both rather than delegated. `sort -V` does not implement the
rule that a pre-release ranks below the release it was cut from, and BSD and GNU builds disagree,
which matters because install.sh runs on macOS as well as Linux. PowerShell's [version] cannot parse
a pre-release suffix at all.
Download URLs and asset names needed no change: ant-cli-v0.3.4-beta.1 already publishes its assets
as ant-{version}-{target}.{tar.gz,zip}, so only resolution differed.
Note that the channel rule now lives in three places — these two scripts and channel.rs — with only
comments binding them together. Worth collapsing if it grows a fourth.
Test evidence:
- 19 logic cases in bash and 14 in PowerShell, both agreeing with channel.rs: -rc rejected on beta,
betamax rejected, 0.3.4 ranked above 0.3.4-beta.1, 0.10.0 above 0.9.0
- live resolution from both scripts: stable -> 0.3.3, beta -> 0.3.4-beta.1, i.e. beta correctly
preferred over the higher-ranked 0.3.4-rc.1
- install.sh run end to end against real GitHub releases into a temp prefix: ANT_CHANNEL=beta
installed a working ant 0.3.4-beta.1, and the default installed ant 0.3.3
- install.ps1 parses clean under pwsh and its helpers were exercised directly; its Windows-only
install body was not run
Also documents ANT_CHANNEL, ANT_VERSION and INSTALL_DIR in the Installation section, and replaces
the manual-download step in the beta walkthrough.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code review — changes requestedReviewed at I found two blocking issues in the opt-in log-forwarding path: 1. Document IDs can collide across installations
All clients write to shared Suggested fix: include a persistent, globally unique installation/source namespace in the document ID, then add a test showing that identical node/file/offset tuples from two installations produce different IDs. 2. Disabling forwarding does not stop in-flight delivery
Uploads can consequently continue after the CLI reports that forwarding is disabled, potentially for multiple request timeouts/retries. For an opt-in telemetry feature, disable should provide a clear revocation boundary. Suggested fix: propagate cancellation through queue draining, HTTP requests and retry sleeps; retain and await the forwarding task's Verification
Current CI has passing format, Clippy, documentation, Ubuntu build and Ubuntu unit-test jobs. The Security Audit fails on Independent review seats timed out without verdicts, so these findings are based on direct code inspection and the executed checks above rather than panel consensus. |
Addresses both blocking findings from review of 8658e16. Document ids could collide across installations. The id was built from node id, filename and byte offset, all of which are local values: every participant has a node 1, writing the same daily filename, whose first line starts at byte 0. Since the whole cohort writes into one shared beta-nodes-YYYY.MM.DD index and the sink counts a 409 as delivered, the second machine to send a given position had its event silently discarded. This was worse than a possibility. Offset 0 of each day's file is reached by every node on every machine, so on any given day the first event from node 1 collided across the entire cohort and exactly one won — and that first event is the startup line carrying version, commit and peer id, which is precisely the record the forwarding exists to collect. Ids are now prefixed with a random 64-bit installation namespace, minted on first enable and persisted alongside the opt-in. It is generated from random bytes rather than derived from hostname, MAC or username, so it separates installations without describing them, and it is deliberately stable: regenerating it would make a replayed batch look like new documents and duplicate them, which is the property the deterministic id exists to provide. Disabling did not stop delivery that was already under way. Cancellation was only observed between poll cycles, so a disable issued mid-flush kept uploading through the rest of the retry ladder — with the default policy, up to three 30s request timeouts plus backoff per batch, repeated for every batch left in the queue — while the CLI had already told the user forwarding had stopped. For a feature whose entire basis is opt-in consent, revocation has to mean something more definite. Cancellation now reaches the delivery loop: checked before taking each batch, and raced against the delivery itself, so the in-flight future is dropped and the HTTP request cancelled with it. The handle retains its JoinHandle and `stop_and_wait` awaits the task, so the disable endpoint returns only once the sender has actually stopped rather than merely having been signalled. Starting a replacement forwarder awaits the old one for the same reason, so two never overlap. Tests: - identical node/file/offset tuples from two installations produce different ids, and an integration test drives two forwarders through the shared mock endpoint to show both events are stored rather than one being swallowed as a conflict - the installation id is minted once, survives save/reload, and is unchanged by re-enabling or rotating the token - a blocking sink holds a request open across a disable: stop_and_wait returns promptly, the blocked send is confirmed never to have completed, and no request starts afterwards. Reverting the mid-delivery cancellation makes this test fail, so it pins the behaviour rather than describing it Full run: 570 lib tests, 9 log-forwarding integration tests, 6 daemon integration, 18 ant-cli, all passing; clippy -D warnings and fmt --check clean. Also documents the installation identifier and the disable guarantee in the README's beta section, since both are things a participant deciding whether to opt in should be told. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — both findings were correct and are fixed in 4bb4689. 1. Document ID collisions across installationsConfirmed, and it was worse than "can collide". Byte offset 0 of each day's file is reached by every node on every machine, so on any given day the first event from node IDs are now prefixed with a random 64-bit installation namespace, minted on first Tests as suggested: identical node/file/offset tuples from two installations produce different IDs, plus an integration test driving two forwarders through the shared mock endpoint showing both events are stored rather than one being swallowed as a 409. 2. Disable does not stop in-flight deliveryConfirmed. Cancellation was only observed between poll cycles, so a disable issued mid-flush kept uploading through the remaining retry ladder — with the default policy up to three 30s timeouts plus backoff per batch, repeated for every queued batch — while the CLI had already reported it stopped. Cancellation now reaches the delivery loop: checked before taking each batch, and raced against the delivery itself so the in-flight future is dropped and the HTTP request cancelled with it, discarding any pending backoff. Tested with a blocking sink as suggested: it holds a request open across a disable, Verification570 lib tests, 9 log-forwarding integration tests, 6 daemon integration, 18 The README's beta section now also documents the installation identifier and the disable guarantee, since both are things someone deciding whether to opt in should be told. Two notes on the rest of your reviewAgreed that Separately, and not raised by you: the release-channel rule now exists in three places — |
dirvine
left a comment
There was a problem hiding this comment.
Re-reviewed at 4bb46899cc19518bf58440eb342deb8352f08084.
The two blocking findings from my earlier review are addressed:
- document IDs now have a stable, random per-installation namespace, with unit and integration coverage for equivalent local positions on separate installations;
- disable now propagates cancellation into delivery, drops an in-flight send/retry, retains the task handle, and waits for task exit before returning.
Local verification passed:
- 113 forwarding unit tests
- 9 log-forwarding integration tests
- 6 daemon integration tests
- 18 CLI tests
- workspace Clippy with all targets/features and
-D warnings - formatting and diff hygiene
I found two non-blocking robustness points: a hand-written/pre-field enabled config can restart with an empty installation ID until enable is run again, and the replacement worker is spawned before the old worker has exited, so the “two never overlap” wording is stronger than the ordering strictly guarantees. Neither creates a release blocker for this newly introduced feature, but both are worth tightening later.
The Security Audit failure remains the base-branch h2 0.4.14 advisory rather than a regression in this PR. macOS and E2E jobs are still queued/pending; this approval does not waive those checks.
Approved.
Resolves the merge-back conflict for the 0.3.4 / 0.7.0 promotion. Only README.md conflicted. Both sides added text at the same point, after the two install commands: `main` gained a note that the `curl | bash` installer is Linux/macOS only and WSL users want the PowerShell one (#169), and the rc branch gained the table of environment variables shared by both installers (#175). They are complementary, so both are kept — the note first, since it decides which installer to run, then the table that applies to either. Everything else merged cleanly, including both manifests and Cargo.lock. Verified after the merge that the promotion survived it: ant-core 0.7.0 and ant-cli 0.3.4, ant-protocol pinned to 2.3.3, both ant-node pins (optional devnet and dev-dependency) to 0.17.2, no `-rc.`/`-beta.` suffixes and no git+branch sources left in the lock. `cargo check --all-targets --all-features` passes against the published crates and left the lock unchanged. The release tags are unaffected: ant-core-v0.7.0 and ant-cli-v0.3.4 point at 7145a0d, which this merge builds on rather than rewrites.
Linear issue
V2-1021 — https://linear.app/autonominetwork/issue/V2-1021
Risk tier
Boundary check: this changes no node behaviour, no wire protocol, no stored-data format, no
payments/economics and no upgrade mechanism. The daemon reads node log files that already exist and
POSTs them to an HTTP endpoint. It never touches a node process, its arguments, its stdio or its
data directory —
build_node_argsis untouched, and enabling or disabling forwarding never restartsa node.
Compatibility
new network traffic is the daemon's own outbound HTTPS to the beta log endpoint, and only after
the user opts in.
node_registry.jsonis unchanged — no newNodeConfigfields. Two new files are written, both created on demand and absent until the feature is used:
config_dir()/log_forward.json(opt-in state and token, mode 0600) anddata_dir()/log_forward_offsets.json(tail positions). An older daemon ignores both.GET /api/v1/logs/forward,POST /api/v1/logs/forward/enable,POST /api/v1/logs/forward/disable) and their OpenAPI schemas.No existing endpoint, request or response shape changes. One new CLI subcommand tree,
ant node logs forward enable|disable|status; no existing command changes.Semver impact
Purely additive: a new module, a new CLI subcommand, new endpoints, and one new
Errorvariant(
Error::LogForward). Nothing existing changed shape or behaviour.Test evidence
Unit —
cargo test -p ant-core --lib: 562 passed, 0 failed (107 new, inforward::). Coversthe config file (0600 permissions preserved across overwrite, corrupt-file handling, token never
serialized), line parsing in both ant-node log layouts, offset persistence and pruning, tailing
across rotation/truncation/partial writes, document construction against the index's field names,
queue bounding and per-position retry, and the bulk response classifier.
Integration —
cargo test -p ant-core --test log_forward_integration: 8 passed. These runagainst a real axum endpoint that reproduces the V2-1016 contract: HTTP 200 for a batch containing
failed documents, per-position
items[].status,createsemantics returning 409 on a repeated_id, and the forcedfilter_pathresponse shape. They assert:createactions withAuthorization: ApiKey,Content-Type: application/x-ndjsonand the required trailing newline;@timestamp,service,binary_version,channel,os,arch), andhost/beta_userare never sent;delivered exactly once, and no
_idis written twice;than storing a second copy;
Other suites, all passing:
daemon_integration6,node_add_integration3,datamap_file20,merkle_unit8,unit_self_encrypt16,ant-cli18.Lint/format:
cargo clippy --all-targets --all-features -- -D warningsclean;cargo fmt --all -- --checkclean.Manual, on a fresh DigitalOcean droplet (Ubuntu 24.04, static musl build of this branch):
daemon starts,
ant node logs forward statusreports off,GET /api/v1/logs/forwardreturns theexpected JSON, and all three paths appear in
/api/v1/openapi.json. Locally, against an isolatedXDG_CONFIG_HOME:enablewithout a token fails with the guidance message,enable --tokenwriteslog_forward.jsonat 0600, andstatusprints a token fingerprint rather than the token.Not run: the
e2e_*suites, which stand up an in-process testnet with Anvil.Note on a pre-existing flake:
data::client::adaptive::tests::controller_perf_overhead_is_boundedasserts 100k controller observations finish inside 500ms and fails under a fully parallel run on a
loaded machine. It is unrelated to this change — it fails identically with all 107 new tests
excluded (
--skip forward::), passes in isolation at 0.12s, andadaptive.rsis untouched here.New dependency
None. The implementation uses
reqwest,tokio,serde,serde_json,futures,blake3andtracing, all already direct dependencies ofant-core.ADR
n/a — Tier 1.
Mitigation / rollback
Forwarding is off unless the user runs
ant node logs forward enable, so the blast radius beforeopt-in is zero. After opt-in,
disablestops it immediately and changes nothing else about anynode. The forwarder only reads log files, on its own task, with a bounded drop-oldest queue and
bounded retries, so a dead or slow endpoint costs dropped log lines and nothing more — it cannot
block, restart or slow a node. Reverting the two commits removes the module, the endpoints and the
subcommand; the two files it writes are inert to any other code path and can be deleted.
Notes for review
Two design points that are load-bearing and easy to miss:
The deterministic
_idand the per-event index derivation are one mechanism, not twochoices.
_iduniqueness is per index, so each document is filed under an index derived fromits own
@timestamprather than the wall clock. Deriving the index from the wall clock wouldsend a batch replayed after midnight to a different daily index, where the duplicate would be
accepted instead of rejected with a 409 — silently breaking the no-duplication guarantee.
The tailer holds a growing file's final event back for one poll so continuation lines can
join it. That is what keeps a panic and its backtrace one document rather than twenty orphans,
at the cost of one poll interval (5s) of latency on the tail of a burst.
Also worth knowing: node file logging is off by default, so
enableforwards only nodes thatalready have a
log_dirand reports the ones it is skipping, pointing at--log-dir-path. Betaonboarding docs need to carry that instruction or most users will forward nothing. Enabling before
starting a node captures its startup line (and so the
version/commit/peer_idfields);enabling afterwards joins the file at its end and skips it.
The beta endpoint was still being provisioned at the time of writing, so a smoke test against the
real
logs.autonomi.comis outstanding. The wire contract itself is confirmed and is documented atthe top of
ant-core/src/node/daemon/forward/es.rs.