Job Logging v2 - #122
Merged
Merged
Conversation
The README's timestamp format table lists both, but they were missing from the TimeLayouts map, so configuring them warned "unknown timestamp format" and silently fell back to RFC3339. The map now covers every named layout constant of the time package, pinned by a parity test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolveTimestampLayout runs on every job start and restart; with timestamps enabled by default that turns the "logging with timestamp layout" info line into per-restart noise for every job. The layout is fully determined by visible configuration, so log the choice at debug level and keep only the unknown-format warning, which indicates a config error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BootJob.Run logged "job failed, but is allowed to fail" with error=<nil> for every successful canFail boot job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A fast-writing child against a persistently broken non-ErrClosed log target was logged once per line, flooding mittnite's own output at the child's write rate. Log the first error of a streak, suppress until a write succeeds, and keep attempting writes so a recovered target (e.g. transient ENOSPC) resumes forwarding. The ErrClosed drain path is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Watch pre/post commands wrote raw to the process-wide streams, leaving their output unattributable while every other line of a decorated job carries the timestamp/name prefix. Run them through the same line forwarder, targeting os.Stdout/os.Stderr (not job.stdout/job.stderr, which are owned and closed by startOnce), with the same bounded drain. Jobs with decoration disabled — and pipe-creation failures — keep the direct attachment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The root command's fallback called up.Run, but the up command switched from Run to RunE in cdb2ecf (2023), so running mittnite without a subcommand crashed on the nil Run function right after the "defaulting to 'up'" warning. Delegate to up.RunE instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Timestamps and job-name prefixes on forwarded job output are now opt-out: the --job-log-timestamps and --job-log-name-prefix defaults flip to true, with unset or unparsable MITTNITE_JOB_LOG_* values falling back to enabled (the startup warning now states the assumed value, since an unparsable opt-out attempt would otherwise silently mean on). Disable globally via --job-log-*=false or MITTNITE_JOB_LOG_*=0; an explicit per-job enableTimestamps/enableNamePrefix — including false — still wins, and jobs with both options disabled keep the raw fd passthrough. BREAKING CHANGE: every line of every job and boot job is now prefixed with [<RFC3339 timestamp>] [<job name>] unless explicitly disabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- raw-passthrough tests now use output without a trailing newline, which the line forwarder would append — byte-identical output proves the raw path, not just an empty decoration - exercise the real executeWatchCommand wiring by swapping the process-wide streams, plus the name-prefix-only and cmd.Start-failure (fast EOF drain) paths of runCommandWithJobDecoration - pin the bare-mittnite fallback to RunE on both root and up - correct the end-to-end test comment: the cmd/up flag wiring is covered by E2E runs, not this test - examples: echoloop_notime opts out of timestamps explicitly again, matching its name under the flipped defaults Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements “Job Logging v2” by making job output decoration (timestamp + job name prefix) the new default, extending the decoration behavior to more execution paths (boot jobs and watch commands), and tightening config/CLI semantics and documentation around the migration.
Changes:
- Make job/boot-job output decoration default-on via new global flags/env vars, with per-job tri-state overrides.
- Replace scanner-based timestamping with a chunk-safe line forwarder (handles >64KiB lines) and apply the same decoration to watch pre/post commands.
- Add migration docs + changelog, fix boot-job output initialization, and add comprehensive tests around the new behavior.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents new default decoration behavior and adds v2 migration guidance. |
| pkg/proc/types.go | Refactors base job initialization and validates log targets for non-boot jobs. |
| pkg/proc/types_test.go | Pins TimeLayouts to the set of named Go time layouts. |
| pkg/proc/runner.go | Improves boot-job init error reporting with job context. |
| pkg/proc/job_common.go | Routes watch command execution through decoration-aware runner. |
| pkg/proc/job_common_test.go | Adds tests ensuring watch command output decoration and raw passthrough behavior. |
| pkg/proc/job_boot.go | Fixes canFail warning to only log when there’s an actual error. |
| pkg/proc/job_boot_test.go | Adds coverage for canFail boot-job warning behavior. |
| pkg/proc/basejob.go | Implements new line-forwarding decoration, bounded draining, and streak-suppressed write-error logging; adds decoration for auxiliary commands. |
| pkg/proc/basejob_test.go | Adds broad tests for boot-job stream init, log target validation semantics, forwarding behavior, and default decoration end-to-end. |
| main.go | Switches logrus timestamp format to RFC3339. |
| internal/config/types.go | Introduces tri-state log options (pointer bools) and accessors. |
| internal/config/types_test.go | Tests HCL tri-state behavior and default materialization. |
| internal/config/ignitionconfig.go | Adds ApplyJobLogDefaults to materialize global defaults onto jobs/boot jobs. |
| go.mod | Bumps indirect deps golang.org/x/sync and golang.org/x/text. |
| go.sum | Updates sums for bumped indirect deps. |
| examples/timestamps.d/timestamps.hcl | Updates timestamp format example and adds opt-out/raw examples for v2 defaults. |
| cmd/up.go | Adds global log-decoration flags with env-derived defaults + env parse warnings; applies defaults to ignition config. |
| cmd/up_test.go | Tests env parsing, env warnings, and default-on flag behavior. |
| cmd/root.go | Fixes bare mittnite fallback to delegate to up via RunE (and keeps profiling support intact). |
| cmd/root_test.go | Adds regression test for RunE delegation to avoid nil Run call. |
| cmd/mittnitectl/main.go | Switches logrus timestamp format to RFC3339. |
| CHANGELOG.md | Adds breaking-change notes for v2.0.0 migration. |
| .goreleaser.yml | Adds release header linking to CHANGELOG for major upgrades. |
Suppressed comments (1)
internal/config/types.go:118
- Same inconsistency as TimestampsEnabled: this comment says an unset enableNamePrefix counts as disabled, but earlier comments describe nil as “follow the global default”. Clarify that defaults must be materialized (and what nil means) to avoid confusion.
// NamePrefixEnabled reports whether the job's output lines should be prefixed
// with the job name; an unset enableNamePrefix counts as disabled.
func (c *BaseJobConfig) NamePrefixEnabled() bool {
return c.EnableNamePrefix != nil && *c.EnableNamePrefix
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The accessor comments contradicted the field doc (unset = follow the global default vs. unset = disabled) without saying when ApplyJobLogDefaults materializes the former into the latter. And the unparsable-env warning claimed "using default" although it runs after flag parsing, where an explicit --job-log-* flag may have overridden the default — it now reports the effective value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
leontappe
force-pushed
the
feat/job-logging-v2
branch
from
August 6, 2026 12:19
744e830 to
40ddbc9
Compare
elenz97
approved these changes
Aug 6, 2026
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
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.
No description provided.