From 39ae0a79f8da37e1b6574ecf1c8d2de8e7970ae3 Mon Sep 17 00:00:00 2001 From: Christopher Wachter Date: Fri, 17 Jul 2026 16:40:46 +0200 Subject: [PATCH] pst-122 honoring coverage upload intervals smaller than 1h only once --- CHANGELOG.md | 1 + .../com/teamscale/jacoco/agent/Agent.kt | 47 ++++++++++++++++--- .../jacoco/agent/options/AgentOptions.kt | 8 +++- .../build.gradle.kts | 8 ++++ .../main/kotlin/dump/test/SystemUnderTest.kt | 12 +++++ .../client/DumpIntervalTooSmallTest.kt | 28 +++++++++++ 6 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 system-tests/dump-interval-too-small-test/build.gradle.kts create mode 100644 system-tests/dump-interval-too-small-test/src/main/kotlin/dump/test/SystemUnderTest.kt create mode 100644 system-tests/dump-interval-too-small-test/src/test/kotlin/com/teamscale/client/DumpIntervalTooSmallTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 9acc98bbc..8ee8da4ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/): - PATCH version when you make backwards compatible bug fixes. # Next version +- [fix] _agent_: A configured dump `interval` smaller than 1h is now applied only once and then raised to 1h to prevent excessive uploads that can overwhelm Teamscale. A warning is logged at startup when this happens. # 37.0.1 - [fix] _agent_: Changed testwise coverage mode to log a warning and finalize an interrupted test as `SKIPPED` (with a calculated duration) if a new test started before the previous `/test/end` event arrived. diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/Agent.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/Agent.kt index 97d470b2c..7b693ceda 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/Agent.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/Agent.kt @@ -1,7 +1,6 @@ package com.teamscale.jacoco.agent import com.teamscale.client.FileSystemUtils -import com.teamscale.client.StringUtils import com.teamscale.jacoco.agent.logging.LoggingUtils import com.teamscale.jacoco.agent.options.AgentOptions import com.teamscale.jacoco.agent.upload.IUploadRetry @@ -17,8 +16,8 @@ import java.io.File import java.io.IOException import java.lang.instrument.Instrumentation import java.nio.file.Files -import java.util.Timer -import kotlin.concurrent.fixedRateTimer +import java.util.* +import kotlin.concurrent.schedule import kotlin.io.path.deleteIfExists import kotlin.io.path.listDirectoryEntries import kotlin.time.DurationUnit @@ -33,6 +32,10 @@ class Agent(options: AgentOptions, instrumentation: Instrumentation?) : AgentBas /** Regular dump task. */ private var timer: Timer? = null + /** Set once the agent is shutting down so no further dumps are rescheduled. */ + @Volatile + private var stopped = false + /** Stores the XML files. */ private val uploader = options.createUploader(instrumentation) @@ -41,17 +44,46 @@ class Agent(options: AgentOptions, instrumentation: Instrumentation?) : AgentBas retryUnsuccessfulUploads(options, uploader) if (options.shouldDumpInIntervals()) { - val period = options.dumpIntervalInMinutes.toDuration(DurationUnit.MINUTES).inWholeMilliseconds - timer = fixedRateTimer("Teamscale-Java-Profiler", true, period, period) { - dumpReport() + timer = Timer("Teamscale-Java-Profiler", true) + // Guard against excessively small intervals (see PST-122): first-time users often set a tiny interval to + // quickly get an upload and then forget to raise it, flooding Teamscale. We warn right away, honor the + // configured interval once, and then raise it to the minimum for all subsequent dumps. + if (options.dumpIntervalInMinutes < AgentOptions.MINIMUM_DUMP_INTERVAL_IN_MINUTES) { + logger.warn( + "You configured an interval smaller than 1h. This causes excessive amounts of data to be sent" + + " to Teamscale. Applying this interval only once, then increasing the interval to 1h." + ) + } else { + logger.info("Dumping every ${options.dumpIntervalInMinutes} minutes.") } - logger.info("Dumping every ${options.dumpIntervalInMinutes} minutes.") + scheduleNextDump() } options.teamscaleServer.partition?.let { partition -> controller.sessionId = partition } } + /** + * Schedules the next interval dump. The configured interval is applied once; when it is below + * [AgentOptions.MINIMUM_DUMP_INTERVAL_IN_MINUTES] it is raised to that minimum after the first dump to avoid + * flooding Teamscale (see PST-122). The warning about this is logged once at startup. + */ + private fun scheduleNextDump() { + if (stopped) return + val delayMillis = options.dumpIntervalInMinutes.toDuration(DurationUnit.MINUTES).inWholeMilliseconds + try { + timer?.schedule(delayMillis) { + dumpReport() + if (options.dumpIntervalInMinutes < AgentOptions.MINIMUM_DUMP_INTERVAL_IN_MINUTES) { + options.dumpIntervalInMinutes = AgentOptions.MINIMUM_DUMP_INTERVAL_IN_MINUTES + } + scheduleNextDump() + } + } catch (_: IllegalStateException) { + // The timer was cancelled (agent shutting down) between our check and scheduling; nothing left to do. + } + } + /** * If we have coverage that was leftover because of previously unsuccessful coverage uploads, we retry to upload * them again with the same configuration as in the previous try. @@ -107,6 +139,7 @@ class Agent(options: AgentOptions, instrumentation: Instrumentation?) : AgentBas } override fun prepareShutdown() { + stopped = true timer?.cancel() if (options.shouldDumpOnExit) dumpReport() diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt index 89342e3b1..797acf77d 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt @@ -16,8 +16,6 @@ import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitProperties import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitSingleProjectPropertiesLocator import com.teamscale.jacoco.agent.commit_resolution.sapnwdi.NwdiMarkerClassLocatingTransformer import com.teamscale.jacoco.agent.configuration.ConfigurationViaTeamscale -import com.teamscale.jacoco.agent.options.AgentOptions.Companion.GIT_PROPERTIES_COMMIT_DATE_FORMAT_OPTION -import com.teamscale.jacoco.agent.options.AgentOptions.Companion.GIT_PROPERTIES_JAR_OPTION import com.teamscale.jacoco.agent.options.sapnwdi.DelayedSapNwdiMultiUploader import com.teamscale.jacoco.agent.options.sapnwdi.SapNwdiApplication import com.teamscale.jacoco.agent.upload.IUploader @@ -673,6 +671,12 @@ open class AgentOptions(private val logger: ILogger) { } else teamscaleProxyOptionsForHttps companion object { + /** + * The smallest allowed value for [dumpIntervalInMinutes] (see PST-122). If a smaller interval is configured, + * it is honored once and then reset to this minimum. + */ + const val MINIMUM_DUMP_INTERVAL_IN_MINUTES = 60 + /** * Can be used to format [java.time.LocalDate] to the format "yyyy-MM-dd-HH-mm-ss.SSS" */ diff --git a/system-tests/dump-interval-too-small-test/build.gradle.kts b/system-tests/dump-interval-too-small-test/build.gradle.kts new file mode 100644 index 000000000..005a3067a --- /dev/null +++ b/system-tests/dump-interval-too-small-test/build.gradle.kts @@ -0,0 +1,8 @@ +plugins { + com.teamscale.`kotlin-convention` + com.teamscale.`system-test-convention` +} + +tasks.test { + teamscaleAgent(mapOf("interval" to "1", "debug" to logFilePath)) +} diff --git a/system-tests/dump-interval-too-small-test/src/main/kotlin/dump/test/SystemUnderTest.kt b/system-tests/dump-interval-too-small-test/src/main/kotlin/dump/test/SystemUnderTest.kt new file mode 100644 index 000000000..60311b860 --- /dev/null +++ b/system-tests/dump-interval-too-small-test/src/main/kotlin/dump/test/SystemUnderTest.kt @@ -0,0 +1,12 @@ +package dump.test + +/** + * A minimal system under test. The agent is attached with a too-small dump `interval`, and we only need the JVM to + * start so the agent logs its startup warning. + */ +object SystemUnderTest { + @JvmStatic + fun main(args: Array) { + // doesn't need to do anything for this test + } +} diff --git a/system-tests/dump-interval-too-small-test/src/test/kotlin/com/teamscale/client/DumpIntervalTooSmallTest.kt b/system-tests/dump-interval-too-small-test/src/test/kotlin/com/teamscale/client/DumpIntervalTooSmallTest.kt new file mode 100644 index 000000000..af68a5cae --- /dev/null +++ b/system-tests/dump-interval-too-small-test/src/test/kotlin/com/teamscale/client/DumpIntervalTooSmallTest.kt @@ -0,0 +1,28 @@ +package com.teamscale.client + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.nio.file.Paths +import kotlin.io.path.exists +import kotlin.io.path.readLines + +/** + * Ensures that the agent warns at startup when a dump `interval` smaller than 1h is configured (see PST-122), so the + * user is told that the interval will be applied only once and then raised to 1h. + */ +class DumpIntervalTooSmallTest { + @Test + @Throws(Exception::class) + fun systemTest() { + assertTrue(LOG_DIRECTORY.exists()) + val logContent = LOG_DIRECTORY.resolve("teamscale-jacoco-agent.log") + .readLines() + .joinToString("\n") + assertThat(logContent).containsPattern("WARN.*You configured an interval smaller than 1h") + } + + companion object { + private val LOG_DIRECTORY = Paths.get("logTest").resolve("logs") + } +}