From a78a682fd41a7204275ebc7b7b59463d4d54f50b Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Sun, 17 Mar 2024 17:53:58 +0100 Subject: [PATCH 1/5] read config file path from TEAMSCALE_JAVA_PROFILER_CONFIG_FILE --- .idea/misc.xml | 1 - .../com/teamscale/jacoco/agent/PreMain.java | 21 ++++++---- .../agent/options/AgentOptionsParser.java | 22 ++++++---- .../agent/options/AgentOptionsParserTest.java | 41 ++++++++++++++++--- .../agent/options/AgentOptionsTest.java | 2 +- .../jacoco/agent/options/agent.properties | 1 + 6 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 agent/src/test/resources/com/teamscale/jacoco/agent/options/agent.properties diff --git a/.idea/misc.xml b/.idea/misc.xml index 3dda0022f..2d60864c7 100644 --- a/.idea/misc.xml +++ b/.idea/misc.xml @@ -1,4 +1,3 @@ - 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 1be6d4b9d..1b8f8e571 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -42,9 +42,12 @@ public class PreMain { */ private static final String LOCKING_SYSTEM_PROPERTY = "TEAMSCALE_JAVA_PROFILER_ATTACHED"; - /** Environment variable from which to read the config file to use. */ + /** Environment variable from which to read the config ID to use. */ private static final String CONFIG_ID_ENVIRONMENT_VARIABLE = "TEAMSCALE_JAVA_PROFILER_CONFIG_ID"; + /** Environment variable from which to read the config file to use. */ + private static final String CONFIG_FILE_ENVIRONMENT_VARIABLE = "TEAMSCALE_JAVA_PROFILER_CONFIG_FILE"; + /** * Entry point for the agent, called by the JVM. */ @@ -55,7 +58,8 @@ public static void premain(String options, Instrumentation instrumentation) thro System.setProperty(LOCKING_SYSTEM_PROPERTY, "true"); String environmentConfigId = System.getenv(CONFIG_ID_ENVIRONMENT_VARIABLE); - if (StringUtils.isEmpty(options) && environmentConfigId == null) { + String environmentConfigFile = System.getenv(CONFIG_FILE_ENVIRONMENT_VARIABLE); + if (StringUtils.isEmpty(options) && environmentConfigId == null && environmentConfigFile == null) { // profiler was registered globally and no config was set explicitly by the user, thus ignore this process // and don't profile anything return; @@ -63,7 +67,7 @@ public static void premain(String options, Instrumentation instrumentation) thro AgentOptions agentOptions; try { - agentOptions = getAndApplyAgentOptions(options, environmentConfigId); + agentOptions = getAndApplyAgentOptions(options, environmentConfigId, environmentConfigFile); } 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) @@ -85,8 +89,9 @@ public static void premain(String options, Instrumentation instrumentation) thro } @NotNull - private static AgentOptions getAndApplyAgentOptions(String options, - String environmentConfigId) throws AgentOptionParseException, IOException, AgentOptionReceiveException { + 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")); @@ -103,7 +108,7 @@ private static AgentOptions getAndApplyAgentOptions(String options, } AgentOptions agentOptions; try { - agentOptions = AgentOptionsParser.parse(options, environmentConfigId, credentials, delayedLogger); + agentOptions = AgentOptionsParser.parse(options, environmentConfigId, environmentConfigFile, credentials, delayedLogger); } catch (AgentOptionParseException e) { try (LoggingUtils.LoggingResources ignored = initializeFallbackLogging(options, delayedLogger)) { delayedLogger.errorAndStdErr("Failed to parse agent options: " + e.getMessage(), e); @@ -112,7 +117,9 @@ private static AgentOptions getAndApplyAgentOptions(String options, } } 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); + delayedLogger.errorAndStdErr( + e.getMessage() + " The application should start up normally, but NO coverage will be collected!", + e); attemptLogAndThrow(delayedLogger); throw e; } 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 d16bdac3a..acd9da767 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 @@ -50,27 +50,30 @@ public class AgentOptionsParser { 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. * - * @param environmentConfigId The Profiler configuration ID given via the - * {@link com.teamscale.jacoco.agent.PreMain#CONFIG_ID_ENVIRONMENT_VARIABLE} environment - * variable. + * @param environmentConfigId The Profiler configuration ID given via an environment variable. + * @param environmentConfigFile The Profiler configuration file given via an environment variable. */ - public static AgentOptions parse(String optionsString, String environmentConfigId, + public static AgentOptions parse(String optionsString, String environmentConfigId, String environmentConfigFile, TeamscaleCredentials credentials, ILogger logger) throws AgentOptionParseException, AgentOptionReceiveException { - return new AgentOptionsParser(logger, environmentConfigId, credentials).parse(optionsString); + return new AgentOptionsParser(logger, environmentConfigId, environmentConfigFile, credentials).parse( + optionsString); } @VisibleForTesting - AgentOptionsParser(ILogger logger, String environmentConfigId, TeamscaleCredentials credentials) { + AgentOptionsParser(ILogger logger, String environmentConfigId, String environmentConfigFile, + TeamscaleCredentials credentials) { this.logger = logger; this.filePatternResolver = new FilePatternResolver(logger); this.teamscaleConfig = new TeamscaleConfig(logger, filePatternResolver); this.environmentConfigId = environmentConfigId; + this.environmentConfigFile = environmentConfigFile; this.credentials = credentials; } @@ -103,6 +106,10 @@ public static AgentOptions parse(String optionsString, String environmentConfigI handleOption(options, "config-id=" + environmentConfigId); } + if (environmentConfigFile != null) { + handleOption(options, "config-file=" + environmentConfigFile); + } + Validator validator = options.getValidator(); if (!validator.isValid()) { throw new AgentOptionParseException("Invalid options given: " + validator.getErrorMessage()); @@ -267,7 +274,8 @@ private void readConfigFromTeamscale(AgentOptions options, options.teamscaleServer.userName, options.teamscaleServer.userAccessToken); options.configurationViaTeamscale = configuration; - logger.debug("Received the following options from Teamscale: " + configuration.getProfilerConfiguration().configurationOptions); + logger.debug( + "Received the following options from Teamscale: " + configuration.getProfilerConfiguration().configurationOptions); readConfigFromString(options, configuration.getProfilerConfiguration().configurationOptions); } 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 f11ed0779..64615679c 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 @@ -15,6 +15,8 @@ import org.junit.jupiter.api.Test; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -25,13 +27,15 @@ public class AgentOptionsParserTest { private TeamscaleCredentials teamscaleCredentials; - private final AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, null); + private final AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, null, null); + private Path configFile; /** The mock server to run requests against. */ protected MockWebServer mockWebServer; /** Starts the mock server. */ @BeforeEach public void setup() throws Exception { + configFile = Paths.get(getClass().getResource("agent.properties").toURI()); mockWebServer = new MockWebServer(); mockWebServer.start(); teamscaleCredentials = new TeamscaleCredentials(mockWebServer.url("/"), "user", "key"); @@ -68,7 +72,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, credentials); + AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), 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( @@ -91,7 +95,7 @@ public void testUploadMethodRecognitionWithTeamscaleProperties() throws Exceptio } @Test - public void environmentConfigOverridesCommandLineOptions() throws Exception { + public void environmentConfigIdOverridesCommandLineOptions() throws Exception { ProfilerRegistration registration = new ProfilerRegistration(); registration.profilerId = UUID.randomUUID().toString(); registration.profilerConfiguration = new ProfilerConfiguration(); @@ -99,12 +103,36 @@ public void environmentConfigOverridesCommandLineOptions() throws Exception { registration.profilerConfiguration.configurationOptions = "teamscale-partition=foo"; mockWebServer.enqueue(new MockResponse().setBody(JsonUtils.serialize(registration))); AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), "my-config", - teamscaleCredentials); + null, teamscaleCredentials); AgentOptions options = parser.parse("teamscale-partition=bar"); assertThat(options.teamscaleServer.partition).isEqualTo("foo"); } + @Test + public void environmentConfigFileOverridesCommandLineOptions() throws Exception { + AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, configFile.toString(), + teamscaleCredentials); + AgentOptions options = parser.parse("teamscale-partition=from-command-line"); + + assertThat(options.teamscaleServer.partition).isEqualTo("from-config-file"); + } + + @Test + public void environmentConfigFileOverridesConfigId() throws Exception { + ProfilerRegistration registration = new ProfilerRegistration(); + registration.profilerId = UUID.randomUUID().toString(); + registration.profilerConfiguration = new ProfilerConfiguration(); + 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(), + teamscaleCredentials); + AgentOptions options = parser.parse("teamscale-partition=from-command-line"); + + assertThat(options.teamscaleServer.partition).isEqualTo("from-config-file"); + } + @Test public void notAllRequiredTeamscaleOptionsSet() { assertThatCode( @@ -183,7 +211,8 @@ 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", teamscaleCredentials).parse( + () -> new AgentOptionsParser(new CommandLineLogger(), "invalid-config-id", null, + teamscaleCredentials).parse( "") ).isInstanceOf(AgentOptionParseException.class).hasMessageContaining("invalid-config-id does not exist"); } @@ -203,7 +232,7 @@ public void mustPreserveDefaultExcludes() throws Exception { @Test public void teamscalePropertiesCredentialsUsedAsDefaultButOverridable() throws Exception { - AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), null, teamscaleCredentials); + AgentOptionsParser parser = new AgentOptionsParser(new CommandLineLogger(), 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 a1fb7e987..96eb3ac4f 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 @@ -347,7 +347,7 @@ private static Predicate excludeFilter(String filterString) throws Excep } private static AgentOptionsParser getAgentOptionsParserWithDummyLogger() { - return new AgentOptionsParser(new CommandLineLogger(), null, null); + return new AgentOptionsParser(new CommandLineLogger(), null, null, null); } /** diff --git a/agent/src/test/resources/com/teamscale/jacoco/agent/options/agent.properties b/agent/src/test/resources/com/teamscale/jacoco/agent/options/agent.properties new file mode 100644 index 000000000..21623d152 --- /dev/null +++ b/agent/src/test/resources/com/teamscale/jacoco/agent/options/agent.properties @@ -0,0 +1 @@ +teamscale-partition=from-config-file \ No newline at end of file From 17c14b9384ce404d25500d49b141c0f4e7eb8651 Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Sun, 17 Mar 2024 18:55:59 +0100 Subject: [PATCH 2/5] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 510065e8a..54b3f79a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/): - PATCH version when you make backwards compatible bug fixes. # Next Release +- [feature] Read configuration file path from `TEAMSCALE_JAVA_PROFILER_CONFIG_FILE` environment variable # 33.1.2 - [fix] _teamscale-maven-plugin_: Revision and end commit could not be set via command line (user property) From 37417f6c55277c37e80976ddbd5b5d94cdf4d493 Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Sun, 17 Mar 2024 18:57:10 +0100 Subject: [PATCH 3/5] update agent README --- agent/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/agent/README.md b/agent/README.md index e606960e0..6c27a05ed 100644 --- a/agent/README.md +++ b/agent/README.md @@ -51,8 +51,9 @@ The following options are available: - `out` (optional): the path to a writable directory where the generated coverage XML files will be stored. (For details see path format section below). Defaults to the subdirectory `coverage` inside the agent's installation directory. - `config-file` (optional): a file which contains one or more of the previously named options as `key=value` entries - which are separated by line breaks. The file may also contain comments starting with `#`. (For details see path format - section below) + which are separated by line breaks. The file may also contain comments starting with `#`. For details see path format + section below. + Alternatively you can also set the `TEAMSCALE_JAVA_PROFILER_CONFIG_FILE` environment variable to that value. - `config-id` (optional): a profiler configuration ID as defined in Teamscale. This allows to centrally manage the profiler configuration in Teamscale's UI (under Project Configuration > Profilers since Teamscale 9.4). Alternatively you can also set the `TEAMSCALE_JAVA_PROFILER_CONFIG_ID` environment variable to that value. From 3d92ea6d1d07e279afc0f655c27347050b8c6980 Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Thu, 28 Mar 2024 11:19:19 +0100 Subject: [PATCH 4/5] rework --- agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java | 3 ++- .../teamscale/jacoco/agent/options/AgentOptionsParser.java | 6 ++++++ 2 files changed, 8 insertions(+), 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 1b8f8e571..d3ef01be0 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -42,7 +42,8 @@ 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. */ + /** 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. */ 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 acd9da767..0af43b276 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 @@ -108,6 +108,12 @@ public static AgentOptions parse(String optionsString, String environmentConfigI if (environmentConfigFile != null) { handleOption(options, "config-file=" + environmentConfigFile); + + if (environmentConfigId != null) { + logger.warn("You specified both an ID for a profiler configuration in Teamscale and a config file." + + " The config file will override the Teamscale configuration." + + " Please use one or the other."); + } } Validator validator = options.getValidator(); From 9831adc6f0a1fec23416331f0889c8b15c91e074 Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Thu, 28 Mar 2024 16:03:46 +0100 Subject: [PATCH 5/5] shorten long method --- .../agent/options/AgentOptionsParser.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) 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 0af43b276..5ff43c19c 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 @@ -102,25 +102,30 @@ public static AgentOptions parse(String optionsString, String environmentConfigI } } + handleConfigFromEnvironment(options); + + Validator validator = options.getValidator(); + if (!validator.isValid()) { + throw new AgentOptionParseException("Invalid options given: " + validator.getErrorMessage()); + } + return options; + } + + private void handleConfigFromEnvironment( + AgentOptions options) throws AgentOptionParseException, AgentOptionReceiveException { if (environmentConfigId != null) { handleOption(options, "config-id=" + environmentConfigId); } if (environmentConfigFile != null) { handleOption(options, "config-file=" + environmentConfigFile); - - if (environmentConfigId != null) { - logger.warn("You specified both an ID for a profiler configuration in Teamscale and a config file." + - " The config file will override the Teamscale configuration." + - " Please use one or the other."); - } } - Validator validator = options.getValidator(); - if (!validator.isValid()) { - throw new AgentOptionParseException("Invalid options given: " + validator.getErrorMessage()); + if (environmentConfigId != null && environmentConfigFile != null) { + logger.warn("You specified both an ID for a profiler configuration in Teamscale and a config file." + + " The config file will override the Teamscale configuration." + + " Please use one or the other."); } - return options; } /**