add log response test to interceptors.

zhumin8 · zhumin8 · commit 7901d8a31c5d · 2024-12-11T10:01:40.000-05:00
diff --git a/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcLoggingInterceptor.java b/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcLoggingInterceptor.java
@@ -85,7 +85,7 @@ public void onMessage(RespT message) {
               @Override
               public void onClose(Status status, Metadata trailers) {
                 try {
-                  logResponse(status.getCode().toString(), logDataBuilder);
+                  logResponse(status, logDataBuilder, LOGGER);
                 } finally {
                   logDataBuilder = null; // release resource
                 }
@@ -154,22 +154,22 @@ <RespT> void recordResponsePayload(RespT message, LogData.Builder logDataBuilder
     }
   }
 
-  void logResponse(String statusCode, LogData.Builder logDataBuilder) {
+  void logResponse(Status status, LogData.Builder logDataBuilder, Logger logger) {
     try {
 
-      if (LOGGER.isInfoEnabled()) {
-        logDataBuilder.responseStatus(statusCode);
+      if (logger.isInfoEnabled()) {
+        logDataBuilder.responseStatus(status.getCode().toString());
       }
-      if (LOGGER.isInfoEnabled() && !LOGGER.isDebugEnabled()) {
+      if (logger.isInfoEnabled() && !logger.isDebugEnabled()) {
         Map<String, String> responseData = logDataBuilder.build().toMapResponse();
-        LoggingUtils.logWithMDC(LOGGER, Level.INFO, responseData, "Received Grpc response");
+        LoggingUtils.logWithMDC(logger, Level.INFO, responseData, "Received Grpc response");
       }
-      if (LOGGER.isDebugEnabled()) {
+      if (logger.isDebugEnabled()) {
         Map<String, String> responsedDetailsMap = logDataBuilder.build().toMapResponse();
-        LoggingUtils.logWithMDC(LOGGER, Level.DEBUG, responsedDetailsMap, "Received Grpc response");
+        LoggingUtils.logWithMDC(logger, Level.DEBUG, responsedDetailsMap, "Received Grpc response");
       }
     } catch (Exception e) {
-      LOGGER.error("Error logging request response", e);
+      logger.error("Error logging request response", e);
     }
   }
 
diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcLoggingInterceptorTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcLoggingInterceptorTest.java
@@ -115,7 +115,7 @@ void testInterceptor_responseListener() {
     // --- Verify that the response listener's methods were called ---
     verify(interceptor).recordResponseHeaders(eq(responseHeaders), any(LogData.Builder.class));
     verify(interceptor).recordResponsePayload(any(), any(LogData.Builder.class));
-    verify(interceptor).logResponse(eq(status.getCode().toString()), any(LogData.Builder.class));
+    verify(interceptor).logResponse(eq(status), any(LogData.Builder.class), any(Logger.class));
   }
 
   @Test
@@ -133,6 +133,20 @@ void testLogRequestInfo() {
     testAppender.stop();
   }
 
+  @Test
+  void TestLogResponseInfo() {
+    TestAppender testAppender = setupTestLogger(GrpcLoggingInterceptorTest.class);
+    GrpcLoggingInterceptor interceptor = new GrpcLoggingInterceptor();
+    interceptor.logResponse(Status.CANCELLED, LogData.builder(), LOGGER);
+
+    Assertions.assertEquals(1, testAppender.events.size());
+    assertEquals(Level.INFO, testAppender.events.get(0).getLevel());
+    assertEquals(
+        "{\"response.status\":\"CANCELLED\",\"message\":\"Received Grpc response\"}",
+        testAppender.events.get(0).getMessage());
+    testAppender.stop();
+  }
+
   private TestAppender setupTestLogger(Class<?> clazz) {
     TestAppender testAppender = new TestAppender();
     testAppender.start();
diff --git a/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonLoggingInterceptor.java b/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonLoggingInterceptor.java
@@ -86,7 +86,7 @@ public void onMessage(RespT message) {
               @Override
               public void onClose(int statusCode, HttpJsonMetadata trailers) {
                 try {
-                  logResponse(statusCode, logDataBuilder);
+                  logResponse(statusCode, logDataBuilder, LOGGER);
                 } finally {
                   logDataBuilder = null; // release resource
                 }
@@ -166,21 +166,21 @@ private <RespT> void recordResponsePayload(RespT message, LogData.Builder logDat
     }
   }
 
-  private void logResponse(int statusCode, LogData.Builder logDataBuilder) {
+  void logResponse(int statusCode, LogData.Builder logDataBuilder, Logger logger) {
     try {
-      if (LOGGER.isInfoEnabled()) {
+      if (logger.isInfoEnabled()) {
         logDataBuilder.responseStatus(String.valueOf(statusCode));
       }
-      if (LOGGER.isInfoEnabled() && !LOGGER.isDebugEnabled()) {
+      if (logger.isInfoEnabled() && !logger.isDebugEnabled()) {
         Map<String, String> responseData = logDataBuilder.build().toMapResponse();
-        LoggingUtils.logWithMDC(LOGGER, Level.INFO, responseData, "Received HTTP response");
+        LoggingUtils.logWithMDC(logger, Level.INFO, responseData, "Received HTTP response");
       }
-      if (LOGGER.isDebugEnabled()) {
+      if (logger.isDebugEnabled()) {
         Map<String, String> responsedDetailsMap = logDataBuilder.build().toMapResponse();
-        LoggingUtils.logWithMDC(LOGGER, Level.DEBUG, responsedDetailsMap, "Received HTTP response");
+        LoggingUtils.logWithMDC(logger, Level.DEBUG, responsedDetailsMap, "Received HTTP response");
       }
     } catch (Exception e) {
-      LOGGER.error("Error logging request response", e);
+      logger.error("Error logging request response", e);
     }
   }
 
diff --git a/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonLoggingInterceptorTest.java b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonLoggingInterceptorTest.java
@@ -71,6 +71,21 @@ void testLogRequestInfo() {
     testAppender.stop();
   }
 
+  @Test
+  void testLogResponseInfo() {
+
+    TestAppender testAppender = setupTestLogger(HttpJsonLoggingInterceptorTest.class);
+    HttpJsonLoggingInterceptor interceptor = new HttpJsonLoggingInterceptor();
+    interceptor.logResponse(200, LogData.builder(), LOGGER);
+
+    Assertions.assertEquals(1, testAppender.events.size());
+    assertEquals(Level.INFO, testAppender.events.get(0).getLevel());
+    assertEquals(
+        "{\"response.status\":\"200\",\"message\":\"Received HTTP response\"}",
+        testAppender.events.get(0).getMessage());
+    testAppender.stop();
+  }
+
   private TestAppender setupTestLogger(Class<?> clazz) {
     TestAppender testAppender = new TestAppender();
     testAppender.start();

Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ public void onMessage(RespT message) {`
`85`	`85`	`@Override`
`86`	`86`	`public void onClose(Status status, Metadata trailers) {`
`87`	`87`	`try {`
`88`		`- logResponse(status.getCode().toString(), logDataBuilder);`
	`88`	`+ logResponse(status, logDataBuilder, LOGGER);`
`89`	`89`	`} finally {`
`90`	`90`	`logDataBuilder = null; // release resource`
`91`	`91`	`}`
`@@ -154,22 +154,22 @@ <RespT> void recordResponsePayload(RespT message, LogData.Builder logDataBuilder`
`154`	`154`	`}`
`155`	`155`	`}`
`156`	`156`
`157`		`- void logResponse(String statusCode, LogData.Builder logDataBuilder) {`
	`157`	`+ void logResponse(Status status, LogData.Builder logDataBuilder, Logger logger) {`
`158`	`158`	`try {`
`159`	`159`
`160`		`- if (LOGGER.isInfoEnabled()) {`
`161`		`- logDataBuilder.responseStatus(statusCode);`
	`160`	`+ if (logger.isInfoEnabled()) {`
	`161`	`+ logDataBuilder.responseStatus(status.getCode().toString());`
`162`	`162`	`}`
`163`		`- if (LOGGER.isInfoEnabled() && !LOGGER.isDebugEnabled()) {`
	`163`	`+ if (logger.isInfoEnabled() && !logger.isDebugEnabled()) {`
`164`	`164`	`Map<String, String> responseData = logDataBuilder.build().toMapResponse();`
`165`		`- LoggingUtils.logWithMDC(LOGGER, Level.INFO, responseData, "Received Grpc response");`
	`165`	`+ LoggingUtils.logWithMDC(logger, Level.INFO, responseData, "Received Grpc response");`
`166`	`166`	`}`
`167`		`- if (LOGGER.isDebugEnabled()) {`
	`167`	`+ if (logger.isDebugEnabled()) {`
`168`	`168`	`Map<String, String> responsedDetailsMap = logDataBuilder.build().toMapResponse();`
`169`		`- LoggingUtils.logWithMDC(LOGGER, Level.DEBUG, responsedDetailsMap, "Received Grpc response");`
	`169`	`+ LoggingUtils.logWithMDC(logger, Level.DEBUG, responsedDetailsMap, "Received Grpc response");`
`170`	`170`	`}`
`171`	`171`	`} catch (Exception e) {`
`172`		`- LOGGER.error("Error logging request response", e);`
	`172`	`+ logger.error("Error logging request response", e);`
`173`	`173`	`}`
`174`	`174`	`}`
`175`	`175`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ public void onMessage(RespT message) {`
`86`	`86`	`@Override`
`87`	`87`	`public void onClose(int statusCode, HttpJsonMetadata trailers) {`
`88`	`88`	`try {`
`89`		`- logResponse(statusCode, logDataBuilder);`
	`89`	`+ logResponse(statusCode, logDataBuilder, LOGGER);`
`90`	`90`	`} finally {`
`91`	`91`	`logDataBuilder = null; // release resource`
`92`	`92`	`}`
`@@ -166,21 +166,21 @@ private <RespT> void recordResponsePayload(RespT message, LogData.Builder logDat`
`166`	`166`	`}`
`167`	`167`	`}`
`168`	`168`
`169`		`- private void logResponse(int statusCode, LogData.Builder logDataBuilder) {`
	`169`	`+ void logResponse(int statusCode, LogData.Builder logDataBuilder, Logger logger) {`
`170`	`170`	`try {`
`171`		`- if (LOGGER.isInfoEnabled()) {`
	`171`	`+ if (logger.isInfoEnabled()) {`
`172`	`172`	`logDataBuilder.responseStatus(String.valueOf(statusCode));`
`173`	`173`	`}`
`174`		`- if (LOGGER.isInfoEnabled() && !LOGGER.isDebugEnabled()) {`
	`174`	`+ if (logger.isInfoEnabled() && !logger.isDebugEnabled()) {`
`175`	`175`	`Map<String, String> responseData = logDataBuilder.build().toMapResponse();`
`176`		`- LoggingUtils.logWithMDC(LOGGER, Level.INFO, responseData, "Received HTTP response");`
	`176`	`+ LoggingUtils.logWithMDC(logger, Level.INFO, responseData, "Received HTTP response");`
`177`	`177`	`}`
`178`		`- if (LOGGER.isDebugEnabled()) {`
	`178`	`+ if (logger.isDebugEnabled()) {`
`179`	`179`	`Map<String, String> responsedDetailsMap = logDataBuilder.build().toMapResponse();`
`180`		`- LoggingUtils.logWithMDC(LOGGER, Level.DEBUG, responsedDetailsMap, "Received HTTP response");`
	`180`	`+ LoggingUtils.logWithMDC(logger, Level.DEBUG, responsedDetailsMap, "Received HTTP response");`
`181`	`181`	`}`
`182`	`182`	`} catch (Exception e) {`
`183`		`- LOGGER.error("Error logging request response", e);`
	`183`	`+ logger.error("Error logging request response", e);`
`184`	`184`	`}`
`185`	`185`	`}`
`186`	`186`