This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-mcp.git
The following commit(s) were added to refs/heads/main by this push:
new 1883124 feat: add SLF4J logging to all service classes (#110)
1883124 is described below
commit 1883124446f71984d2ea50fb117044e68f8fb3c8
Author: Aditya Parikh <[email protected]>
AuthorDate: Fri Sep 11 10:29:07 2026 -0400
feat: add SLF4J logging to all service classes (#110)
* 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 <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: adityamparikh <[email protected]>
* refactor(collection): narrow the metrics catch clauses to SolrException
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 <[email protected]>
* refactor(search): log query failures before rethrowing with a hint
SearchService gained a logger in the SLF4J commit but never used it:
#166 landed afterwards and made every SolrException rethrow wrapped in
an IllegalArgumentException carrying a remediation hint, so nothing is
swallowed there any more.
The MCP client sees only the exception message, so the server kept no
record of a failed query at all. Log one debug line in
withRemediationHint covering every path, including the no-hint
fallback. Debug rather than warn because the failure is already
reported to the caller; the stdio profile defines no appenders, so
this cannot reach stdout.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_015rXANeQAujgxhEz1uAWFm9
Signed-off-by: Aditya Parikh <[email protected]>
---------
Signed-off-by: Aditya Parikh <[email protected]>
Signed-off-by: adityamparikh <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
---
.../mcp/server/collection/CollectionService.java | 22 +++++++++++++++-------
.../solr/mcp/server/indexing/IndexingService.java | 15 +++++++++++++--
.../solr/mcp/server/schema/SchemaService.java | 5 +++++
.../solr/mcp/server/search/SearchService.java | 8 ++++++++
.../org/apache/solr/mcp/server/util/JsonUtils.java | 5 +++++
5 files changed, 46 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 011d278..6786082 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,11 +40,14 @@ import
org.apache.solr.client.solrj.response.CollectionAdminResponse;
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;
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 +139,8 @@ import org.springframework.stereotype.Service;
@Observed
public class CollectionService {
+ private static final Logger logger =
LoggerFactory.getLogger(CollectionService.class);
+
// ========================================
// Constants for API Parameters and Paths
// ========================================
@@ -683,16 +688,17 @@ public class CollectionService {
* 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<Object> coreMetrics =
fetchMetrics(collection, CACHE_METRIC_PREFIX);
+ NamedList<Object> 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 | SolrException e) {
+ logger.debug("Cache metrics unavailable for collection:
{}", collectionName, e);
return null;
}
}
@@ -799,18 +805,19 @@ public class CollectionService {
* 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 | SolrException e) {
+ logger.debug("Handler metrics unavailable for
collection: {}", collectionName, e);
return null;
}
}
@@ -1080,6 +1087,7 @@ public class CollectionService {
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 5ac3704..1350496 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.common.SolrInputDocument;
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;
@@ -116,6 +118,8 @@ import org.xml.sax.SAXException;
@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 */
@@ -578,12 +582,14 @@ public class IndexingService {
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
@@ -592,7 +598,12 @@ public class IndexingService {
}
}
- 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 3f3bb96..73bb340 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.FieldTypeDefinition;
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 @@ import org.springframework.stereotype.Service;
@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 class SchemaService {
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 2fb9800..c9dda13 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.SolrException;
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 @@ import org.springframework.util.StringUtils;
@Observed
public class SearchService {
+ private static final Logger logger =
LoggerFactory.getLogger(SearchService.class);
+
/**
* Fragments of Solr's own error text that identify a failure we can
advise on.
*
@@ -370,6 +374,10 @@ public class SearchService {
private static RuntimeException withRemediationHint(SolrException e,
String collection) {
final String message = String.valueOf(e.getMessage());
+ // The MCP client only ever sees the exception message, so
without this the
+ // server keeps no record of a failed query.
+ logger.debug("Solr query failed on collection {}", collection,
e);
+
// An unknown collection is a 404 whose body is Solr's HTML
"not found" page,
// so SolrJ reports it as a mime-type mismatch and leaves
getMetadata() null.
// The status code is the only signal that survives; match it
rather than the
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 6ecc3bc..44c36a8 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 @@ package org.apache.solr.mcp.server.util;
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 @@ import com.fasterxml.jackson.databind.ObjectMapper;
*/
public final class JsonUtils {
+ private static final Logger logger =
LoggerFactory.getLogger(JsonUtils.class);
+
private JsonUtils() {
// Utility class - prevent instantiation
}
@@ -52,6 +56,7 @@ public final class JsonUtils {
try {
return objectMapper.writeValueAsString(obj);
} catch (JsonProcessingException e) {
+ logger.error("Failed to serialize response", e);
return "{\"error\": \"Failed to serialize response\"}";
}
}
