From 1464f5696024db5f6fe17b997c5de76be884e46f Mon Sep 17 00:00:00 2001 From: adityamparikh Date: Fri, 24 Apr 2026 14:13:26 -0400 Subject: [PATCH 1/2] feat: add SLF4J logging to all service classes 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. Closes #1 Signed-off-by: Aditya Parikh Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: adityamparikh --- .../server/collection/CollectionService.java | 21 ++++++++++++------- .../mcp/server/indexing/IndexingService.java | 15 +++++++++++-- .../solr/mcp/server/schema/SchemaService.java | 5 +++++ .../solr/mcp/server/search/SearchService.java | 4 ++++ .../solr/mcp/server/util/JsonUtils.java | 5 +++++ 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java index 011d278e..a94b3819 100644 --- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java +++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java @@ -45,6 +45,8 @@ import org.apache.solr.mcp.server.config.SolrConfigurationProperties; import org.apache.solr.mcp.server.util.PromptNames; import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springaicommunity.mcp.annotation.McpArg; import org.springaicommunity.mcp.annotation.McpComplete; import org.springaicommunity.mcp.annotation.McpPrompt; @@ -136,6 +138,8 @@ @Observed public class CollectionService { + private static final Logger logger = LoggerFactory.getLogger(CollectionService.class); + // ======================================== // Constants for API Parameters and Paths // ======================================== @@ -683,16 +687,17 @@ public QueryStats buildQueryStats(QueryResponse response) { * Internal cache metrics fetch that assumes the collection has already been * validated and the name has been extracted from any shard identifier. */ - private @Nullable CacheStats fetchCacheMetrics(String collection) { + private @Nullable CacheStats fetchCacheMetrics(String collectionName) { try { - NamedList coreMetrics = fetchMetrics(collection, CACHE_METRIC_PREFIX); + NamedList coreMetrics = fetchMetrics(collectionName, CACHE_METRIC_PREFIX); if (coreMetrics == null) { return null; } CacheStats stats = extractCacheStats(coreMetrics); return isCacheStatsEmpty(stats) ? null : stats; - } catch (SolrServerException | IOException | RuntimeException _) { + } catch (SolrServerException | IOException | RuntimeException e) { + logger.debug("Cache metrics unavailable for collection: {}", collectionName, e); return null; } } @@ -799,18 +804,19 @@ private CacheStats extractCacheStats(NamedList coreMetrics) { * Internal handler metrics fetch that assumes the collection has already been * validated and the name has been extracted from any shard identifier. */ - private @Nullable HandlerStats fetchHandlerMetrics(String collection) { + private @Nullable HandlerStats fetchHandlerMetrics(String collectionName) { try { // Handler metrics are flat keys (e.g. QUERY./select.requests) so we // fetch each handler prefix separately and reconstruct HandlerInfo - HandlerInfo selectHandler = fetchFlatHandlerInfo(collection, SELECT_HANDLER_METRIC_PREFIX, + HandlerInfo selectHandler = fetchFlatHandlerInfo(collectionName, SELECT_HANDLER_METRIC_PREFIX, SELECT_HANDLER_KEY); - HandlerInfo updateHandler = fetchFlatHandlerInfo(collection, UPDATE_HANDLER_METRIC_PREFIX, + HandlerInfo updateHandler = fetchFlatHandlerInfo(collectionName, UPDATE_HANDLER_METRIC_PREFIX, UPDATE_HANDLER_KEY); HandlerStats stats = new HandlerStats(selectHandler, updateHandler); return isHandlerStatsEmpty(stats) ? null : stats; - } catch (SolrServerException | IOException | RuntimeException _) { + } catch (SolrServerException | IOException | RuntimeException e) { + logger.debug("Handler metrics unavailable for collection: {}", collectionName, e); return null; } } @@ -1080,6 +1086,7 @@ public SolrHealthStatus checkHealth(@McpToolParam(description = "Solr collection statsResponse.getResults().getNumFound(), Instant.now(), actualCollection); } catch (Exception e) { + logger.warn("Health check failed for collection: {}", collection, e); return new SolrHealthStatus(false, e.getMessage(), null, null, Instant.now(), actualCollection); } } diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java index 34674852..1f08076f 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java @@ -29,6 +29,8 @@ import org.apache.solr.mcp.server.indexing.documentcreator.IndexingDocumentCreator; import org.apache.solr.mcp.server.util.PromptNames; import org.apache.solr.mcp.server.util.PromptText; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springaicommunity.mcp.annotation.McpArg; import org.springaicommunity.mcp.annotation.McpPrompt; import org.springaicommunity.mcp.annotation.McpTool; @@ -114,6 +116,8 @@ @Observed public class IndexingService { + private static final Logger logger = LoggerFactory.getLogger(IndexingService.class); + private static final int DEFAULT_BATCH_SIZE = 1000; /** SolrJ client for communicating with Solr server */ @@ -501,12 +505,14 @@ public int indexDocuments(String collection, List documents) solrClient.add(collection, batch); successCount += batch.size(); } catch (SolrServerException | IOException | RuntimeException e) { + logger.warn("Batch indexing failed, retrying individually", e); // Try indexing documents individually to identify problematic ones for (SolrInputDocument doc : batch) { try { solrClient.add(collection, doc); successCount++; - } catch (SolrServerException | IOException | RuntimeException _) { + } catch (SolrServerException | IOException | RuntimeException e2) { + logger.debug("Failed to index individual document", e2); // Document failed to index - this is expected behavior for problematic // documents // We continue processing the rest of the batch @@ -515,7 +521,12 @@ public int indexDocuments(String collection, List documents) } } - solrClient.commit(collection); + try { + solrClient.commit(collection); + } catch (SolrServerException | IOException e) { + logger.error("Failed to commit after indexing to collection: {}", collection, e); + throw e; + } return successCount; } diff --git a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java index 3f3bb96a..73bb3405 100644 --- a/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java +++ b/src/main/java/org/apache/solr/mcp/server/schema/SchemaService.java @@ -33,6 +33,8 @@ import org.apache.solr.client.solrj.request.schema.SchemaRequest; import org.apache.solr.client.solrj.response.schema.SchemaRepresentation; import org.apache.solr.mcp.server.util.PromptNames; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springaicommunity.mcp.annotation.McpArg; import org.springaicommunity.mcp.annotation.McpPrompt; import org.springaicommunity.mcp.annotation.McpResource; @@ -137,6 +139,8 @@ @Observed public class SchemaService { + private static final Logger logger = LoggerFactory.getLogger(SchemaService.class); + /** SolrJ client for communicating with Solr server */ private final SolrClient solrClient; @@ -185,6 +189,7 @@ public String getSchemaResource(String collection) { try { return toJson(objectMapper, getSchema(collection)); } catch (Exception e) { + logger.error("Failed to get schema for collection: {}", collection, e); // Serialise via Jackson rather than concatenating: an exception message // containing a quote, backslash or newline would otherwise emit invalid // JSON to the MCP client. diff --git a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java index cff51681..6307e0d8 100644 --- a/src/main/java/org/apache/solr/mcp/server/search/SearchService.java +++ b/src/main/java/org/apache/solr/mcp/server/search/SearchService.java @@ -34,6 +34,8 @@ import org.apache.solr.common.params.FacetParams; import org.apache.solr.mcp.server.util.PromptNames; import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springaicommunity.mcp.annotation.McpArg; import org.springaicommunity.mcp.annotation.McpPrompt; import org.springaicommunity.mcp.annotation.McpTool; @@ -108,6 +110,8 @@ @Observed public class SearchService { + private static final Logger logger = LoggerFactory.getLogger(SearchService.class); + /** Key for the field name within a sort clause map. */ public static final String SORT_ITEM = "item"; /** diff --git a/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java b/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java index 6ecc3bc1..44c36a8d 100644 --- a/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java +++ b/src/main/java/org/apache/solr/mcp/server/util/JsonUtils.java @@ -18,6 +18,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Utility class for JSON serialization operations. @@ -31,6 +33,8 @@ */ public final class JsonUtils { + private static final Logger logger = LoggerFactory.getLogger(JsonUtils.class); + private JsonUtils() { // Utility class - prevent instantiation } @@ -52,6 +56,7 @@ public static String toJson(ObjectMapper objectMapper, Object obj) { try { return objectMapper.writeValueAsString(obj); } catch (JsonProcessingException e) { + logger.error("Failed to serialize response", e); return "{\"error\": \"Failed to serialize response\"}"; } } From 506f2bba7da79a5f6584bb40afb71d3f189c89fc Mon Sep 17 00:00:00 2001 From: Aditya Parikh Date: Thu, 20 Aug 2026 11:04:23 -0400 Subject: [PATCH 2/2] refactor(collection): narrow the metrics catch clauses to SolrException MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopts the narrowing from #111 so the two PRs compose instead of colliding. Both PRs rewrite the same two catch clauses in fetchCacheMetrics and fetchHandlerMetrics. #111 narrows RuntimeException to SolrException; this PR was binding the exception for logging while leaving RuntimeException in place. Whichever merged second would either conflict or silently revert the other's intent — so this branch now carries the narrowed form too, and the end state is the same in either merge order. RemoteSolrException extends SolrException (verified against solrj 10.0.0), so the Solr 10 path where /admin/mbeans is gone still degrades to null rather than propagating. What no longer gets swallowed is unrelated RuntimeExceptions -- which is the point of #111, and is what the new debug logging is there to surface. Signed-off-by: Aditya Parikh --- .../apache/solr/mcp/server/collection/CollectionService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java index a94b3819..67860822 100644 --- a/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java +++ b/src/main/java/org/apache/solr/mcp/server/collection/CollectionService.java @@ -40,6 +40,7 @@ import org.apache.solr.client.solrj.response.LukeResponse; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.SolrPingResponse; +import org.apache.solr.common.SolrException; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.mcp.server.config.SolrConfigurationProperties; @@ -696,7 +697,7 @@ public QueryStats buildQueryStats(QueryResponse response) { CacheStats stats = extractCacheStats(coreMetrics); return isCacheStatsEmpty(stats) ? null : stats; - } catch (SolrServerException | IOException | RuntimeException e) { + } catch (SolrServerException | IOException | SolrException e) { logger.debug("Cache metrics unavailable for collection: {}", collectionName, e); return null; } @@ -815,7 +816,7 @@ private CacheStats extractCacheStats(NamedList coreMetrics) { HandlerStats stats = new HandlerStats(selectHandler, updateHandler); return isHandlerStatsEmpty(stats) ? null : stats; - } catch (SolrServerException | IOException | RuntimeException e) { + } catch (SolrServerException | IOException | SolrException e) { logger.debug("Handler metrics unavailable for collection: {}", collectionName, e); return null; }