From 2204c1107e8edb85dcdffc515c67b20f0f03d8a1 Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 15:33:43 -0400 Subject: [PATCH 1/7] activate cloud profile and set sentry logs level --- web/service/Dockerfile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/service/Dockerfile b/web/service/Dockerfile index bf04982bd1..c62e5ecbb3 100644 --- a/web/service/Dockerfile +++ b/web/service/Dockerfile @@ -26,4 +26,8 @@ COPY ${JAR_FILES} / ADD ./newrelic/newrelic.jar /newrelic/newrelic.jar +ENV SPRING_PROFILES_ACTIVE=cloud +ENV SENTRY_LOGGING_MINIMUM_EVENT_LEVEL=error +ENV SENTRY_LOGGING_MINIMUM_BREADCRUMB_LEVEL=info + ENTRYPOINT exec java -Xmx12g -javaagent:/newrelic/newrelic.jar -jar /service-${CURRENT_VERSION}.jar From 9189828d60e9951037e6684896cdf6bb8eab8359 Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 17:10:52 -0400 Subject: [PATCH 2/7] remove base logging to avoid logs duplication --- .../src/main/resources/{logback.xml => logback-spring.xml} | 2 -- 1 file changed, 2 deletions(-) rename web/service/src/main/resources/{logback.xml => logback-spring.xml} (94%) diff --git a/web/service/src/main/resources/logback.xml b/web/service/src/main/resources/logback-spring.xml similarity index 94% rename from web/service/src/main/resources/logback.xml rename to web/service/src/main/resources/logback-spring.xml index 209422a0a4..cc600a1420 100644 --- a/web/service/src/main/resources/logback.xml +++ b/web/service/src/main/resources/logback-spring.xml @@ -1,7 +1,5 @@ - - From 627df4cfb06511b1e86bbb86459c26f78b9854ee Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 18:37:57 -0400 Subject: [PATCH 3/7] add job id in logs change levels to finest --- .../performance/MemoryUsageRegister.java | 2 +- .../gtfsvalidator/util/VersionResolver.java | 2 +- .../controller/ValidationController.java | 22 +++++++++++++++---- .../web/service/util/StorageHelper.java | 4 ++-- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java index 93e7c6980e..d3be3c2762 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/performance/MemoryUsageRegister.java @@ -82,7 +82,7 @@ public MemoryUsage registerMemoryUsage(String key, MemoryUsage previous) { */ public void registerMemoryUsage(MemoryUsage memoryUsage) { registry.add(memoryUsage); - logger.atInfo().log(memoryUsage.humanReadablePrint()); + logger.atFinest().log(memoryUsage.humanReadablePrint()); } /** Clears the memory usage registry. */ diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/util/VersionResolver.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/util/VersionResolver.java index b6b527fada..04afef795f 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/util/VersionResolver.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/util/VersionResolver.java @@ -171,7 +171,7 @@ private Optional resolveLatestReleaseVersion(Optional currentVer Gson gson = new GsonBuilder().create(); VersionResponse response = gson.fromJson(in, VersionResponse.class); if (response != null && !Strings.isNullOrEmpty(response.version)) { - logger.atInfo().log("resolved release version=%s", response.version); + logger.atFinest().log("resolved release version=%s", response.version); return Optional.of(response.version); } } diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java index ef1f49c3d1..845f12e5d6 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java @@ -30,6 +30,7 @@ import org.mobilitydata.gtfsvalidator.web.service.util.ValidationJobMetaData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.slf4j.MDC; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -39,6 +40,7 @@ @RestController public class ValidationController { + public static final String JOB_ID = "job_id"; private final Logger logger = LoggerFactory.getLogger(ValidationController.class); @Autowired private StorageHelper storageHelper; @@ -110,6 +112,9 @@ public ResponseEntity runValidator( ValidationJobMetaData jobData = getFeedFileMetaData(message); jobId = jobData.getJobId(); + MDC.put(JOB_ID, jobId); + + logger.info("Validation started for job ID: {}", jobId); var fileName = jobData.getFileName(); @@ -142,12 +147,21 @@ public ResponseEntity runValidator( Sentry.captureException(exc); throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error", exc); } finally { + MDC.remove(JOB_ID); // delete the temp file and directory - if (tempFile != null) { - tempFile.delete(); - } + safeDeleteFile(tempFile); if (outputPath != null) { - outputPath.toFile().delete(); + safeDeleteFile(outputPath.toFile()); + } + } + } + + private void safeDeleteFile(File file) { + if (file != null && file.exists()) { + try { + file.delete(); + } catch (Exception e) { + logger.warn("Error deleting file: {}", file.getAbsolutePath(), e); } } } diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java index 6ea6b8210a..3bf8991e71 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java @@ -204,10 +204,10 @@ public void writeExecutionResultFile( Path executionResultPath = outputPath.resolve(executionResultFile); try { - logger.info("Writing executionResult file to " + executionResultFile); + logger.debug("Writing executionResult file to " + executionResultFile); Files.write( executionResultPath, gson.toJson(executionResult).getBytes(StandardCharsets.UTF_8)); - logger.info(executionResultFile + " file written successfully"); + logger.debug(executionResultFile + " file written successfully"); } catch (IOException e) { logger.error("Error writing to file " + executionResultFile); e.printStackTrace(); From 9548e53946c1ad27aebf8fd0c92aa8e55c91dcd1 Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 18:56:16 -0400 Subject: [PATCH 4/7] fix failing test --- .../web/service/controller/RunValidatorEndpointTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/RunValidatorEndpointTest.java b/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/RunValidatorEndpointTest.java index 0873a34cfb..c12f88a2f6 100644 --- a/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/RunValidatorEndpointTest.java +++ b/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/RunValidatorEndpointTest.java @@ -83,6 +83,8 @@ public void runValidatorSuccess() throws Exception { .when(storageHelper) .downloadFeedFileFromStorage(anyString(), anyString()); + doReturn(true).when(mockFeedFile).exists(); + doReturn(true).when(mockOutputPathToFile).exists(); mockMvc .perform( MockMvcRequestBuilders.post("/run-validator") @@ -101,7 +103,9 @@ public void runValidatorSuccess() throws Exception { verify(storageHelper, times(1)).uploadFilesToStorage(testJobId, mockOutputPath); // verify that the temp files and directory are deleted + verify(mockFeedFile, times(1)).exists(); verify(mockFeedFile, times(1)).delete(); + verify(mockOutputPathToFile, times(1)).exists(); verify(mockOutputPathToFile, times(1)).delete(); } From 0e87cbfdcf5dd5343a965b2c51d124921a8f6069 Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 20:05:29 -0400 Subject: [PATCH 5/7] reduce noise in logs --- .../web/service/util/StorageHelper.java | 6 +++--- .../web/service/util/ValidationHandler.java | 2 +- web/service/src/main/resources/logback-spring.xml | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java index 3bf8991e71..d4b2146a47 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java @@ -73,7 +73,7 @@ public void saveJobMetadata(JobMetadata metadata) throws Exception { var jobBlobInfo = BlobInfo.newBuilder(jobBlobId).setContentType("application/json").build(); var om = new ObjectMapper(); var json = om.writeValueAsString(metadata); - logger.info("Saving job metadata: " + json); + logger.debug("Saving job metadata: {}", json); storage.create(jobBlobInfo, json.getBytes()); } catch (Exception exc) { logger.error("Error setting country code", exc); @@ -93,13 +93,13 @@ public JobMetadata getJobMetadata(String jobId) { var jobBlobId = BlobId.of(JOB_INFO_BUCKET_NAME, jobInfoPath); Blob blob = storage.get(jobBlobId); var json = new String(blob.getContent()); - logger.info("Loading job metadata: " + json); + logger.debug("Loading job metadata: {}", json); var objectMapper = new ObjectMapper(); JobMetadata jobMetadata = objectMapper.readValue(json, JobMetadata.class); return jobMetadata; } catch (Exception exc) { - logger.error("Error could not load remote file, using default country code", exc); + logger.debug("No metadata found using default country code"); return new JobMetadata(jobId, ""); } } diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/ValidationHandler.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/ValidationHandler.java index 1765cbd7b3..aae34de976 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/ValidationHandler.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/ValidationHandler.java @@ -35,7 +35,7 @@ public void validateFeed(@NonNull File feedFile, @NonNull Path outputPath, Strin .setOutputDirectory(outputPath); if (!countryCode.isEmpty()) { var country = CountryCode.forStringOrUnknown(countryCode); - logger.info("setting country code: " + country.getCountryCode()); + logger.debug("setting country code: {}", country.getCountryCode()); configBuilder.setCountryCode(CountryCode.forStringOrUnknown(countryCode)); } var config = configBuilder.build(); diff --git a/web/service/src/main/resources/logback-spring.xml b/web/service/src/main/resources/logback-spring.xml index cc600a1420..aac6513670 100644 --- a/web/service/src/main/resources/logback-spring.xml +++ b/web/service/src/main/resources/logback-spring.xml @@ -24,6 +24,20 @@ + + + + + + + + + + + + + + From 17061eb000355873380b3faded8ab63176ed39c3 Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Mon, 5 May 2025 20:34:13 -0400 Subject: [PATCH 6/7] remove Spring starting logs --- web/service/Dockerfile | 1 + .../web/service/util/LoggingBridgeConfig.java | 21 +++++++++++++++++++ .../src/main/resources/logback-spring.xml | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java diff --git a/web/service/Dockerfile b/web/service/Dockerfile index c62e5ecbb3..5e3c2ed7a1 100644 --- a/web/service/Dockerfile +++ b/web/service/Dockerfile @@ -27,6 +27,7 @@ COPY ${JAR_FILES} / ADD ./newrelic/newrelic.jar /newrelic/newrelic.jar ENV SPRING_PROFILES_ACTIVE=cloud +ENV SPRING_MAIN_BANNER-MODE=off ENV SENTRY_LOGGING_MINIMUM_EVENT_LEVEL=error ENV SENTRY_LOGGING_MINIMUM_BREADCRUMB_LEVEL=info diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java new file mode 100644 index 0000000000..328f54d02c --- /dev/null +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java @@ -0,0 +1,21 @@ +package org.mobilitydata.gtfsvalidator.web.service.util; + +import java.util.logging.LogManager; +import java.util.logging.Logger; +import javax.annotation.PostConstruct; +import org.slf4j.bridge.SLF4JBridgeHandler; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class LoggingBridgeConfig { + + @PostConstruct + public void setupJulToSlf4jBridge() { + // Reset JUL root logger and install SLF4J bridge + LogManager.getLogManager().reset(); + SLF4JBridgeHandler.removeHandlersForRootLogger(); + SLF4JBridgeHandler.install(); + + Logger rootLogger = Logger.getLogger(""); + } +} diff --git a/web/service/src/main/resources/logback-spring.xml b/web/service/src/main/resources/logback-spring.xml index aac6513670..e6dcef720d 100644 --- a/web/service/src/main/resources/logback-spring.xml +++ b/web/service/src/main/resources/logback-spring.xml @@ -34,7 +34,7 @@ - + From 10f43a6b4573024daf99872c4b42d3e736446e1a Mon Sep 17 00:00:00 2001 From: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com> Date: Tue, 6 May 2025 11:25:36 -0400 Subject: [PATCH 7/7] add methods documentation --- .../web/service/controller/ValidationController.java | 6 ++++++ .../web/service/util/LoggingBridgeConfig.java | 12 ++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java index 845f12e5d6..3377b6a146 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java @@ -156,6 +156,12 @@ public ResponseEntity runValidator( } } + /** + * Deletes the temp file and directory. Exception is logged but not thrown as it's not considered + * a validation error. + * + * @param file to be deleted + */ private void safeDeleteFile(File file) { if (file != null && file.exists()) { try { diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java index 328f54d02c..0a4afd9f89 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/LoggingBridgeConfig.java @@ -1,21 +1,25 @@ package org.mobilitydata.gtfsvalidator.web.service.util; import java.util.logging.LogManager; -import java.util.logging.Logger; import javax.annotation.PostConstruct; import org.slf4j.bridge.SLF4JBridgeHandler; import org.springframework.context.annotation.Configuration; +/** + * This class sets up a bridge between Java Util Logging (JUL) and SLF4J. It resets the JUL root + * logger and installs the SLF4J bridge handler to redirect JUL log messages to SLF4J. + */ @Configuration public class LoggingBridgeConfig { + /** + * This method is called after the bean is constructed. It resets the JUL root logger and installs + * the SLF4J bridge handler. + */ @PostConstruct public void setupJulToSlf4jBridge() { - // Reset JUL root logger and install SLF4J bridge LogManager.getLogManager().reset(); SLF4JBridgeHandler.removeHandlersForRootLogger(); SLF4JBridgeHandler.install(); - - Logger rootLogger = Logger.getLogger(""); } }