Build report: structured JSON report with per-mojo log capture

[gnodet]

Summary

Part 2 of the logging feature chain (depends on #12694 — logging foundation).

Adds a structured build report that captures per-module and per-mojo execution results, timing, log events, and failures as a JSON file at the end of every build.

15 files changed, ~2700 insertions — focused on report data model, collection, and serialization.

What's in this PR

Layer	Files	Description
API	BuildReport, BuildStatus, ModuleReport, MojoReport, FailureReport	Immutable interfaces for the report data model
Impl	DefaultBuildReport, DefaultModuleReport, DefaultMojoReport, DefaultFailureReport	Record implementations
Collector	BuildReportCollector	EventSpy that tracks lifecycle events and captures log output via LogEventSink, routing to mojo/module/build-level buffers
Writer	BuildReportJsonWriter	Zero-dependency JSON serializer with stable field order
Tests	3 test classes, 17 tests	Unit tests for collector, JSON writer, and integration

Key design decisions

• EventSpy pattern: BuildReportCollector is a @Named @Singleton that extends AbstractEventSpy, discovered automatically — no wiring changes needed
• Thread-based log routing: Uses ConcurrentHashMap<Long, String> (thread ID → mojo/project key) to associate log events with the correct scope in parallel builds
• Dual sink architecture: Uses LogEventSink (4-arg) independently from the existing LogSink (5-arg) used by ProjectBuildLogAppender — no interference with console output
• Atomic writes: Reports are written to a temp file, then atomic-moved into place with a timestamped filename and a build-report-latest.json symlink
• Defensive: onSessionEnded wraps report generation in try-catch so report failures never crash the build

What's NOT in this PR (deferred to later PRs)

• BuilderProblem enrichments (key, suggestion, documentationUrl) — Warning mode, diagnostic collector, BuilderProblem enrichments #12698
• Diagnostic collection and warning deduplication — Warning mode, diagnostic collector, BuilderProblem enrichments #12698
• --warning-mode CLI flag — Warning mode, diagnostic collector, BuilderProblem enrichments #12698
• Console modes (--console=plain/rich/machine) — Console modes: --console=plain/rich/verbose/machine #12697
• mvnlog viewer tool — mvnlog: build log viewer, integration tests, script routing #12699

PR chain

#	PR	Feature
1	#12694	Logging foundation
2	This PR	Build report
3	#12697	Console modes
4	#12698	Warning mode + diagnostics
5	#12699	mvnlog viewer
6	#12702	Structured problems pipeline
7	#12714	TRACE level migration

Test plan

• 17 new unit/integration tests pass
• Full Maven test suite passes (same results as base branch)
• CI validation

🤖 Generated with Claude Code

[gnodet]

Well-designed build report feature with clean API/impl separation and comprehensive tests. A few issues noted below.

Also noted:

• The architecture is solid: clean API interfaces in maven-api-core, record-based implementations in maven-core, EventSpy pattern for automatic discovery, thread-based log routing for parallel-build safety, atomic file writes with symlink swap.
• The PR correctly depends on PR #12694 (logging foundation) — should not be merged until #12694 lands.
• No test for the captureLogEvent routing logic (mojo-level vs module-level vs build-level buffers). This is the core routing mechanism and warrants at least one test exercising the dispatch.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

The Javadoc says "Beyond this, events are dropped and a truncation notice is appended" but captureLogEvent silently drops events without appending any truncation notice. Either append a synthetic LogEvent indicating truncation (e.g. with level WARN and message "... N events truncated"), or update the Javadoc to say events are silently dropped.

[gnodet]

The hasMore parameter is unused (annotated @SuppressWarnings("unused")) and all call sites pass true. If trailing comma control is no longer needed, the parameter should be removed to reduce confusion.

Suggested change

	private static void writeModule(StringBuilder sb, ModuleReport module, int indent) {
	private static void writeNullableField(StringBuilder sb, int indent, String key, String value) {

[gnodet]

Well-designed build report feature with clean API/impl separation, solid thread safety, and comprehensive tests. A few issues worth addressing:

Confirmed findings (verified independently):

1. [Medium] BuildReportCollector.java — The Javadoc on MAX_LOG_EVENTS_PER_SCOPE states "events are dropped and a truncation notice is appended," but captureLogEvent silently drops events without ever appending a truncation notice. Either implement the truncation notice (e.g., append a synthetic LogEvent like "... N events truncated") or correct the Javadoc to say events are silently dropped.

2. [Low] BuildReportJsonWriter.java — The writeNullableField method has an unused boolean hasMore parameter annotated with @SuppressWarnings("unused"). The parameter is never read and the method always emits a trailing comma regardless. Remove it to avoid confusion.

3. [Low] BuildReportJsonWriter.java — writeProblem uses sb.lastIndexOf(",\n") to remove trailing commas (searches entire buffer backwards), while writeLogEvent uses the dedicated removeTrailingComma helper (checks only last two characters). Use removeTrailingComma consistently — it's safer since lastIndexOf could theoretically match an earlier ,\n if future refactors change field order.

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

[Medium] The Javadoc here says "events are dropped and a truncation notice is appended," but captureLogEvent (below) silently drops events when the limit is reached — no truncation notice is ever appended.

Either implement the truncation notice (e.g., append a synthetic LogEvent like "... N events truncated") or correct this Javadoc to say events are silently dropped.

[gnodet]

[Low] The hasMore parameter is annotated @SuppressWarnings("unused") and is indeed never read — the method always emits a trailing comma regardless of its value. Consider removing it:

Suggested change

	StringBuilder sb, int indent, String key, String value, @SuppressWarnings("unused") boolean hasMore) {
	private static void writeNullableField(StringBuilder sb, int indent, String key, String value) {

[gnodet]

Well-structured addition of a JSON build report feature with clean API design, good test coverage (17 tests), and defensive error handling. The zero-dependency JSON writer is appropriate for Maven's philosophy. A few design items:

Medium severity:

1. No opt-out mechanism (BuildReportCollector.java): The collector is unconditionally active for every build — every mvn invocation writes a JSON file to disk with no system property to disable it. Consider adding -Dmaven.build.report.skip=true for environments where this is undesirable (read-only filesystems, embedded invocations, CI runners).

2. MojoSkipped events not handled (BuildReportCollector.java): ExecutionEvent.Type.MojoSkipped (fired e.g. when a mojo requires online mode but Maven is offline) silently vanishes from the report. Inconsistent with ProjectSkipped which IS handled. Skipped mojos should be tracked with BuildStatus.SKIPPED.

Low severity:

3. Failure timestamp inaccuracy (BuildReportCollector.java line 1110): failureTimestamp is set to MonotonicClock.now() at report-assembly time, not at actual failure time. The mojo's timing data does capture the real timing — worth documenting in the FailureReport.timestamp() Javadoc.

4. Inconsistent trailing comma removal (BuildReportJsonWriter.java): writeProblem uses sb.lastIndexOf(",\n") which searches backwards through the entire buffer, while writeLogEvent uses the more robust removeTrailingComma(sb) which checks only the end. Consider using removeTrailingComma consistently.

5. Unused hasMore parameter (BuildReportJsonWriter.java line 1553): Annotated @SuppressWarnings("unused") and never referenced. Either use it to control comma behavior or remove it.

6. ~70 lines of duplicated test helpers: BuildReportCollectorTest and BuildReportIntegrationTest share identical createProject, createSession, createMojoExecution, and createEvent methods. Consider extracting to a shared test utility.

The API design (immutable interfaces in maven-api-core, record implementations in maven-core, @Experimental markers) follows Maven's established patterns. Thread safety approach is sound. The atomic file write + symlink pattern is well-implemented with proper fallbacks.

---

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Well-structured build report feature with clean API/impl separation, solid thread safety, and good test coverage. The core architecture (EventSpy-based collector, ConcurrentHashMap log routing, atomic file write with symlink swap) is sound. The 271-file diff is mostly site doc conversions and dependency bumps from rebasing — the actual feature is ~15 new files.

Nine items to address before the chain merges to master:

1. NPE risk: null logger-name guard removed in MavenJulHandler (high)

In MavenJulHandler.java line ~124, the null-to-empty-string guard for record.getLoggerName() was removed. JUL spec explicitly allows null logger names, and LoggerFactory.getLogger(null) will throw NPE/IllegalArgumentException. Restore the guard or move the LoggerFactory.getLogger() call inside the else branch.

2. No opt-out mechanism for BuildReportCollector (medium)

The collector is unconditionally active (@Singleton @Named extending AbstractEventSpy) — every mvn invocation incurs the overhead with no way to disable it. Read-only filesystems, embedded invocations, and CI runners that don't want report files have no escape hatch. Consider a system property like -Dmaven.build.report.skip=true.

3. MojoSkipped events not handled (medium)

BuildReportCollector.onEvent() handles MojoStarted/MojoSucceeded/MojoFailed but not MojoSkipped. Skipped mojos silently vanish from the report. This is inconsistent with ProjectSkipped which IS handled.

4. Unconditional StackWalker overhead (medium)

DefaultLog.withMetadata() now calls StackWalker.walk() unconditionally for every Log API call (~1-5μs per log statement). The previous hasReportCapture() guard was removed. If an opt-out is added (finding #2), the conditional StackWalker should be restored.

5. Javadoc/code mismatch on truncation (medium)

MAX_LOG_EVENTS_PER_SCOPE Javadoc says "events are dropped and a truncation notice is appended," but the code silently drops events beyond the limit without appending a notice. Align the docs or implement the truncation notice.

6-9. Lower-severity items:

• BuildReportJsonWriter.writeNullableField has unused boolean hasMore parameter with @SuppressWarnings("unused") — remove it
• writeProblem uses sb.lastIndexOf(",\n") (O(n) search) while writeLogEvent uses the dedicated removeTrailingComma helper (O(1)) — use the helper consistently
• failureTimestamp is set at report-assembly time (MonotonicClock.now() in buildReport()), not at actual failure time — could derive from the failed MojoTiming
• BuildReportCollectorTest and BuildReportIntegrationTest share ~70 lines of identical helper methods — consider extracting to a shared test utility

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet