Skip to content

Commit 1aa952b

Browse files
rpanackalrpanackal
committed
Add review suggestions
1 parent 6720c2e commit 1aa952b

File tree

6 files changed

+183
-189
lines changed

6 files changed

+183
-189
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import com.fasterxml.jackson.core.JsonProcessingException;
66
import com.fasterxml.jackson.databind.ObjectMapper;
77
import com.google.common.annotations.Beta;
8-
import com.sap.ai.sdk.core.common.MdcHelper.RequestContext;
98
import io.vavr.control.Try;
109
import java.nio.charset.StandardCharsets;
1110
import java.util.Optional;
@@ -69,6 +68,7 @@ public ClientResponseHandler<T, R, E> objectMapper(@Nonnull final ObjectMapper j
6968
@Override
7069
public T handleResponse(@Nonnull final ClassicHttpResponse response) throws E {
7170
if (response.getCode() >= 300) {
71+
7272
buildAndThrowException(response);
7373
}
7474
return parseSuccess(response);
@@ -87,7 +87,7 @@ private T parseSuccess(@Nonnull final ClassicHttpResponse response) throws E {
8787
val content =
8888
tryGetContent(responseEntity)
8989
.getOrElseThrow(e -> exceptionFactory.build(message, e).setHttpResponse(response));
90-
RequestContext.logResponseSuccess(response);
90+
RequestLogContext.logResponseSuccess(response);
9191

9292
try {
9393
return objectMapper.readValue(content, successType);

‎core/src/main/java/com/sap/ai/sdk/core/common/MdcHelper.java‎

Lines changed: 0 additions & 145 deletions
This file was deleted.

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

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
package com.sap.ai.sdk.core.common;
2+
3+
import com.google.common.annotations.Beta;
4+
import java.util.Optional;
5+
import java.util.UUID;
6+
import javax.annotation.Nonnull;
7+
import lombok.Getter;
8+
import lombok.RequiredArgsConstructor;
9+
import lombok.experimental.UtilityClass;
10+
import lombok.extern.slf4j.Slf4j;
11+
import lombok.val;
12+
import org.apache.hc.core5.http.ClassicHttpResponse;
13+
import org.apache.hc.core5.http.HttpEntity;
14+
import org.slf4j.MDC;
15+
16+
/**
17+
* Utility for managing MDC (Mapped Diagnostic Context) for logging of AI Core requests.
18+
*
19+
* <p>This class is intended for internal use only.
20+
*/
21+
@Slf4j
22+
@UtilityClass
23+
@Beta
24+
public class RequestLogContext {
25+
26+
private static void setRequestId(@Nonnull final String requestId) {
27+
MDC.put(MdcKeys.REQUEST_ID, requestId);
28+
}
29+
30+
/**
31+
* Set the endpoint for the current request context.
32+
*
33+
* @param endpoint the endpoint URL
34+
*/
35+
public static void setEndpoint(@Nonnull final String endpoint) {
36+
MDC.put(MdcKeys.ENDPOINT, endpoint);
37+
}
38+
39+
/**
40+
* Set the destination for the current request context.
41+
*
42+
* @param destination the destination name
43+
*/
44+
public static void setDestination(@Nonnull final String destination) {
45+
MDC.put(MdcKeys.DESTINATION, destination);
46+
}
47+
48+
/**
49+
* Set the mode for the current request context.
50+
*
51+
* @param mode the request mode
52+
*/
53+
public static void setMode(@Nonnull final Mode mode) {
54+
MDC.put(MdcKeys.MODE, mode.getValue());
55+
}
56+
57+
/**
58+
* Set the service for the current request context.
59+
*
60+
* @param service the service type
61+
*/
62+
public static void setService(@Nonnull final Service service) {
63+
MDC.put(MdcKeys.SERVICE, service.getValue());
64+
}
65+
66+
/** Clear all MDC request context information. */
67+
public static void clear() {
68+
MDC.remove(MdcKeys.REQUEST_ID);
69+
MDC.remove(MdcKeys.ENDPOINT);
70+
MDC.remove(MdcKeys.DESTINATION);
71+
MDC.remove(MdcKeys.MODE);
72+
MDC.remove(MdcKeys.SERVICE);
73+
}
74+
75+
/** Log the start of a request with generated request ID. */
76+
public static void logRequestStart() {
77+
val reqId = UUID.randomUUID().toString().substring(0, 8);
78+
RequestLogContext.setRequestId(reqId);
79+
80+
val message = "[reqId={}] Starting {} {} request to {}, destination={}";
81+
log.debug(
82+
message,
83+
reqId,
84+
MDC.get(MdcKeys.SERVICE),
85+
MDC.get(MdcKeys.MODE),
86+
MDC.get(MdcKeys.ENDPOINT),
87+
MDC.get(MdcKeys.DESTINATION));
88+
}
89+
90+
/**
91+
* Log successful response with duration and size information.
92+
*
93+
* @param response the HTTP response
94+
*/
95+
public static void logResponseSuccess(@Nonnull final ClassicHttpResponse response) {
96+
if (!log.isDebugEnabled()) {
97+
return;
98+
}
99+
100+
val headerTime = Optional.ofNullable(response.getFirstHeader("x-upstream-service-time"));
101+
val duration = headerTime.map(h -> h.getValue() + "ms").orElse("unknown");
102+
val sizeInfo =
103+
Optional.ofNullable(response.getEntity())
104+
.map(HttpEntity::getContentLength)
105+
.filter(length -> length >= 0)
106+
.map(length -> "%.1fKB".formatted(length / 1024.0))
107+
.orElse("unknown");
108+
val message = "[reqId={}] {} request completed successfully with duration={}, size={}.";
109+
log.debug(message, MDC.get(MdcKeys.REQUEST_ID), MDC.get(MdcKeys.SERVICE), duration, sizeInfo);
110+
}
111+
112+
@UtilityClass
113+
private static class MdcKeys {
114+
private static final String REQUEST_ID = "reqId";
115+
private static final String ENDPOINT = "endpoint";
116+
private static final String DESTINATION = "destination";
117+
private static final String MODE = "mode";
118+
private static final String SERVICE = "service";
119+
}
120+
121+
/** Request execution modes. */
122+
@RequiredArgsConstructor
123+
public enum Mode {
124+
/** Synchronous request mode */
125+
SYNCHRONOUS("synchronous"),
126+
/** Streaming request mode */
127+
STREAMING("streaming");
128+
@Getter private final String value;
129+
}
130+
131+
/** AI service types. */
132+
@RequiredArgsConstructor
133+
public enum Service {
134+
/** OpenAI service */
135+
OPENAI("openai"),
136+
/** Orchestration service */
137+
ORCHESTRATION("orchestration");
138+
@Getter private final String value;
139+
}
140+
}

core/src/test/java/com/sap/ai/sdk/core/common/MdcHelperTest.java renamed to core/src/test/java/com/sap/ai/sdk/core/common/RequestLogContextTest.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,33 @@
11
package com.sap.ai.sdk.core.common;
22

3-
import static com.sap.ai.sdk.core.common.MdcHelper.Mode.STREAMING;
4-
import static com.sap.ai.sdk.core.common.MdcHelper.Service.OPENAI;
3+
import static com.sap.ai.sdk.core.common.RequestLogContext.Mode.STREAMING;
4+
import static com.sap.ai.sdk.core.common.RequestLogContext.Service.OPENAI;
55
import static org.assertj.core.api.Assertions.assertThat;
66

7-
import com.sap.ai.sdk.core.common.MdcHelper.RequestContext;
87
import org.junit.jupiter.api.Test;
98
import org.slf4j.MDC;
109

11-
class MdcHelperTest {
10+
class RequestLogContextTest {
1211

1312
@Test
14-
void testRequestContextLifecycle() {
13+
void testRequestLogContextLifecycle() {
1514
// Setup customer MDC entries (to test clear() safety)
1615
MDC.put("consumer-key", "consumer-value");
1716

18-
RequestContext.setService(OPENAI);
19-
RequestContext.setMode(STREAMING);
20-
RequestContext.setEndpoint("/api/endpoint");
21-
RequestContext.setDestination("http://localhost:8000");
17+
RequestLogContext.setService(OPENAI);
18+
RequestLogContext.setMode(STREAMING);
19+
RequestLogContext.setEndpoint("/api/endpoint");
20+
RequestLogContext.setDestination("http://localhost:8000");
2221

2322
assertThat(MDC.get("service")).isEqualTo("openai");
2423
assertThat(MDC.get("mode")).isEqualTo("streaming");
2524
assertThat(MDC.get("endpoint")).isEqualTo("/api/endpoint");
2625
assertThat(MDC.get("destination")).isEqualTo("http://localhost:8000");
2726

28-
RequestContext.logRequestStart();
27+
RequestLogContext.logRequestStart();
2928
assertThat(MDC.get("reqId")).isNotNull().hasSize(8);
3029

31-
RequestContext.clear();
30+
RequestLogContext.clear();
3231
assertThat(MDC.get("service")).isNull();
3332
assertThat(MDC.get("mode")).isNull();
3433
assertThat(MDC.get("endpoint")).isNull();

0 commit comments

Comments
 (0)