Fix enabling / disabling of APM agent "recording" in APMAgentSettings (…

…#104324)
elastic · Jan 30, 2024 · 9ea187d · 9ea187d
1 parent e74a79f
commit 9ea187d
Show file tree

Hide file tree

Showing 3 changed files with 173 additions and 48 deletions.
diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/APM.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/APM.java
@@ -70,8 +70,7 @@ public Collection<?> createComponents(PluginServices services) {
         apmTracer.setNodeName(services.clusterService().getNodeName());
 
         final APMAgentSettings apmAgentSettings = new APMAgentSettings();
-        apmAgentSettings.syncAgentSystemProperties(settings);
-
+        apmAgentSettings.initAgentSystemProperties(settings);
         apmAgentSettings.addClusterSettingsListeners(services.clusterService(), telemetryProvider.get());
         logger.info("Sending apm metrics is {}", APMAgentSettings.TELEMETRY_METRICS_ENABLED_SETTING.get(settings) ? "enabled" : "disabled");
         logger.info("Sending apm tracing is {}", APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.get(settings) ? "enabled" : "disabled");

diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java
@@ -45,12 +45,17 @@ public void addClusterSettingsListeners(ClusterService clusterService, APMTeleme
         clusterSettings.addSettingsUpdateConsumer(TELEMETRY_TRACING_ENABLED_SETTING, enabled -> {
             apmTracer.setEnabled(enabled);
             this.setAgentSetting("instrument", Boolean.toString(enabled));
+            // The agent records data other than spans, e.g. JVM metrics, so we toggle this setting in order to
+            // minimise its impact to a running Elasticsearch.
+            boolean recording = enabled || clusterSettings.get(TELEMETRY_METRICS_ENABLED_SETTING);
+            this.setAgentSetting("recording", Boolean.toString(recording));
         });
         clusterSettings.addSettingsUpdateConsumer(TELEMETRY_METRICS_ENABLED_SETTING, enabled -> {
             apmMeterService.setEnabled(enabled);
             // The agent records data other than spans, e.g. JVM metrics, so we toggle this setting in order to
             // minimise its impact to a running Elasticsearch.
-            this.setAgentSetting("recording", Boolean.toString(enabled));
+            boolean recording = enabled || clusterSettings.get(TELEMETRY_TRACING_ENABLED_SETTING);
+            this.setAgentSetting("recording", Boolean.toString(recording));
         });
         clusterSettings.addSettingsUpdateConsumer(TELEMETRY_TRACING_NAMES_INCLUDE_SETTING, apmTracer::setIncludeNames);
         clusterSettings.addSettingsUpdateConsumer(TELEMETRY_TRACING_NAMES_EXCLUDE_SETTING, apmTracer::setExcludeNames);
@@ -59,11 +64,16 @@ public void addClusterSettingsListeners(ClusterService clusterService, APMTeleme
     }
 
     /**
-     * Copies APM settings from the provided settings object into the corresponding system properties.
+     * Initialize APM settings from the provided settings object into the corresponding system properties.
+     * Later updates to these settings are synchronized using update consumers.
      * @param settings the settings to apply
      */
-    public void syncAgentSystemProperties(Settings settings) {
-        this.setAgentSetting("recording", Boolean.toString(TELEMETRY_TRACING_ENABLED_SETTING.get(settings)));
+    public void initAgentSystemProperties(Settings settings) {
+        boolean tracing = TELEMETRY_TRACING_ENABLED_SETTING.get(settings);
+        boolean metrics = TELEMETRY_METRICS_ENABLED_SETTING.get(settings);
+
+        this.setAgentSetting("recording", Boolean.toString(tracing || metrics));
+        this.setAgentSetting("instrument", Boolean.toString(tracing));
         // Apply values from the settings in the cluster state
         APM_AGENT_SETTINGS.getAsMap(settings).forEach(this::setAgentSetting);
     }