From 3abf8b6b515c3c534eb951486db6095643fa0baa Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Tue, 8 Oct 2024 17:29:00 +0200 Subject: [PATCH 01/10] WIP --- .../agent/logging/DelayedLogAppender.java | 40 +++++++++++++++++++ .../jacoco/agent/logging/LoggingUtils.java | 1 + .../com/teamscale/jacoco/agent/logback.xml | 12 ++++++ 3 files changed, 53 insertions(+) create mode 100644 agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java create mode 100644 agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java new file mode 100644 index 000000000..a65e426ff --- /dev/null +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java @@ -0,0 +1,40 @@ +package com.teamscale.jacoco.agent.logging; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.AppenderBase; + +import java.util.ArrayList; +import java.util.List; + +public class DelayedLogAppender extends AppenderBase { + + private static final List BUFFER = new ArrayList<>(); + + @Override + protected void append(ILoggingEvent eventObject) { + synchronized (BUFFER) { + BUFFER.add(eventObject); + } + } + + public static void logTo(LoggerContext context) { + synchronized (BUFFER) { + for (ILoggingEvent event : BUFFER) { + context.getLogger(event.getLoggerName()).callAppenders(event); + } + BUFFER.clear(); + } + } + + public static void addAppenderTo(LoggerContext context) { + DelayedLogAppender appender = new DelayedLogAppender(); + appender.setContext(context); + appender.start(); + + Logger rootLogger = context.getLogger(Logger.ROOT_LOGGER_NAME); + rootLogger.addAppender(appender); + } + +} diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java index 5f90300e0..988dfb7ea 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java @@ -75,6 +75,7 @@ private static void reconfigureLoggerContext(InputStream stream) { // StatusPrinter will handle this } StatusPrinter.printInCaseOfErrorsOrWarnings(loggerContext); + DelayedLogAppender.logTo(loggerContext); } /** diff --git a/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml b/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml new file mode 100644 index 000000000..50e7b0bdd --- /dev/null +++ b/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + From ebc6884c11dbc9579c3188af171c69d2cb23ba4b Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Tue, 3 Dec 2024 16:25:35 +0100 Subject: [PATCH 02/10] TS-40806 replace usages of delayedLogger with DelayedLogAppender --- .../com/teamscale/jacoco/agent/PreMain.java | 59 +++++++++---------- .../ConfigurationViaTeamscale.java | 5 +- .../ProcessInformationRetriever.java | 8 +-- .../jacoco/agent/convert/ConvertCommand.java | 3 +- .../agent/logging/DelayedLogAppender.java | 10 ++++ .../jacoco/agent/logging/LoggingUtils.java | 4 +- .../jacoco/agent/options/AgentOptions.java | 8 +-- .../agent/options/AgentOptionsParser.java | 24 ++++---- .../agent/options/FilePatternResolver.java | 11 ++-- .../upload/teamscale/TeamscaleConfig.java | 8 +-- .../agent/options/AgentOptionsParserTest.java | 15 +++-- .../agent/options/AgentOptionsTest.java | 3 +- .../options/FilePatternResolverTest.java | 3 +- .../options/TestAgentOptionsBuilder.java | 5 +- 14 files changed, 82 insertions(+), 84 deletions(-) diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index f9d7931ed..18791700e 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/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,7 +12,6 @@ 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; @@ -39,6 +37,8 @@ /** Container class for the premain entry point for the agent. */ public class PreMain { + private static Logger logger = LoggingUtils.getLogger(PreMain.class); + private static LoggingUtils.LoggingResources loggingResources = null; /** @@ -102,31 +102,29 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir List javaAgents = CollectionUtils.filter(ManagementFactory.getRuntimeMXBean().getInputArguments(), s -> s.contains("-javaagent")); if (javaAgents.size() > 1) { - delayedLogger.warn("Using multiple java agents could interfere with coverage recording."); + logger.warn("Using multiple java agents could interfere with coverage recording."); } if (!javaAgents.get(0).contains("teamscale-jacoco-agent.jar")) { - delayedLogger.warn("For best results consider registering the Teamscale JaCoCo Agent first."); + logger.warn("For best results consider registering the Teamscale JaCoCo Agent first."); } TeamscaleCredentials credentials = TeamscalePropertiesUtils.parseCredentials(); if (credentials == null) { - delayedLogger.warn("Did not find a teamscale.properties file!"); + logger.warn("Did not find a teamscale.properties file!"); } AgentOptions agentOptions; try { - agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials, delayedLogger); + agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials); } catch (AgentOptionParseException e) { try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) { - delayedLogger.errorAndStdErr("Failed to parse agent options: " + e.getMessage(), e); - attemptLogAndThrow(delayedLogger); + String message = "Failed to parse agent options: " + e.getMessage(); + logAndPrintError(e, message); throw e; } } catch (AgentOptionReceiveException e) { try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) { - delayedLogger.errorAndStdErr( - e.getMessage() + " The application should start up normally, but NO coverage will be collected!", - e); - attemptLogAndThrow(delayedLogger); + String message = e.getMessage() + " The application should start up normally, but NO coverage will be collected!"; + logAndPrintError(e, message); throw e; } } @@ -138,14 +136,6 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir return agentOptions; } - private static void attemptLogAndThrow(DelayedLogger delayedLogger) { - // We perform actual logging output after writing to console to - // ensure the console is reached even in case of logging issues - // (see TS-23151). We use the Agent class here (same as below) - Logger logger = LoggingUtils.getLogger(Agent.class); - delayedLogger.logTo(logger); - } - /** Initializes logging during {@link #premain(String, Instrumentation)} and also logs the log directory. */ private static void initializeLogging(AgentOptions agentOptions, DelayedLogger logger) throws IOException { if (agentOptions.isDebugLogging()) { @@ -204,24 +194,24 @@ private static LoggingUtils.LoggingResources initializeFallbackLogging(String pr } for (String optionPart : premainOptions.split(",")) { if (optionPart.startsWith(AgentOptionsParser.LOGGING_CONFIG_OPTION + "=")) { - return createFallbackLoggerFromConfig(optionPart.split("=", 2)[1], delayedLogger); + return createFallbackLoggerFromConfig(optionPart.split("=", 2)[1]); } if (optionPart.startsWith(AgentOptionsParser.CONFIG_FILE_OPTION + "=")) { String configFileValue = optionPart.split("=", 2)[1]; Optional loggingConfigLine = Optional.empty(); try { - File configFile = new FilePatternResolver(delayedLogger).parsePath( + File configFile = new FilePatternResolver().parsePath( AgentOptionsParser.CONFIG_FILE_OPTION, configFileValue).toFile(); loggingConfigLine = FileSystemUtils.readLinesUTF8(configFile).stream() .filter(line -> line.startsWith(AgentOptionsParser.LOGGING_CONFIG_OPTION + "=")) .findFirst(); } catch (IOException | AgentOptionParseException e) { - delayedLogger.error("Failed to load configuration from " + configFileValue + ": " + e.getMessage(), - e); + String message = "Failed to load configuration from " + configFileValue + ": " + e.getMessage(); + logAndPrintError(e, message); } if (loggingConfigLine.isPresent()) { - return createFallbackLoggerFromConfig(loggingConfigLine.get().split("=", 2)[1], delayedLogger); + return createFallbackLoggerFromConfig(loggingConfigLine.get().split("=", 2)[1]); } } } @@ -230,19 +220,24 @@ 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) { + private static LoggingUtils.LoggingResources createFallbackLoggerFromConfig(String configLocation) { try { return LoggingUtils.initializeLogging( - new FilePatternResolver(delayedLogger).parsePath(AgentOptionsParser.LOGGING_CONFIG_OPTION, + new FilePatternResolver().parsePath(AgentOptionsParser.LOGGING_CONFIG_OPTION, configLocation)); } catch (IOException | AgentOptionParseException e) { String message = "Failed to load log configuration from location " + configLocation + ": " + e.getMessage(); - delayedLogger.error(message, e); - // output the message to console as well, as this might - // otherwise not make it to the user - System.err.println(message); + logAndPrintError(e, message); return LoggingUtils.initializeDefaultLogging(); } } + + /** + * Log the error and also print it to System Error, as the error might prevent the initialization of a real logger. + */ + private static void logAndPrintError(Exception e, String message) { + logger.error(message, + e); + System.err.println(message); + } } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ConfigurationViaTeamscale.java b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ConfigurationViaTeamscale.java index 48ee78655..ca25d8225 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ConfigurationViaTeamscale.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ConfigurationViaTeamscale.java @@ -8,7 +8,6 @@ import com.teamscale.client.TeamscaleServiceGenerator; import com.teamscale.jacoco.agent.options.AgentOptionParseException; import com.teamscale.jacoco.agent.logging.LoggingUtils; -import com.teamscale.report.util.ILogger; import okhttp3.HttpUrl; import okhttp3.ResponseBody; import retrofit2.Response; @@ -51,13 +50,13 @@ public ConfigurationViaTeamscale(ITeamscaleService teamscaleClient, ProfilerRegi * Tries to retrieve the profiler configuration from Teamscale. In case retrieval fails the method throws a * {@link AgentOptionReceiveException}. */ - public static ConfigurationViaTeamscale retrieve(ILogger logger, String configurationId, HttpUrl url, + public static ConfigurationViaTeamscale retrieve(String configurationId, HttpUrl url, String userName, String userAccessToken) throws AgentOptionReceiveException, AgentOptionParseException { ITeamscaleService teamscaleClient = TeamscaleServiceGenerator .createService(ITeamscaleService.class, url, userName, userAccessToken, LONG_TIMEOUT, LONG_TIMEOUT); try { - ProcessInformation processInformation = new ProcessInformationRetriever(logger).getProcessInformation(); + ProcessInformation processInformation = new ProcessInformationRetriever().getProcessInformation(); Response response = teamscaleClient.registerProfiler(configurationId, processInformation).execute(); if (!response.isSuccessful()) { diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java index 63a34f4cf..e79eeeab5 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java @@ -1,7 +1,8 @@ package com.teamscale.jacoco.agent.configuration; import com.teamscale.client.ProcessInformation; -import com.teamscale.report.util.ILogger; +import com.teamscale.jacoco.agent.logging.LoggingUtils; +import org.slf4j.Logger; import java.lang.management.ManagementFactory; import java.lang.reflect.InvocationTargetException; @@ -13,10 +14,9 @@ */ public class ProcessInformationRetriever { - private final ILogger logger; + private final Logger logger = LoggingUtils.getLogger(this); - public ProcessInformationRetriever(ILogger logger) { - this.logger = logger; + public ProcessInformationRetriever() { } /** diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/convert/ConvertCommand.java b/agent/src/main/java/com/teamscale/jacoco/agent/convert/ConvertCommand.java index 72b68ea05..ed37a9c1f 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/convert/ConvertCommand.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/convert/ConvertCommand.java @@ -14,7 +14,6 @@ import com.teamscale.jacoco.agent.options.ClasspathUtils; import com.teamscale.jacoco.agent.options.FilePatternResolver; import com.teamscale.report.EDuplicateClassFileBehavior; -import com.teamscale.report.util.CommandLineLogger; import org.conqat.lib.commons.assertion.CCSMAssert; import org.conqat.lib.commons.filesystem.FileSystemUtils; import org.conqat.lib.commons.string.StringUtils; @@ -95,7 +94,7 @@ public class ConvertCommand implements ICommand { /** @see #classDirectoriesOrZips */ public List getClassDirectoriesOrZips() throws AgentOptionParseException { return ClasspathUtils - .resolveClasspathTextFiles("class-dir", new FilePatternResolver(new CommandLineLogger()), + .resolveClasspathTextFiles("class-dir", new FilePatternResolver(), classDirectoriesOrZips); } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java index a65e426ff..19cee0a15 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java @@ -8,10 +8,16 @@ import java.util.ArrayList; import java.util.List; +/** + * An appender that collects log statements in its buffer until it's asked to empty it into a given + * {@link LoggerContext}. It can be used to log log-messages before an actual logging appender has been configured using + * the options passed to the agent. + * */ public class DelayedLogAppender extends AppenderBase { private static final List BUFFER = new ArrayList<>(); + /** Appends the given {@link ILoggingEvent} to logger's buffer. */ @Override protected void append(ILoggingEvent eventObject) { synchronized (BUFFER) { @@ -19,6 +25,10 @@ protected void append(ILoggingEvent eventObject) { } } + /** + * logs the log-messages currently stored in the buffer into the given {@link LoggerContext} and empties the + * buffer. + * */ public static void logTo(LoggerContext context) { synchronized (BUFFER) { for (ILoggingEvent event : BUFFER) { diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java index 988dfb7ea..58e482b97 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java @@ -15,10 +15,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; +import java.nio.file.Files; import java.nio.file.Path; /** @@ -87,7 +87,7 @@ public static LoggingResources initializeLogging(Path loggingConfigFile) throws return initializeDefaultLogging(); } - reconfigureLoggerContext(new FileInputStream(loggingConfigFile.toFile())); + reconfigureLoggerContext(Files.newInputStream(loggingConfigFile.toFile().toPath())); return new LoggingResources(); } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java index f2855083b..eb12f1840 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java @@ -18,6 +18,7 @@ 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.logging.LoggingUtils; import com.teamscale.jacoco.agent.options.sapnwdi.DelayedSapNwdiMultiUploader; import com.teamscale.jacoco.agent.options.sapnwdi.SapNwdiApplication; import com.teamscale.jacoco.agent.upload.IUploader; @@ -34,11 +35,11 @@ import com.teamscale.jacoco.agent.util.AgentUtils; import com.teamscale.report.EDuplicateClassFileBehavior; import com.teamscale.report.util.ClasspathWildcardIncludeFilter; -import com.teamscale.report.util.ILogger; import org.conqat.lib.commons.assertion.CCSMAssert; import org.conqat.lib.commons.collections.PairList; import org.jacoco.core.runtime.WildcardMatcher; import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; import java.io.File; import java.io.IOException; @@ -73,7 +74,7 @@ public class AgentOptions { */ public static final String DEFAULT_EXCLUDES = "shadow.*:com.sun.*:sun.*:org.eclipse.*:org.junit.*:junit.*:org.apache.*:org.slf4j.*:javax.*:org.gradle.*:java.*:org.jboss.*:org.wildfly.*:org.springframework.*:com.fasterxml.*:jakarta.*:org.aspectj.*:org.h2.*:org.hibernate.*:org.assertj.*:org.mockito.*:org.thymeleaf.*"; - private final ILogger logger; + private final Logger logger = LoggingUtils.getLogger(this); /** See {@link AgentOptions#GIT_PROPERTIES_JAR_OPTION} */ /* package */ File gitPropertiesJar; @@ -207,8 +208,7 @@ public class AgentOptions { */ public ConfigurationViaTeamscale configurationViaTeamscale; - public AgentOptions(ILogger logger) { - this.logger = logger; + public AgentOptions() { setParentOutputDirectory(AgentUtils.getMainTempDirectory().resolve("coverage")); } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java index a91a626ad..57b8b13a4 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java @@ -10,16 +10,17 @@ import com.teamscale.jacoco.agent.commandline.Validator; import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException; import com.teamscale.jacoco.agent.configuration.ConfigurationViaTeamscale; +import com.teamscale.jacoco.agent.logging.LoggingUtils; import com.teamscale.jacoco.agent.options.sapnwdi.SapNwdiApplication; import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig; import com.teamscale.jacoco.agent.upload.azure.AzureFileStorageConfig; import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig; import com.teamscale.report.EDuplicateClassFileBehavior; -import com.teamscale.report.util.ILogger; import okhttp3.HttpUrl; import org.conqat.lib.commons.collections.CollectionUtils; import org.conqat.lib.commons.collections.Pair; import org.conqat.lib.commons.filesystem.FileSystemUtils; +import org.slf4j.Logger; import java.io.File; import java.io.FileNotFoundException; @@ -45,13 +46,14 @@ public class AgentOptionsParser { /** Character which starts a comment in the config file. */ private static final String COMMENT_PREFIX = "#"; - private final ILogger logger; + private final Logger logger = LoggingUtils.getLogger(this); private final FilePatternResolver filePatternResolver; private final TeamscaleConfig teamscaleConfig; private final String environmentConfigId; private final String environmentConfigFile; private final TeamscaleCredentials credentials; + /** * Parses the given command-line options. * @@ -59,18 +61,16 @@ public class AgentOptionsParser { * @param environmentConfigFile The Profiler configuration file given via an environment variable. */ public static AgentOptions parse(String optionsString, String environmentConfigId, String environmentConfigFile, - TeamscaleCredentials credentials, - ILogger logger) throws AgentOptionParseException, AgentOptionReceiveException { - return new AgentOptionsParser(logger, environmentConfigId, environmentConfigFile, credentials).parse( + TeamscaleCredentials credentials) throws AgentOptionParseException, AgentOptionReceiveException { + return new AgentOptionsParser(environmentConfigId, environmentConfigFile, credentials).parse( optionsString); } @VisibleForTesting - AgentOptionsParser(ILogger logger, String environmentConfigId, String environmentConfigFile, + AgentOptionsParser(String environmentConfigId, String environmentConfigFile, TeamscaleCredentials credentials) { - this.logger = logger; - this.filePatternResolver = new FilePatternResolver(logger); - this.teamscaleConfig = new TeamscaleConfig(logger, filePatternResolver); + this.filePatternResolver = new FilePatternResolver(); + this.teamscaleConfig = new TeamscaleConfig(filePatternResolver); this.environmentConfigId = environmentConfigId; this.environmentConfigFile = environmentConfigFile; this.credentials = credentials; @@ -84,8 +84,8 @@ public static AgentOptions parse(String optionsString, String environmentConfigI if (optionsString == null) { optionsString = ""; } - logger.debug("Parsing options: " + optionsString); - AgentOptions options = new AgentOptions(logger); + logger.debug("Parsing options: {}", optionsString); + AgentOptions options = new AgentOptions(); options.originalOptionsString = optionsString; if (credentials != null) { @@ -280,7 +280,7 @@ private void readConfigFromTeamscale(AgentOptions options, "Has specified config-id '" + configId + "' without teamscale url/user/accessKey! The options need to be defined in teamscale.properties."); } options.teamscaleServer.configId = configId; - ConfigurationViaTeamscale configuration = ConfigurationViaTeamscale.retrieve(logger, configId, + ConfigurationViaTeamscale configuration = ConfigurationViaTeamscale.retrieve(configId, options.teamscaleServer.url, options.teamscaleServer.userName, options.teamscaleServer.userAccessToken); diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java index 6a5411cbd..f801a7ce2 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java @@ -1,10 +1,12 @@ package com.teamscale.jacoco.agent.options; +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.AntPatternUtils; import org.conqat.lib.commons.filesystem.FileSystemUtils; +import org.slf4j.Logger; import java.io.File; import java.io.IOException; @@ -29,10 +31,9 @@ public class FilePatternResolver { /** Stand-in for the asterisk operator. */ private static final String ASTERISK_REPLACEMENT = "#@"; - private final ILogger logger; + private final Logger logger = LoggingUtils.getLogger(this); - public FilePatternResolver(ILogger logger) { - this.logger = logger; + public FilePatternResolver() { } /** @@ -120,9 +121,9 @@ private static class FilePatternResolverRun { private String suffixPattern; private Path basePath; private List matchingPaths; - private final ILogger logger; + private final Logger logger; - private FilePatternResolverRun(ILogger logger, String optionName, String pattern, File workingDirectory) { + private FilePatternResolverRun(Logger logger, String optionName, String pattern, File workingDirectory) { this.logger = logger; this.optionName = optionName; this.pattern = pattern; diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/TeamscaleConfig.java b/agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/TeamscaleConfig.java index 3ed0964a4..fe6d435c7 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/TeamscaleConfig.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/upload/teamscale/TeamscaleConfig.java @@ -9,11 +9,12 @@ import com.teamscale.client.CommitDescriptor; import com.teamscale.client.StringUtils; import com.teamscale.client.TeamscaleServer; +import com.teamscale.jacoco.agent.logging.LoggingUtils; import com.teamscale.jacoco.agent.options.AgentOptionParseException; import com.teamscale.jacoco.agent.options.AgentOptionsParser; import com.teamscale.jacoco.agent.options.FilePatternResolver; import com.teamscale.report.util.BashFileSkippingInputStream; -import com.teamscale.report.util.ILogger; +import org.slf4j.Logger; /** Config necessary for direct Teamscale upload. */ public class TeamscaleConfig { @@ -30,11 +31,10 @@ public class TeamscaleConfig { /** Option name that allows to specify a jar file that contains the branch name and timestamp in a MANIFEST.MF file. */ public static final String TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION = "teamscale-commit-manifest-jar"; - private final ILogger logger; + private final Logger logger = LoggingUtils.getLogger(this); private final FilePatternResolver filePatternResolver; - public TeamscaleConfig(ILogger logger, FilePatternResolver filePatternResolver) { - this.logger = logger; + public TeamscaleConfig(FilePatternResolver filePatternResolver) { this.filePatternResolver = filePatternResolver; } diff --git a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java index 64615679c..237092a19 100644 --- a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java +++ b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java @@ -5,7 +5,6 @@ import com.teamscale.client.ProfilerRegistration; import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig; import com.teamscale.jacoco.agent.util.TestUtils; -import com.teamscale.report.util.CommandLineLogger; import okhttp3.HttpUrl; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; @@ -27,7 +26,7 @@ public class AgentOptionsParserTest { private TeamscaleCredentials teamscaleCredentials; - private final AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, null, null); + private final AgentOptionsParser parser = new AgentOptionsParser(null, null, null); private Path configFile; /** The mock server to run requests against. */ protected MockWebServer mockWebServer; @@ -72,7 +71,7 @@ public void testUploadMethodRecognition() throws Exception { @Test public void testUploadMethodRecognitionWithTeamscaleProperties() throws Exception { TeamscaleCredentials credentials = new TeamscaleCredentials(HttpUrl.get("http://localhost"), "user", "key"); - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, null, credentials); + AgentOptionsParser parser = new AgentOptionsParser(null, null, credentials); assertThat(parser.parse(null).determineUploadMethod()).isEqualTo(AgentOptions.EUploadMethod.LOCAL_DISK); assertThat(parser.parse("azure-url=azure.com,azure-key=key").determineUploadMethod()).isEqualTo( @@ -102,7 +101,7 @@ public void environmentConfigIdOverridesCommandLineOptions() throws Exception { registration.profilerConfiguration.configurationId = "my-config"; registration.profilerConfiguration.configurationOptions = "teamscale-partition=foo"; mockWebServer.enqueue(new MockResponse().setBody(JsonUtils.serialize(registration))); - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), "my-config", + AgentOptionsParser parser = new AgentOptionsParser("my-config", null, teamscaleCredentials); AgentOptions options = parser.parse("teamscale-partition=bar"); @@ -111,7 +110,7 @@ public void environmentConfigIdOverridesCommandLineOptions() throws Exception { @Test public void environmentConfigFileOverridesCommandLineOptions() throws Exception { - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, configFile.toString(), + AgentOptionsParser parser = new AgentOptionsParser(null, configFile.toString(), teamscaleCredentials); AgentOptions options = parser.parse("teamscale-partition=from-command-line"); @@ -126,7 +125,7 @@ public void environmentConfigFileOverridesConfigId() throws Exception { registration.profilerConfiguration.configurationId = "my-config"; registration.profilerConfiguration.configurationOptions = "teamscale-partition=from-config-id"; mockWebServer.enqueue(new MockResponse().setBody(JsonUtils.serialize(registration))); - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), "my-config", configFile.toString(), + AgentOptionsParser parser = new AgentOptionsParser("my-config", configFile.toString(), teamscaleCredentials); AgentOptions options = parser.parse("teamscale-partition=from-command-line"); @@ -211,7 +210,7 @@ public void revisionOrCommitRequireProject() { public void environmentConfigIdDoesNotExist() { mockWebServer.enqueue(new MockResponse().setResponseCode(404).setBody("invalid-config-id does not exist")); assertThatThrownBy( - () -> new AgentOptionsParser(new CommandLineLogger(), "invalid-config-id", null, + () -> new AgentOptionsParser("invalid-config-id", null, teamscaleCredentials).parse( "") ).isInstanceOf(AgentOptionParseException.class).hasMessageContaining("invalid-config-id does not exist"); @@ -232,7 +231,7 @@ public void mustPreserveDefaultExcludes() throws Exception { @Test public void teamscalePropertiesCredentialsUsedAsDefaultButOverridable() throws Exception { - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials); + AgentOptionsParser parser = new AgentOptionsParser(null, null, teamscaleCredentials); assertThat(parser.parse("teamscale-project=p,teamscale-partition=p").teamscaleServer.userName).isEqualTo( "user"); diff --git a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java index d0020f8fa..82f10fa4f 100644 --- a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java +++ b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java @@ -4,7 +4,6 @@ import com.teamscale.client.TeamscaleServer; import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig; import com.teamscale.jacoco.agent.util.TestUtils; -import com.teamscale.report.util.CommandLineLogger; import okhttp3.HttpUrl; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; @@ -339,7 +338,7 @@ private static Predicate excludeFilter(String filterString) throws Excep } private static AgentOptionsParser getAgentOptionsParserWithDummyLogger() { - return new AgentOptionsParser(new CommandLineLogger(), null, null, null); + return new AgentOptionsParser(null, null, null); } /** diff --git a/agent/src/test/java/com/teamscale/jacoco/agent/options/FilePatternResolverTest.java b/agent/src/test/java/com/teamscale/jacoco/agent/options/FilePatternResolverTest.java index a652ebce3..0915f4667 100644 --- a/agent/src/test/java/com/teamscale/jacoco/agent/options/FilePatternResolverTest.java +++ b/agent/src/test/java/com/teamscale/jacoco/agent/options/FilePatternResolverTest.java @@ -1,7 +1,6 @@ package com.teamscale.jacoco.agent.options; import com.teamscale.jacoco.agent.util.TestUtils; -import com.teamscale.report.util.CommandLineLogger; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -84,7 +83,7 @@ void testPathResolutionWithPatternErrorCases() { } private static FilePatternResolver getFilePatternResolverWithDummyLogger() { - return new FilePatternResolver(new CommandLineLogger()); + return new FilePatternResolver(); } @Test diff --git a/agent/src/test/java/com/teamscale/jacoco/agent/options/TestAgentOptionsBuilder.java b/agent/src/test/java/com/teamscale/jacoco/agent/options/TestAgentOptionsBuilder.java index 081c64efd..097b05a83 100644 --- a/agent/src/test/java/com/teamscale/jacoco/agent/options/TestAgentOptionsBuilder.java +++ b/agent/src/test/java/com/teamscale/jacoco/agent/options/TestAgentOptionsBuilder.java @@ -4,11 +4,8 @@ import com.teamscale.client.TeamscaleServer; import com.teamscale.jacoco.agent.commit_resolution.git_properties.CommitInfo; import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig; -import com.teamscale.report.util.ILogger; import okhttp3.HttpUrl; -import static org.mockito.Mockito.mock; - /** * Builds {@link AgentOptions} for test purposes */ @@ -106,7 +103,7 @@ public TestAgentOptionsBuilder withMinimalArtifactoryConfig(String apiKey, Strin * Builds the {@link AgentOptions}. **/ public AgentOptions create() { - AgentOptions agentOptions = new AgentOptions(mock(ILogger.class)); + AgentOptions agentOptions = new AgentOptions(); agentOptions.teamscaleServer = teamscaleServer; agentOptions.httpServerPort = httpServerPort; agentOptions.artifactoryConfig = artifactoryConfig; From 6ef19dc102a70bb0e4f42a15d7f5185aae347101 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Tue, 3 Dec 2024 16:30:07 +0100 Subject: [PATCH 03/10] TS-40806 clean-up --- .../com/teamscale/jacoco/agent/PreMain.java | 20 ++++++++----------- .../agent/logging/DelayedLogAppender.java | 11 ---------- .../agent/options/FilePatternResolver.java | 1 - 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index 18791700e..ec515fde6 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -37,7 +37,7 @@ /** Container class for the premain entry point for the agent. */ public class PreMain { - private static Logger logger = LoggingUtils.getLogger(PreMain.class); + private static final Logger logger = LoggingUtils.getLogger(PreMain.class); private static LoggingUtils.LoggingResources loggingResources = null; @@ -98,7 +98,6 @@ public static void premain(String options, Instrumentation instrumentation) thro private static AgentOptions getAndApplyAgentOptions(String options, String environmentConfigId, String environmentConfigFile) throws AgentOptionParseException, IOException, AgentOptionReceiveException { - DelayedLogger delayedLogger = new DelayedLogger(); List javaAgents = CollectionUtils.filter(ManagementFactory.getRuntimeMXBean().getInputArguments(), s -> s.contains("-javaagent")); if (javaAgents.size() > 1) { @@ -116,30 +115,28 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir try { agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials); } catch (AgentOptionParseException e) { - try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) { + try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options)) { String message = "Failed to parse agent options: " + e.getMessage(); logAndPrintError(e, message); throw e; } } catch (AgentOptionReceiveException e) { - try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) { + try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options)) { String message = e.getMessage() + " The application should start up normally, but NO coverage will be collected!"; logAndPrintError(e, message); throw e; } } - initializeLogging(agentOptions, delayedLogger); - Logger logger = LoggingUtils.getLogger(Agent.class); - delayedLogger.logTo(logger); + initializeLogging(agentOptions); HttpUtils.setShouldValidateSsl(agentOptions.shouldValidateSsl()); return agentOptions; } /** Initializes logging during {@link #premain(String, Instrumentation)} and also logs the log directory. */ - private static void initializeLogging(AgentOptions agentOptions, DelayedLogger logger) throws IOException { + private static void initializeLogging(AgentOptions agentOptions) throws IOException { if (agentOptions.isDebugLogging()) { - initializeDebugLogging(agentOptions, logger); + initializeDebugLogging(agentOptions); } else { loggingResources = LoggingUtils.initializeLogging(agentOptions.getLoggingConfig()); logger.info("Logging to " + new LogDirectoryPropertyDefiner().getPropertyValue()); @@ -172,7 +169,7 @@ private static AgentBase createAgent(AgentOptions agentOptions, * Initializes debug logging during {@link #premain(String, Instrumentation)} and also logs the log directory if * given. */ - private static void initializeDebugLogging(AgentOptions agentOptions, DelayedLogger logger) { + private static void initializeDebugLogging(AgentOptions agentOptions) { loggingResources = LoggingUtils.initializeDebugLogging(agentOptions.getDebugLogDirectory()); Path logDirectory = Paths.get(new DebugLogDirectoryPropertyDefiner().getPropertyValue()); if (FileSystemUtils.isValidPath(logDirectory.toString()) && Files.isWritable(logDirectory)) { @@ -187,8 +184,7 @@ private static void initializeDebugLogging(AgentOptions agentOptions, DelayedLog * {@link #premain(String, Instrumentation)} (see TS-23151). This tries to extract the logging configuration and use * this and falls back to the default logger. */ - private static LoggingUtils.LoggingResources initializeFallbackLogging(String premainOptions, - DelayedLogger delayedLogger) { + private static LoggingUtils.LoggingResources initializeFallbackLogging(String premainOptions) { if (premainOptions == null) { return LoggingUtils.initializeDefaultLogging(); } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java index 19cee0a15..64ba37cff 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java @@ -1,6 +1,5 @@ package com.teamscale.jacoco.agent.logging; -import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.AppenderBase; @@ -37,14 +36,4 @@ public static void logTo(LoggerContext context) { BUFFER.clear(); } } - - public static void addAppenderTo(LoggerContext context) { - DelayedLogAppender appender = new DelayedLogAppender(); - appender.setContext(context); - appender.start(); - - Logger rootLogger = context.getLogger(Logger.ROOT_LOGGER_NAME); - rootLogger.addAppender(appender); - } - } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java index f801a7ce2..04ddfed9a 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java @@ -1,7 +1,6 @@ package com.teamscale.jacoco.agent.options; 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.AntPatternUtils; From e8cdeeae577dab181a965ece388cb87ca97196fa Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Mon, 9 Dec 2024 14:03:04 +0100 Subject: [PATCH 04/10] TS-40806 clean-up --- .../teamscale/jacoco/agent/DelayedLogger.java | 70 ------------------- .../com/teamscale/jacoco/agent/PreMain.java | 18 ++--- .../ProcessInformationRetriever.java | 3 - .../agent/options/FilePatternResolver.java | 3 - 4 files changed, 9 insertions(+), 85 deletions(-) delete mode 100644 agent/src/main/java/com/teamscale/jacoco/agent/DelayedLogger.java diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/DelayedLogger.java b/agent/src/main/java/com/teamscale/jacoco/agent/DelayedLogger.java deleted file mode 100644 index 574389c58..000000000 --- a/agent/src/main/java/com/teamscale/jacoco/agent/DelayedLogger.java +++ /dev/null @@ -1,70 +0,0 @@ -package com.teamscale.jacoco.agent; - -import com.teamscale.report.util.ILogger; -import org.slf4j.Logger; - -import java.util.ArrayList; -import java.util.List; - -/** - * A logger that buffers logs in memory and writes them to the actual logger at a later point. This is needed when stuff - * needs to be logged before the actual logging framework is initialized. - */ -public class DelayedLogger implements ILogger { - - /** List of log actions that will be executed once the logger is initialized. */ - private final List logActions = new ArrayList<>(); - - @Override - public void debug(String message) { - logActions.add(logger -> logger.debug(message)); - } - - @Override - public void info(String message) { - logActions.add(logger -> logger.info(message)); - } - - @Override - public void warn(String message) { - logActions.add(logger -> logger.warn(message)); - } - - @Override - public void warn(String message, Throwable throwable) { - logActions.add(logger -> logger.warn(message, throwable)); - } - - @Override - public void error(Throwable throwable) { - logActions.add(logger -> logger.error(throwable.getMessage(), throwable)); - } - - @Override - public void error(String message, Throwable throwable) { - logActions.add(logger -> logger.error(message, throwable)); - } - - /** - * Logs an error and also writes the message to {@link System#err} to ensure the message is even logged in case - * setting up the logger itself fails for some reason (see TS-23151). - */ - public void errorAndStdErr(String message, Throwable throwable) { - System.err.println(message); - logActions.add(logger -> logger.error(message, throwable)); - } - - /** Writes the logs to the given slf4j logger. */ - public void logTo(Logger logger) { - logActions.forEach(action -> action.log(logger)); - } - - /** An action to be executed on a logger. */ - private interface ILoggerAction { - - /** Executes the action on the given logger. */ - void log(Logger logger); - - } -} - diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index ec515fde6..bb8c776da 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -37,7 +37,7 @@ /** Container class for the premain entry point for the agent. */ public class PreMain { - private static final Logger logger = LoggingUtils.getLogger(PreMain.class); + private static final Logger LOGGER = LoggingUtils.getLogger(PreMain.class); private static LoggingUtils.LoggingResources loggingResources = null; @@ -82,7 +82,7 @@ public static void premain(String options, Instrumentation instrumentation) thro Logger logger = LoggingUtils.getLogger(Agent.class); - logger.info("Teamscale Java profiler version " + AgentUtils.VERSION); + logger.info("Teamscale Java profiler version {}", AgentUtils.VERSION); logger.info("Starting JaCoCo's agent"); JacocoAgentOptionsBuilder agentBuilder = new JacocoAgentOptionsBuilder(agentOptions); JaCoCoPreMain.premain(agentBuilder.createJacocoAgentOptions(), instrumentation, logger); @@ -101,15 +101,15 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir List javaAgents = CollectionUtils.filter(ManagementFactory.getRuntimeMXBean().getInputArguments(), s -> s.contains("-javaagent")); if (javaAgents.size() > 1) { - logger.warn("Using multiple java agents could interfere with coverage recording."); + LOGGER.warn("Using multiple java agents could interfere with coverage recording."); } if (!javaAgents.get(0).contains("teamscale-jacoco-agent.jar")) { - logger.warn("For best results consider registering the Teamscale JaCoCo Agent first."); + LOGGER.warn("For best results consider registering the Teamscale JaCoCo Agent first."); } TeamscaleCredentials credentials = TeamscalePropertiesUtils.parseCredentials(); if (credentials == null) { - logger.warn("Did not find a teamscale.properties file!"); + LOGGER.warn("Did not find a teamscale.properties file!"); } AgentOptions agentOptions; try { @@ -139,7 +139,7 @@ private static void initializeLogging(AgentOptions agentOptions) throws IOExcept initializeDebugLogging(agentOptions); } else { loggingResources = LoggingUtils.initializeLogging(agentOptions.getLoggingConfig()); - logger.info("Logging to " + new LogDirectoryPropertyDefiner().getPropertyValue()); + LOGGER.info("Logging to " + new LogDirectoryPropertyDefiner().getPropertyValue()); } if (agentOptions.getTeamscaleServerOptions().isConfiguredForServerConnection()) { @@ -173,9 +173,9 @@ private static void initializeDebugLogging(AgentOptions agentOptions) { loggingResources = LoggingUtils.initializeDebugLogging(agentOptions.getDebugLogDirectory()); Path logDirectory = Paths.get(new DebugLogDirectoryPropertyDefiner().getPropertyValue()); if (FileSystemUtils.isValidPath(logDirectory.toString()) && Files.isWritable(logDirectory)) { - logger.info("Logging to " + logDirectory); + LOGGER.info("Logging to " + logDirectory); } else { - logger.warn("Could not create " + logDirectory + ". Logging to console only."); + LOGGER.warn("Could not create " + logDirectory + ". Logging to console only."); } } @@ -232,7 +232,7 @@ private static LoggingUtils.LoggingResources createFallbackLoggerFromConfig(Stri * Log the error and also print it to System Error, as the error might prevent the initialization of a real logger. */ private static void logAndPrintError(Exception e, String message) { - logger.error(message, + LOGGER.error(message, e); System.err.println(message); } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java index e79eeeab5..2e35628bf 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/configuration/ProcessInformationRetriever.java @@ -16,9 +16,6 @@ public class ProcessInformationRetriever { private final Logger logger = LoggingUtils.getLogger(this); - public ProcessInformationRetriever() { - } - /** * Retrieves the process information, including the host name and process ID. */ diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java index 04ddfed9a..a1664b999 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/FilePatternResolver.java @@ -32,9 +32,6 @@ public class FilePatternResolver { private final Logger logger = LoggingUtils.getLogger(this); - public FilePatternResolver() { - } - /** * Interprets the given pattern as an Ant pattern and resolves it to one existing {@link Path}. If the given path is * relative, it is resolved relative to the current working directory. If more than one file matches the pattern, From a53d202b10b843ae412b14a44b2f05f5d1a17dc1 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Mon, 9 Dec 2024 14:08:05 +0100 Subject: [PATCH 05/10] TS-40806 adjust FilePatternResolver in Maven Plugin --- .../java/com/teamscale/maven/tia/TiaCoverageConvertMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teamscale-maven-plugin/src/main/java/com/teamscale/maven/tia/TiaCoverageConvertMojo.java b/teamscale-maven-plugin/src/main/java/com/teamscale/maven/tia/TiaCoverageConvertMojo.java index 294699b66..9516454aa 100644 --- a/teamscale-maven-plugin/src/main/java/com/teamscale/maven/tia/TiaCoverageConvertMojo.java +++ b/teamscale-maven-plugin/src/main/java/com/teamscale/maven/tia/TiaCoverageConvertMojo.java @@ -151,7 +151,7 @@ private JaCoCoTestwiseReportGenerator createJaCoCoTestwiseReportGenerator(List getClassDirectoriesOrZips(String projectBuildDir) throws AgentOptionParseException { List classDirectoriesOrZips = new ArrayList<>(); classDirectoriesOrZips.add(projectBuildDir); - return ClasspathUtils.resolveClasspathTextFiles("classes", new FilePatternResolver(new CommandLineLogger()), + return ClasspathUtils.resolveClasspathTextFiles("classes", new FilePatternResolver(), classDirectoriesOrZips); } } From 9436f0142ab6e2de659d92c6eed22d70605f8597 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Tue, 10 Dec 2024 14:55:40 +0100 Subject: [PATCH 06/10] TS-40806 new todo --- agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index bb8c776da..49c900531 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -235,5 +235,6 @@ private static void logAndPrintError(Exception e, String message) { LOGGER.error(message, e); System.err.println(message); + // TODO } } From 9aec90b00672c5f445007c28d9595939ffeb0ac7 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Mon, 16 Dec 2024 12:02:02 +0100 Subject: [PATCH 07/10] TS-40806 clean-up --- agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index 49c900531..bb8c776da 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -235,6 +235,5 @@ private static void logAndPrintError(Exception e, String message) { LOGGER.error(message, e); System.err.println(message); - // TODO } } From b073c2e5e56943110f80a7a32f638e6949b83153 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Thu, 13 Feb 2025 13:26:40 +0100 Subject: [PATCH 08/10] TS-40806 Rework --- .../com/teamscale/jacoco/agent/PreMain.java | 16 +++++-- .../jacoco/agent/logging/CloseableBuffer.java | 42 +++++++++++++++++++ .../agent/logging/DelayedLogAppender.java | 22 +++++++--- .../jacoco/agent/logging/LoggingUtils.java | 6 +++ 4 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index bb8c776da..983be4bbc 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -2,6 +2,7 @@ import com.teamscale.client.HttpUtils; import com.teamscale.jacoco.agent.configuration.AgentOptionReceiveException; +import com.teamscale.jacoco.agent.logging.DelayedLogAppender; import com.teamscale.jacoco.agent.logging.LogToTeamscaleAppender; import com.teamscale.jacoco.agent.options.AgentOptionParseException; import com.teamscale.jacoco.agent.options.AgentOptions; @@ -37,7 +38,7 @@ /** Container class for the premain entry point for the agent. */ public class PreMain { - private static final Logger LOGGER = LoggingUtils.getLogger(PreMain.class); + private static Logger LOGGER; private static LoggingUtils.LoggingResources loggingResources = null; @@ -58,6 +59,7 @@ public class PreMain { * Entry point for the agent, called by the JVM. */ public static void premain(String options, Instrumentation instrumentation) throws Exception { + initializeDelayedLogging(); if (System.getProperty(LOCKING_SYSTEM_PROPERTY) != null) { return; } @@ -94,6 +96,11 @@ public static void premain(String options, Instrumentation instrumentation) thro agent.registerShutdownHook(); } + private static void initializeDelayedLogging() { + loggingResources = LoggingUtils.initializeDelayedLogging(); + LOGGER = LoggingUtils.getLoggerContext().getLogger(PreMain.class); + } + @NotNull private static AgentOptions getAndApplyAgentOptions(String options, String environmentConfigId, String environmentConfigFile) throws AgentOptionParseException, IOException, AgentOptionReceiveException { @@ -135,10 +142,13 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir /** Initializes logging during {@link #premain(String, Instrumentation)} and also logs the log directory. */ private static void initializeLogging(AgentOptions agentOptions) throws IOException { + closeLoggingResources(); + DelayedLogAppender.close(); if (agentOptions.isDebugLogging()) { initializeDebugLogging(agentOptions); } else { loggingResources = LoggingUtils.initializeLogging(agentOptions.getLoggingConfig()); + LOGGER = LoggingUtils.getLogger(PreMain.class); LOGGER.info("Logging to " + new LogDirectoryPropertyDefiner().getPropertyValue()); } @@ -171,6 +181,7 @@ private static AgentBase createAgent(AgentOptions agentOptions, */ private static void initializeDebugLogging(AgentOptions agentOptions) { loggingResources = LoggingUtils.initializeDebugLogging(agentOptions.getDebugLogDirectory()); + LOGGER = LoggingUtils.getLogger(PreMain.class); Path logDirectory = Paths.get(new DebugLogDirectoryPropertyDefiner().getPropertyValue()); if (FileSystemUtils.isValidPath(logDirectory.toString()) && Files.isWritable(logDirectory)) { LOGGER.info("Logging to " + logDirectory); @@ -232,8 +243,7 @@ private static LoggingUtils.LoggingResources createFallbackLoggerFromConfig(Stri * Log the error and also print it to System Error, as the error might prevent the initialization of a real logger. */ private static void logAndPrintError(Exception e, String message) { - LOGGER.error(message, - e); + LOGGER.error(message, e); System.err.println(message); } } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java new file mode 100644 index 000000000..fe950568b --- /dev/null +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java @@ -0,0 +1,42 @@ +package com.teamscale.jacoco.agent.logging; + +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** A Buffer that cn be closed and ignores all inputs once closed. */ +class CloseableBuffer implements Iterable { + private final List BUFFER = new ArrayList<>(); + + private boolean closed = false; + + /** Append to the buffer. + * + * @return whether the given object was appended to the buffer. Returns {@code false} if the buffer is closed. + * */ + public boolean append(T object) { + if (closed) { + return BUFFER.add(object); + } + return false; + } + + /** Close the buffer. */ + public void close() { + closed = true; + } + + /** Clear the buffer. */ + public void clear() { + BUFFER.clear(); + } + + + @NotNull + @Override + public Iterator iterator() { + return BUFFER.iterator(); + } +} diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java index 64ba37cff..77a2f0305 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java @@ -4,23 +4,25 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.AppenderBase; -import java.util.ArrayList; -import java.util.List; - /** * An appender that collects log statements in its buffer until it's asked to empty it into a given * {@link LoggerContext}. It can be used to log log-messages before an actual logging appender has been configured using * the options passed to the agent. - * */ + * + * Logback picks this appender up as the default appender because it searches for a default in logback.xml, which in this case + * references the {@link DelayedLogAppender}, as described here. + */ public class DelayedLogAppender extends AppenderBase { - private static final List BUFFER = new ArrayList<>(); + private static final CloseableBuffer BUFFER = new CloseableBuffer<>(); /** Appends the given {@link ILoggingEvent} to logger's buffer. */ @Override protected void append(ILoggingEvent eventObject) { synchronized (BUFFER) { - BUFFER.add(eventObject); + if (!BUFFER.append(eventObject)) { + System.err.println("Attempted to log to a closed delayed log buffer: " + eventObject); + } } } @@ -36,4 +38,12 @@ public static void logTo(LoggerContext context) { BUFFER.clear(); } } + + /** Close the Buffer of the DelayedLogAppender. */ + public static void close() { + synchronized (BUFFER) { + BUFFER.close(); + } + } + } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java index 58e482b97..3c322da9b 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java @@ -91,6 +91,12 @@ public static LoggingResources initializeLogging(Path loggingConfigFile) throws return new LoggingResources(); } + public static LoggingResources initializeDelayedLogging() { + InputStream stream = Agent.class.getResourceAsStream("logback.xml"); + reconfigureLoggerContext(stream); + return new LoggingResources(); + } + /** Initializes debug logging. */ public static LoggingResources initializeDebugLogging(Path logDirectory) { if (logDirectory != null) { From 80d3b228bac5d2677f03747a26c9d19a1c13520d Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Sat, 22 Feb 2025 14:35:36 +0100 Subject: [PATCH 09/10] TS-40806 add DelayedLogAppender to existing logging context. --- .../com/teamscale/jacoco/agent/PreMain.java | 6 +-- .../jacoco/agent/logging/CloseableBuffer.java | 2 +- .../agent/logging/DelayedLogAppender.java | 43 +++++++++++++++++-- .../jacoco/agent/logging/LoggingUtils.java | 6 --- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index 983be4bbc..ccfdd7c23 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -97,8 +97,8 @@ public static void premain(String options, Instrumentation instrumentation) thro } private static void initializeDelayedLogging() { - loggingResources = LoggingUtils.initializeDelayedLogging(); - LOGGER = LoggingUtils.getLoggerContext().getLogger(PreMain.class); + DelayedLogAppender.addDelayedAppenderTo(getLoggerContext()); + LOGGER = LoggingUtils.getLogger(PreMain.class); } @NotNull @@ -118,6 +118,7 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir if (credentials == null) { LOGGER.warn("Did not find a teamscale.properties file!"); } + AgentOptions agentOptions; try { agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials); @@ -142,7 +143,6 @@ private static AgentOptions getAndApplyAgentOptions(String options, String envir /** Initializes logging during {@link #premain(String, Instrumentation)} and also logs the log directory. */ private static void initializeLogging(AgentOptions agentOptions) throws IOException { - closeLoggingResources(); DelayedLogAppender.close(); if (agentOptions.isDebugLogging()) { initializeDebugLogging(agentOptions); diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java index fe950568b..13c4cd842 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/CloseableBuffer.java @@ -17,7 +17,7 @@ class CloseableBuffer implements Iterable { * @return whether the given object was appended to the buffer. Returns {@code false} if the buffer is closed. * */ public boolean append(T object) { - if (closed) { + if (!closed) { return BUFFER.add(object); } return false; diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java index 77a2f0305..2225021ed 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/DelayedLogAppender.java @@ -1,21 +1,27 @@ package com.teamscale.jacoco.agent.logging; +import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.AppenderBase; +import ch.qos.logback.core.spi.FilterReply; /** * An appender that collects log statements in its buffer until it's asked to empty it into a given * {@link LoggerContext}. It can be used to log log-messages before an actual logging appender has been configured using * the options passed to the agent. - * - * Logback picks this appender up as the default appender because it searches for a default in logback.xml, which in this case - * references the {@link DelayedLogAppender}, as described here. */ public class DelayedLogAppender extends AppenderBase { private static final CloseableBuffer BUFFER = new CloseableBuffer<>(); + + /** Override this to make sure every log message is passed to real logger, which does the filtering on its own. */ + @Override + public FilterReply getFilterChainDecision(ILoggingEvent event) { + return FilterReply.ACCEPT; + } + /** Appends the given {@link ILoggingEvent} to logger's buffer. */ @Override protected void append(ILoggingEvent eventObject) { @@ -33,7 +39,27 @@ protected void append(ILoggingEvent eventObject) { public static void logTo(LoggerContext context) { synchronized (BUFFER) { for (ILoggingEvent event : BUFFER) { - context.getLogger(event.getLoggerName()).callAppenders(event); + Logger logger = context.getLogger(event.getLoggerName()); + String formattedMessage = event.getFormattedMessage(); + switch (event.getLevel().levelStr) { + case "ERROR": + logger.error(formattedMessage); + break; + case "WARN": + logger.warn(formattedMessage); + break; + case "INFO": + logger.info(formattedMessage); + break; + case "DEBUG": + logger.debug(formattedMessage); + break; + case "TRACE": + logger.trace(formattedMessage); + break; + default: + break; + } } BUFFER.clear(); } @@ -46,4 +72,13 @@ public static void close() { } } + /** Add the {@link DelayedLogAppender} to the given {@link LoggerContext}. */ + public static void addDelayedAppenderTo(LoggerContext context) { + DelayedLogAppender logToTeamscaleAppender = new DelayedLogAppender(); + logToTeamscaleAppender.setContext(context); + logToTeamscaleAppender.start(); + + Logger rootLogger = context.getLogger(Logger.ROOT_LOGGER_NAME); + rootLogger.addAppender(logToTeamscaleAppender); + } } diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java index 3c322da9b..58e482b97 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/logging/LoggingUtils.java @@ -91,12 +91,6 @@ public static LoggingResources initializeLogging(Path loggingConfigFile) throws return new LoggingResources(); } - public static LoggingResources initializeDelayedLogging() { - InputStream stream = Agent.class.getResourceAsStream("logback.xml"); - reconfigureLoggerContext(stream); - return new LoggingResources(); - } - /** Initializes debug logging. */ public static LoggingResources initializeDebugLogging(Path logDirectory) { if (logDirectory != null) { From d00f793af3674d97bcf26b948fbf506093bf5750 Mon Sep 17 00:00:00 2001 From: Jonas Bogenberger Date: Sat, 22 Feb 2025 14:36:26 +0100 Subject: [PATCH 10/10] TS-40806 delete logback.xml --- .../resources/com/teamscale/jacoco/agent/logback.xml | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml diff --git a/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml b/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml deleted file mode 100644 index 50e7b0bdd..000000000 --- a/agent/src/main/resources/com/teamscale/jacoco/agent/logback.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - - - - - -