From 40889e2ab1f6f0cb5c7e0c852ec14c57c424591f Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 21 Aug 2024 15:01:47 +0200 Subject: [PATCH 1/7] Small cleanup --- examples/src/test/java/org/mapfish/print/PrintApiTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/src/test/java/org/mapfish/print/PrintApiTest.java b/examples/src/test/java/org/mapfish/print/PrintApiTest.java index 9664b64a3a..fbde22862c 100644 --- a/examples/src/test/java/org/mapfish/print/PrintApiTest.java +++ b/examples/src/test/java/org/mapfish/print/PrintApiTest.java @@ -44,7 +44,7 @@ public void testListApps() throws Exception { assertEquals(HttpStatus.OK, response.getStatusCode()); assertEquals(getJsonMediaType(), response.getHeaders().getContentType()); final JSONArray appIdsJson = new JSONArray(getBodyAsText(response)); - assertTrue(appIdsJson.length() > 0); + assertFalse(appIdsJson.isEmpty()); } } From b153f53b878eb79610b8c58015fa0645372268be Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 21 Aug 2024 20:17:45 +0200 Subject: [PATCH 2/7] Initial App Status implementation --- .../print/metrics/ApplicationStatus.java | 22 ++++++++++++++++ .../print/metrics/HealthCheckingRegistry.java | 13 ++++++++++ .../mapfish-spring-application-context.xml | 3 ++- .../org/mapfish/print/MetricsApiTest.java | 26 ++++++++++--------- 4 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java create mode 100644 core/src/main/java/org/mapfish/print/metrics/HealthCheckingRegistry.java diff --git a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java new file mode 100644 index 0000000000..cf262c976e --- /dev/null +++ b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java @@ -0,0 +1,22 @@ +package org.mapfish.print.metrics; + +import com.codahale.metrics.health.HealthCheck; +import org.mapfish.print.servlet.job.JobQueue; +import org.springframework.beans.factory.annotation.Autowired; + +class ApplicationStatus extends HealthCheck { + public static final int MAX_WAITING_JOBS_TILL_UNHEALTHY = 5; + @Autowired private JobQueue jobQueue; + + @Override + protected Result check() throws Exception { + long waitingJobsCount = jobQueue.getWaitingJobsCount(); + String health = "Number of jobs waiting is " + waitingJobsCount; + + if (waitingJobsCount >= MAX_WAITING_JOBS_TILL_UNHEALTHY) { + return Result.unhealthy(health + ". It is too high."); + } else { + return Result.healthy(health); + } + } +} diff --git a/core/src/main/java/org/mapfish/print/metrics/HealthCheckingRegistry.java b/core/src/main/java/org/mapfish/print/metrics/HealthCheckingRegistry.java new file mode 100644 index 0000000000..4e1f4f3b59 --- /dev/null +++ b/core/src/main/java/org/mapfish/print/metrics/HealthCheckingRegistry.java @@ -0,0 +1,13 @@ +package org.mapfish.print.metrics; + +import javax.annotation.PostConstruct; +import org.springframework.beans.factory.annotation.Autowired; + +public class HealthCheckingRegistry extends com.codahale.metrics.health.HealthCheckRegistry { + @Autowired private ApplicationStatus applicationStatus; + + @PostConstruct + public void registerHealthCheck() { + register("application", applicationStatus); + } +} diff --git a/core/src/main/resources/mapfish-spring-application-context.xml b/core/src/main/resources/mapfish-spring-application-context.xml index ff4ced5242..6583901b77 100644 --- a/core/src/main/resources/mapfish-spring-application-context.xml +++ b/core/src/main/resources/mapfish-spring-application-context.xml @@ -57,7 +57,8 @@ - + + diff --git a/examples/src/test/java/org/mapfish/print/MetricsApiTest.java b/examples/src/test/java/org/mapfish/print/MetricsApiTest.java index 2cbc6f2e1a..778d6c2f77 100644 --- a/examples/src/test/java/org/mapfish/print/MetricsApiTest.java +++ b/examples/src/test/java/org/mapfish/print/MetricsApiTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -26,7 +27,7 @@ public class MetricsApiTest extends AbstractApiTest { @Test public void testMetrics() throws Exception { - ClientHttpRequest request = getMetricsRequest("metrics", HttpMethod.GET); + ClientHttpRequest request = getMetricsRequest("metrics"); try (ClientHttpResponse response = request.execute()) { assertEquals(HttpStatus.OK, response.getStatusCode()); assertEquals(MediaType.APPLICATION_JSON, response.getHeaders().getContentType()); @@ -37,7 +38,7 @@ public void testMetrics() throws Exception { @Test public void testPing() throws Exception { - ClientHttpRequest request = getMetricsRequest("ping", HttpMethod.GET); + ClientHttpRequest request = getMetricsRequest("ping"); try (ClientHttpResponse response = request.execute()) { assertEquals(HttpStatus.OK, response.getStatusCode()); assertEquals("pong", getBodyAsText(response).trim()); @@ -46,7 +47,7 @@ public void testPing() throws Exception { @Test public void testThreads() throws Exception { - ClientHttpRequest request = getMetricsRequest("threads", HttpMethod.GET); + ClientHttpRequest request = getMetricsRequest("threads"); try (ClientHttpResponse response = request.execute()) { assertEquals(HttpStatus.OK, response.getStatusCode()); assertEquals(MediaType.TEXT_PLAIN, response.getHeaders().getContentType()); @@ -56,18 +57,19 @@ public void testThreads() throws Exception { @Test public void testHealthcheck() throws Exception { - ClientHttpRequest request = getMetricsRequest("healthcheck", HttpMethod.GET); + ClientHttpRequest request = getMetricsRequest("healthcheck"); try (ClientHttpResponse response = request.execute()) { - // TODO not implemented? - assertEquals(HttpStatus.NOT_IMPLEMENTED, response.getStatusCode()); - // assertEquals(HttpStatus.OK, response.getStatusCode()); - // assertEquals(MediaType.APPLICATION_JSON, response.getHeaders().getContentType()); - // assertNotNull(new JSONObject(getBodyAsText(response))); + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertEquals(MediaType.APPLICATION_JSON, response.getHeaders().getContentType()); + String bodyAsText = getBodyAsText(response); + assertNotNull(bodyAsText); + JSONObject healthcheck = new JSONObject(bodyAsText); + JSONObject application = healthcheck.getJSONObject("application"); + assertTrue(application.getBoolean("healthy")); } } - private ClientHttpRequest getMetricsRequest(String path, HttpMethod method) - throws IOException, URISyntaxException { - return getRequest("metrics/" + path, method); + private ClientHttpRequest getMetricsRequest(String path) throws IOException, URISyntaxException { + return getRequest("metrics/" + path, HttpMethod.GET); } } From b777400fc55fbebfd407c167443789d8d92a4a50 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Thu, 22 Aug 2024 08:40:59 +0200 Subject: [PATCH 3/7] CI check container health --- ci/validate-container | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/validate-container b/ci/validate-container index d98df53e7a..c9ca9ce984 100755 --- a/ci/validate-container +++ b/ci/validate-container @@ -23,7 +23,7 @@ fi docker run --detach --rm --name $container_name camptocamp/mapfish_print:latest sleep 15 http_code=$(docker exec $container_name curl -sL -w "%{http_code}" -I localhost:8080/metrics/healthcheck -o /dev/null) -if [[ $http_code = 501 ]]; then +if [[ $http_code = 200 ]]; then echo "container healthy" cleanup exit 0 From a2f751e5da17be0194f1a5d94e95daa1a11e0f35 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Sat, 24 Aug 2024 04:32:28 +0200 Subject: [PATCH 4/7] Validate server health ensuring print job are executed. --- .../print/metrics/ApplicationStatus.java | 48 +++++++++++++++++-- .../mapfish/print/servlet/job/JobManager.java | 14 ++++-- .../job/impl/ThreadPoolJobManager.java | 7 +++ .../main/resources/mapfish-spring.properties | 3 ++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java index cf262c976e..84da854684 100644 --- a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java +++ b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java @@ -1,22 +1,60 @@ package org.mapfish.print.metrics; import com.codahale.metrics.health.HealthCheck; +import java.time.Instant; +import java.time.temporal.TemporalAmount; +import java.util.Date; import org.mapfish.print.servlet.job.JobQueue; +import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; class ApplicationStatus extends HealthCheck { - public static final int MAX_WAITING_JOBS_TILL_UNHEALTHY = 5; + @Value("${expectedMaxTime.sinceLastPrint.InSeconds}") + private int secondsInFloatingWindow; + + private final TemporalAmount timeWindowSize = + java.time.Duration.ofSeconds(secondsInFloatingWindow); @Autowired private JobQueue jobQueue; + @Autowired private ThreadPoolJobManager jobManager; + + private long previousNumberOfWaitingJobs = 0L; @Override protected Result check() throws Exception { long waitingJobsCount = jobQueue.getWaitingJobsCount(); - String health = "Number of jobs waiting is " + waitingJobsCount; + if (waitingJobsCount == 0) { + previousNumberOfWaitingJobs = waitingJobsCount; + return Result.healthy("No print job is waiting in the queue."); + } - if (waitingJobsCount >= MAX_WAITING_JOBS_TILL_UNHEALTHY) { - return Result.unhealthy(health + ". It is too high."); + String health = "Number of print jobs waiting is " + waitingJobsCount; + + if (jobManager.getLastExecutedJobTimestamp() == null) { + return Result.unhealthy("No print job was ever processed by this server. " + health); + } else if (jobManager + .getLastExecutedJobTimestamp() + .toInstant() + .isAfter(getBeginningOfTimeWindow())) { + if (waitingJobsCount > previousNumberOfWaitingJobs) { + previousNumberOfWaitingJobs = waitingJobsCount; + return Result.unhealthy( + "Number of print jobs queued is increasing. But this server is processing them. " + + health); + } else { + previousNumberOfWaitingJobs = waitingJobsCount; + return Result.healthy( + "Print jobs are being dequeued. Number of print jobs waiting is " + waitingJobsCount); + } } else { - return Result.healthy(health); + previousNumberOfWaitingJobs = waitingJobsCount; + throw new RuntimeException( + "No print job was processed by this server, in the last (seconds): " + + secondsInFloatingWindow); } } + + private Instant getBeginningOfTimeWindow() { + return new Date().toInstant().minus(timeWindowSize); + } } diff --git a/core/src/main/java/org/mapfish/print/servlet/job/JobManager.java b/core/src/main/java/org/mapfish/print/servlet/job/JobManager.java index 7ea15b8d52..79b0a8a8ed 100644 --- a/core/src/main/java/org/mapfish/print/servlet/job/JobManager.java +++ b/core/src/main/java/org/mapfish/print/servlet/job/JobManager.java @@ -1,8 +1,9 @@ package org.mapfish.print.servlet.job; +import java.util.Date; + /** Manages and Executes Print Jobs. */ public interface JobManager { - /** * Submit a new job for execution. * @@ -14,7 +15,7 @@ public interface JobManager { * Cancel a job. * * @param referenceId The referenceId of the job to cancel. - * @throws NoSuchReferenceException + * @throws NoSuchReferenceException When trying to cancel an unknown referenceId */ void cancel(String referenceId) throws NoSuchReferenceException; @@ -22,7 +23,14 @@ public interface JobManager { * Get the status for a job. * * @param referenceId The referenceId of the job to check. - * @throws NoSuchReferenceException + * @throws NoSuchReferenceException When requesting status of an unknown referenceId. */ PrintJobStatus getStatus(String referenceId) throws NoSuchReferenceException; + + /** + * Instant at which a job was executed by this manager. + * + * @return the timestamp as a Date. + */ + Date getLastExecutedJobTimestamp(); } diff --git a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java b/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java index 8307e2dcb0..20dfe2b28d 100644 --- a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java +++ b/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.util.Collections; import java.util.Comparator; +import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -118,6 +119,7 @@ public class ThreadPoolJobManager implements JobManager { @Autowired private MetricRegistry metricRegistry; private boolean requestedToStop = false; + private Date lastExecutedJobTimestamp; public final void setMaxNumberOfRunningPrintJobs(final int maxNumberOfRunningPrintJobs) { this.maxNumberOfRunningPrintJobs = maxNumberOfRunningPrintJobs; @@ -290,7 +292,12 @@ public final void shutdown() { } } + public Date getLastExecutedJobTimestamp() { + return lastExecutedJobTimestamp; + } + private void executeJob(final PrintJob job) { + lastExecutedJobTimestamp = new Date(); LOGGER.debug( "executeJob {}, PoolSize {}, CorePoolSize {}, Active {}, Completed {}, Task {}, isShutdown" + " {}, isTerminated {}", diff --git a/core/src/main/resources/mapfish-spring.properties b/core/src/main/resources/mapfish-spring.properties index 7d59290bee..76f7af38c7 100644 --- a/core/src/main/resources/mapfish-spring.properties +++ b/core/src/main/resources/mapfish-spring.properties @@ -52,3 +52,6 @@ httpRequest.fetchRetry.maxNumber=3 # Number of milliseconds between 2 executions of the same request httpRequest.fetchRetry.intervalMillis=100 + +# Amount of time in the past where we check if a print job was executed by this server +expectedMaxTime.sinceLastPrint.InSeconds=300 From d50c8c8469cfce044b2a855af88b3596c63c4a49 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Wed, 28 Aug 2024 18:05:20 +0200 Subject: [PATCH 5/7] Ensure Time window is initialized and tested --- .../print/metrics/ApplicationStatus.java | 17 ++-- .../print/metrics/ApplicationStatusTest.java | 84 +++++++++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java diff --git a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java index 84da854684..0d6b7cc18b 100644 --- a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java +++ b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java @@ -1,8 +1,8 @@ package org.mapfish.print.metrics; import com.codahale.metrics.health.HealthCheck; +import java.time.Duration; import java.time.Instant; -import java.time.temporal.TemporalAmount; import java.util.Date; import org.mapfish.print.servlet.job.JobQueue; import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; @@ -13,8 +13,6 @@ class ApplicationStatus extends HealthCheck { @Value("${expectedMaxTime.sinceLastPrint.InSeconds}") private int secondsInFloatingWindow; - private final TemporalAmount timeWindowSize = - java.time.Duration.ofSeconds(secondsInFloatingWindow); @Autowired private JobQueue jobQueue; @Autowired private ThreadPoolJobManager jobManager; @@ -32,10 +30,7 @@ protected Result check() throws Exception { if (jobManager.getLastExecutedJobTimestamp() == null) { return Result.unhealthy("No print job was ever processed by this server. " + health); - } else if (jobManager - .getLastExecutedJobTimestamp() - .toInstant() - .isAfter(getBeginningOfTimeWindow())) { + } else if (hasJobExecutedRecently()) { if (waitingJobsCount > previousNumberOfWaitingJobs) { previousNumberOfWaitingJobs = waitingJobsCount; return Result.unhealthy( @@ -54,7 +49,13 @@ protected Result check() throws Exception { } } + private boolean hasJobExecutedRecently() { + final Instant lastExecutedJobTime = jobManager.getLastExecutedJobTimestamp().toInstant(); + final Instant beginningOfTimeWindow = getBeginningOfTimeWindow(); + return lastExecutedJobTime.isAfter(beginningOfTimeWindow); + } + private Instant getBeginningOfTimeWindow() { - return new Date().toInstant().minus(timeWindowSize); + return new Date().toInstant().minus(Duration.ofSeconds(secondsInFloatingWindow)); } } diff --git a/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java b/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java new file mode 100644 index 0000000000..6a5f4f0274 --- /dev/null +++ b/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java @@ -0,0 +1,84 @@ +package org.mapfish.print.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.when; + +import com.codahale.metrics.health.HealthCheck; +import java.util.Date; +import org.junit.Before; +import org.junit.Test; +import org.mapfish.print.AbstractMapfishSpringTest; +import org.mapfish.print.servlet.job.JobQueue; +import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.beans.factory.annotation.Autowired; + +public class ApplicationStatusTest extends AbstractMapfishSpringTest { + + @Mock private JobQueue jobQueue; + + @Mock private ThreadPoolJobManager jobManager; + + @Autowired @InjectMocks private ApplicationStatus applicationStatus; + + @Before + public void setUp() throws Exception { + // Initialize mocks created above + MockitoAnnotations.openMocks(this); + } + + @Test + public void testCheck_Success_NoPrintJobs() throws Exception { + when(jobQueue.getWaitingJobsCount()).thenReturn(0L); + + HealthCheck.Result result = applicationStatus.check(); + + assertTrue(result.isHealthy()); + assertEquals("No print job is waiting in the queue.", result.getMessage()); + } + + @Test + public void testCheck_Failed_NoPrintJobs() throws Exception { + when(jobQueue.getWaitingJobsCount()).thenReturn(1L); + when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date(0L)); + + try { + applicationStatus.check(); + fail("Expected exception not thrown"); + } catch (RuntimeException e) { + assertEquals( + "No print job was processed by this server, in the last (seconds): 300", e.getMessage()); + } catch (Exception e) { + fail("Incorrect Exception thrown: " + e); + } + } + + @Test + public void testCheck_Success_PrintJobs() throws Exception { + when(jobQueue.getWaitingJobsCount()).thenReturn(10L, 9L); + when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date()); + + applicationStatus.check(); + HealthCheck.Result result = applicationStatus.check(); + + assertTrue(result.isHealthy()); + assertTrue(result.getMessage().contains("Print jobs are being dequeued")); + } + + @Test + public void testCheck_Fail_PrintJobsButIncreasing() throws Exception { + when(jobQueue.getWaitingJobsCount()).thenReturn(0L, 1L); + when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date()); + + applicationStatus.check(); + HealthCheck.Result result = applicationStatus.check(); + + assertFalse(result.isHealthy()); + assertTrue(result.getMessage().contains("Number of print jobs queued is increasing.")); + } +} From 0e982200a84e7da72590765c1a29542d138e708d Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 30 Aug 2024 00:49:12 +0200 Subject: [PATCH 6/7] Introduce new printing queue threshold --- .../print/metrics/ApplicationStatus.java | 42 ++++++++++--------- .../main/resources/mapfish-spring.properties | 5 ++- .../print/metrics/ApplicationStatusTest.java | 32 +++++++------- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java index 0d6b7cc18b..a5bc3527e0 100644 --- a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java +++ b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java @@ -10,46 +10,50 @@ import org.springframework.beans.factory.annotation.Value; class ApplicationStatus extends HealthCheck { - @Value("${expectedMaxTime.sinceLastPrint.InSeconds}") + @Value("${healthStatus.expectedMaxTime.sinceLastPrint.InSeconds}") private int secondsInFloatingWindow; + @Value("${healthStatus.unhealthyThreshold.maxNbrPrintJobQueued}") + private int maxNbrPrintJobQueued; + @Autowired private JobQueue jobQueue; @Autowired private ThreadPoolJobManager jobManager; - private long previousNumberOfWaitingJobs = 0L; - + /** + * When a Result is returned it can be healthy or unhealthy. In both cases it is associated to a + * Http 200 status. When an exception is thrown it means the server is no longer working at all, + * it is associated to a Http 500 status. + */ @Override protected Result check() throws Exception { long waitingJobsCount = jobQueue.getWaitingJobsCount(); if (waitingJobsCount == 0) { - previousNumberOfWaitingJobs = waitingJobsCount; return Result.healthy("No print job is waiting in the queue."); } - String health = "Number of print jobs waiting is " + waitingJobsCount; + String health = ". Number of print jobs waiting is " + waitingJobsCount; if (jobManager.getLastExecutedJobTimestamp() == null) { - return Result.unhealthy("No print job was ever processed by this server. " + health); - } else if (hasJobExecutedRecently()) { - if (waitingJobsCount > previousNumberOfWaitingJobs) { - previousNumberOfWaitingJobs = waitingJobsCount; + return Result.unhealthy("No print job was ever processed by this server" + health); + } else if (hasThisServerPrintedRecently()) { + if (waitingJobsCount > maxNbrPrintJobQueued) { return Result.unhealthy( - "Number of print jobs queued is increasing. But this server is processing them. " - + health); + "Number of print jobs queued is above threshold: " + maxNbrPrintJobQueued + health); } else { - previousNumberOfWaitingJobs = waitingJobsCount; - return Result.healthy( - "Print jobs are being dequeued. Number of print jobs waiting is " + waitingJobsCount); + return Result.healthy("This server instance is printing" + health); } } else { - previousNumberOfWaitingJobs = waitingJobsCount; - throw new RuntimeException( - "No print job was processed by this server, in the last (seconds): " - + secondsInFloatingWindow); + throw notificationForBrokenServer(); } } - private boolean hasJobExecutedRecently() { + private RuntimeException notificationForBrokenServer() { + return new RuntimeException( + "None of the print job queued was processed by this server, in the last (seconds): " + + secondsInFloatingWindow); + } + + private boolean hasThisServerPrintedRecently() { final Instant lastExecutedJobTime = jobManager.getLastExecutedJobTimestamp().toInstant(); final Instant beginningOfTimeWindow = getBeginningOfTimeWindow(); return lastExecutedJobTime.isAfter(beginningOfTimeWindow); diff --git a/core/src/main/resources/mapfish-spring.properties b/core/src/main/resources/mapfish-spring.properties index 76f7af38c7..9bc9b61347 100644 --- a/core/src/main/resources/mapfish-spring.properties +++ b/core/src/main/resources/mapfish-spring.properties @@ -54,4 +54,7 @@ httpRequest.fetchRetry.maxNumber=3 httpRequest.fetchRetry.intervalMillis=100 # Amount of time in the past where we check if a print job was executed by this server -expectedMaxTime.sinceLastPrint.InSeconds=300 +healthStatus.expectedMaxTime.sinceLastPrint.InSeconds=300 + +# Maximum number of Print Jobs queued before raising it i +healthStatus.unhealthyThreshold.maxNbrPrintJobQueued=4 diff --git a/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java b/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java index 6a5f4f0274..e98c8a7737 100644 --- a/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java +++ b/core/src/test/java/org/mapfish/print/metrics/ApplicationStatusTest.java @@ -2,8 +2,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrowsExactly; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.when; import com.codahale.metrics.health.HealthCheck; @@ -47,38 +47,40 @@ public void testCheck_Failed_NoPrintJobs() throws Exception { when(jobQueue.getWaitingJobsCount()).thenReturn(1L); when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date(0L)); - try { - applicationStatus.check(); - fail("Expected exception not thrown"); - } catch (RuntimeException e) { - assertEquals( - "No print job was processed by this server, in the last (seconds): 300", e.getMessage()); - } catch (Exception e) { - fail("Incorrect Exception thrown: " + e); - } + RuntimeException rte = + assertThrowsExactly( + RuntimeException.class, + () -> { + // WHEN + applicationStatus.check(); + }); + + assertEquals( + "None of the print job queued was processed by this server, in the last (seconds): 300", + rte.getMessage()); } @Test public void testCheck_Success_PrintJobs() throws Exception { - when(jobQueue.getWaitingJobsCount()).thenReturn(10L, 9L); + when(jobQueue.getWaitingJobsCount()).thenReturn(5L, 4L); when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date()); applicationStatus.check(); HealthCheck.Result result = applicationStatus.check(); assertTrue(result.isHealthy()); - assertTrue(result.getMessage().contains("Print jobs are being dequeued")); + assertTrue(result.getMessage().contains("This server instance is printing.")); } @Test - public void testCheck_Fail_PrintJobsButIncreasing() throws Exception { - when(jobQueue.getWaitingJobsCount()).thenReturn(0L, 1L); + public void testCheck_Fail_TooManyJobsAreQueued() throws Exception { + when(jobQueue.getWaitingJobsCount()).thenReturn(4L, 5L); when(jobManager.getLastExecutedJobTimestamp()).thenReturn(new Date()); applicationStatus.check(); HealthCheck.Result result = applicationStatus.check(); assertFalse(result.isHealthy()); - assertTrue(result.getMessage().contains("Number of print jobs queued is increasing.")); + assertTrue(result.getMessage().contains("Number of print jobs queued is above threshold: ")); } } From 01314cc43f8b6ba1bf997f5b567d4762d4750a32 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Tue, 3 Sep 2024 12:50:31 +0200 Subject: [PATCH 7/7] WIP: see MFP issue 3393 to add missing observability. --- .../java/org/mapfish/print/metrics/ApplicationStatus.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java index a5bc3527e0..96b214924f 100644 --- a/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java +++ b/core/src/main/java/org/mapfish/print/metrics/ApplicationStatus.java @@ -36,9 +36,12 @@ protected Result check() throws Exception { if (jobManager.getLastExecutedJobTimestamp() == null) { return Result.unhealthy("No print job was ever processed by this server" + health); } else if (hasThisServerPrintedRecently()) { + // WIP (See issue https://github.com/mapfish/mapfish-print/issues/3393) if (waitingJobsCount > maxNbrPrintJobQueued) { return Result.unhealthy( - "Number of print jobs queued is above threshold: " + maxNbrPrintJobQueued + health); + "WIP: Number of print jobs queued is above threshold: " + + maxNbrPrintJobQueued + + health); } else { return Result.healthy("This server instance is printing" + health); }