Review suggestions

- adding . at the end
- reqId explanation
- 'size' renaming in log for clarity
- MDc examplee
- "object creation" with example instead of vague computaiton
- remove mention of "placeholder" warnings

Author: rpanackal

core/src/main/java/com/sap/ai/sdk/core/AiCoreServiceKeyAccessor.java

@@ -52,7 +52,7 @@ public List<ServiceBinding> getServiceBindings() throws ServiceBindingAccessExce
       throw new ServiceBindingAccessException("Failed to load service key from environment", e);
     }
     if (serviceKey == null) {
-      log.debug("No service key found in environment variable {}", ENV_VAR_KEY);
+      log.debug("No service key found in environment variable {}.", ENV_VAR_KEY);
       return List.of();
     }
     final String logMessage =

core/src/main/java/com/sap/ai/sdk/core/DeploymentResolver.java

@@ -46,7 +46,7 @@ void reloadDeployments(@Nonnull final String resourceGroup) {
     try {
       val apiClient = new DeploymentApi(service);
       val deployments = new HashSet<>(apiClient.query(resourceGroup).getResources());
-      log.info("Found {} deployments in resource group '{}'", deployments.size(), resourceGroup);
+      log.info("Found {} deployments in resource group '{}'.", deployments.size(), resourceGroup);
       cache.put(resourceGroup, deployments);
     } catch (final RuntimeException e) {
       throw new DeploymentResolutionException(

core/src/main/java/com/sap/ai/sdk/core/common/ClientResponseHandler.java

@@ -91,7 +91,7 @@ private T parseSuccess(@Nonnull final ClassicHttpResponse response) throws E {
     try {
       return objectMapper.readValue(content, successType);
     } catch (final JsonProcessingException e) {
-      log.error("Failed to parse response to type {}", successType);
+      log.error("Failed to parse response to type {}.", successType);
       throw exceptionFactory.build("Failed to parse response", e).setHttpResponse(response);
     }
   }

core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java

@@ -85,7 +85,7 @@ public Stream<D> handleStreamingResponse(@Nonnull final ClassicHttpResponse resp
                 }
                 return objectMapper.treeToValue(jsonNode, successType);
               } catch (final IOException e) {
-                log.error("Failed to parse delta chunk to type {}", successType);
+                log.error("Failed to parse delta chunk to type {}.", successType);
                 final String message = "Failed to parse delta chunk";
                 throw exceptionFactory.build(message, e).setHttpResponse(response);
               }

core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java

@@ -35,8 +35,8 @@ class IterableStreamConverter<T> implements Iterator<T> {
   static final int BUFFER_SIZE = 8192;
 
   private static final String ERR_CONTENT = "Failed to read response content.";
-  private static final String ERR_INTERRUPTED = "Parsing response content was interrupted";
-  private static final String ERR_CLOSE = "Could not close input stream with error: {} (ignored)";
+  private static final String ERR_INTERRUPTED = "Parsing response content was interrupted.";
+  private static final String ERR_CLOSE = "Could not close input stream with error: {} (ignored).";
 
   /** Read next entry for Stream or {@code null} when no further entry can be read. */
   private final Callable<T> readHandler;
@@ -70,7 +70,7 @@ public boolean hasNext() {
     } catch (final Exception e) {
       isDone = true;
       stopHandler.run();
-      log.debug("Reading next element failed with error {})", e.getClass().getSimpleName());
+      log.debug("Reading next element failed with error {}.", e.getClass().getSimpleName());
       throw errorHandler.apply(e);
     }
     return !isDone;

core/src/main/java/com/sap/ai/sdk/core/common/RequestLogContext.java

@@ -77,7 +77,7 @@ public static void logRequestStart() {
     val reqId = UUID.randomUUID().toString().substring(0, 8);
     RequestLogContext.setRequestId(reqId);
 
-    val message = "[reqId={}] Starting {} {} request to {}, destination={}";
+    val message = "[reqId={}] Starting {} {} request to {}, destination={}.";
     log.debug(
         message,
         reqId,

docs/adrs/006-logging-strategy.md

@@ -38,10 +38,12 @@ This approach mandates descriptive, human-readable logs with structured request
   Logs must be clear enough for a developer to understand what happened without checking the code.
   Use the `metric=value` pattern to include structured details with extensibility in mind.
 
-  ```[reqId=e3eaa45c] OpenAI request completed successfully with duration=1628ms, size=1,2KB.```
+  ```[reqId=e3eaa45c] OpenAI request completed successfully with duration=1628ms, responseSize=1.2KB.```
 
 * **Correlate logs.**
   Include a request identifier (e.g., `reqId`) in per-request logs to assist with correlation and debugging.
+  In this SDK, a "request" represents a single AI service operation (e.g., one chat completion call, one embedding generation).
+  The `reqId` is generated for each AI service call and is distinct from any HTTP request tracking in user's application framework.
 
 * **Exception logging.**
   When logging exceptions, use standard logging methods (e.g., `log.error("Operation failed", exception)`) rather than serializing exception objects.
@@ -59,16 +61,42 @@ This approach mandates descriptive, human-readable logs with structured request
 
 * **Avoid unnecessary warnings.**
   Use the WARN level only for actionable or genuinely concerning conditions.
-  Do not use it as a placeholder or for expected transient states.
+  Expected transient states (retries, fallbacks, cache misses) should not generate a warning.
 
 * **Explicit request logging.**
   Always log at **request start** to provide immediate visibility that an operation has begun.
   This helps users understand that their request is being processed even before a result is available.
   Do not rely solely on response-time logging — requests may fail, hang, or take long durations.
   This approach also avoids the need for stack-trace investigation when surface error responses are ambiguous.
 
+  ```[reqId=e3eaa45c] Starting OpenAI synchronous request to /v1/chat/completions, destination=<some-uri>.```
+
 * **Performance-aware logging.**
-  If a log statement requires expensive computation, guard it with a log-level check (e.g., `if (log.isDebugEnabled())`) to prevent performance degradation.
+  If a log statement requires any object creation, guard it with a log-level check to prevent performance degradation.
+
+  ``` java
+  Optional<Destination> maybeDestination;
+  Destination fallbackDestination = /* existing instance */;
+  
+  // Bad: Creates objects or invokes methods even when logging is disabled
+  log.debug("Destination: {}", maybeDestination.toString());
+  log.debug("Destination: {}", maybeDestination.orElseGet(() -> new Destination()));
+  log.debug("Destination: {}", maybeDestination.orElseGet(this::getFallback));
+  log.debug("Destination: {}", maybeDestination.orElse(getFallback()));
+  
+  // Good: No object creation or method invocation
+  log.debug("Destination: {}", maybeDestination);
+  log.debug("Destination: {}", maybeDestination.orElse(fallbackDestination));
+  
+  // Good: Guard object creation
+  if (log.isDebugEnabled()) {
+    log.debug("Destination: {}", maybeDestination.orElse(getFallback()));
+  }
+  
+  // Exception: Singletons require no guarding (no object creation)
+  Optional<List<Destination>> maybeDestinations;
+  log.debug("Destinations: {}", maybeDestinations.orElse(Collections.emptyList()));
+  ```
 
 ---
 
@@ -82,13 +110,35 @@ This approach mandates descriptive, human-readable logs with structured request
   Per-request MDC context must be cleared when the response completes.
   Avoid setting per-request values in long-lived objects that outlive the request lifecycle, as this can result in corrupted or incomplete log context.
 
-* **Granular clearing only.**
-  Never clear the entire MDC context.
-  Instead, remove entries key-by-key to preserve unrelated context items that may remain valid for longer periods.
-
 * **Centralized MDC management.**
   Avoid using magic strings for MDC keys or values.
   Define them in a dedicated structure or utility (e.g., `RequestLogContext` class) to ensure discoverability and prevent errors during refactoring.
+  ```java
+  // Bad: Magic strings scattered in code
+  MDC.put("service", "OpenAI");
+  log.debug("Service {}", MDC.get("service"));
+  
+  // Good: Centralized in utility class
+  RequestLogContext.setService(Service.OPENAI);
+  log.debug("Service {}", RequestLogContext.get(MdcKeys.SERVICE));
+  ```
+
+* **Granular clearing only.**
+  Never clear the entire MDC context.
+  Instead, remove entries key-by-key to preserve unrelated context items that may remain valid for longer periods.
+  ```java
+  // Bad: Risk clearing useful context entries from other components
+  MDC.clear();
+
+  // Good: Clear only your own entries, centralized in utility class
+  class RequestLogContext { 
+     //...
+     static void clear(){
+      MDC.remove(MdcKeys.REQUEST_ID);
+      MDC.remove(MdcKeys.SERVICE);
+    }
+  }
+  ```
 
 * **Responsibility and ownership.**
   The component or class that sets MDC context values is also responsible for clearing them.

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionResponse.java

@@ -116,7 +116,7 @@ public List<OpenAiToolMessage> executeTools() {
     final var tools = Optional.ofNullable(originalRequest.getToolsExecutable()).orElseGet(List::of);
     if (log.isDebugEnabled() && !tools.isEmpty()) {
       final var toolNames = tools.stream().map(OpenAiTool::getName).toList();
-      log.debug("Executing {} tool call(s) - {}", toolNames.size(), toolNames);
+      log.debug("Executing {} tool call(s) - {}.", toolNames.size(), toolNames);
     }
     return OpenAiTool.execute(tools, getMessage());
   }

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiTool.java

@@ -193,7 +193,7 @@ private static Map<OpenAiFunctionCall, Object> executeInternal(
       if (toolCall instanceof OpenAiFunctionCall functionCall) {
         final var tool = toolMap.get(functionCall.getName());
         if (tool == null) {
-          log.warn("Tool not found for function call: {}", functionCall.getName());
+          log.warn("Tool not found for function call: {}.", functionCall.getName());
           continue;
         }
         final var toolResult = executeFunction(tool, functionCall);

foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/spring/OpenAiChatModel.java

@@ -71,7 +71,7 @@ public ChatResponse call(@Nonnull final Prompt prompt) {
     if (options != null && isInternalToolExecutionEnabled(options) && response.hasToolCalls()) {
       val toolCalls =
           response.getResult().getOutput().getToolCalls().stream().map(ToolCall::name).toList();
-      log.info("Executing {} tool call(s) - {}", toolCalls.size(), toolCalls);
+      log.info("Executing {} tool call(s) - {}.", toolCalls.size(), toolCalls);
       val toolExecutionResult = toolCallingManager.executeToolCalls(prompt, response);
       // Send the tool execution result back to the model.
       log.debug("Re-invoking model with tool execution results.");
@@ -219,7 +219,7 @@ private static List<ChatCompletionTool> extractTools(final ToolCallingChatOption
         val tool = new ChatCompletionTool().type(toolType).function(toolFunction);
         tools.add(tool);
       } catch (JsonProcessingException e) {
-        log.warn("Failed to add tool to the chat request: {}", e.getMessage());
+        log.warn("Failed to add tool to the chat request: {}.", e.getMessage());
       }
     }
     return tools;