Merge pull request #1915 from kekey1/OCD-5174

fix: Add logging, exception/null handling to SBUL creator job

Author: kekey1

chpl/chpl-api/pom.xml

@@ -126,13 +126,6 @@
             </exclusions>
         </dependency>
 
-        <!-- Rate limiting -->
-        <dependency>
-            <groupId>com.github.vladimir-bukhtoyarov</groupId>
-            <artifactId>bucket4j-core</artifactId>
-            <version>7.6.0</version>
-        </dependency>
-
         <!-- provided dependencies -->
         <dependency>
             <groupId>org.projectlombok</groupId>

chpl/chpl-api/src/main/java/gov/healthit/chpl/ratelimiting/RateLimitingInterceptor.java

@@ -6,18 +6,15 @@
 import java.util.List;
 import java.util.Map;
 
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.http.HttpStatus;
 import org.springframework.web.servlet.HandlerInterceptor;
 
 import gov.healthit.chpl.api.dao.ApiKeyDAO;
 import gov.healthit.chpl.util.ErrorMessageUtil;
-import io.github.bucket4j.Bandwidth;
 import io.github.bucket4j.Bucket;
-import io.github.bucket4j.Refill;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import lombok.extern.log4j.Log4j2;
 
 @Log4j2
@@ -66,10 +63,12 @@ public boolean preHandle(final HttpServletRequest request,
 
     private Bucket getBucketForApiKey(String apiKey) {
         if (!rateLimitingBuckets.containsKey(apiKey)) {
+
             rateLimitingBuckets.put(apiKey, Bucket.builder()
-                    .addLimit(Bandwidth.classic(rateLimitRequestCount,
-                            Refill.intervally(rateLimitRequestCount, Duration.ofSeconds(rateLimitTimePeriod))))
-            .build());
+                    .addLimit(limit -> limit
+                            .capacity(rateLimitRequestCount)
+                            .refillIntervally(rateLimitRequestCount, Duration.ofSeconds(rateLimitTimePeriod)))
+                            .build());
         }
         return rateLimitingBuckets.get(apiKey);
     }

chpl/chpl-service/pom.xml

@@ -181,17 +181,6 @@
               </exclusion>
             </exclusions>
         </dependency>
-        <!--dependency>
-            <groupId>commons-fileupload</groupId>
-            <artifactId>commons-fileupload</artifactId>
-            <version>${commons.fileupload.version}</version>
-            <exclusions>
-              <exclusion>
-                <groupId>commons-io</groupId>
-                <artifactId>commons-io</artifactId>
-              </exclusion>
-            </exclusions>
-        </dependency-->
         <dependency>
             <groupId>commons-io</groupId>
             <artifactId>commons-io</artifactId>
@@ -361,6 +350,14 @@
             <artifactId>datadog-api-client</artifactId>
             <version>2.48.0</version>
         </dependency>
+        <!-- Provides throttling and rate-limiting behavior-->
+        <!-- Specifically being used to slow down our requests to the Datadog API to avoid being rate-limited -->
+        <dependency>
+            <groupId>com.bucket4j</groupId>
+            <artifactId>bucket4j_jdk17-core</artifactId>
+            <version>8.16.1</version>
+            <scope>compile</scope>
+        </dependency>
 
         <!-- jars provided by the system and not included in the packaged jar -->
         <dependency>

chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/urluptime/DatadogSyntheticsTestResultService.java

@@ -70,7 +70,8 @@ public SyntheticsAPITestResultFull getDetailedTestResult(String publicTestKey, S
         try {
             return apiProvider.getApiInstance().getAPITestResult(publicTestKey, resultId);
         } catch (ApiException e) {
-            LOGGER.error("Could not retrieve test result details for: {} | {}", publicTestKey, resultId);
+            LOGGER.error("Could not retrieve test result details for: {} | {}", publicTestKey, resultId, e);
+            LOGGER.error("Datadog API error response: " + e.getResponseBody());
             return null;
         }
     }

chpl/chpl-service/src/main/java/gov/healthit/chpl/scheduler/job/urluptime/DatadogUrlUptimeSynchonizer.java

@@ -1,6 +1,7 @@
 package gov.healthit.chpl.scheduler.job.urluptime;
 
 import java.time.DayOfWeek;
+import java.time.Duration;
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -30,24 +31,28 @@
 import gov.healthit.chpl.domain.CertificationBody;
 import gov.healthit.chpl.domain.Developer;
 import gov.healthit.chpl.exception.ValidationException;
+import io.github.bucket4j.Bucket;
 import lombok.extern.log4j.Log4j2;
 
 @Log4j2(topic = "serviceBaseUrlListUptimeCreatorJobLogger")
 @Component
 public class DatadogUrlUptimeSynchonizer {
     private static final Long DAYS_TO_LOOK_BACK_FOR_RESULTS = 7L;
+    private static final long DATADOG_REQUESTS_PER_MINUTE = 90; //the actual limit is 100 but I don't feel like we need to cut it that close
+    private static final long DATADOG_REQUESTS_BURST_LIMIT = 5;
 
     // VARIABLE NAMING CONVENTION
     // ServiceBaseUrlList - these are service base url lists collected from the listings in CHPL, grouped by developer
     // DatadogSyntheticsTest - these are synthetic tests that exist in Datadog
-    // UrlUptimeMonitor - these are reporting entities that are store in the table url_uptime_monitors
+    // UrlUptimeMonitor - these are reporting entities that are store in the table url_uptime_monitor
 
     private DatadogSyntheticsTestService datadogSyntheticsTestService;
     private DatadogSyntheticsTestResultService datadogSyntheticsTestResultService;
     private ServiceBaseUrlListService serviceBaseUrlListService;
     private UrlUptimeMonitorDAO urlUptimeMonitorDAO;
     private UrlUptimeMonitorTestDAO urlUptimeMonitorTestDAO;
     private DeveloperSearchService developerSearchService;
+    private Bucket bucket;
 
     private List<String> errorsToIgnore;
 
@@ -61,7 +66,16 @@ public DatadogUrlUptimeSynchonizer(DatadogSyntheticsTestService datadogSynthetic
         this.urlUptimeMonitorDAO = urlUptimeMonitorDAO;
         this.urlUptimeMonitorTestDAO = urlUptimeMonitorTestDAO;
         this.developerSearchService = developerSearchService;
-
+        this.bucket = Bucket.builder()
+                // 1. Sustained Limit: 90 requests per 1 minute
+                .addLimit(limit -> limit
+                    .capacity(DATADOG_REQUESTS_PER_MINUTE)
+                    .refillGreedy(DATADOG_REQUESTS_PER_MINUTE, Duration.ofMinutes(1)))
+                // 2. Burst Limit: Maximum 5 requests per 1 second
+                .addLimit(limit -> limit
+                    .capacity(DATADOG_REQUESTS_BURST_LIMIT)
+                    .refillGreedy(DATADOG_REQUESTS_BURST_LIMIT, Duration.ofSeconds(1)))
+                .build();
         errorsToIgnore = List.of("BODY_TOO_LARGE_TO_PROCESS");
     }
 
@@ -82,12 +96,18 @@ private void synchronizeUrlUptimeMonitorTestsWithDatadogSyntheticsTestResults()
                 .forEach(testDate -> urlUptimeMonitorDAO.getAll().forEach(urlUptimeMonitor -> {
                         String publicId = getDatadogPublicId(syntheticsTestDetails, urlUptimeMonitor.getUrl(), urlUptimeMonitor.getDeveloper().getId());
                         datadogSyntheticsTestResultService.getSyntheticsTestResults(publicId, testDate).forEach(syntheticsTestResult -> {
+                            try {
+                                //Blocks until tokens for BOTH limits are available
+                                bucket.asBlocking().consumeUninterruptibly(1);
                                 urlUptimeMonitorTestDAO.create(UrlUptimeMonitorTest.builder()
                                         .urlUptimeMonitorId(urlUptimeMonitor.getId())
                                         .datadogTestKey(syntheticsTestResult.getResultId())
                                         .checkTime(toLocalDateTime(syntheticsTestResult.getCheckTime().longValue()))
                                         .passed(calculatePassed(syntheticsTestResult, publicId))
                                         .build());
+                            } catch (Exception ex) {
+                                LOGGER.error("Could not process url_uptime_monitor " + publicId + " for url " + urlUptimeMonitor.getUrl() + " and developer " + urlUptimeMonitor.getDeveloper().getId());
+                            }
                         });
                 }));
     }
@@ -97,7 +117,7 @@ private boolean calculatePassed(SyntheticsAPITestResultShort result, String publ
             return true;
         } else {
             SyntheticsAPITestResultFull detailedResult = datadogSyntheticsTestResultService.getDetailedTestResult(publicId, result.getResultId());
-            return isErrorIgnorable(detailedResult.getResult().getFailure().getCode());
+            return detailedResult != null && isErrorIgnorable(detailedResult.getResult().getFailure().getCode());
         }
     }
 
@@ -199,7 +219,7 @@ private void removeOutdatedUrlMonitors(List<UrlUptimeMonitor> existing, List<Url
         existing.stream()
                 .filter(uum -> !contains(expected, uum))
                 .forEach(urlMonitor -> {
-                        LOGGER.info("Removing the following URL to url_uptime_monitor table: {}, {}", urlMonitor.getUrl(), urlMonitor.getDeveloper().getId());
+                        LOGGER.info("Removing the following URL to url_uptime_monitor table: {}, {}, {}", urlMonitor.getUrl(), urlMonitor.getDeveloper().getId(), urlMonitor.getDatadogPublicId());
                         urlUptimeMonitorDAO.delete(urlMonitor);
                 });
     }