PI-2851 Improve Telemetry for cron jobs + fix hanging jobs (#4693)

ministryofjustice · Feb 20, 2025 · 03acdb4 · 03acdb4
1 parent e47aa70
commit 03acdb4
Show file tree

Hide file tree

Showing 10 changed files with 105 additions and 22 deletions.
diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/job/CronJobHelper.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/job/CronJobHelper.kt
@@ -0,0 +1,24 @@
+package uk.gov.justice.digital.hmpps.job
+
+import io.opentelemetry.api.trace.Span
+import io.sentry.Sentry
+import org.springframework.boot.SpringApplication.exit
+import org.springframework.context.ApplicationContext
+import kotlin.system.exitProcess
+
+object CronJobHelper {
+    fun ApplicationContext.runThenExit(
+        exit: (code: Int) -> Unit = { exitProcess(exit(this, { it })) },
+        job: () -> Unit
+    ) {
+        try {
+            job.invoke()
+            exit(0)
+        } catch (e: Exception) {
+            Sentry.captureException(e)
+            Span.current().recordException(e)
+        } finally {
+            exit(1)
+        }
+    }
+}
diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/logging/LazyLogger.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/logging/LazyLogger.kt
@@ -3,6 +3,8 @@ package uk.gov.justice.digital.hmpps.logging
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
-fun <T : Any> T.logger(): Lazy<Logger> {
-    return lazy { LoggerFactory.getLogger(this::class.java) }
-}
+object LazyLogger {
+    fun <T : Any> T.logger(): Lazy<Logger> {
+        return lazy { LoggerFactory.getLogger(this::class.java) }
+    }
+}
diff --git a/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/logging/Logger.kt b/libs/commons/src/main/kotlin/uk/gov/justice/digital/hmpps/logging/Logger.kt
@@ -0,0 +1,8 @@
+package uk.gov.justice.digital.hmpps.logging
+
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+object Logger {
+    fun <T : Any> T.logger(): Logger = LoggerFactory.getLogger(this::class.java)
+}
diff --git a/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/job/CronJobHelperTest.kt b/libs/commons/src/test/kotlin/uk/gov/justice/digital/hmpps/job/CronJobHelperTest.kt
@@ -0,0 +1,45 @@
+package uk.gov.justice.digital.hmpps.job
+
+import io.sentry.Sentry
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.mockito.Mock
+import org.mockito.Mockito.mockStatic
+import org.mockito.junit.jupiter.MockitoExtension
+import org.mockito.kotlin.any
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
+import org.mockito.kotlin.verify
+import org.springframework.context.ApplicationContext
+import uk.gov.justice.digital.hmpps.job.CronJobHelper.runThenExit
+
+@ExtendWith(MockitoExtension::class)
+class CronJobHelperTest {
+    @Mock
+    private lateinit var applicationContext: ApplicationContext
+
+    @Test
+    fun `captures exception and exits with code 1 on error`() {
+        mockStatic(Sentry::class.java).use { sentry ->
+            val exception = RuntimeException("error")
+            val exitMock = mock<(code: Int) -> Unit>()
+
+            applicationContext.runThenExit(exit = exitMock) { throw exception }
+
+            sentry.verify { Sentry.captureException(exception) }
+            verify(exitMock).invoke(1)
+        }
+    }
+
+    @Test
+    fun `runs successfully`() {
+        mockStatic(Sentry::class.java).use { sentry ->
+            val exitMock = mock<(code: Int) -> Unit>()
+
+            applicationContext.runThenExit(exit = exitMock) { }
+
+            sentry.verify({ Sentry.captureException(any()) }, never())
+            verify(exitMock).invoke(0)
+        }
+    }
+}
diff --git a/projects/appointment-reminders-and-delius/deploy/values-dev.yml b/projects/appointment-reminders-and-delius/deploy/values-dev.yml
@@ -20,6 +20,7 @@ jobs:
       templates:
         - 9405e109-1dcf-4df1-8788-5ec9b72db8a0
     wales:
+      schedule: '0 10 * * *' # 10am UTC every day
       templates:
         - 9405e109-1dcf-4df1-8788-5ec9b72db8a0 # English
         - c29e156e-45d6-478a-a110-71d4796cbc29 # Welsh
diff --git a/...delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/UpwAppointmentRemindersJob.kt b/...delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/UpwAppointmentRemindersJob.kt
@@ -1,14 +1,14 @@
 package uk.gov.justice.digital.hmpps.cronjob
 
+import io.opentelemetry.instrumentation.annotations.WithSpan
 import org.springframework.beans.factory.annotation.Value
-import org.springframework.boot.SpringApplication.exit
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.event.ApplicationStartedEvent
 import org.springframework.context.ApplicationContext
 import org.springframework.context.ApplicationListener
 import org.springframework.stereotype.Component
+import uk.gov.justice.digital.hmpps.job.CronJobHelper.runThenExit
 import uk.gov.justice.digital.hmpps.service.UnpaidWorkAppointmentsService
-import kotlin.system.exitProcess
 
 @Component
 @ConditionalOnProperty("jobs.unpaid-work-appointment-reminders.enabled")
@@ -19,8 +19,8 @@ class UpwAppointmentRemindersJob(
     @Value("\${jobs.unpaid-work-appointment-reminders.templates}") private val templateIds: List<String>,
     @Value("\${jobs.unpaid-work-appointment-reminders.days-in-advance}") private val daysInAdvance: Long,
 ) : ApplicationListener<ApplicationStartedEvent> {
-    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) {
+    @WithSpan("JOB unpaid-work-appointment-reminders")
+    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) = applicationContext.runThenExit {
         service.sendUnpaidWorkAppointmentReminders(providerCode, templateIds, daysInAdvance)
-        exitProcess(exit(applicationContext, { 0 }))
     }
 }
diff --git a/...ius/src/main/kotlin/uk/gov/justice/digital/hmpps/service/UnpaidWorkAppointmentsService.kt b/...ius/src/main/kotlin/uk/gov/justice/digital/hmpps/service/UnpaidWorkAppointmentsService.kt
@@ -1,6 +1,7 @@
 package uk.gov.justice.digital.hmpps.service
 
 import org.springframework.stereotype.Service
+import uk.gov.justice.digital.hmpps.logging.Logger.logger
 import uk.gov.justice.digital.hmpps.properties.UpwAppointmentRemindersJobProperties
 import uk.gov.justice.digital.hmpps.repository.UpwAppointmentRepository
 import uk.gov.justice.digital.hmpps.telemetry.TelemetryService
@@ -25,6 +26,7 @@ class UnpaidWorkAppointmentsService(
                 )
                 if (it.crn !in properties.excludedCrns) {
                     val responses = templateIds.map { templateId ->
+                        log.info("Sending SMS template $templateId to ${it.mobileNumber}")
                         val templateValues = mapOf("FirstName" to it.firstName, "NextWorkSession" to it.appointmentDate)
                         notificationClient.sendSms(templateId, it.mobileNumber, templateValues, it.crn)
                     }
@@ -35,4 +37,8 @@ class UnpaidWorkAppointmentsService(
                 } else telemetryService.trackEvent("UnpaidWorkAppointmentReminderNotSent", telemetryProperties)
             }
     }
+
+    companion object {
+        private val log = logger()
+    }
 }
diff --git a/...dates-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt b/...dates-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt
@@ -1,14 +1,14 @@
 package uk.gov.justice.digital.hmpps.cronjob
 
+import io.opentelemetry.instrumentation.annotations.WithSpan
 import org.springframework.beans.factory.annotation.Value
-import org.springframework.boot.SpringApplication.exit
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.event.ApplicationStartedEvent
 import org.springframework.context.ApplicationContext
 import org.springframework.context.ApplicationListener
 import org.springframework.stereotype.Component
+import uk.gov.justice.digital.hmpps.job.CronJobHelper.runThenExit
 import uk.gov.justice.digital.hmpps.messaging.Notifier
-import kotlin.system.exitProcess
 
 @Component
 @ConditionalOnProperty("bulk.update.enabled")
@@ -17,9 +17,8 @@ class BulkUpdateRunner(
     private val applicationContext: ApplicationContext,
     private val notifier: Notifier,
 ) : ApplicationListener<ApplicationStartedEvent> {
-
-    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) {
+    @WithSpan("JOB bulk-update-key-dates")
+    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) = applicationContext.runThenExit {
         notifier.requestBulkUpdate(listOf(), dryRun)
-        exitProcess(exit(applicationContext, { 0 }))
     }
 }
diff --git a/...cases-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt b/...cases-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt
@@ -1,14 +1,14 @@
 package uk.gov.justice.digital.hmpps.cronjob
 
+import io.opentelemetry.instrumentation.annotations.WithSpan
 import org.springframework.beans.factory.annotation.Value
-import org.springframework.boot.SpringApplication.exit
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.event.ApplicationStartedEvent
 import org.springframework.context.ApplicationContext
 import org.springframework.context.ApplicationListener
 import org.springframework.stereotype.Component
+import uk.gov.justice.digital.hmpps.job.CronJobHelper.runThenExit
 import uk.gov.justice.digital.hmpps.messaging.Notifier
-import kotlin.system.exitProcess
 
 @Component
 @ConditionalOnProperty("bulk.update.enabled")
@@ -17,9 +17,8 @@ class BulkUpdateRunner(
     private val applicationContext: ApplicationContext,
     private val notifier: Notifier,
 ) : ApplicationListener<ApplicationStartedEvent> {
-
-    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) {
+    @WithSpan("JOB bulk-update-pom")
+    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) = applicationContext.runThenExit {
         notifier.requestBulkUpdate(dryRun)
-        exitProcess(exit(applicationContext, { 0 }))
     }
 }
diff --git a/...ifier-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt b/...ifier-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/cronjob/BulkUpdateRunner.kt
@@ -1,14 +1,14 @@
 package uk.gov.justice.digital.hmpps.cronjob
 
+import io.opentelemetry.instrumentation.annotations.WithSpan
 import org.springframework.beans.factory.annotation.Value
-import org.springframework.boot.SpringApplication.exit
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.event.ApplicationStartedEvent
 import org.springframework.context.ApplicationContext
 import org.springframework.context.ApplicationListener
 import org.springframework.stereotype.Component
+import uk.gov.justice.digital.hmpps.job.CronJobHelper.runThenExit
 import uk.gov.justice.digital.hmpps.messaging.Notifier
-import kotlin.system.exitProcess
 
 @Component
 @ConditionalOnProperty("bulk.update.enabled")
@@ -17,9 +17,8 @@ class BulkUpdateRunner(
     private val applicationContext: ApplicationContext,
     private val notifier: Notifier,
 ) : ApplicationListener<ApplicationStartedEvent> {
-
-    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) {
+    @WithSpan("JOB bulk-update-prison-identifiers")
+    override fun onApplicationEvent(applicationStartedEvent: ApplicationStartedEvent) = applicationContext.runThenExit {
         notifier.requestPrisonMatching(listOf(), dryRun)
-        exitProcess(exit(applicationContext, { 0 }))
     }
 }