Skip to content

libutil: add machine origin field to Logger activity events#15599

Open
lovesegfault wants to merge 1 commit intomasterfrom
logger-machine-origin
Open

libutil: add machine origin field to Logger activity events#15599
lovesegfault wants to merge 1 commit intomasterfrom
logger-machine-origin

Conversation

@lovesegfault
Copy link
Copy Markdown
Member

@lovesegfault lovesegfault commented Mar 30, 2026

Motivation

When using remote builders (via --builders or --store ssh-ng://), build monitors like nix-output-monitor receive degraded structured logging compared to local builds.

This PR adds an optional machine parameter to Logger::log() and startActivity() so that log messages and activities forwarded from a remote store connection can be tagged with their origin (e.g. "ssh-ng://host").

Context

Split from #15215 into smaller reviewable changes. Related to #15055 — adopts the same machine origin string approach reviewed favorably there.

What changed

  • Logger::log() and startActivity() gain an optional machine parameter
  • JSONLogger emits a "machine" field on msg/start events
  • Progress bar falls back to the machine parameter when actBuild fields lack a machine name
  • TeeLogger forwards the parameter to all sub-loggers
  • TunnelLogger accepts the parameter but does not serialize it (would need a worker protocol version bump)
  • BasicClientConnection gains a machineName field populated by RemoteStore::initConnection() from getHumanReadableURI()
  • Build hook paths convert empty machineName to nullopt before forwarding

Activity ID remapping

ActivityId is (pid << 32) | counter, so IDs from a remote daemon are not guaranteed unique against local activities (or other remotes). BasicClientConnection now allocates fresh local IDs when forwarding STDERR_START_ACTIVITY and remaps subsequent stop/result/parent references through a per-connection map — mirroring what handleJSONLogMessage already does for the build-hook path. This means stopActivity()/result() don't need a machine parameter; loggers correlate via the activity map populated at start time. nextActivityId() is extracted so the connection allocates from the same sequence as Activity.

@github-actions github-actions Bot added the store Issues and pull requests concerning the Nix store label Mar 30, 2026
Comment thread src/libmain/progress-bar.cc Outdated
Add an optional "machine" parameter to Logger::log() and startActivity()
to identify messages forwarded from a remote store connection (e.g.
"ssh-ng://host"). This lets the progress bar and JSON consumers
attribute forwarded build activity to its source machine.

- JSONLogger emits a "machine" field on msg/start events
- Progress bar falls back to the machine parameter when actBuild fields
  lack a machine name
- TeeLogger forwards the parameter to all sub-loggers
- TunnelLogger accepts but does not serialize it (would need a worker
  protocol version bump)
- handleJSONLogMessage forwards machine through Activity constructors
- BasicClientConnection gains a machineName field set by
  RemoteStore::initConnection()

Since ActivityId is (pid << 32) | counter and thus not unique across
machines, BasicClientConnection now remaps remote activity IDs to
freshly-allocated local IDs when forwarding STDERR_START_ACTIVITY, and
translates subsequent stop/result/parent references through a
per-connection map. This mirrors what handleJSONLogMessage already does
for the build-hook path, and means stopActivity()/result() don't need a
machine parameter — loggers correlate via the activity map populated at
start time. nextActivityId() is extracted so the connection can allocate
from the same sequence as Activity.
@lovesegfault lovesegfault force-pushed the logger-machine-origin branch from f743ee1 to 09ffa11 Compare April 3, 2026 20:21
@lovesegfault lovesegfault requested a review from edolstra April 3, 2026 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants