From 30ced43110cd0de8f7e8b33110b9bd673a7cb42f Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 10 Feb 2021 09:04:26 +0100 Subject: [PATCH 1/2] Add more trace logging when installing monitor watches and (#68752) unmute TransportMonitoringMigrateAlertsActionTests#testLocalAlertsRemoval and TransportMonitoringMigrateAlertsActionTests#testRepeatedLocalAlertsRemoval tests Somehow during these tests the monitor watches are not installed. Both tests use the local exporter and this exporter only installs the watches under specific conditions via the elected master node. I suspect the conditions are never met. The http exporter is more relaxed when attempting to install monitor watches and the tests using the http exporter seem not to be prone by the fact that tests fail because monitor watches have not been installed. Relates to #66586 --- .../monitoring/exporter/local/LocalExporter.java | 12 ++++++++++++ .../TransportMonitoringMigrateAlertsActionTests.java | 8 +++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java index 5c383bd4e1706..92b8ef1de4be5 100644 --- a/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java +++ b/x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java @@ -430,9 +430,20 @@ private void setupClusterAlertsTasks(ClusterState clusterState, boolean clusterS if (watches != null && watches.allPrimaryShardsActive() == false) { logger.trace("cannot manage cluster alerts because [.watches] index is not allocated"); } else if ((watches == null || indexExists) && watcherSetup.compareAndSet(false, true)) { + logger.trace("installing monitoring watches"); getClusterAlertsInstallationAsyncActions(indexExists, asyncActions, pendingResponses); + } else { + logger.trace("skipping installing monitoring watches, watches=[{}], indexExists=[{}], watcherSetup=[{}]", + watches, indexExists, watcherSetup.get()); } + } else { + logger.trace("watches shouldn't be setup, because state=[{}] and clusterStateChange=[{}]", state.get(), clusterStateChange); } + } else { + logger.trace("watches can't be used, because xpack.watcher.enabled=[{}] and " + + "xpack.monitoring.exporters._local.cluster_alerts.management.enabled=[{}]", + XPackSettings.WATCHER_ENABLED.get(config.settings()), + CLUSTER_ALERTS_MANAGEMENT_SETTING.getConcreteSettingForNamespace(config.name()).get(config.settings())); } } @@ -581,6 +592,7 @@ private void getClusterAlertsInstallationAsyncActions(final boolean indexExists, new ResponseActionListener<>("watch", uniqueWatchId, pendingResponses))); } } else if (addWatch) { + logger.trace("adding monitoring watch [{}]", uniqueWatchId); asyncActions.add(() -> putWatch(watcher, watchId, uniqueWatchId, pendingResponses)); } } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java index e97c0f251de67..4cd5aa1b3952a 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java @@ -35,6 +35,7 @@ import org.elasticsearch.test.http.MockRequest; import org.elasticsearch.test.http.MockResponse; import org.elasticsearch.test.http.MockWebServer; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsAction; import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsRequest; @@ -124,7 +125,9 @@ private void stopMonitoring() { )); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/66586") + @TestLogging( + value = "org.elasticsearch.xpack.monitoring.exporter.local:trace", + reason = "to ensure we log local exporter on trace level") public void testLocalAlertsRemoval() throws Exception { try { // start monitoring service @@ -159,6 +162,9 @@ public void testLocalAlertsRemoval() throws Exception { } } + @TestLogging( + value = "org.elasticsearch.xpack.monitoring.exporter.local:trace", + reason = "to ensure we log local exporter on trace level") public void testRepeatedLocalAlertsRemoval() throws Exception { try { // start monitoring service From cfcb8c6576a22590703097de978c3d6315c31b03 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 18 Feb 2021 08:55:40 +0100 Subject: [PATCH 2/2] Manually trigger local exporter to open a bulk in some monitor tests. (#69139) Change tests to use monitor bulk api on elected master node before verifying watcher index exists. Sometimes the monitor service on the elected master doesn't yet export monitor documents resulting in tests using the `ensureInitialLocalResources(...)` method to fail. Cluster alerts watcher are only installed when local exporter tries to resolve local bulk. Relates to #66586 --- ...sportMonitoringMigrateAlertsActionTests.java | 17 +++++++++++++++++ .../exporter/local/LocalExporterIntegTests.java | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java index 4cd5aa1b3952a..e60ca4a584ec3 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/action/TransportMonitoringMigrateAlertsActionTests.java @@ -31,12 +31,16 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.http.MockRequest; import org.elasticsearch.test.http.MockResponse; import org.elasticsearch.test.http.MockWebServer; import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.core.XPackSettings; +import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction; +import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkRequest; +import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkResponse; import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsAction; import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsRequest; import org.elasticsearch.xpack.core.monitoring.action.MonitoringMigrateAlertsResponse; @@ -50,6 +54,7 @@ import org.elasticsearch.xpack.monitoring.exporter.ClusterAlertsUtil; import org.elasticsearch.xpack.monitoring.exporter.http.HttpExporter; import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporter; +import org.elasticsearch.xpack.monitoring.exporter.local.LocalExporterIntegTests; import org.elasticsearch.xpack.monitoring.test.MonitoringIntegTestCase; import org.junit.After; import org.junit.Before; @@ -480,6 +485,18 @@ public void testRemoteAlertsRemoteDisallowsWatcher() throws Exception { } private void ensureInitialLocalResources() throws Exception { + // Should trigger setting up alert watches via LocalExporter#openBulk(...) and + // then eventually to LocalExporter#setupIfElectedMaster(...) + // Sometimes this last method doesn't install watches, because elected master node doesn't export monitor documents. + // and then these assertions here fail. + { + MonitoringBulkRequest request = new MonitoringBulkRequest(); + request.add(LocalExporterIntegTests.createMonitoringBulkDoc()); + String masterNode = internalCluster().getMasterName(); + MonitoringBulkResponse response = client(masterNode).execute(MonitoringBulkAction.INSTANCE, request).actionGet(); + assertThat(response.status(), equalTo(RestStatus.OK)); + } + waitForWatcherIndices(); assertBusy(() -> { assertThat(indexExists(".monitoring-*"), is(true)); diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java index 5aa6e729e72eb..d66f768f22151 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporterIntegTests.java @@ -303,7 +303,7 @@ private void checkMonitoringDocs() { } } - private static MonitoringBulkDoc createMonitoringBulkDoc() throws IOException { + public static MonitoringBulkDoc createMonitoringBulkDoc() throws IOException { final MonitoredSystem system = randomFrom(BEATS, KIBANA, LOGSTASH); final XContentType xContentType = randomFrom(XContentType.values()); final BytesReference source;