Draft: TS-40806 replace the delayed logger with a delayed appender

CHANGELOG.md

@@ -5,7 +5,26 @@ We use [semantic versioning](http://semver.org/):
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next version
+- [fix] _agent_: Warning about multiple agents being present is misleading  
+
+# 34.2.0
+- [fix] _agent_: `search-git-properties-recursively` was not considered when jar was given via `artifactory-git-properties-jar`
+- [deprecation] _agent_: `artifactory-git-properties-jar` is deprecated now. Replace the option with `git-properties-jar`. The old option still works but is an alias now for `git-properties-jar`.
+- [feature] _agent_: Added `git-properties-commit-date-format` (replaces `artifactory-git-properties-commit-date-format`), which now also can be used in non-artifactory cases.
+- [fix] _agent_: `search-git-properties-recursively` did only consider nested jar files, but no `war`, `ear`, `aar`
+
+# 34.1.1
+- [fix] _agent_: Loading a profiler configuration from Teamscale was not possible if the potentially necessary proxy settings were not set yet.
+
+# 34.1.0
+- [feature] _agent_: New options `proxy-http(s)-host`/`-port`/`-user`/`-password` allow user to specify teamscale-specific proxy settings.
+- [fix] _teamscale-maven-plugin_: NPE when no commit, nor revision was configured
+- [fix] _teamscale-maven-plugin_: Fixed commit was ignored in commit resolution
+
+# 34.0.2
 - [feature] _agent_: Prevent uploading coverage to the same project + revision or branch@timestamp when doing multi project upload via git.properties
+- [fix] _impacted-test-engine_: Remove duplicated slashes in uniform paths and cluster ids for Cucumber Tests
+- [fix] _impacted-test-engine_: Ensure that the config file specified via `java.util.logging.config.file` is used when specified
 
 # 34.0.1
 - [fix] _agent_: Error was reported when the system under test used logback

agent/README.md

@@ -63,6 +63,7 @@ The following options are available:
   `logging` folder, e.g. to enable debug logging or log directly to the console. (For details see path format section
   below)
 - `proxy-password-file` (optional): path to a file that contains the password for a proxy server authentication. This file may only contain the password and nothing else.
+- `proxy-` (optional): For other options regarding teamscale-specific proxy settings see [Teamscale-Specific-Proxy Section](#teamscale-specific-proxy-settings).
 - `mode` (optional): which coverage collection mode to use. Can be either `normal` or `testwise` (Default is `normal`)
 - `debug` (optional): `true`, `false` or a path to which the logs should be written to. `true` if no explicit value given.
   This option turns on debug mode. The logs will be written to console and the given file path. If no file path is given,
@@ -87,6 +88,20 @@ Note that defining `-Djava.io.tmpdir` will change the temp directory that is bei
 If there is no log file at that location, it means the agent didn't even start and you have not configured it correctly.
 Check your applications console output for error messages.
 
+#### Teamscale-Specific Proxy Settings
+It is possible to specify teamscale-specific proxy settings that take prevalence over the system properties of the JVM.
+If no or not all teamscale-specific proxy settings are provided the agent will fall back to the system properties of the
+JVM.
+
+It is also possible to specify these options by prefixing the JVM flags for proxies with `teamscale.`,
+for example `-Dteamscale.https.proxyHost`.
+
+The following options are available both for `https` and `http`:
+- `proxy-https-host`/`proxy-http-host`: The host name of the proxy server.
+- `proxy-https-port`/`proxy-http-port`: The port of the proxy server.
+- `proxy-https-user`/`proxy-http-user`: The username for the proxy server.
+- `proxy-https-password`/`proxy-http-password`: The password for the proxy user.
+
 #### Testwise coverage
 
 If you want to collect testwise coverage, please have a look below in the [Testwise mode section](#testwise-coverage-modes).
@@ -161,6 +176,7 @@ patterns with `*`, `**` and `?`.
   - the properties `git.branch` and `git.commit.time` (in the format `yyyy-MM-dd'T'HH:mm:ssZ` or `yyyy-MM-dd'T'HH:mm:ssXXX`) or
   - the properties `teamscale.commit.branch` and `teamscale.commit.time` (either as an epoch timestamp or in one of the two formats above)
 - `search-git-properties-recursively` Specifies whether to search for git.properties files recursively in folders or archive (jar, war, ear, aar) files. Default: true.
+- `git-properties-commit-date-format` The Java data pattern `git.commit.time` is encoded with in `git.properties`. Defaults to `yyyy-MM-dd'T'HH:mm:ssZ`.
 - `teamscale-message` (optional): the commit message shown within Teamscale for the coverage upload (Default is "Agent
   coverage upload").
 - `config-file` (optional): a file which contains one or more of the previously named options as `key=value` entries
@@ -217,11 +233,6 @@ patterns with `*`, `**` and `?`.
   This can be used to encode e.g. a partition name that is parsed later on via Teamscale Artifactory connector options.
 - `artifactory-path-suffix` (optional): The path within the storage location between
   the default path and the uploaded artifact.
-- `artifactory-git-properties-jar` (optional): Specify a Jar to search a `git.properties` file within.
-  If not specified, Git commit information is extracted from the first found `git.properties` file.
-  See `git-properties-jar` for details.
-- `artifactory-git-properties-commit-date-format` (optional):
-  The Java data pattern `git.commit.time` is encoded with in `git.properties`. Defaults to `yyyy-MM-dd'T'HH:mm:ssZ`.
 
 ### The new standard upload schema
 ```

agent/build.gradle.kts

@@ -107,9 +107,10 @@ tasks.shadowDistZip {
 }
 
 tasks.processResources {
+	inputs.property("version", project.version)
 	filesMatching("**/app.properties") {
 		filter {
-			it.replace("%APP_VERSION_TOKEN_REPLACED_DURING_BUILD%", appVersion)
+			it.replace("%APP_VERSION_TOKEN_REPLACED_DURING_BUILD%", version.toString())
 		}
 	}
 }

agent/src/main/java/com/teamscale/jacoco/agent/AgentBase.java

@@ -1,9 +1,7 @@
 package com.teamscale.jacoco.agent;
 
-import com.teamscale.client.ProxySystemProperties;
 import com.teamscale.jacoco.agent.options.AgentOptions;
 import com.teamscale.jacoco.agent.logging.LoggingUtils;
-import org.conqat.lib.commons.filesystem.FileSystemUtils;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.servlet.ServletContextHandler;
@@ -14,9 +12,7 @@
 import org.jacoco.agent.rt.RT;
 import org.slf4j.Logger;
 
-import java.io.IOException;
 import java.lang.management.ManagementFactory;
-import java.nio.file.Path;
 
 /**
  * Base class for agent implementations. Handles logger shutdown, store creation and instantiation of the
@@ -40,7 +36,7 @@ public abstract class AgentBase {
 	/** Constructor. */
 	public AgentBase(AgentOptions options) throws IllegalStateException {
 		this.options = options;
-		setProxyPasswordFromFile(options.getProxyPasswordPath());
+
 		try {
 			controller = new JacocoRuntimeController(RT.getAgent());
 		} catch (IllegalStateException e) {
@@ -60,22 +56,7 @@ public AgentBase(AgentOptions options) throws IllegalStateException {
 		}
 	}
 
-	/** Sets the proxy password JVM property from a file for both http and https. */
-	private void setProxyPasswordFromFile(Path proxyPasswordFilePath) {
-		if (proxyPasswordFilePath == null) {
-			return;
-		}
-		try {
-			String proxyPassword = FileSystemUtils.readFileUTF8(proxyPasswordFilePath.toFile()).trim();
-			new ProxySystemProperties(ProxySystemProperties.Protocol.HTTP).setProxyPassword(proxyPassword);
-			new ProxySystemProperties(ProxySystemProperties.Protocol.HTTPS).setProxyPassword(proxyPassword);
-		} catch (IOException e) {
-			logger.error(
-					"Unable to open file containing proxy password. Please make sure the file exists and the user has the permissions to read the file.",
-					e);
-		}
 
-	}
 
 	/**
 	 * Lazily generated string representation of the command line arguments to print to the log.
@@ -133,10 +114,17 @@ private ServletContextHandler buildUsingResourceConfig() {
 	 */
 	void registerShutdownHook() {
 		Runtime.getRuntime().addShutdownHook(new Thread(() -> {
-			stopServer();
-			prepareShutdown();
-			logger.info("CQSE JaCoCo agent successfully shut down.");
-			PreMain.closeLoggingResources();
+			try {
+				logger.info("CQSE JaCoCo agent is shutting down...");
+				stopServer();
+				prepareShutdown();
+				logger.info("CQSE JaCoCo agent successfully shut down.");
+			} catch (Exception e) {
+				logger.error("Exception during agent shutdown.", e);
+			} finally {
+				// Try to flush logging resources also in case of an exception during shutdown
+				PreMain.closeLoggingResources();
+			}
 		}));
 	}
 

agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java

@@ -1,7 +1,6 @@
 package com.teamscale.jacoco.agent;
 
 import com.teamscale.client.HttpUtils;
-import com.teamscale.client.TeamscaleServer;
 import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException;
 import com.teamscale.jacoco.agent.logging.LogToTeamscaleAppender;
 import com.teamscale.jacoco.agent.options.AgentOptionParseException;
@@ -13,15 +12,15 @@
 import com.teamscale.jacoco.agent.options.TeamscalePropertiesUtils;
 import com.teamscale.jacoco.agent.testimpact.TestwiseCoverageAgent;
 import com.teamscale.jacoco.agent.upload.UploaderException;
-import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
 import com.teamscale.jacoco.agent.util.AgentUtils;
 import com.teamscale.jacoco.agent.logging.DebugLogDirectoryPropertyDefiner;
 import com.teamscale.jacoco.agent.logging.LogDirectoryPropertyDefiner;
 import com.teamscale.jacoco.agent.logging.LoggingUtils;
+import com.teamscale.report.util.ILogger;
 import org.conqat.lib.commons.collections.CollectionUtils;
+import org.conqat.lib.commons.collections.Pair;
 import org.conqat.lib.commons.filesystem.FileSystemUtils;
 import org.conqat.lib.commons.string.StringUtils;
-import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 
 import java.io.File;
@@ -47,8 +46,10 @@ public class PreMain {
 	 */
 	private static final String LOCKING_SYSTEM_PROPERTY = "TEAMSCALE_JAVA_PROFILER_ATTACHED";
 
-	/** Environment variable from which to read the config ID to use.
-	 * This is an ID for a profiler configuration that is stored in Teamscale. */
+	/**
+	 * Environment variable from which to read the config ID to use. This is an ID for a profiler configuration that is
+	 * stored in Teamscale.
+	 */
 	private static final String CONFIG_ID_ENVIRONMENT_VARIABLE = "TEAMSCALE_JAVA_PROFILER_CONFIG_ID";
 
 	/** Environment variable from which to read the config file to use. */
@@ -71,9 +72,29 @@ public static void premain(String options, Instrumentation instrumentation) thro
 			return;
 		}
 
-		AgentOptions agentOptions;
+		AgentOptions agentOptions = null;
 		try {
-			agentOptions = getAndApplyAgentOptions(options, environmentConfigId, environmentConfigFile);
+			Pair<AgentOptions, List<Exception>> parseResult = getAndApplyAgentOptions(options, environmentConfigId,
+					environmentConfigFile);
+			agentOptions = parseResult.getFirst();
+
+			// After parsing everything and configuring logging, we now
+			// can throw the caught exceptions.
+			for (Exception exception : parseResult.getSecond()) {
+				throw exception;
+			}
+		} catch (AgentOptionParseException e) {
+			getLoggerContext().getLogger(PreMain.class).error(e.getMessage(), e);
+
+			// Flush logs to Teamscale, if configured.
+			closeLoggingResources();
+
+			// Unregister the profiler from Teamscale.
+			if (agentOptions != null && agentOptions.configurationViaTeamscale != null) {
+				agentOptions.configurationViaTeamscale.unregisterProfiler();
+			}
+
+			throw e;
 		} catch (AgentOptionReceiveException e) {
 			// When Teamscale is not available, we don't want to fail hard to still allow for testing even if no
 			// coverage is collected (see TS-33237)
@@ -94,14 +115,14 @@ public static void premain(String options, Instrumentation instrumentation) thro
 		agent.registerShutdownHook();
 	}
 
-	@NotNull
-	private static AgentOptions getAndApplyAgentOptions(String options, String environmentConfigId,
-														String environmentConfigFile) throws AgentOptionParseException, IOException, AgentOptionReceiveException {
+	private static Pair<AgentOptions, List<Exception>> getAndApplyAgentOptions(String options, String environmentConfigId,
+			String environmentConfigFile) throws AgentOptionParseException, IOException, AgentOptionReceiveException {
 
 		DelayedLogger delayedLogger = new DelayedLogger();
 		List<String> javaAgents = CollectionUtils.filter(ManagementFactory.getRuntimeMXBean().getInputArguments(),
 				s -> s.contains("-javaagent"));
-		if (javaAgents.size() > 1) {
+		// We allow multiple instances of the teamscale-jacoco-agent as we ensure with the #LOCKING_SYSTEM_PROPERTY to only use it once
+		if (!javaAgents.stream().allMatch(javaAgent -> javaAgent.contains("teamscale-jacoco-agent.jar"))) {
 			delayedLogger.warn("Using multiple java agents could interfere with coverage recording.");
 		}
 		if (!javaAgents.get(0).contains("teamscale-jacoco-agent.jar")) {
@@ -112,9 +133,14 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir
 		if (credentials == null) {
 			delayedLogger.warn("Did not find a teamscale.properties file!");
 		}
+
+		Pair<AgentOptions, List<Exception>> parseResult;
 		AgentOptions agentOptions;
 		try {
-			agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials, delayedLogger);
+			parseResult = AgentOptionsParser.parse(
+					options, environmentConfigId, environmentConfigFile, credentials,
+					delayedLogger);
+			agentOptions = parseResult.getFirst();
 		} catch (AgentOptionParseException e) {
 			try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) {
 				delayedLogger.errorAndStdErr("Failed to parse agent options: " + e.getMessage(), e);
@@ -135,7 +161,8 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir
 		Logger logger = LoggingUtils.getLogger(Agent.class);
 		delayedLogger.logTo(logger);
 		HttpUtils.setShouldValidateSsl(agentOptions.shouldValidateSsl());
-		return agentOptions;
+
+		return parseResult;
 	}
 
 	private static void attemptLogAndThrow(DelayedLogger delayedLogger) {
@@ -156,7 +183,9 @@ private static void initializeLogging(AgentOptions agentOptions, DelayedLogger l
 		}
 
 		if (agentOptions.getTeamscaleServerOptions().isConfiguredForServerConnection()) {
-			LogToTeamscaleAppender.addTeamscaleAppenderTo(getLoggerContext(), agentOptions);
+			if (LogToTeamscaleAppender.addTeamscaleAppenderTo(getLoggerContext(), agentOptions)) {
+				logger.info("Logs are being forwarded to Teamscale at " + agentOptions.getTeamscaleServerOptions().url);
+			}
 		}
 	}
 
@@ -170,7 +199,7 @@ static void closeLoggingResources() {
 	 * the HTTP server is used.
 	 */
 	private static AgentBase createAgent(AgentOptions agentOptions,
-										 Instrumentation instrumentation) throws UploaderException, IOException {
+			Instrumentation instrumentation) throws UploaderException, IOException {
 		if (agentOptions.useTestwiseCoverageMode()) {
 			return TestwiseCoverageAgent.create(agentOptions);
 		} else {
@@ -198,7 +227,7 @@ private static void initializeDebugLogging(AgentOptions agentOptions, DelayedLog
 	 * this and falls back to the default logger.
 	 */
 	private static LoggingUtils.LoggingResources initializeFallbackLogging(String premainOptions,
-																		   DelayedLogger delayedLogger) {
+			DelayedLogger delayedLogger) {
 		if (premainOptions == null) {
 			return LoggingUtils.initializeDefaultLogging();
 		}
@@ -231,7 +260,7 @@ private static LoggingUtils.LoggingResources initializeFallbackLogging(String pr
 
 	/** Creates a fallback logger using the given config file. */
 	private static LoggingUtils.LoggingResources createFallbackLoggerFromConfig(String configLocation,
-																				DelayedLogger delayedLogger) {
+			ILogger delayedLogger) {
 		try {
 			return LoggingUtils.initializeLogging(
 					new FilePatternResolver(delayedLogger).parsePath(AgentOptionsParser.LOGGING_CONFIG_OPTION,

agent/src/main/java/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.java

@@ -4,11 +4,13 @@
 import com.teamscale.jacoco.agent.upload.teamscale.DelayedTeamscaleMultiProjectUploader;
 import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
 import com.teamscale.jacoco.agent.logging.LoggingUtils;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 
 import java.io.File;
 import java.io.IOException;
+import java.time.format.DateTimeFormatter;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -27,19 +29,22 @@ public class GitMultiProjectPropertiesLocator implements IGitPropertiesLocator {
 
 	private final boolean recursiveSearch;
 
-	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, boolean recursiveSearch) {
+	private final @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat;
+
+	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, boolean recursiveSearch, @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		// using a single threaded executor allows this class to be lock-free
 		this(uploader, Executors
 				.newSingleThreadExecutor(
 						new DaemonThreadFactory(GitMultiProjectPropertiesLocator.class,
-								"git.properties Jar scanner thread")), recursiveSearch);
+								"git.properties Jar scanner thread")), recursiveSearch, gitPropertiesCommitTimeFormat);
 	}
 
 	public GitMultiProjectPropertiesLocator(DelayedTeamscaleMultiProjectUploader uploader, Executor executor,
-			boolean recursiveSearch) {
+			boolean recursiveSearch, @Nullable DateTimeFormatter gitPropertiesCommitTimeFormat) {
 		this.uploader = uploader;
 		this.executor = executor;
 		this.recursiveSearch = recursiveSearch;
+		this.gitPropertiesCommitTimeFormat = gitPropertiesCommitTimeFormat;
 	}
 
 	/**
@@ -62,7 +67,7 @@ void searchFile(File file, boolean isJarFile) {
 			List<ProjectAndCommit> projectAndCommits = GitPropertiesLocatorUtils.getProjectRevisionsFromGitProperties(
 					file,
 					isJarFile,
-					recursiveSearch);
+					recursiveSearch, gitPropertiesCommitTimeFormat);
 			if (projectAndCommits.isEmpty()) {
 				logger.debug("No git.properties file found in {}", file);
 				return;