From 16649bb93cf6002f07796c7faa49179c3cd5f652 Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Fri, 10 Jan 2025 08:43:42 +0100 Subject: [PATCH 1/8] 279 build(gradle): upgrade Spring Boot to 3.4.1 --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 63068418..f4c95a35 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -11,7 +11,7 @@ plugins { id("org.jooq.jooq-codegen-gradle") version "3.19.15" id("org.openapi.generator") version "7.10.0" // id("org.owasp.dependencycheck") version "10.0.3" - id("org.springframework.boot") version "3.4.0" + id("org.springframework.boot") version "3.4.1" } repositories { From a4349ff4fe7e31119a64495a084e4745ab538525 Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Sun, 12 Jan 2025 21:55:02 +0100 Subject: [PATCH 2/8] 279 feat(logging): deduplicate array `plIds` --- .../java/nl/rijksoverheid/mev/logging/LoggingContext.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java index 2f40911e..f10f699e 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java @@ -12,12 +12,12 @@ public class LoggingContext { private static final Logger logger = LoggerFactory.getLogger(LoggingContext.class); private final Instant eventStart; - private final List plIds; + private final Set plIds; private final Map gezagResultaten; public LoggingContext() { this.eventStart = Instant.now(); - this.plIds = new ArrayList<>(); + this.plIds = new HashSet<>(); this.gezagResultaten = new HashMap<>(); } @@ -61,8 +61,8 @@ public Instant getEventStart() { return eventStart; } - public List getPlIds() { - return Collections.unmodifiableList(plIds); + public Set getPlIds() { + return Collections.unmodifiableSet(plIds); } public Collection getGezagResultaten() { From 5316ada80f2ab6cacc3d6525cf28b8ee413d7828 Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Sun, 12 Jan 2025 23:58:45 +0100 Subject: [PATCH 3/8] 279 feat(logging): always log `response.body` to enable analysis on returned results --- src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java index c9115aed..b721369f 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java @@ -154,12 +154,10 @@ private void processMetadata(HttpServletRequest request, HttpServletResponse res } var responseHeaders = cachedResponse == null ? null : headersToMap(cachedResponse.getHeaderNames(), cachedResponse::getHeader); - var isError = response.getStatus() >= 400; - var metadata = new HashMap(); metadata.put("gezag_resultaten", loggingContext.getGezagResultaten()); metadata.put("pl_ids", loggingContext.getPlIds()); - if (isError) metadata.put("response.body", responseBodyAsJsonString); + metadata.put("response.body", responseBodyAsJsonString); metadata.put("response.headers", responseHeaders); metadata.put("request.body", requestBodyAsJsonString); metadata.put("request.headers", requestHeaders); From 5e3252ace9a4df49e6cd11ae0876a259ae49e16e Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Mon, 13 Jan 2025 00:02:14 +0100 Subject: [PATCH 4/8] 279 feat(logging): log `gezag_resultaten` per gezag deduction instead of per request --- .../gezagsmodule/service/GezagService.java | 21 ++++++-- .../mev/gmapi/BevoegdheidTotGezagService.java | 1 - .../mev/logging/GezagResultaat.java | 2 + .../mev/logging/LoggingContext.java | 49 ++++--------------- .../mev/logging/LoggingFilter.java | 1 - 5 files changed, 28 insertions(+), 46 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/gezagsmodule/service/GezagService.java b/src/main/java/nl/rijksoverheid/mev/gezagsmodule/service/GezagService.java index 9f28fcb8..760eaa1b 100644 --- a/src/main/java/nl/rijksoverheid/mev/gezagsmodule/service/GezagService.java +++ b/src/main/java/nl/rijksoverheid/mev/gezagsmodule/service/GezagService.java @@ -9,6 +9,7 @@ import nl.rijksoverheid.mev.gezagsmodule.domain.Persoon; import nl.rijksoverheid.mev.gezagsmodule.domain.Persoonslijst; import nl.rijksoverheid.mev.gezagsmodule.domain.gezagvraag.GezagsBepaling; +import nl.rijksoverheid.mev.logging.GezagResultaat; import nl.rijksoverheid.mev.logging.LoggingContext; import org.openapitools.model.AbstractGezagsrelatie; import org.openapitools.model.GezagNietTeBepalen; @@ -18,6 +19,8 @@ import java.util.*; +import static net.logstash.logback.argument.StructuredArguments.value; + /** * Service voor bepalen gezag */ @@ -124,10 +127,20 @@ public List getGezagResultaat(final String burgerservicen gezagsRelaties = gezagsrelatieService.bepaalGezagsrelaties(arAntwoordenModel, gezagsBepaling); } - loggingContext.addGezagType(arAntwoordenModel.getSoortGezag(), burgerservicenummer); - loggingContext.addRoute(route, burgerservicenummer); - loggingContext.addToelichting(arAntwoordenModel.getUitleg(), burgerservicenummer); - logger.info("Gezag bepaald voor persoon \"{}\": {}", burgerservicenummer, arAntwoordenModel); + if (logger.isInfoEnabled()) { + var gezagResultaat = new GezagResultaat( + loggingContext.getPlIdBy(burgerservicenummer), + arAntwoordenModel.getSoortGezag(), + arAntwoordenModel.getUitleg(), + route + ); + logger.info( + """ + Gezag bepaald voor persoon "{}": {} + {}""", + burgerservicenummer, value("gezag_resultaat", gezagResultaat), value("antwoorden_model", arAntwoordenModel) + ); + } return gezagsRelaties; } diff --git a/src/main/java/nl/rijksoverheid/mev/gmapi/BevoegdheidTotGezagService.java b/src/main/java/nl/rijksoverheid/mev/gmapi/BevoegdheidTotGezagService.java index 129ba104..d9e089a2 100644 --- a/src/main/java/nl/rijksoverheid/mev/gmapi/BevoegdheidTotGezagService.java +++ b/src/main/java/nl/rijksoverheid/mev/gmapi/BevoegdheidTotGezagService.java @@ -52,7 +52,6 @@ public BevoegdheidTotGezagService( */ public List bepaalBevoegdheidTotGezag(final GezagRequest gezagRequest) throws GezagException { List burgerservicenummers = gezagRequest.getBurgerservicenummer(); - loggingContext.addBurgerservicenummers(burgerservicenummers); return burgerservicenummers.stream() .map(this::bepaalGezagVoorPersoon) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java index 4c90d048..e161650b 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java @@ -1,8 +1,10 @@ package nl.rijksoverheid.mev.logging; +import lombok.AllArgsConstructor; import lombok.Data; @Data +@AllArgsConstructor public final class GezagResultaat { private long plId; private String type; diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java index f10f699e..86d0de13 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java @@ -6,55 +6,27 @@ import java.time.Instant; import java.util.*; +import java.util.stream.Collectors; public class LoggingContext { private static final Logger logger = LoggerFactory.getLogger(LoggingContext.class); private final Instant eventStart; - private final Set plIds; - private final Map gezagResultaten; + private final Map plIdByBurgerservicenummer; public LoggingContext() { this.eventStart = Instant.now(); - this.plIds = new HashSet<>(); - this.gezagResultaten = new HashMap<>(); - } - - public void addBurgerservicenummers(Collection burgerservicenummers) { - burgerservicenummers.forEach(burgerservicenummer -> - gezagResultaten.put(burgerservicenummer, new GezagResultaat()) - ); + this.plIdByBurgerservicenummer = new HashMap<>(); } public void addPlId(long plId, Burgerservicenummer burgerservicenummer) { - gezagResultaten.computeIfPresent(burgerservicenummer.asString(), (unused, gezagResultaat) -> { - gezagResultaat.setPlId(plId); - return gezagResultaat; - }); - plIds.add(plId); - } - - public void addGezagType(String gezagType, String burgerservicenummer) { - gezagResultaten.computeIfPresent(burgerservicenummer, (unused, gezagResultaat) -> { - var translatedGezagType = mapGezagType(gezagType); - gezagResultaat.setType(translatedGezagType); - return gezagResultaat; - }); + plIdByBurgerservicenummer.put(burgerservicenummer.value(), plId); } - public void addRoute(String route, String burgerservicenummer) { - gezagResultaten.computeIfPresent(burgerservicenummer, (unused, gezagResultaat) -> { - gezagResultaat.setRoute(route); - return gezagResultaat; - }); - } - - public void addToelichting(String toelichting, String burgerservicenummer) { - gezagResultaten.computeIfPresent(burgerservicenummer, (unused, gezagResultaat) -> { - gezagResultaat.setToelichting(toelichting); - return gezagResultaat; - }); + public long getPlIdBy(String burgerservicenummerAsString) { + var burgerservicenummerAsLong = Long.valueOf(burgerservicenummerAsString); + return plIdByBurgerservicenummer.get(burgerservicenummerAsLong); } public Instant getEventStart() { @@ -62,11 +34,8 @@ public Instant getEventStart() { } public Set getPlIds() { - return Collections.unmodifiableSet(plIds); - } - - public Collection getGezagResultaten() { - return Collections.unmodifiableCollection(gezagResultaten.values()); + return plIdByBurgerservicenummer.values().stream() + .collect(Collectors.toUnmodifiableSet()); } private static String mapGezagType(String gezagType) { diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java index b721369f..c247d56e 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java @@ -155,7 +155,6 @@ private void processMetadata(HttpServletRequest request, HttpServletResponse res var responseHeaders = cachedResponse == null ? null : headersToMap(cachedResponse.getHeaderNames(), cachedResponse::getHeader); var metadata = new HashMap(); - metadata.put("gezag_resultaten", loggingContext.getGezagResultaten()); metadata.put("pl_ids", loggingContext.getPlIds()); metadata.put("response.body", responseBodyAsJsonString); metadata.put("response.headers", responseHeaders); From 43787641473a871708ef7090444d9d6a0682fcaf Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Mon, 13 Jan 2025 00:02:42 +0100 Subject: [PATCH 5/8] 279 refactor: remove unused field and method --- .../mev/logging/LoggingContext.java | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java index 86d0de13..df197bec 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java @@ -1,17 +1,15 @@ package nl.rijksoverheid.mev.logging; import nl.rijksoverheid.mev.gezagsmodule.model.Burgerservicenummer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.time.Instant; -import java.util.*; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; public class LoggingContext { - private static final Logger logger = LoggerFactory.getLogger(LoggingContext.class); - private final Instant eventStart; private final Map plIdByBurgerservicenummer; @@ -37,21 +35,4 @@ public Set getPlIds() { return plIdByBurgerservicenummer.values().stream() .collect(Collectors.toUnmodifiableSet()); } - - private static String mapGezagType(String gezagType) { - // temporary until GezagService returns typed responses - return switch (gezagType) { - case "OG1" -> "EenhoofdigOuderlijkGezag"; - case "OG2" -> "TweehoofdigOuderlijkGezag"; - case "GG" -> "GezamenlijkGezag"; - case "V" -> "Voogdij"; - case "G" -> "TijdelijkGeenGezag"; - case "N" -> "GezagNietTeBepalen"; - case "NVT" -> "GezagNietVanToepassing*"; - default -> { // should never happen - logger.error("Unknown gezag type: {}", gezagType); - yield "OnbekendGezagType"; - } - }; - } } From e706a2e7ee0d3dc66cbf294d4fcada4b155df1ee Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Mon, 13 Jan 2025 00:13:29 +0100 Subject: [PATCH 6/8] 279 refactor: rewrite `GezagResultaat` as record --- .../mev/logging/GezagResultaat.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java index e161650b..3971ff6b 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java @@ -1,13 +1,9 @@ package nl.rijksoverheid.mev.logging; -import lombok.AllArgsConstructor; -import lombok.Data; - -@Data -@AllArgsConstructor -public final class GezagResultaat { - private long plId; - private String type; - private String toelichting; - private String route; +public record GezagResultaat( + long plId, + String type, + String toelichting, + String route +) { } From 5de90ef74fed825c172009e643682d914e489b69 Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Mon, 13 Jan 2025 08:34:57 +0100 Subject: [PATCH 7/8] 279 bug: resolve metadata and nested objects written as stringified objects --- .../mev/logging/LoggingFilter.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java index c247d56e..369de97f 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingFilter.java @@ -29,6 +29,8 @@ import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import static net.logstash.logback.marker.Markers.append; + public class LoggingFilter extends OncePerRequestFilter implements ApplicationContextAware { /** @@ -81,10 +83,10 @@ private void afterRequest(HttpServletRequest request, HttpServletResponse respon var event = processEvent(); var http = processHttp(request, response); var url = processUrl(request); - - processMetadata(request, response); + var metadata = processMetadata(request, response); log.info( + append("metadata", metadata), "HTTP {} {} responded {} in {} ms", http.requestMethod, url.path, http.responseStatusCode, event.duration.toMillis() ); @@ -136,7 +138,7 @@ private Url processUrl(HttpServletRequest request) { private record Url(String path) { } - private void processMetadata(HttpServletRequest request, HttpServletResponse response) { + private Map processMetadata(HttpServletRequest request, HttpServletResponse response) { var loggingContext = applicationContext.getBean(LoggingContext.class); var requestHeaders = headersToMap(Collections.list(request.getHeaderNames()), request::getHeader); @@ -156,18 +158,22 @@ private void processMetadata(HttpServletRequest request, HttpServletResponse res var metadata = new HashMap(); metadata.put("pl_ids", loggingContext.getPlIds()); - metadata.put("response.body", responseBodyAsJsonString); metadata.put("response.headers", responseHeaders); - metadata.put("request.body", requestBodyAsJsonString); metadata.put("request.headers", requestHeaders); try { - var metadataAsJsonString = objectMapper.writeValueAsString(metadata); - MDC.put("metadata", metadataAsJsonString); + var responseBody = objectMapper.readValue(responseBodyAsJsonString, Object.class); + var requestBody = objectMapper.readValue(requestBodyAsJsonString, Object.class); + + metadata.put("response.body", responseBody); + metadata.put("request.body", requestBody); } catch (JsonProcessingException e) { - log.error("Failed to serialize headers to JSON; fallback to writing headers as Java object", e); - MDC.put("metadata", metadata.toString()); + log.error("Failed to serialize HTTP body to JSON; fallback to writing headers as Java object", e); + metadata.put("response.body", responseBodyAsJsonString); + metadata.put("request.body", requestBodyAsJsonString); } + + return metadata; } private Map headersToMap(Collection headerNames, UnaryOperator headerValueResolver) { From de15e16604e830995354ef1eb0ee902ee2c07847 Mon Sep 17 00:00:00 2001 From: Leroy Truong Date: Mon, 13 Jan 2025 12:35:07 +0100 Subject: [PATCH 8/8] 279 bug: mitigate potential NullPointerException by using `Long` instead of `long` --- src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java | 2 +- src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java index 3971ff6b..a6e3fe0e 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/GezagResultaat.java @@ -1,7 +1,7 @@ package nl.rijksoverheid.mev.logging; public record GezagResultaat( - long plId, + Long plId, String type, String toelichting, String route diff --git a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java index df197bec..99c9ef73 100644 --- a/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java +++ b/src/main/java/nl/rijksoverheid/mev/logging/LoggingContext.java @@ -22,7 +22,7 @@ public void addPlId(long plId, Burgerservicenummer burgerservicenummer) { plIdByBurgerservicenummer.put(burgerservicenummer.value(), plId); } - public long getPlIdBy(String burgerservicenummerAsString) { + public Long getPlIdBy(String burgerservicenummerAsString) { var burgerservicenummerAsLong = Long.valueOf(burgerservicenummerAsString); return plIdByBurgerservicenummer.get(burgerservicenummerAsLong); }