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 index 9f268a522fa64..698b067b3ffc3 100644 --- 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 index c929d1c484be1..0f44115d92e08 100644 --- 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); } diff --git a/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettingsTests.java b/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettingsTests.java index 52607a79fe69d..d7ae93aded3de 100644 --- a/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettingsTests.java +++ b/modules/apm/src/test/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettingsTests.java @@ -8,81 +8,190 @@ package org.elasticsearch.telemetry.apm.internal; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; +import org.mockito.Mockito; import java.util.List; - +import java.util.Set; + +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.APM_AGENT_SETTINGS; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_API_KEY_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_METRICS_ENABLED_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_SECRET_TOKEN_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_TRACING_NAMES_EXCLUDE_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_TRACING_NAMES_INCLUDE_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TELEMETRY_TRACING_SANITIZE_FIELD_NAMES; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_API_KEY_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_ENABLED_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_NAMES_EXCLUDE_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_NAMES_INCLUDE_SETTING; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_SANITIZE_FIELD_NAMES; +import static org.elasticsearch.telemetry.apm.internal.APMAgentSettings.TRACING_APM_SECRET_TOKEN_SETTING; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItem; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class APMAgentSettingsTests extends ESTestCase { + APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); + APMTelemetryProvider apmTelemetryProvider = mock(Mockito.RETURNS_DEEP_STUBS); /** * Check that when the tracer is enabled, it also sets the APM agent's recording system property to true. */ public void testEnableTracing() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); - Settings settings = Settings.builder().put(APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true).build(); - apmAgentSettings.syncAgentSystemProperties(settings); - - verify(apmAgentSettings).setAgentSetting("recording", "true"); + for (boolean metricsEnabled : List.of(true, false)) { + clearInvocations(apmAgentSettings, apmTelemetryProvider.getTracer()); + + Settings update = Settings.builder() + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true) + .put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), metricsEnabled) + .build(); + apmAgentSettings.initAgentSystemProperties(update); + + verify(apmAgentSettings).setAgentSetting("recording", "true"); + verify(apmAgentSettings).setAgentSetting("instrument", "true"); + clearInvocations(apmAgentSettings); + + Settings initial = Settings.builder().put(update).put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), false).build(); + triggerUpdateConsumer(initial, update); + verify(apmAgentSettings).setAgentSetting("recording", "true"); + verify(apmAgentSettings).setAgentSetting("instrument", "true"); + verify(apmTelemetryProvider.getTracer()).setEnabled(true); + } } public void testEnableTracingUsingLegacySetting() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); - Settings settings = Settings.builder().put(APMAgentSettings.TRACING_APM_ENABLED_SETTING.getKey(), true).build(); - apmAgentSettings.syncAgentSystemProperties(settings); + Settings settings = Settings.builder().put(TRACING_APM_ENABLED_SETTING.getKey(), true).build(); + apmAgentSettings.initAgentSystemProperties(settings); verify(apmAgentSettings).setAgentSetting("recording", "true"); + verify(apmAgentSettings).setAgentSetting("instrument", "true"); + } + + public void testEnableMetrics() { + for (boolean tracingEnabled : List.of(true, false)) { + clearInvocations(apmAgentSettings, apmTelemetryProvider.getMeterService()); + + Settings update = Settings.builder() + .put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), true) + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), tracingEnabled) + .build(); + apmAgentSettings.initAgentSystemProperties(update); + + verify(apmAgentSettings).setAgentSetting("recording", "true"); + verify(apmAgentSettings).setAgentSetting("instrument", Boolean.toString(tracingEnabled)); + clearInvocations(apmAgentSettings); + + Settings initial = Settings.builder().put(update).put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), false).build(); + triggerUpdateConsumer(initial, update); + verify(apmAgentSettings).setAgentSetting("recording", "true"); + verify(apmTelemetryProvider.getMeterService()).setEnabled(true); + } } /** - * Check that when the tracer is disabled, it also sets the APM agent's recording system property to false. + * Check that when the tracer is disabled, it also sets the APM agent's recording system property to false unless metrics are enabled. */ public void testDisableTracing() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); - Settings settings = Settings.builder().put(APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.getKey(), false).build(); - apmAgentSettings.syncAgentSystemProperties(settings); - - verify(apmAgentSettings).setAgentSetting("recording", "false"); + for (boolean metricsEnabled : List.of(true, false)) { + clearInvocations(apmAgentSettings, apmTelemetryProvider.getTracer()); + + Settings update = Settings.builder() + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), false) + .put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), metricsEnabled) + .build(); + apmAgentSettings.initAgentSystemProperties(update); + + verify(apmAgentSettings).setAgentSetting("recording", Boolean.toString(metricsEnabled)); + verify(apmAgentSettings).setAgentSetting("instrument", "false"); + clearInvocations(apmAgentSettings); + + Settings initial = Settings.builder().put(update).put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true).build(); + triggerUpdateConsumer(initial, update); + verify(apmAgentSettings).setAgentSetting("recording", Boolean.toString(metricsEnabled)); + verify(apmAgentSettings).setAgentSetting("instrument", "false"); + verify(apmTelemetryProvider.getTracer()).setEnabled(false); + } } public void testDisableTracingUsingLegacySetting() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); - Settings settings = Settings.builder().put(APMAgentSettings.TRACING_APM_ENABLED_SETTING.getKey(), false).build(); - apmAgentSettings.syncAgentSystemProperties(settings); + Settings settings = Settings.builder().put(TRACING_APM_ENABLED_SETTING.getKey(), false).build(); + apmAgentSettings.initAgentSystemProperties(settings); verify(apmAgentSettings).setAgentSetting("recording", "false"); + verify(apmAgentSettings).setAgentSetting("instrument", "false"); + } + + public void testDisableMetrics() { + for (boolean tracingEnabled : List.of(true, false)) { + clearInvocations(apmAgentSettings, apmTelemetryProvider.getMeterService()); + + Settings update = Settings.builder() + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), tracingEnabled) + .put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), false) + .build(); + apmAgentSettings.initAgentSystemProperties(update); + + verify(apmAgentSettings).setAgentSetting("recording", Boolean.toString(tracingEnabled)); + verify(apmAgentSettings).setAgentSetting("instrument", Boolean.toString(tracingEnabled)); + clearInvocations(apmAgentSettings); + + Settings initial = Settings.builder().put(update).put(TELEMETRY_METRICS_ENABLED_SETTING.getKey(), true).build(); + triggerUpdateConsumer(initial, update); + verify(apmAgentSettings).setAgentSetting("recording", Boolean.toString(tracingEnabled)); + verify(apmTelemetryProvider.getMeterService()).setEnabled(false); + } + } + + private void triggerUpdateConsumer(Settings initial, Settings update) { + ClusterService clusterService = mock(); + ClusterSettings clusterSettings = new ClusterSettings( + initial, + Set.of( + TELEMETRY_TRACING_ENABLED_SETTING, + TELEMETRY_METRICS_ENABLED_SETTING, + TELEMETRY_TRACING_NAMES_INCLUDE_SETTING, + TELEMETRY_TRACING_NAMES_EXCLUDE_SETTING, + TELEMETRY_TRACING_SANITIZE_FIELD_NAMES, + APM_AGENT_SETTINGS + ) + ); + when(clusterService.getClusterSettings()).thenReturn(clusterSettings); + apmAgentSettings.addClusterSettingsListeners(clusterService, apmTelemetryProvider); + clusterSettings.applySettings(update); } /** * Check that when cluster settings are synchronised with the system properties, agent settings are set. */ public void testSetAgentSettings() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); Settings settings = Settings.builder() - .put(APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true) - .put(APMAgentSettings.APM_AGENT_SETTINGS.getKey() + "span_compression_enabled", "true") + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true) + .put(APM_AGENT_SETTINGS.getKey() + "span_compression_enabled", "true") .build(); - apmAgentSettings.syncAgentSystemProperties(settings); + apmAgentSettings.initAgentSystemProperties(settings); verify(apmAgentSettings).setAgentSetting("recording", "true"); verify(apmAgentSettings).setAgentSetting("span_compression_enabled", "true"); } public void testSetAgentsSettingsWithLegacyPrefix() { - APMAgentSettings apmAgentSettings = spy(new APMAgentSettings()); Settings settings = Settings.builder() - .put(APMAgentSettings.TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true) + .put(TELEMETRY_TRACING_ENABLED_SETTING.getKey(), true) .put("tracing.apm.agent.span_compression_enabled", "true") .build(); - apmAgentSettings.syncAgentSystemProperties(settings); + apmAgentSettings.initAgentSystemProperties(settings); verify(apmAgentSettings).setAgentSetting("recording", "true"); verify(apmAgentSettings).setAgentSetting("span_compression_enabled", "true"); @@ -92,57 +201,54 @@ public void testSetAgentsSettingsWithLegacyPrefix() { * Check that invalid or forbidden APM agent settings are rejected. */ public void testRejectForbiddenOrUnknownAgentSettings() { - List prefixes = List.of(APMAgentSettings.APM_AGENT_SETTINGS.getKey(), "tracing.apm.agent."); + List prefixes = List.of(APM_AGENT_SETTINGS.getKey(), "tracing.apm.agent."); for (String prefix : prefixes) { Settings settings = Settings.builder().put(prefix + "unknown", "true").build(); - Exception exception = expectThrows( - IllegalArgumentException.class, - () -> APMAgentSettings.APM_AGENT_SETTINGS.getAsMap(settings) - ); + Exception exception = expectThrows(IllegalArgumentException.class, () -> APM_AGENT_SETTINGS.getAsMap(settings)); assertThat(exception.getMessage(), containsString("[" + prefix + "unknown]")); } // though, accept / ignore nested global_labels for (String prefix : prefixes) { Settings settings = Settings.builder().put(prefix + "global_labels." + randomAlphaOfLength(5), "123").build(); - APMAgentSettings.APM_AGENT_SETTINGS.getAsMap(settings); + APM_AGENT_SETTINGS.getAsMap(settings); } } public void testTelemetryTracingNamesIncludeFallback() { - Settings settings = Settings.builder().put(APMAgentSettings.TRACING_APM_NAMES_INCLUDE_SETTING.getKey(), "abc,xyz").build(); + Settings settings = Settings.builder().put(TRACING_APM_NAMES_INCLUDE_SETTING.getKey(), "abc,xyz").build(); - List included = APMAgentSettings.TELEMETRY_TRACING_NAMES_INCLUDE_SETTING.get(settings); + List included = TELEMETRY_TRACING_NAMES_INCLUDE_SETTING.get(settings); assertThat(included, containsInAnyOrder("abc", "xyz")); } public void testTelemetryTracingNamesExcludeFallback() { - Settings settings = Settings.builder().put(APMAgentSettings.TRACING_APM_NAMES_EXCLUDE_SETTING.getKey(), "abc,xyz").build(); + Settings settings = Settings.builder().put(TRACING_APM_NAMES_EXCLUDE_SETTING.getKey(), "abc,xyz").build(); - List included = APMAgentSettings.TELEMETRY_TRACING_NAMES_EXCLUDE_SETTING.get(settings); + List included = TELEMETRY_TRACING_NAMES_EXCLUDE_SETTING.get(settings); assertThat(included, containsInAnyOrder("abc", "xyz")); } public void testTelemetryTracingSanitizeFieldNamesFallback() { - Settings settings = Settings.builder().put(APMAgentSettings.TRACING_APM_SANITIZE_FIELD_NAMES.getKey(), "abc,xyz").build(); + Settings settings = Settings.builder().put(TRACING_APM_SANITIZE_FIELD_NAMES.getKey(), "abc,xyz").build(); - List included = APMAgentSettings.TELEMETRY_TRACING_SANITIZE_FIELD_NAMES.get(settings); + List included = TELEMETRY_TRACING_SANITIZE_FIELD_NAMES.get(settings); assertThat(included, containsInAnyOrder("abc", "xyz")); } public void testTelemetryTracingSanitizeFieldNamesFallbackDefault() { - List included = APMAgentSettings.TELEMETRY_TRACING_SANITIZE_FIELD_NAMES.get(Settings.EMPTY); + List included = TELEMETRY_TRACING_SANITIZE_FIELD_NAMES.get(Settings.EMPTY); assertThat(included, hasItem("password")); // and more defaults } public void testTelemetrySecretTokenFallback() { MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString(APMAgentSettings.TRACING_APM_SECRET_TOKEN_SETTING.getKey(), "verysecret"); + secureSettings.setString(TRACING_APM_SECRET_TOKEN_SETTING.getKey(), "verysecret"); Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - try (SecureString secureString = APMAgentSettings.TELEMETRY_SECRET_TOKEN_SETTING.get(settings)) { + try (SecureString secureString = TELEMETRY_SECRET_TOKEN_SETTING.get(settings)) { assertEquals("verysecret", secureString.toString()); } @@ -150,12 +256,22 @@ public void testTelemetrySecretTokenFallback() { public void testTelemetryApiKeyFallback() { MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString(APMAgentSettings.TRACING_APM_API_KEY_SETTING.getKey(), "abc"); + secureSettings.setString(TRACING_APM_API_KEY_SETTING.getKey(), "abc"); Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); - try (SecureString secureString = APMAgentSettings.TELEMETRY_API_KEY_SETTING.get(settings)) { + try (SecureString secureString = TELEMETRY_API_KEY_SETTING.get(settings)) { assertEquals("abc", secureString.toString()); } } + + /** + * Check that invalid or forbidden APM agent settings are rejected if their last part resembles an allowed setting. + */ + public void testRejectUnknownSettingResemblingAnAllowedOne() { + Settings settings = Settings.builder().put(APM_AGENT_SETTINGS.getKey() + "unknown.service_name", "true").build(); + + Exception exception = expectThrows(IllegalArgumentException.class, () -> APM_AGENT_SETTINGS.getAsMap(settings)); + assertThat(exception.getMessage(), containsString("[telemetry.agent.unknown.service_name]")); + } }