Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [P4PU-687] Added feign custom logger #158

Merged
merged 9 commits into from
Nov 14, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package it.gov.pagopa.arc.config;

import feign.Logger;
import feign.Request;
import feign.Response;
import feign.Util;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;

import java.io.IOException;
import java.nio.charset.StandardCharsets;

@Slf4j
public class CustomFeignClientLogger extends Logger {

@Override
protected void logRequest(String configKey, Level logLevel, Request request) {
/*
* This method is intentionally left blank to prevent it
* from logging the request. We will log both the request and the response
* in the logAndRebufferResponse method.
*/
}

@Override
protected Response logAndRebufferResponse(
String configKey, Level logLevel, Response response, long elapsedTime) throws IOException {
int status = response.status();
Request request = response.request();

log(configKey,"[FEIGN_CLIENT_REQUEST] ---> %s %s", request.httpMethod(), request.url());

if(status >= HttpStatus.BAD_REQUEST.value() && response.body() != null){
byte[] bodyData = Util.toByteArray(response.body().asInputStream());
if (bodyData.length > 0) {
String responseString = new String(bodyData, StandardCharsets.UTF_8);

log(configKey,"[FEIGN_CLIENT_RESPONSE] <--- Status: %d, response reason: %s, elapsed time (%d ms), response: %s", status, response.reason(), elapsedTime, responseString);
}
}else{
log(configKey,"[FEIGN_CLIENT_RESPONSE] <--- Status: %d, response reason: %s, elapsed time (%d ms)", status, response.reason(), elapsedTime);
}

return response;
}

@Override
protected void log(String configKey, String format, Object... args) {
log.info(String.format(methodTag(configKey).concat(format), args));
}
}
20 changes: 20 additions & 0 deletions src/main/java/it/gov/pagopa/arc/config/FeignLoggingConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package it.gov.pagopa.arc.config;

import feign.Logger;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class FeignLoggingConfig {
@Bean
Logger feignLogger() {
return new CustomFeignClientLogger();
}

@Bean
Logger.Level feignLoggerLevel(){
return Logger.Level.FULL;
}


}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package it.gov.pagopa.arc.connector.bizevents.paidnotice;

import feign.Response;
import it.gov.pagopa.arc.config.FeignLoggingConfig;
import it.gov.pagopa.arc.connector.bizevents.dto.paidnotice.BizEventsPaidNoticeDetailsDTO;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.core.io.Resource;
Expand All @@ -12,7 +13,7 @@

@FeignClient(
name = "biz-events-paid-notice",
url = "${rest-client.biz-events.paid-notice.baseUrl}")
url = "${rest-client.biz-events.paid-notice.baseUrl}",configuration = FeignLoggingConfig.class)
public interface BizEventsPaidNoticeRestClient {

@GetMapping(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package it.gov.pagopa.arc.connector.pullpayment;

import it.gov.pagopa.arc.config.FeignLoggingConfig;
import it.gov.pagopa.arc.connector.pullpayment.dto.PullPaymentNoticeDTO;
import org.springframework.cloud.openfeign.FeignClient;
import org.springframework.format.annotation.DateTimeFormat;
Expand All @@ -12,7 +13,7 @@

@FeignClient(
name = "pull-payment",
url = "${rest-client.pull-payment.baseUrl}")
url = "${rest-client.pull-payment.baseUrl}", configuration = FeignLoggingConfig.class)
public interface PullPaymentRestClient {
@GetMapping(
value = "/payment-notices/v1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import it.gov.pagopa.arc.controller.generated.ArcPaymentNoticesApi;
import it.gov.pagopa.arc.model.generated.PaymentNoticesListDTO;
import it.gov.pagopa.arc.service.PaymentNoticesService;
import it.gov.pagopa.arc.utils.SecurityUtils;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.RestController;
Expand All @@ -19,7 +20,9 @@ public PaymentNoticesControllerImpl(PaymentNoticesService paymentNoticesService)

@Override
public ResponseEntity<PaymentNoticesListDTO> getPaymentNotices(LocalDate dueDate, Integer size, Integer page) {
PaymentNoticesListDTO paymentNoticesListDTO = paymentNoticesService.retrievePaymentNotices(dueDate, size, page);
String userFiscalCode = SecurityUtils.getUserFiscalCode();
String userId = SecurityUtils.getUserId();
PaymentNoticesListDTO paymentNoticesListDTO = paymentNoticesService.retrievePaymentNotices(userId, userFiscalCode, dueDate, size, page);
return new ResponseEntity<>(paymentNoticesListDTO, HttpStatus.OK);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ public ResponseEntity<ErrorDTO> handleGenericRuntimeException(RuntimeException e

private static ResponseEntity<ErrorDTO> handleArcErrorException(RuntimeException ex, HttpServletRequest request, HttpStatus httpStatus, ErrorDTO.ErrorEnum errorEnum) {
String message = ex.getMessage();
log.info("A {} occurred handling request {}: HttpStatus {} - {}",
log.error("A {} occurred handling request {}: HttpStatus {} - {}",
ex.getClass(),
getRequestDetails(request),
httpStatus.value(),
message);

log.error("Exception occurred: {} - {}", ex.getClass().getSimpleName(), ex.getMessage(), ex);
message,
ex);

return ResponseEntity
.status(httpStatus)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
import java.time.LocalDate;

public interface PaymentNoticesService {
PaymentNoticesListDTO retrievePaymentNotices(LocalDate dueDate, Integer size, Integer page);
PaymentNoticesListDTO retrievePaymentNotices(String userId, String userFiscalCode, LocalDate dueDate, Integer size, Integer page);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public PaymentNoticesServiceImpl(PullPaymentService pullPaymentService) {
}

@Override
public PaymentNoticesListDTO retrievePaymentNotices(LocalDate dueDate, Integer size, Integer page) {
log.info("[GET_PAYMENT_NOTICES] The current user has requested to retrieve payment notices, with the current parameters: dueDate {}, size {} and page {}", dueDate, size, page);
return pullPaymentService.retrievePaymentNoticesListFromPullPayment(dueDate,size,page);
public PaymentNoticesListDTO retrievePaymentNotices(String userId, String userFiscalCode, LocalDate dueDate, Integer size, Integer page) {
log.info("[GET_PAYMENT_NOTICES] The current user with user id : {}, has requested to retrieve payment notices, with the current parameters: dueDate {}, size {} and page {}", userId, dueDate, size, page);
return pullPaymentService.retrievePaymentNoticesListFromPullPayment(userFiscalCode, dueDate,size,page);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
import java.time.LocalDate;

public interface PullPaymentService {
PaymentNoticesListDTO retrievePaymentNoticesListFromPullPayment(LocalDate dueDate, Integer size, Integer page);
PaymentNoticesListDTO retrievePaymentNoticesListFromPullPayment(String userFiscalCode, LocalDate dueDate, Integer size, Integer page);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import it.gov.pagopa.arc.dto.mapper.pullpayment.PullPaymentNoticeDTO2PaymentNoticeDTOMapper;
import it.gov.pagopa.arc.model.generated.PaymentNoticeDTO;
import it.gov.pagopa.arc.model.generated.PaymentNoticesListDTO;
import it.gov.pagopa.arc.utils.SecurityUtils;
import org.springframework.stereotype.Service;

import java.time.LocalDate;
Expand All @@ -28,9 +27,8 @@ public PullPaymentServiceImpl(PullPaymentConnector pullPaymentConnector,
}

@Override
public PaymentNoticesListDTO retrievePaymentNoticesListFromPullPayment(LocalDate dueDate, Integer size, Integer page) {
String retrievedUserFiscalCode = SecurityUtils.getUserFiscalCode();
List<PullPaymentNoticeDTO> pullPaymentNoticeDTOList = pullPaymentConnector.getPaymentNotices(retrievedUserFiscalCode, dueDate, size, page);
public PaymentNoticesListDTO retrievePaymentNoticesListFromPullPayment(String userFiscalCode, LocalDate dueDate, Integer size, Integer page) {
List<PullPaymentNoticeDTO> pullPaymentNoticeDTOList = pullPaymentConnector.getPaymentNotices(userFiscalCode, dueDate, size, page);
List<PaymentNoticeDTO> paymentNoticeDTOFilteredList;

if(!pullPaymentNoticeDTOList.isEmpty()){
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ rest-client:

logging:
level:
it.gov.pagopa.arc.connector: \${CONNECTOR_LOGGING_LEVEL:DEBUG}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for logging levels, let's use a common prefix

Suggested change
it.gov.pagopa.arc.connector: \${CONNECTOR_LOGGING_LEVEL:DEBUG}
it.gov.pagopa.arc.connector: \${LOGGING_LEVEL_ARC_CONNECTOR:DEBUG}

org:
springframework:
security: INFO
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package it.gov.pagopa.arc.config;

import feign.Request;
import feign.Response;
import feign.Util;
import it.gov.pagopa.arc.utils.MemoryAppender;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;

import static org.junit.jupiter.api.Assertions.*;

class CustomFeignClientLoggerTest {
private static final String CONFIG_KEY = "ExampleClass#exampleMethod(String p)";
private final CustomFeignClientLogger customFeignClientLogger = new CustomFeignClientLogger();
private Request request;
private MemoryAppender memoryAppender;
@BeforeEach
void setUp() {
request = Request.create(
Request.HttpMethod.GET,
"https://api.example.com/test",
new HashMap<>(),
null,
StandardCharsets.UTF_8,
null
);

ch.qos.logback.classic.Logger logger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger("it.gov.pagopa.arc.config.CustomFeignClientLogger");
memoryAppender = new MemoryAppender();
logger.setLevel(ch.qos.logback.classic.Level.INFO);
logger.addAppender(memoryAppender);
memoryAppender.start();
}

@Test
void givenRequestWhenLogRequestThenLogNothing() {
//given
//when
customFeignClientLogger.logRequest(CONFIG_KEY, feign.Logger.Level.FULL, request);
//then
assertEquals(0, memoryAppender.getLoggedEvents().size());

}

@Test
void givenResponseWhenLogAndRebufferResponseThenLog() throws IOException {
//given
String responseBody = "{\"notices\": []}";
Response response = Response.builder()
.status(200)
.reason("OK")
.request(request)
.body(responseBody, StandardCharsets.UTF_8)
.build();
//when
Response clonedResponse = customFeignClientLogger.logAndRebufferResponse(CONFIG_KEY, feign.Logger.Level.FULL, response, 100);
//then
byte[] bodyData = Util.toByteArray(clonedResponse.body().asInputStream());
String responseString = new String(bodyData, StandardCharsets.UTF_8);

assertEquals(2, memoryAppender.getLoggedEvents().size());
assertEquals(responseBody, responseString);
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(0).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_REQUEST] ---> GET https://api.example.com/test"));
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(1).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_RESPONSE] <--- Status: 200, response reason: OK, elapsed time (100 ms)"));
}

@Test
void givenErrorResponseWhenLogAndRebufferResponseThenLogError() throws IOException {
//given
String responseBody = "{\"error\":\"No records found for the requested user\"}";
Response response = Response.builder()
.status(404)
.reason("Not Found")
.request(request)
.body(responseBody, StandardCharsets.UTF_8)
.build();
//when
Response clonedResponse = customFeignClientLogger.logAndRebufferResponse(CONFIG_KEY, feign.Logger.Level.FULL, response, 100);
//then
byte[] bodyData = Util.toByteArray(clonedResponse.body().asInputStream());
String responseString = new String(bodyData, StandardCharsets.UTF_8);

assertEquals(2, memoryAppender.getLoggedEvents().size());
assertEquals(responseBody, responseString);
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(0).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_REQUEST] ---> GET https://api.example.com/test"));
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(1).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_RESPONSE] <--- Status: 404, response reason: Not Found, elapsed time (100 ms), response: {\"error\":\"No records found for the requested user\"}"));
}

@Test
void givenEmptyBodyWhenLogAndRebufferResponseThenLog() throws IOException {
//given
Response response = Response.builder()
.status(200)
.reason("OK")
.request(request)
.build();
//when
Response clonedResponse = customFeignClientLogger.logAndRebufferResponse(CONFIG_KEY, feign.Logger.Level.FULL, response, 100);
//then

assertEquals(2, memoryAppender.getLoggedEvents().size());

assertNull(clonedResponse.body());
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(0).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_REQUEST] ---> GET https://api.example.com/test"));
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(1).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_RESPONSE] <--- Status: 200, response reason: OK, elapsed time (100 ms)"));
}

@Test
void givenEmptyBodyAnd400ErrorWhenLogAndRebufferResponseThenLog() throws IOException {
//given
Response response = Response.builder()
.status(400)
.reason("BAD REQUEST")
.request(request)
.build();
//when
Response clonedResponse = customFeignClientLogger.logAndRebufferResponse(CONFIG_KEY, feign.Logger.Level.FULL, response, 100);
//then

assertEquals(2, memoryAppender.getLoggedEvents().size());

assertNull(clonedResponse.body());
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(0).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_REQUEST] ---> GET https://api.example.com/test"));
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(1).getFormattedMessage().contains("[ExampleClass#exampleMethod] [FEIGN_CLIENT_RESPONSE] <--- Status: 400, response reason: BAD REQUEST, elapsed time (100 ms)"));
}

@Test
void givenConfigKeyAndFormatStringWhenLogThenReturnLogInfo() {
//given
String formatString = "This is an example string %s";
//when
customFeignClientLogger.log(CONFIG_KEY, formatString, "Arg1");
//then
assertEquals(1, memoryAppender.getLoggedEvents().size());
Assertions.assertTrue(memoryAppender.getLoggedEvents().get(0).getFormattedMessage().contains("[ExampleClass#exampleMethod] This is an example string Arg1"));
}
}
Loading
Loading