You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "adityamparikh (via GitHub)" <gi...@apache.org> on 2026/04/24 21:02:58 UTC
[PR] feat: add SLF4J logging to all service classes [solr-mcp]
adityamparikh opened a new pull request, #110:
URL: https://github.com/apache/solr-mcp/pull/110
## Summary
- Add SLF4J loggers to `CollectionService`, `IndexingService`, `SchemaService`, `SearchService`, and `JsonUtils`
- Log exceptions in all catch blocks instead of silently swallowing them
- Use appropriate log levels: `error` for operational failures, `warn` for recoverable issues, `debug` for expected conditions (Solr 10 metrics unavailability, individual doc failures)
- Safe for STDIO mode: logback-spring.xml already suppresses console logging in the stdio profile
## Test plan
- [x] `./gradlew build` passes
- [x] `./gradlew nativeTest -Pnative` passes (119/119 tests)
- [x] STDIO mode unaffected (logging suppressed by logback config)
- [x] No regressions
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] feat: add SLF4J logging to all service classes [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #110:
URL: https://github.com/apache/solr-mcp/pull/110#issuecomment-5341806477
Rebased onto `main` (`a84033b`). This branch was stale enough that the conflicts mattered — it still carried pre-#176 code, so a naive resolution would have reverted three upstream fixes. Taking `main`'s body and adding only the logging:
- `checkHealth`: kept `Instant.now()` and the 6-arg `SolrHealthStatus`; the branch still had `new Date()` and the old 8-arg form (#113 replaced `Date` with `Instant`). Added the `logger.warn` on top.
- `SchemaService`: kept `toJson(objectMapper, Map.of("error", String.valueOf(e.getMessage())))`; the branch still had the string-concatenated `"{\"error\": \"" + e.getMessage()` that #176 replaced because a message containing a quote or newline emitted invalid JSON. Added `logger.error` on top.
- `fetchCacheMetrics`/`fetchHandlerMetrics`: kept `main`'s `@Nullable` return annotations alongside the branch's `collectionName` parameter rename and `logger.debug` calls.
- Import blocks: kept `org.jspecify.annotations.Nullable` next to the new slf4j imports rather than letting one side win.
Net effect is unchanged from the original intent — loggers on the five service classes, exceptions logged rather than silently swallowed — with none of #113/#176 undone.
Verified: `./gradlew build` green on JDK 25.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] feat: add SLF4J logging to all service classes [solr-mcp]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #110:
URL: https://github.com/apache/solr-mcp/pull/110
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] feat: add SLF4J logging to all service classes [solr-mcp]
Posted by "adityamparikh (via GitHub)" <gi...@apache.org>.
adityamparikh commented on PR #110:
URL: https://github.com/apache/solr-mcp/pull/110#issuecomment-5357781388
**Adopted #111's narrowing (506f2bb) so these two PRs no longer collide.**
Both touch the same two catch clauses in `fetchCacheMetrics` / `fetchHandlerMetrics`. This PR was
binding the exception for logging while keeping `RuntimeException`; #111 narrows it to
`SolrException`. Whichever merged second would have conflicted, or silently reverted the other's
intent — the worse outcome, since nothing would look wrong.
This branch now carries the narrowed catch as well, so the end state is the same in either merge
order. If #111 merges first this rebases cleanly; if this merges first, #111 becomes a no-op on
these lines and keeps its `AGENTS.md` correction.
Verified: `RemoteSolrException extends SolrException` in solrj 10.0.0, so Solr 10 still degrades
to `null` where `/admin/mbeans` is gone. Full build green — 372 tests, 0 failures.
Worth noting the two changes reinforce each other: narrowing the catch means unrelated runtime
failures propagate instead of being reported as "metrics unavailable", and the `debug` logging
added here is what makes the genuinely-unavailable case diagnosable rather than silent.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org