adityamparikh opened a new pull request, #193:
URL: https://github.com/apache/solr-mcp/pull/193
## Problem
After #189 the logging setup had three coupled pieces and a test whose only
job was to keep them coupled:
| Piece | Job |
|---|---|
| `logback.xml` | a `NopStatusListener` and nothing else, so logback's *own*
pre-Spring init stays off stdout in the native image |
| `logback-spring.xml` | the real configuration, with the `<springProfile>`
appenders |
| `logging.config=classpath:logback-spring.xml` in `application.properties`
| steer Boot past `logback.xml`, which `initializeWithConventions()` would
otherwise stop at, never loading the `-spring` variant |
That is not how Boot expects to be configured. The reference documentation
asks for **one** file, resolved by convention, and reserves `logging.config`
for pointing at an *external* file. Two classpath files disambiguated by a
property is a workaround, and `LoggingConfigurationTest` existed to stop anyone
from "cleaning it up" — which #104 had in fact done in a stray commit that
would now break native STDIO.
## Change
The only thing `logback.xml` did was install a status listener before Boot
exists. Logback has a first-class knob for exactly that, the
`logback.statusListenerClass` system property, and I verified the ordering in
logback 1.5.32 (Boot 3.5.14's BOM): `ContextInitializer.autoConfig()` runs
`checkVersions()` (the source of the native-only `|-WARN`), then
`StatusListenerConfigHelper.installIfAsked()` (reads the property), and
`LogbackServiceProvider` calls `StatusPrinter.printInCaseOfErrorsOrWarnings()`
only when `StatusUtil.contextHasStatusListener()` is false. So the listener
does not need to exist before the WARN, only before the print, and the property
is read in between.
```
main() sets logback.statusListenerClass (unless
already set)
└─ SpringApplication.<clinit> first LoggerFactory touch
└─ ContextInitializer checkVersions() → |-WARN (native only)
installIfAsked() → NopStatusListener ←
property read here
BasicConfigurator (no logback.xml)
└─ LogbackServiceProvider contextHasStatusListener() == true →
nothing printed
└─ LoggingApplicationListener logback-spring.xml by convention →
<springProfile> appenders
```
- `Main.main()` sets the property as its first statement. An operator's
explicit `-Dlogback.statusListenerClass=...OnConsoleStatusListener` wins, so
logback itself can still be debugged.
- `logback.xml` deleted.
- `logging.config` removed from `application.properties`. `LOGGING_CONFIG`
in the environment still works as Boot's normal override for an external file.
- `SolrNativeHints` no longer registers `logback.xml`; the
`logback-spring.xml` hint stays as belt-and-braces for the non-AOT path.
- `LoggingConfigurationTest` now pins the *convention* instead of the
coupling: no standard-location logback file on the classpath, no
`logging.config` in `application.properties`, and `Main` installs a listener
that `StatusListenerConfigHelper` honours. Written first and watched fail on
`main` before the change.
- `AGENTS.md`, `dev-docs/graalvm-native-image.md`, `docs/security/stdio.md`
and the `keycloak.md` troubleshooting section (which still described the
pre-#189 "no HTTP logs" state) updated.
`logging.pattern.console=` in `application-stdio.properties` is untouched:
it is the idiom Spring AI documents for STDIO servers and does not depend on
any of the above.
## Verification
| Check | Result |
|---|---|
| `LoggingConfigurationTest` on `main` before the change | 2 of 5 fail
(`logback.xml` present, `logging.config` set), compile error for the missing
`Main` API |
| `./gradlew build` (JDK 25) | **403 tests, 0 failures, 7 skipped**;
`McpClientStdioIntegrationTest` (JVM `java -jar` over MCP STDIO) 40/40 |
| `./gradlew dockerIntegrationTest -Pnative` (native STDIO image driven over
MCP STDIO) | running — result will be posted in a comment |
## Related
- Supersedes the `logback.xml` removal commit (`777168e8`) carried by #104;
that PR should drop the commit on its next rebase, since with this change there
is nothing left to remove.
- #189 introduced the coupling this replaces; the native-only failure it
fixed is covered by the same Docker test.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
