From b9ec57b96ccb747dba546fda304b6be5190fd668 Mon Sep 17 00:00:00 2001 From: ddsha441981 Date: Mon, 24 Aug 2026 13:00:40 +0530 Subject: [PATCH 1/3] fix: render startup-time numbers instead of literal SLF4J specifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three log templates used {:+d} / {:.1f}. SLF4J only interpolates {}, so the specifier was printed literally and every following argument shifted by one: ⏱ Startup Time: 6687ms -> ms ({:+d}ms, {:.1f}% 7178) The worst one was the startup-time gate's log.error, which is exactly the line a CI reader lands on. Numbers are now pre-formatted through signed()/oneDecimal() and passed as plain {} arguments: ⏱ Startup Time: 6687ms -> 7418ms (+731ms, 10.9% slower) Also routes the percent change written into wiredoctor-gate.status through oneDecimal(), which pins Locale.ROOT — that file is machine-read, so the decimal separator must not follow the build machine's default locale. Docs, found while reproducing on spring-petclinic: - ci-gating.md prescribed recording the baseline with `spring-boot:run` while gating on `java -jar`. devtools is on the classpath for one and excluded from the repackaged jar for the other, which on petclinic is 12 removed beans and a 31% startup delta — a startup-time failure on a build where nobody changed a line. Step 1 now records from the jar, with a callout on keeping profiles and web-application-type identical across both runs. - ci-gating.md now warns that gating through `spring-boot:run` cannot fail a build: devtools runs main on its own restart thread, so the gate trips, BUILD SUCCESS prints, and the job exits 0. Falls back to grepping wiredoctor-gate.status for anyone stuck on a Maven goal. - configuration.md documents output-path for projects whose build lints the source tree (nohttp rejects the vendored http:// license headers in the report), and that scan-packages also keeps WireDoctor's own beans out of the smell rankings. - _config.yml declares the warning callout the docs reference; Just the Docs renders it unstyled otherwise. Adds WireDoctorLogFormattingTest, including a guard that greps every log template in the module for a non-{} specifier so the whole defect class stays fixed. --- docs/_config.yml | 8 ++ docs/ci-gating.md | 45 ++++++++- docs/configuration.md | 40 +++++++- .../com/wiredoctor/WireDoctorAnalyzer.java | 31 ++++-- .../WireDoctorLogFormattingTest.java | 96 +++++++++++++++++++ 5 files changed, 208 insertions(+), 12 deletions(-) create mode 100644 wiredoctor-autoconfigure/src/test/java/com/wiredoctor/WireDoctorLogFormattingTest.java diff --git a/docs/_config.yml b/docs/_config.yml index 6930f76..daec5b2 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -16,6 +16,14 @@ lang: en-US remote_theme: just-the-docs/just-the-docs@v0.12.0 color_scheme: dark +# Callouts referenced as `{: .warning }` in the docs. Just the Docs renders these +# unstyled unless they are declared here. +callouts_level: quiet +callouts: + warning: + title: Warning + color: red + plugins: # Required for remote_theme to resolve. Pages injects it on its own builds, # but once an explicit plugins list exists it must be named here or the theme diff --git a/docs/ci-gating.md b/docs/ci-gating.md index d3d4f0d..272fb71 100644 --- a/docs/ci-gating.md +++ b/docs/ci-gating.md @@ -37,13 +37,32 @@ app unless you explicitly asked it to via `fail-on`. ## Step 1 — create and commit the baseline -Run your app once in baseline-write mode (locally or in a one-off CI job): +**Record the baseline the same way CI will run the app.** The graph WireDoctor sees +depends on what is on the classpath, so a baseline captured under one launch method +and gated under another produces a diff full of differences you did not make. + +CI runs the packaged jar (Step 3), so build and record from the jar: ```bash -./mvnw spring-boot:run \ - -Dspring-boot.run.arguments="--wiredoctor.baseline=wiredoctor-baseline.json --wiredoctor.baseline-write=true" +./mvnw -DskipTests package +java -jar target/*.jar \ + --wiredoctor.baseline=wiredoctor-baseline.json \ + --wiredoctor.baseline-write=true ``` +{: .warning } +> Do not record the baseline with `./mvnw spring-boot:run` if CI gates on the jar. +> `spring-boot:run` keeps `spring-boot-devtools` on the classpath while +> `spring-boot-maven-plugin` excludes it from the repackaged jar. On +> spring-petclinic that single difference shows up as **12 removed beans** +> (`classPathFileSystemWatcher`, `LocalDevToolsAutoConfiguration`, +> `DevToolsDataSourceAutoConfiguration`, …) and a **31% startup-time delta** — +> enough to trip `startup-time` on a build where nobody changed a line of code. +> +> Same rule for anything else that moves the graph: keep the active profiles and +> `spring.main.web-application-type` identical between the baseline run and the +> gate run. + Then commit the file: ```bash @@ -72,6 +91,26 @@ The gate trips at `ApplicationReadyEvent`, so any way of fully starting the context works. The simplest is to boot the packaged jar and let the exit code speak: +{: .warning } +> **Do not gate through `./mvnw spring-boot:run`.** With devtools on the classpath +> it launches `main` on its own restart thread; `WireDoctorRegressionException` is +> logged, that thread dies, and the Maven build still reports `BUILD SUCCESS` and +> exits **0**. The gate fires and CI goes green anyway: +> +> ``` +> [WireDoctor] REGRESSION GATE TRIPPED (wiredoctor.fail-on=new-cycle): +> 2 new cycle(s) introduced vs baseline. +> ... +> [INFO] BUILD SUCCESS +> ``` +> +> Booting the jar gives the exit code 1 that CI needs. If you must use a Maven +> goal, gate on the verdict file instead — it is written before the exception: +> +> ```bash +> grep -q '^FAIL' target/wiredoctor-gate.status && exit 1 +> ``` + ```yaml name: Architecture Gate diff --git a/docs/configuration.md b/docs/configuration.md index 1d21829..64634db 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,7 +12,7 @@ Every WireDoctor property in one place. All properties are optional — WireDoct | Property | Default | Description | |----------|---------|-------------| | `wiredoctor.enabled` | `true` | Master switch. `false` completely disables the analyzer — no analysis, no reports, no bean-structure exposure. Set this in `application-prod.properties` if the dependency ships to production. | -| `wiredoctor.output-path` | project root | Directory where `wiredoctor-report.json` and `wiredoctor-report.html` are written. | +| `wiredoctor.output-path` | project root | Directory where `wiredoctor-report.json` and `wiredoctor-report.html` are written. Set it to `target` (or `build`) if your build lints the source tree — see the note below. | | `wiredoctor.scan-packages` | *(auto)* | Comma-separated package prefixes to analyze for orphan beans. By default, framework packages (`org.springframework`, `java.`, `org.apache`, …) are filtered out automatically. | | `wiredoctor.slow-bean-threshold-ms` | `100` | Beans taking longer than this to instantiate are flagged as slow (report + console). | | `wiredoctor.max-graph-nodes` | `2000` | Above this many beans, the *serialized* graph (JSON + HTML view) is capped to top-N by fan-in (cycle members always kept) so the browser doesn't freeze. Analysis itself — cycles, smells, critical path, baseline diff — always runs on the full graph. `0` = unlimited. | @@ -20,11 +20,47 @@ Every WireDoctor property in one place. All properties are optional — WireDoct ```properties wiredoctor.scan-packages=com.yourcompany.app,io.yourteam.service -wiredoctor.output-path=/path/to/your/reports +wiredoctor.output-path=target wiredoctor.slow-bean-threshold-ms=50 wiredoctor.max-graph-nodes=2000 ``` +### Set `output-path` if your build lints the source tree + +By default the report lands in the project root, where source-tree linters will +find it. The HTML embeds vis.js and egjs, whose license headers contain `http://` +URLs, so any project using Spring's `nohttp-checkstyle` — that is, every Spring +project and every build that inherited Spring's parent — fails its **next** build +on a file nobody wrote: + +``` +[ERROR] wiredoctor-report.html:[17,43] (extension) NoHttp: http:// URLs are not + allowed but got 'http://almende.com'. Use https:// instead. +[ERROR] Failed to execute goal maven-checkstyle-plugin:check + (nohttp-checkstyle-validation): You have 11 Checkstyle violations. +``` + +```properties +wiredoctor.output-path=target +``` + +`target/` is already outside the lint scope and already ignored by git, so this +also keeps the report out of commits and diffs. + +### `scan-packages` also cleans the smell rankings + +Naming your own packages does more than filter orphan beans: without it, +WireDoctor's own beans are ranked in your architecture report +(`com.wiredoctor.WireDoctorAutoConfiguration` as a coupling hotspot, +`wireDoctorAnalyzer` as unstable), alongside framework beans you cannot refactor. + +```properties +wiredoctor.scan-packages=com.yourcompany.app +``` + +With that set, every ranked bean is one you own. It is the single highest-leverage +property for first-run signal quality. + ## Regression Guard & Gates (opt-in — CI only) | Property | Default | Description | diff --git a/wiredoctor-autoconfigure/src/main/java/com/wiredoctor/WireDoctorAnalyzer.java b/wiredoctor-autoconfigure/src/main/java/com/wiredoctor/WireDoctorAnalyzer.java index 060509d..3626edb 100644 --- a/wiredoctor-autoconfigure/src/main/java/com/wiredoctor/WireDoctorAnalyzer.java +++ b/wiredoctor-autoconfigure/src/main/java/com/wiredoctor/WireDoctorAnalyzer.java @@ -918,17 +918,17 @@ private WireDoctorRegressionException runRegressionGuard(Map g // v0.7.0: timing diff summary — only when both sides carried timing data if (diff.hasStartupTimeRegression()) { WireDoctorBaselineDiff.StartupTimeRegression regression = diff.startupTimeRegression(); - log.info("⏱ Startup Time: {}ms -> ms ({:+d}ms, {:.1f}% {})", + log.info("⏱ Startup Time: {}ms -> {}ms ({}ms, {}% {})", regression.baselineMs(), regression.currentMs(), - regression.deltaMs(), - Math.abs(regression.percentChange() * 100), + signed(regression.deltaMs()), + oneDecimal(Math.abs(regression.percentChange() * 100)), regression.deltaMs() >= 0 ? "slower" : "faster"); } else if (baseline.timing() != null && totalStartupMs != null) { // Both sides have timing, but no regression (faster or within noise) long baselineMs = baseline.timing().totalStartupMs(); long delta = totalStartupMs - baselineMs; - log.info("⏱ Startup Time: {}ms -> {}ms ({:+d}ms)", baselineMs, totalStartupMs, delta); + log.info("⏱ Startup Time: {}ms -> {}ms ({}ms)", baselineMs, totalStartupMs, signed(delta)); } else if (baseline.timing() == null && totalStartupMs != null) { log.info("⏱ Startup Time: {}ms (baseline predates timing tracking, diff skipped)", totalStartupMs); } @@ -1027,10 +1027,10 @@ private WireDoctorRegressionException runRegressionGuard(Map g && regression.percentChange() >= relativeThreshold; if (tripsDual) { log.error("WireDoctor regression gate 'startup-time' tripped: startup time " - + "increased by {}ms ({:.1f}%) vs baseline {} ({}ms -> {}ms). " + + "increased by {}ms ({}%) vs baseline {} ({}ms -> {}ms). " + "Thresholds: >={}ms AND >={}%", regression.deltaMs(), - regression.percentChange() * 100, + oneDecimal(regression.percentChange() * 100), baselineFile.getName(), regression.baselineMs(), regression.currentMs(), @@ -1152,7 +1152,7 @@ private void writeGateStatus(File gateStatusFile, WireDoctorBaselineDiff.StartupTimeRegression regression = diff.startupTimeRegression(); status.append("startupTimeDeltaMs=").append(regression.deltaMs()).append('\n'); status.append("startupTimePercentChange=") - .append(String.format("%.1f", regression.percentChange() * 100)).append('\n'); + .append(oneDecimal(regression.percentChange() * 100)).append('\n'); } if (diff.hasNewSlowBeans()) { status.append("newSlowBeansCount=").append(diff.newSlowBeans().size()).append('\n'); @@ -1171,4 +1171,21 @@ private void writeGateStatus(File gateStatusFile, gateStatusFile.getAbsolutePath(), e.getMessage()); } } + + /** + * SLF4J templates only understand {}; format specifiers such as {@code %+d} or + * {@code {:+d}} are not interpolated, so signed/decimal numbers are pre-formatted + * here and passed as ordinary {} arguments. + *

+ * {@link Locale#ROOT} is deliberate: this output is grepped from CI logs and read + * out of {@code wiredoctor-gate.status}, so the decimal separator must not follow + * the machine's default locale. + */ + static String signed(long value) { + return String.format(Locale.ROOT, "%+d", value); + } + + static String oneDecimal(double value) { + return String.format(Locale.ROOT, "%.1f", value); + } } diff --git a/wiredoctor-autoconfigure/src/test/java/com/wiredoctor/WireDoctorLogFormattingTest.java b/wiredoctor-autoconfigure/src/test/java/com/wiredoctor/WireDoctorLogFormattingTest.java new file mode 100644 index 0000000..4208dd9 --- /dev/null +++ b/wiredoctor-autoconfigure/src/test/java/com/wiredoctor/WireDoctorLogFormattingTest.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2026 Deendayal Kumawat + * + * SPDX-License-Identifier: MIT OR Apache-2.0 + */ +package com.wiredoctor; + +import org.junit.jupiter.api.Test; +import org.slf4j.helpers.MessageFormatter; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards the number formatting used in console output and in the machine-readable + * gate status file. + *

+ * v1.1.0 shipped three log templates containing Python-style specifiers + * ({@code {:+d}}, {@code {:.1f}}). SLF4J only interpolates {@code {}}, so those were + * printed literally and every following argument shifted one slot, producing + * {@code "Startup Time: 6687ms -> ms ({:+d}ms, {:.1f}% 7178)"}. These tests fail if + * that defect class returns. + */ +class WireDoctorLogFormattingTest { + + @Test + void signedRendersAnExplicitSign() { + assertThat(WireDoctorAnalyzer.signed(491)).isEqualTo("+491"); + assertThat(WireDoctorAnalyzer.signed(-491)).isEqualTo("-491"); + assertThat(WireDoctorAnalyzer.signed(0)).isEqualTo("+0"); + } + + @Test + void oneDecimalKeepsADotSeparatorUnderAnyDefaultLocale() { + Locale original = Locale.getDefault(); + try { + // GERMANY formats decimals with a comma; wiredoctor-gate.status is grepped + // by CI, so the separator must not follow the machine's locale. + Locale.setDefault(Locale.GERMANY); + assertThat(WireDoctorAnalyzer.oneDecimal(7.34260505458352)).isEqualTo("7.3"); + assertThat(WireDoctorAnalyzer.oneDecimal(31.1)).isEqualTo("31.1"); + } + finally { + Locale.setDefault(original); + } + } + + @Test + void startupTimeSummaryRendersEveryValue() { + WireDoctorBaselineDiff.StartupTimeRegression regression = + new WireDoctorBaselineDiff.StartupTimeRegression(6687, 7178); + + String rendered = MessageFormatter.arrayFormat( + "⏱ Startup Time: {}ms -> {}ms ({}ms, {}% {})", + new Object[] { + regression.baselineMs(), + regression.currentMs(), + WireDoctorAnalyzer.signed(regression.deltaMs()), + WireDoctorAnalyzer.oneDecimal(Math.abs(regression.percentChange() * 100)), + regression.deltaMs() >= 0 ? "slower" : "faster" }) + .getMessage(); + + assertThat(rendered).isEqualTo("⏱ Startup Time: 6687ms -> 7178ms (+491ms, 7.3% slower)"); + } + + @Test + void noLogTemplateInTheModuleUsesANonSlf4jSpecifier() throws IOException { + Path sources = Path.of("src", "main", "java"); + assertThat(sources).as("module source root — test must not silently pass").isDirectory(); + + // log.info("... {:+d} ...") and friends: a '{' followed by anything but '}'. + Pattern badTemplate = Pattern.compile("log\\.(?:trace|debug|info|warn|error)\\(\\s*\"[^\"]*\\{[^}]"); + List offenders = new ArrayList<>(); + try (Stream files = Files.walk(sources)) { + for (Path file : files.filter(f -> f.toString().endsWith(".java")).toList()) { + Matcher m = badTemplate.matcher(Files.readString(file)); + while (m.find()) { + offenders.add(file.getFileName() + ": " + m.group()); + } + } + } + assertThat(offenders) + .as("SLF4J only interpolates {} — a specifier inside the braces is printed " + + "literally and shifts every later argument") + .isEmpty(); + } +} From 137af0b6019a3c2f003598bc878423883e88dad0 Mon Sep 17 00:00:00 2001 From: ddsha441981 Date: Mon, 24 Aug 2026 13:22:55 +0530 Subject: [PATCH 2/3] docs: changelog entry for the startup-time formatting fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filed under [Unreleased] — the fix sits on main and the docs corrections go live via Pages, but there is no reason to cut a patch release to Central for console formatting alone. It rides along in the next release that has its own reason to exist. --- CHANGELOG.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5257551..3f9a7df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,68 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +Correctness pass on startup-time reporting, found by running the published 1.1.0 +artifact unmodified against spring-petclinic (Spring Boot 4.1.0, Framework 7.0.8, +Java 25.0.2, 470 beans). No API, schema, or configuration changes. + +### Fixed + +- **Startup-time numbers rendered as literal format specifiers.** Three log + templates used `{:+d}` / `{:.1f}`. SLF4J interpolates only `{}`, so the + specifier was printed verbatim and every following argument shifted by one + slot — the summary line read + `⏱ Startup Time: 6687ms -> ms ({:+d}ms, {:.1f}% 7178)` instead of + `⏱ Startup Time: 6687ms -> 7418ms (+731ms, 10.9% slower)`. Affected the diff + summary, its no-regression variant, and the `startup-time` gate's `log.error` + — the last being the line a CI reader lands on first. Numbers are now + pre-formatted through `signed(long)` / `oneDecimal(double)` and passed as plain + `{}` arguments. The `WireDoctorRegressionException` message was already + correct and is unchanged. +- **`wiredoctor-gate.status` percent change is now locale-independent.** + `startupTimePercentChange` was written with a default-locale + `String.format("%.1f", …)`, so a build machine set to a comma-decimal locale + emitted `4,3`. It now goes through `oneDecimal(…)`, which pins `Locale.ROOT` — + the file is machine-read, so its decimal separator must not follow the host. + +### Documentation + +- **`ci-gating.md` no longer prescribes a baseline/gate mismatch.** Step 1 said + to record the baseline with `./mvnw spring-boot:run` while Step 3 gates on + `java -jar`. `spring-boot-devtools` is on the classpath for the former and + excluded from the repackaged jar by `spring-boot-maven-plugin` for the latter; + on spring-petclinic that single difference is 12 removed beans and a 31% + startup-time delta — enough to trip the `startup-time` gate on a build where + nobody changed a line of code. Step 1 now records from the jar, with a callout + on keeping active profiles and `spring.main.web-application-type` identical + across both runs. +- **`ci-gating.md` warns that a Maven-goal gate cannot fail a build.** With + devtools present, `spring-boot:run` launches `main` on its own restart thread: + `WireDoctorRegressionException` is logged, that thread dies, and Maven still + prints `BUILD SUCCESS` and exits `0`. Documents booting the jar for a real exit + code, and `grep -q '^FAIL' target/wiredoctor-gate.status && exit 1` as the + fallback for builds pinned to a Maven goal. +- **`configuration.md`**: documents `wiredoctor.output-path` for projects whose + build lints the source tree — `nohttp` (a Spring build convention) rejects the + `http://` license headers vendored inside the self-contained report, failing + the *next* build after a WireDoctor run — and notes that + `wiredoctor.scan-packages` also keeps WireDoctor's own beans out of the smell + rankings. +- **`_config.yml`** declares the `warning` callout the docs reference; Just the + Docs renders `{: .warning }` unstyled unless it is declared. + +### Tests + +- 257 tests green — 246 autoconfigure + 11 actuator (+4 new: `signed` sign + rendering, `oneDecimal` separator stability under a comma-decimal default + locale, full startup-time summary rendering asserted through + `MessageFormatter.arrayFormat(…)`, and a guard that scans every `log.*` + template in the module for a non-`{}` specifier so the whole defect class stays + closed). + +--- + ## [1.1.0] - 2026-08-22 **Longitudinal Visibility**: two focused features extending the existing baseline From e3f15c4e985835bde110576fb63ce302f39b07f9 Mon Sep 17 00:00:00 2001 From: ddsha441981 Date: Mon, 24 Aug 2026 13:29:45 +0530 Subject: [PATCH 3/3] chore: bump to 1.1.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parent version bumped in all three child poms, not just the root — a partial bump is what broke CI on the v0.7.0 release. Install snippets in README.md, docs/index.md and docs/why-wiredoctor.md stay on 1.1.0 deliberately: 1.1.1 is a source tag, and the docs must only ever point at a coordinate that actually resolves from Maven Central. --- CHANGELOG.md | 2 +- pom.xml | 2 +- wiredoctor-actuator/pom.xml | 2 +- wiredoctor-autoconfigure/pom.xml | 2 +- wiredoctor-test/pom.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f9a7df..519af50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [1.1.1] - 2026-08-24 Correctness pass on startup-time reporting, found by running the published 1.1.0 artifact unmodified against spring-petclinic (Spring Boot 4.1.0, Framework 7.0.8, diff --git a/pom.xml b/pom.xml index e02a5cd..a8febdb 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ 4.0.0 io.github.ddsha441981 wiredoctor-parent - 1.1.0 + 1.1.1 pom WireDoctor diff --git a/wiredoctor-actuator/pom.xml b/wiredoctor-actuator/pom.xml index 705f979..8a75ee5 100644 --- a/wiredoctor-actuator/pom.xml +++ b/wiredoctor-actuator/pom.xml @@ -6,7 +6,7 @@ io.github.ddsha441981 wiredoctor-parent - 1.1.0 + 1.1.1 wiredoctor-actuator WireDoctor Actuator diff --git a/wiredoctor-autoconfigure/pom.xml b/wiredoctor-autoconfigure/pom.xml index 08e7839..b5b0963 100644 --- a/wiredoctor-autoconfigure/pom.xml +++ b/wiredoctor-autoconfigure/pom.xml @@ -6,7 +6,7 @@ io.github.ddsha441981 wiredoctor-parent - 1.1.0 + 1.1.1 wiredoctor-autoconfigure WireDoctor AutoConfiguration diff --git a/wiredoctor-test/pom.xml b/wiredoctor-test/pom.xml index a0cc104..ff7afb7 100644 --- a/wiredoctor-test/pom.xml +++ b/wiredoctor-test/pom.xml @@ -6,7 +6,7 @@ io.github.ddsha441981 wiredoctor-parent - 1.1.0 + 1.1.1 wiredoctor-test