Browse Source

Cleanup apm logging config (#101291)

This commit makes the apm log a little more usable. First, it makes the
path to the log explicit by passing the ES logs dir through, instead of
relying on the location of the apm jar file. Second, it tweaks the log
level to be warn, not error. Third, it switches the apm log file to be
json, which is more easily processable.
Ryan Ernst 2 years ago
parent
commit
d0571b6c22

+ 10 - 8
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/APMJvmOptions.java

@@ -45,12 +45,6 @@ class APMJvmOptions {
         // Identifies the version of Elasticsearch in the captured trace data.
         "service_version", Build.current().version(),
 
-        // Configures a log file to write to. `_AGENT_HOME_` is a placeholder used
-        // by the agent. Don't disable writing to a log file, as the agent will then
-        // require extra Security Manager permissions when it tries to do something
-        // else, and it's just painful.
-        "log_file", "_AGENT_HOME_/../../logs/apm.log",
-
         // ES does not use auto-instrumentation.
         "instrument", "false",
         "enable_experimental_instrumentations", "true"
@@ -80,7 +74,8 @@ class APMJvmOptions {
 
         // Logging configuration. Unless you need detailed logs about what the APM
         // is doing, leave this value alone.
-        "log_level", "error",
+        "log_level", "warn",
+        "log_format_file", "JSON",
         "application_packages", "org.elasticsearch,org.apache.lucene",
         "metrics_interval", "120s",
         "breakdown_metrics", "false",
@@ -134,9 +129,11 @@ class APMJvmOptions {
      *
      * @param settings the Elasticsearch settings to consider
      * @param secrets a wrapper to access the secrets, or null if there is no secrets
+     * @param logsDir the directory to write the apm log into
      * @param tmpdir Elasticsearch's temporary directory, where the config file will be written
      */
-    static List<String> apmJvmOptions(Settings settings, @Nullable SecureSettings secrets, Path tmpdir) throws UserException, IOException {
+    static List<String> apmJvmOptions(Settings settings, @Nullable SecureSettings secrets, Path logsDir, Path tmpdir) throws UserException,
+        IOException {
         final Path agentJar = findAgentJar();
 
         if (agentJar == null) {
@@ -145,6 +142,11 @@ class APMJvmOptions {
 
         final Map<String, String> propertiesMap = extractApmSettings(settings);
 
+        // Configures a log file to write to. Don't disable writing to a log file,
+        // as the agent will then require extra Security Manager permissions when
+        // it tries to do something else, and it's just painful.
+        propertiesMap.put("log_file", logsDir.resolve("apm-agent.log").toString());
+
         // No point doing anything if we don't have a destination for the trace data, and it can't be configured dynamically
         if (propertiesMap.containsKey("server_url") == false && propertiesMap.containsKey("server_urls") == false) {
             return List.of();

+ 1 - 1
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java

@@ -139,7 +139,7 @@ final class JvmOptionsParser {
         final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions);
         final List<String> systemJvmOptions = SystemJvmOptions.systemJvmOptions();
 
-        final List<String> apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), tmpDir);
+        final List<String> apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), args.logsDir(), tmpDir);
 
         final List<String> finalJvmOptions = new ArrayList<>(
             systemJvmOptions.size() + substitutedJvmOptions.size() + ergonomicJvmOptions.size() + apmOptions.size()

+ 1 - 1
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java

@@ -219,7 +219,7 @@ class ServerCli extends EnvironmentAwareCommand {
             }
             validatePidFile(pidFile);
         }
-        return new ServerArgs(daemonize, quiet, pidFile, secrets, env.settings(), env.configFile());
+        return new ServerArgs(daemonize, quiet, pidFile, secrets, env.settings(), env.configFile(), env.logsFile());
     }
 
     @Override

+ 9 - 1
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java

@@ -195,7 +195,15 @@ public class ServerProcessTests extends ESTestCase {
 
     ServerProcess startProcess(boolean daemonize, boolean quiet, String keystorePassword) throws Exception {
         var pinfo = new ProcessInfo(Map.copyOf(sysprops), Map.copyOf(envVars), esHomeDir);
-        var args = new ServerArgs(daemonize, quiet, null, secrets, nodeSettings.build(), esHomeDir.resolve("config"));
+        var args = new ServerArgs(
+            daemonize,
+            quiet,
+            null,
+            secrets,
+            nodeSettings.build(),
+            esHomeDir.resolve("config"),
+            esHomeDir.resolve("logs")
+        );
         ServerProcess.ProcessStarter starter = pb -> {
             if (processValidator != null) {
                 processValidator.validate(pb);

+ 1 - 1
distribution/tools/windows-service-cli/src/main/java/org/elasticsearch/windows/service/WindowsServiceDaemon.java

@@ -37,7 +37,7 @@ class WindowsServiceDaemon extends EnvironmentAwareCommand {
     public void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
         // the Windows service daemon doesn't support secure settings implementations other than the keystore
         try (var loadedSecrets = KeyStoreWrapper.bootstrap(env.configFile(), () -> new SecureString(new char[0]))) {
-            var args = new ServerArgs(false, true, null, loadedSecrets, env.settings(), env.configFile());
+            var args = new ServerArgs(false, true, null, loadedSecrets, env.settings(), env.configFile(), env.logsFile());
             this.server = ServerProcess.start(terminal, processInfo, args);
             // start does not return until the server is ready, and we do not wait for the process
         }

+ 12 - 3
server/src/main/java/org/elasticsearch/bootstrap/ServerArgs.java

@@ -29,10 +29,17 @@ import java.nio.file.Path;
  * @param secrets the provided secure settings implementation
  * @param nodeSettings the node settings read from {@code elasticsearch.yml}, the cli and the process environment
  * @param configDir the directory where {@code elasticsearch.yml} and other config exists
+ * @param logsDir the directory where log files should be written
  */
-public record ServerArgs(boolean daemonize, boolean quiet, Path pidFile, SecureSettings secrets, Settings nodeSettings, Path configDir)
-    implements
-        Writeable {
+public record ServerArgs(
+    boolean daemonize,
+    boolean quiet,
+    Path pidFile,
+    SecureSettings secrets,
+    Settings nodeSettings,
+    Path configDir,
+    Path logsDir
+) implements Writeable {
 
     /**
      * Arguments for running Elasticsearch.
@@ -59,6 +66,7 @@ public record ServerArgs(boolean daemonize, boolean quiet, Path pidFile, SecureS
             readPidFile(in),
             readSecureSettingsFromStream(in),
             Settings.readSettingsFromStream(in),
+            resolvePath(in.readString()),
             resolvePath(in.readString())
         );
     }
@@ -82,6 +90,7 @@ public record ServerArgs(boolean daemonize, boolean quiet, Path pidFile, SecureS
         secrets.writeTo(out);
         nodeSettings.writeTo(out);
         out.writeString(configDir.toString());
+        out.writeString(logsDir.toString());
     }
 
     private static SecureSettings readSecureSettingsFromStream(StreamInput in) throws IOException {