TS-46436 improve log messages

CHANGELOG.md

@@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [security fix] _agent_: The Azure shared-key error messages no longer include the configured `azure-key` value in the exception text (previously leaked the access key into stack traces and logs)
 
 # 36.5.2
 - [security fix] _agent_: The Teamscale access token was logged in clear text in DEBUG-level logs (e.g., when `debug=true` was set) and in the WARN-level log emitted when multiple `-javaagent` arguments are present. The token is now obfuscated in those logs as well, matching INFO-level behavior.

agent/src/main/kotlin/com/teamscale/jacoco/agent/Agent.kt

@@ -92,7 +92,10 @@
 			}
 			Files.deleteIfExists(file.toPath())
 		} catch (e: IOException) {
-			logger.error("Reuploading coverage failed. $e")
+			logger.error(
+				"Reuploading cached coverage from {} failed. The file is kept and the upload will be retried on" +
+						" the next agent restart.", file, e
+			)
 		}
 	}
 
@@ -140,32 +143,43 @@
 	}
 
 	private fun dumpReportUnsafe() {
 		val dump: Dump
 		try {
 			dump = controller.dumpAndReset()
 		} catch (e: JacocoRuntimeController.DumpException) {
-			logger.error("Dumping failed, retrying later", e)
+			logger.error(
+				"Dumping coverage data failed. The agent will retry on the next dump interval." +
+						" If this persists, report a bug.", e
+			)
 			return
 		}
 
 		val generator = JaCoCoXmlReportGenerator(
 			options.classDirectoriesOrZips,
 			options.locationIncludeFilter,
 			options.duplicateClassFileBehavior,
 			options.ignoreUncoveredClasses,
 			LoggingUtils.wrap(logger)
 		)
 
 		try {
 			benchmark("Generating the XML report") {
 				val outputFile = options.createNewFileInOutputDirectory("jacoco", "xml")
 				val coverageFile = generator.convertSingleDumpToReport(dump, outputFile)
 				uploader.upload(coverageFile)
 			}
 		} catch (e: IOException) {
-			logger.error("Converting binary dump to XML failed", e)
+			logger.error(
+				"Converting the binary JaCoCo dump to XML failed. This dump's coverage will be skipped." +
+						" If you set 'class-dir', ensure it points to your compiled classes and they are readable.", e
+			)
 		} catch (e: EmptyReportException) {
-			logger.error("No coverage was collected. ${e.message}", e)
+			logger.warn(
+				"No coverage was collected in this dump interval: {}." +
+						" This is normal if no profiled code ran." +
+						" Check the 'includes'/'excludes' patterns if you expected coverage.",
+				e.message
+			)
 		}
 	}
 }

agent/src/main/kotlin/com/teamscale/jacoco/agent/AgentBase.kt

@@ -60,8 +60,11 @@ abstract class AgentBase(
 			try {
 				initServer()
 			} catch (e: Exception) {
-				logger.error("Could not start http server on port $port. Please check if the port is blocked.")
-				throw IllegalStateException("Control server not started.", e)
+				throw IllegalStateException(
+					"Could not start the agent control HTTP server on port $port." +
+							" Check that the port is not already in use and try a different value for the" +
+							" 'http-server-port' option.", e
+				)
 			}
 		}
 	}
@@ -117,7 +120,11 @@ abstract class AgentBase(
 				prepareShutdown()
 				logger.info("Teamscale Java Profiler successfully shut down.")
 			} catch (e: Exception) {
-				logger.error("Exception during profiler shutdown.", e)
+				logger.error(
+					"Error while shutting down the Teamscale Java Profiler. Coverage may not have been flushed;" +
+							" check the log for details. If this keeps happening, report a bug.",
+					e
+				)
 			} finally {
 				// Try to flush logging resources also in case of an exception during shutdown
 				PreMain.closeLoggingResources()
@@ -131,7 +138,10 @@ abstract class AgentBase(
 			try {
 				server.stop()
 			} catch (e: Exception) {
-				logger.error("Could not stop server so it is killed now.", e)
+				logger.error(
+					"Could not gracefully stop the agent control HTTP server on port {}; forcing shutdown." +
+							" Pending test events may be lost.", options.httpServerPort, e
+				)
 			} finally {
 				server.destroy()
 			}

agent/src/main/kotlin/com/teamscale/jacoco/agent/JacocoRuntimeController.kt

@@ -1,5 +1,6 @@
 package com.teamscale.jacoco.agent
 
+import com.teamscale.client.BugReportMessages
 import com.teamscale.report.jacoco.dump.Dump
 import org.jacoco.agent.rt.IAgent
 import org.jacoco.core.data.ExecutionDataReader
@@ -47,7 +48,10 @@ class JacocoRuntimeController
 				}
 			}
 		} catch (e: IOException) {
-			throw DumpException("should never happen for the ByteArrayInputStream", e)
+			throw DumpException(
+				"Unexpected IOException while reading the in-memory coverage dump." +
+						" ${BugReportMessages.REPORT_TO_CQSE}", e
+			)
 		}
 	}
 
@@ -75,7 +79,10 @@ class JacocoRuntimeController
 		try {
 			agent.dump(true)
 		} catch (e: IOException) {
-			throw DumpException(e.message, e)
+			throw DumpException(
+				"Failed to dump execution data: ${e.message ?: e.javaClass.simpleName}." +
+						" The dump will be retried on the next interval.", e
+			)
 		}
 	}
 

agent/src/main/kotlin/com/teamscale/jacoco/agent/PreMain.kt

@@ -247,7 +247,10 @@ object PreMain {
 		if (FileSystemUtils.isValidPath(logDirectory.toString()) && Files.isWritable(logDirectory)) {
 			logger.info("Logging to $logDirectory")
 		} else {
-			logger.warn("Could not create $logDirectory. Logging to console only.")
+			logger.warn(
+				"Could not create debug log directory '$logDirectory'" +
+						" (check permissions and free disk space). Falling back to console-only logging."
+			)
 		}
 	}
 

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt

@@ -87,9 +87,19 @@ class GitMultiProjectPropertiesLocator(
 				uploader.addTeamscaleProjectAndCommit(file, projectAndCommit)
 			}
 		} catch (e: IOException) {
-			logger.error("Error during asynchronous search for git.properties in {}", file, e)
+			logger.error(
+				"Could not search for git.properties in {}." +
+						" Auto-detection of the Teamscale project/commit for code loaded from this location will be skipped" +
+						" — set 'teamscale-project' and 'teamscale-revision' manually if no other git.properties is found.",
+				file, e
+			)
 		} catch (e: InvalidGitPropertiesException) {
-			logger.error("Error during asynchronous search for git.properties in {}", file, e)
+			logger.error(
+				"Could not search for git.properties in {}." +
+						" Auto-detection of the Teamscale project/commit for code loaded from this location will be skipped" +
+						" — set 'teamscale-project' and 'teamscale-revision' manually if no other git.properties is found.",
+				file, e
+			)
 		}
 	}
 }

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatingTransformer.kt

@@ -69,7 +69,11 @@ class GitPropertiesLocatingTransformer(
 		} catch (e: Throwable) {
 			// we catch Throwable to be sure that we log all errors as anything thrown from this method is
 			// silently discarded by the JVM
-			logger.error("Failed to process class {} in search of git.properties", className, e)
+			logger.error(
+				"Failed to process class {} while searching for git.properties. This class will be skipped" +
+						" — coverage detection should still work via other classes from the same JAR.",
+				className, e
+			)
 		}
 		return null
 	}

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitPropertiesLocatorUtils.kt

@@ -261,7 +261,9 @@ object GitPropertiesLocatorUtils {
 			}
 		} catch (e: IOException) {
 			throw IOException(
-				"Reading jar ${file.absolutePath} for obtaining commit descriptor from git.properties failed", e
+				"Reading JAR file ${file.absolutePath} for git.properties failed: ${e.message}." +
+						" Verify the file is a valid JAR and is readable by the user running the JVM.",
+				e
 			)
 		}
 	}
@@ -316,7 +318,9 @@ object GitPropertiesLocatorUtils {
 				}
 			} catch (e: IOException) {
 				throw IOException(
-					"Reading directory ${gitPropertiesFile.absolutePath} for obtaining commit descriptor from git.properties failed", e
+					"Reading directory ${gitPropertiesFile.absolutePath} for git.properties failed: ${e.message}." +
+							" Verify the path is readable by the user running the JVM.",
+					e
 				)
 			}
 		}
@@ -359,7 +363,10 @@ object GitPropertiesLocatorUtils {
 		}
 
 		if (isEmpty && isRootArchive) {
-			throw IOException("No entries in Jar file $archiveName. Is this a valid jar file?. If so, please report to CQSE.")
+			throw IOException(
+				"No entries found in JAR file $archiveName. Verify this is a valid JAR file." +
+						" If it is, report a bug."
+			)
 		}
 
 		return result

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt

@@ -76,9 +76,19 @@ class GitSingleProjectPropertiesLocator<T>(
 			jarFileWithGitProperties = file
 			uploader.setCommitAndTriggerAsynchronousUpload(dataEntry)
 		} catch (e: IOException) {
-			logger.error("Error during asynchronous search for git.properties in {}", file.toString(), e)
+			logger.error(
+				"Could not search for git.properties in {}." +
+						" Auto-detection of the Teamscale project/commit for code loaded from this location will be skipped" +
+						" — set 'teamscale-project' and 'teamscale-revision' manually if no other git.properties is found.",
+				file.toString(), e
+			)
 		} catch (e: InvalidGitPropertiesException) {
-			logger.error("Error during asynchronous search for git.properties in {}", file.toString(), e)
+			logger.error(
+				"Could not search for git.properties in {}." +
+						" Auto-detection of the Teamscale project/commit for code loaded from this location will be skipped" +
+						" — set 'teamscale-project' and 'teamscale-revision' manually if no other git.properties is found.",
+				file.toString(), e
+			)
 		}
 	}
 

agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/sapnwdi/NwdiMarkerClassLocatingTransformer.kt

@@ -64,7 +64,9 @@ class NwdiMarkerClassLocatingTransformer(
 			// we catch Throwable to be sure that we log all errors as anything thrown from this method is
 			// silently discarded by the JVM
 			logger.error(
-				"Failed to process class {} trying to determine its last modification timestamp.", className, e
+				"Failed to determine the last-modified timestamp of class {} for SAP NWDI commit auto-detection." +
+						" This class will be skipped; auto-detection should still succeed via other marker classes.",
+				className, e
 			)
 		}
 		return null

agent/src/main/kotlin/com/teamscale/jacoco/agent/configuration/ConfigurationViaTeamscale.kt

@@ -61,8 +61,10 @@ class ConfigurationViaTeamscale(
 		try {
 			val response = teamscaleClient.sendHeartbeat(profilerId!!, profilerInfo).execute()
 			if (!response.isSuccessful) {
-				LoggingUtils.getLogger(this)
-					.error("Failed to send heartbeat. Teamscale responded with: ${response.errorBody()?.string()}")
+				LoggingUtils.getLogger(this).error(
+					"Failed to send heartbeat to Teamscale: HTTP ${response.code()} ${response.errorBody()?.string()}." +
+							" If heartbeats keep failing, Teamscale will mark this profiler as offline."
+				)
 			}
 		} catch (e: IOException) {
 			LoggingUtils.getLogger(this).error("Failed to send heartbeat to Teamscale!", e)
@@ -78,11 +80,17 @@ class ConfigurationViaTeamscale(
 				response = teamscaleClient.unregisterProfilerLegacy(profilerId).execute()
 			}
 			if (!response.isSuccessful) {
-				LoggingUtils.getLogger(this)
-					.error("Failed to unregister profiler. Teamscale responded with: ${response.errorBody()?.string()}")
+				LoggingUtils.getLogger(this).error(
+					"Failed to unregister profiler with Teamscale: HTTP ${response.code()} ${response.errorBody()?.string()}." +
+							" The profiler will be marked offline automatically after the heartbeat timeout."
+				)
 			}
 		} catch (e: IOException) {
-			LoggingUtils.getLogger(this).error("Failed to unregister profiler!", e)
+			LoggingUtils.getLogger(this).error(
+				"Failed to unregister profiler with Teamscale (network error)." +
+						" The profiler will be marked offline automatically after the heartbeat timeout.",
+				e
+			)
 		}
 	}
 
@@ -122,12 +130,10 @@ class ConfigurationViaTeamscale(
 				val body = response.body()
 				return parseProfilerRegistration(body!!, response, teamscaleClient, processInformation)
 			} catch (e: IOException) {
-				// we include the causing error message in this exception's message since this causes it to be printed
-				// to stderr which is much more helpful than just saying "something didn't work"
 				throw AgentOptionReceiveException(
-					"Failed to retrieve profiler configuration from Teamscale due to network error: ${
-						LoggingUtils.getStackTraceAsString(e)
-					}", e
+					"Failed to retrieve profiler configuration from Teamscale due to network error: ${e.message}." +
+							" Verify the Teamscale URL ($url) is reachable from this machine and the configured user has access.",
+					e
 				)
 			}
 		}

agent/src/main/kotlin/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.kt

@@ -24,7 +24,11 @@ class ProcessInformationRetriever(private val logger: ILogger) {
 			try {
 				return InetAddress.getLocalHost().hostName
 			} catch (e: UnknownHostException) {
-				logger.error("Failed to determine hostname!", e)
+				logger.warn(
+					"Could not determine the local hostname; using an empty value." +
+							" This only affects the display name shown in Teamscale's profiler list.",
+					e
+				)
 				return ""
 			}
 		}

agent/src/main/kotlin/com/teamscale/jacoco/agent/convert/Converter.kt

@@ -46,7 +46,11 @@ class Converter
 				generator.convertExecFilesToReport(jacocoExecutionDataList, Paths.get(arguments.outputFile).toFile())
 			}
 		} catch (e: EmptyReportException) {
-			logger.warn("Converted report was empty.", e)
+			logger.warn(
+				"Converted report was empty — no coverage in the input .exec files." +
+						" If you set 'class-dir', verify it points to compiled classes matching the recorded coverage.",
+				e
+			)
 		}
 	}
 

agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt

@@ -259,7 +259,10 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 				try {
 					options.additionalMetaDataFiles = splitMultiOptionValue(value).map { path -> Paths.get(path) }
 				} catch (e: InvalidPathException) {
-					throw AgentOptionParseException("Invalid path given for option 'upload-metadata'", e)
+					throw AgentOptionParseException(
+						"Invalid path given for option 'upload-metadata': '$value' (${e.message})." +
+								" Use a semicolon-separated list of file paths.", e
+					)
 				}
 				return true
 			}
@@ -326,7 +329,9 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 						key, filePatternResolver, list
 					)
 				} catch (e: IOException) {
-					throw AgentOptionParseException(e)
+					throw AgentOptionParseException(
+						"Failed to resolve class directories for option 'class-dir': ${e.message}", e
+					)
 				}
 				return true
 			}
@@ -504,7 +509,9 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 			try {
 				return filePatternResolver.parsePath(optionName, pattern)
 			} catch (e: IOException) {
-				throw AgentOptionParseException(e)
+				throw AgentOptionParseException(
+					"Invalid path or pattern given for option '$optionName': '$pattern' (${e.message})", e
+				)
 			}
 		}
 
@@ -533,7 +540,10 @@ class AgentOptionsParser @VisibleForTesting internal constructor(
 
 		@Throws(AgentOptionParseException::class)
 		private fun getUrl(key: String?, value: String) =
-			value.toHttpUrlOrNull() ?: throw AgentOptionParseException("Invalid URL given for option '$key'")
+			value.toHttpUrlOrNull() ?: throw AgentOptionParseException(
+				"Invalid URL given for option '$key': '$value'." +
+						" The URL must be of the form http(s)://host[:port]/."
+			)
 
 		/**
 		 * Splits the given value at semicolons.