Switched to slf4j (#64)

ONSdigital · Apr 30, 2024 · abb2cc7 · abb2cc7
1 parent 77d2b65
commit abb2cc7
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 46 deletions.
diff --git a/pom.xml b/pom.xml
@@ -115,9 +115,19 @@
             <artifactId>spring-cloud-gcp-starter-firestore</artifactId>
         </dependency>
         <dependency>
-            <groupId>com.godaddy</groupId>
-            <artifactId>logging</artifactId>
-            <version>1.2.5</version>
+            <groupId>ch.qos.logback</groupId>
+            <artifactId>logback-classic</artifactId>
+            <version>1.5.3</version>
+        </dependency>
+        <dependency>
+            <groupId>net.logstash.logback</groupId>
+            <artifactId>logstash-logback-encoder</artifactId>
+            <version>7.4</version>
+        </dependency>
+        <dependency>
+            <groupId>ch.qos.logback</groupId>
+            <artifactId>logback-core</artifactId>
+            <version>1.5.3</version>
         </dependency>
         <dependency>
             <groupId>com.nimbusds</groupId>

diff --git a/src/main/java/uk/gov/ons/ssdc/rhservice/config/AppConfig.java b/src/main/java/uk/gov/ons/ssdc/rhservice/config/AppConfig.java
@@ -1,6 +1,5 @@
 package uk.gov.ons.ssdc.rhservice.config;
 
-import com.godaddy.logging.LoggingConfigs;
 import com.google.cloud.spring.pubsub.core.PubSubTemplate;
 import com.google.cloud.spring.pubsub.support.PublisherFactory;
 import com.google.cloud.spring.pubsub.support.SubscriberFactory;
@@ -45,10 +44,6 @@ public SimplePubSubMessageConverter messageConverter() {
 
   @PostConstruct
   public void init() {
-    if ("STRUCTURED".equals(loggingProfile)) {
-      LoggingConfigs.setCurrent(LoggingConfigs.getCurrent().useJson());
-    }
-
     TimeZone.setDefault(TimeZone.getTimeZone("UTC"));
   }
 

diff --git a/src/main/java/uk/gov/ons/ssdc/rhservice/messaging/ManagedMessageRecoverer.java b/src/main/java/uk/gov/ons/ssdc/rhservice/messaging/ManagedMessageRecoverer.java
@@ -1,11 +1,11 @@
 package uk.gov.ons.ssdc.rhservice.messaging;
 
-import com.godaddy.logging.Logger;
-import com.godaddy.logging.LoggerFactory;
 import com.google.cloud.spring.pubsub.support.BasicAcknowledgeablePubsubMessage;
 import com.google.cloud.spring.pubsub.support.GcpPubSubHeaders;
 import com.google.protobuf.ByteString;
-import net.logstash.logback.encoder.org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageHandlingException;
@@ -35,9 +35,10 @@ public ManagedMessageRecoverer(ExceptionManagerClient exceptionManagerClient) {
   @Override
   public Object recover(RetryContext retryContext) {
     if (!(retryContext.getLastThrowable() instanceof MessagingException)) {
-      log.error(
-          "Super duper unexpected kind of error, so going to fail very noisily",
-          retryContext.getLastThrowable());
+      log.atError()
+          .setMessage("Super duper unexpected kind of error, so going to fail very noisily")
+          .setCause(retryContext.getLastThrowable())
+          .log();
       throw new RuntimeException(retryContext.getLastThrowable());
     }
 
@@ -87,9 +88,11 @@ private ExceptionReportResponse getExceptionReportResponse(
           exceptionManagerClient.reportException(
               messageHash, SERVICE_NAME, subscriptionName, cause, stackTraceRootCause);
     } catch (Exception exceptionManagerClientException) {
-      log.with("reason", exceptionManagerClientException.getMessage())
-          .warn(
-              "Could not report to Exception Manager. There will be excessive logging until resolved");
+      log.atWarn()
+          .setMessage(
+              "Could not report to Exception Manager. There will be excessive logging until resolved")
+          .addKeyValue("reason", exceptionManagerClientException.getMessage())
+          .log();
     }
     return reportResult;
   }
@@ -119,15 +122,16 @@ private boolean skipMessage(
       exceptionManagerClient.storeMessageBeforeSkipping(skippedMessage);
       result = true;
     } catch (Exception exceptionManagerClientException) {
-      log.with("message_hash", messageHash)
-          .warn(
-              "Unable to store a copy of the message. Will NOT be quarantining",
-              exceptionManagerClientException);
+      log.atWarn()
+          .setMessage("Unable to store a copy of the message. Will NOT be quarantining")
+          .setCause(exceptionManagerClientException)
+          .addKeyValue("message_hash", messageHash)
+          .log();
     }
 
     // If the quarantined message is persisted OK then we can ACK the message
     if (result) {
-      log.with("message_hash", messageHash).warn("Quarantined message");
+      log.atWarn().setMessage("Quarantined message").addKeyValue("message_hash", messageHash).log();
     }
 
     return result;
@@ -157,12 +161,18 @@ private void logMessage(
     }
 
     if (logStackTraces) {
-      log.with("message_hash", messageHash).error("Could not process message", cause);
+      log.atError()
+          .setMessage("Could not process message")
+          .setCause(cause)
+          .addKeyValue("message_hash", messageHash)
+          .log();
     } else {
-      log.with("message_hash", messageHash)
-          .with("cause", cause.getMessage())
-          .with("root_cause", stackTraceRootCause)
-          .error("Could not process message");
+      log.atError()
+          .setMessage("Could not process message")
+          .addKeyValue("message_hash", messageHash)
+          .addKeyValue("cause", cause.getMessage())
+          .addKeyValue("root_cause", stackTraceRootCause)
+          .log();
     }
   }
 

diff --git a/src/main/java/uk/gov/ons/ssdc/rhservice/service/CloudRetryListener.java b/src/main/java/uk/gov/ons/ssdc/rhservice/service/CloudRetryListener.java
@@ -1,7 +1,7 @@
 package uk.gov.ons.ssdc.rhservice.service;
 
-import com.godaddy.logging.Logger;
-import com.godaddy.logging.LoggerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.retry.RetryCallback;
 import org.springframework.retry.RetryContext;
 import org.springframework.retry.RetryListener;
@@ -14,7 +14,7 @@ public class CloudRetryListener implements RetryListener {
   public <T, E extends Throwable> void onError(
       RetryContext context, RetryCallback<T, E> callback, Throwable throwable) {
     Object operationName = context.getAttribute(RetryContext.NAME);
-    log.warn("Retry failed: " + operationName);
+    log.atWarn().setMessage("Retry failed: " + operationName).log();
   }
 
   @Override
@@ -29,8 +29,11 @@ public <T, E extends Throwable> void close(
 
         // On failure the retryCount actually holds the number of attempts
         int numAttempts = context.getRetryCount();
-        log.warn(
-            String.format("%s Transaction failed after %s attempts", operationName, numAttempts));
+        log.atWarn()
+            .setMessage(
+                String.format(
+                    "%s Transaction failed after %s attempts", operationName, numAttempts))
+            .log();
       }
     }
   }

diff --git a/src/main/java/uk/gov/ons/ssdc/rhservice/service/UacValidationService.java b/src/main/java/uk/gov/ons/ssdc/rhservice/service/UacValidationService.java
@@ -2,12 +2,12 @@
 
 import static uk.gov.ons.ssdc.rhservice.utils.Constants.*;
 
-import com.godaddy.logging.Logger;
-import com.godaddy.logging.LoggerFactory;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.util.Optional;
 import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Service;
@@ -86,9 +86,11 @@ private CollectionExerciseUpdateDTO getCollectionExerciseFromCase(CaseUpdateDTO
         .readCollectionExerciseUpdate(caseUpdateDTO.getCollectionExerciseId())
         .orElseThrow(
             () -> {
-              log.with("collectionExerciseId", caseUpdateDTO.getCollectionExerciseId())
-                  .with("caseId", caseUpdateDTO.getCaseId())
-                  .error("collectionExerciseId not found for caseId");
+              log.atError()
+                  .setMessage("collectionExerciseId not found for caseId")
+                  .addKeyValue("collectionExerciseId", caseUpdateDTO.getCollectionExerciseId())
+                  .addKeyValue("caseId", caseUpdateDTO.getCaseId())
+                  .log();
               return new RuntimeException("Collection exercise not found for case");
             });
   }
@@ -101,14 +103,18 @@ private boolean collectionExerciseResponseExpiresAtDateHasPassed(
         collectionExerciseEndDate.plusWeeks(RESPONSE_EXPIRES_AT_WEEK_INCREMENT);
 
     if (collectionExerciseEndDateWithWeekIncrement.isBefore(OffsetDateTime.now(ZoneOffset.UTC))) {
-      log.with("collectionExerciseId", collectionExerciseUpdateDTO.getCollectionExerciseId())
-          .with("caseId", caseUpdateDTO.getCaseId())
-          .with("collectionExerciseEndDate", collectionExerciseEndDate.toString())
-          .with("collectionExerciseWeeksInFutureIncrement", RESPONSE_EXPIRES_AT_WEEK_INCREMENT)
-          .with(
+      log.atWarn()
+          .setMessage("Collection exercise response expiry end date has already passed for case")
+          .addKeyValue(
+              "collectionExerciseId", collectionExerciseUpdateDTO.getCollectionExerciseId())
+          .addKeyValue("caseId", caseUpdateDTO.getCaseId())
+          .addKeyValue("collectionExerciseEndDate", collectionExerciseEndDate.toString())
+          .addKeyValue(
+              "collectionExerciseWeeksInFutureIncrement", RESPONSE_EXPIRES_AT_WEEK_INCREMENT)
+          .addKeyValue(
               "collectionExerciseEndDateWithWeekIncrement",
               collectionExerciseEndDateWithWeekIncrement.toString())
-          .warn("Collection exercise response expiry end date has already passed for case");
+          .log();
       return true;
     }
     return false;
@@ -125,7 +131,10 @@ private CaseUpdateDTO getCaseFromUac(UacUpdateDTO uacUpdateDTO) {
         .readCaseUpdate(caseId)
         .orElseThrow(
             () -> {
-              log.with("caseId", uacUpdateDTO.getCaseId()).error("caseId not found for UAC");
+              log.atError()
+                  .setMessage("caseId not found for UAC")
+                  .addKeyValue("caseId", uacUpdateDTO.getCaseId())
+                  .log();
               return new RuntimeException("Case Not Found for UAC");
             });
   }

diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml
@@ -9,9 +9,9 @@
     <springProperty name="springAppName" scope="context"
                     source="spring.application.name" />
     <property name="CONSOLE_LOG_PATTERN"
-              value="%clr(%d{yyyy-MM-dd HH:mm:ss.SSS}){faint} %clr(${LOG_LEVEL_PATTERN:-%5p})  %clr(${PID:- }){magenta} %clr(---){faint} %clr([%15.15t]){faint} %clr(%-40.40logger{39}){cyan} %clr(:){faint} %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}" />
+              value="%clr(%d{yyyy-MM-dd HH:mm:ss.SSS}){faint} %clr(${LOG_LEVEL_PATTERN:-%5p})  %clr(${PID:- }){magenta} %clr(---){faint} %clr([%15.15t]){faint} %clr(%-40.40logger{39}){cyan} %clr(:){faint} %m %kvp%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}" />
     <property name="SYSLOG_PATTERN"
-              value="${LOG_LEVEL_PATTERN:-%5level} %-40.40logger{39} : %message%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}" />
+              value="${LOG_LEVEL_PATTERN:-%5level} %-40.40logger{39} : %message %kvp%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}" />
     <property name="ISO8601_DATE_FORMAT"
               value="yyyy-MM-dd'T'HH:mm:ss'Z'" />
 
@@ -63,6 +63,8 @@
                             <nonStructuredArgumentsFieldPrefix>prefix
                             </nonStructuredArgumentsFieldPrefix>
                         </arguments>
+                        <keyValuePairs>
+                        </keyValuePairs>
                         <tags />
                         <logstashMarkers />
                     </providers>