From 54bbe7da1db636ad5a74475129cbe2658b8117bd Mon Sep 17 00:00:00 2001 From: andsel Date: Fri, 18 Oct 2024 15:57:07 +0200 Subject: [PATCH 01/14] Introduce a new flag setting 'legacy.monitoring.enabled' which eventually enable the legacy monitoring collector --- config/logstash.yml | 5 +++++ docs/static/settings-file.asciidoc | 4 ++++ logstash-core/lib/logstash/environment.rb | 3 ++- x-pack/lib/monitoring/monitoring.rb | 3 ++- x-pack/qa/integration/management/multiple_pipelines_spec.rb | 1 + x-pack/qa/integration/monitoring/direct_shipping_spec.rb | 1 + .../monitoring/es_documents_structure_validation_spec.rb | 1 + .../qa/integration/monitoring/multiple_host_defined_spec.rb | 1 + .../monitoring/no_ssl_create_monitoring_indexes_spec.rb | 1 + .../monitoring/persisted_queue_is_enabled_spec.rb | 1 + x-pack/qa/integration/support/helpers.rb | 1 + 11 files changed, 20 insertions(+), 2 deletions(-) diff --git a/config/logstash.yml b/config/logstash.yml index 59703aa54f6..5fbca366156 100644 --- a/config/logstash.yml +++ b/config/logstash.yml @@ -334,6 +334,11 @@ # Default to direct, optionally can be switched to heap to select Java heap space. # pipeline.buffer.type: direct # +# +# Flag to enable the legacy internal monitoring. +# Default is false +# legacy.monitoring.enabled false +# # ------------ X-Pack Settings (not applicable for OSS build)-------------- # # X-Pack Monitoring diff --git a/docs/static/settings-file.asciidoc b/docs/static/settings-file.asciidoc index bc43cec6741..5a4003572c7 100644 --- a/docs/static/settings-file.asciidoc +++ b/docs/static/settings-file.asciidoc @@ -365,4 +365,8 @@ separating each log lines per pipeline could be helpful in case you need to trou | Determine where to allocate memory buffers, for plugins that leverage them. Currently defaults to `direct` but can be switched to `heap` to select Java heap space, which will become the default in the future. | `direct` Check out <> for more info. + +| `monitoring.legacy.enabled` +| Setting to explicitly enable <>. +| `false` |======================================================================= diff --git a/logstash-core/lib/logstash/environment.rb b/logstash-core/lib/logstash/environment.rb index d8d7033f210..907b7d470ee 100644 --- a/logstash-core/lib/logstash/environment.rb +++ b/logstash-core/lib/logstash/environment.rb @@ -110,7 +110,8 @@ module Environment Setting::String.new("keystore.classname", "org.logstash.secret.store.backend.JavaKeyStore"), Setting::String.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on Setting::NullableString.new("monitoring.cluster_uuid"), - Setting::String.new("pipeline.buffer.type", "direct", true, ["direct", "heap"]) + Setting::String.new("pipeline.buffer.type", "direct", true, ["direct", "heap"]), + Setting::Boolean.new("legacy.monitoring.enabled", false) # post_process ].each {|setting| SETTINGS.register(setting) } diff --git a/x-pack/lib/monitoring/monitoring.rb b/x-pack/lib/monitoring/monitoring.rb index fac0b7565bf..3e914d29caf 100644 --- a/x-pack/lib/monitoring/monitoring.rb +++ b/x-pack/lib/monitoring/monitoring.rb @@ -155,8 +155,9 @@ def after_agent(runner) # For versions prior to 6.3 the default value of "xpack.monitoring.enabled" was true # For versions 6.3+ the default of "xpack.monitoring.enabled" is false. # To help keep passivity, assume that if "xpack.monitoring.elasticsearch.hosts" has been set that monitoring should be enabled. - # return true if xpack.monitoring.enabled=true (explicitly) or xpack.monitoring.elasticsearch.hosts is configured + # return true if legacy.monitoring.enabled=true and xpack.monitoring.enabled=true (explicitly) or xpack.monitoring.elasticsearch.hosts is configured def monitoring_enabled?(settings) + return false unless settings.get_value("legacy.monitoring.enabled") return settings.get_value("monitoring.enabled") if settings.set?("monitoring.enabled") return settings.get_value("xpack.monitoring.enabled") if settings.set?("xpack.monitoring.enabled") diff --git a/x-pack/qa/integration/management/multiple_pipelines_spec.rb b/x-pack/qa/integration/management/multiple_pipelines_spec.rb index 0511da0bfac..83efb88596f 100644 --- a/x-pack/qa/integration/management/multiple_pipelines_spec.rb +++ b/x-pack/qa/integration/management/multiple_pipelines_spec.rb @@ -33,6 +33,7 @@ "xpack.management.elasticsearch.hosts" => ["http://localhost:9200"], "xpack.management.elasticsearch.username" => "elastic", "xpack.management.elasticsearch.password" => elastic_password, + "legacy.monitoring.enabled" => true, "xpack.monitoring.elasticsearch.username" => "elastic", "xpack.monitoring.elasticsearch.password" => elastic_password diff --git a/x-pack/qa/integration/monitoring/direct_shipping_spec.rb b/x-pack/qa/integration/monitoring/direct_shipping_spec.rb index bded6b36100..1be3ab9170c 100644 --- a/x-pack/qa/integration/monitoring/direct_shipping_spec.rb +++ b/x-pack/qa/integration/monitoring/direct_shipping_spec.rb @@ -15,6 +15,7 @@ @logstash_service = logstash_with_empty_default("bin/logstash -e '#{config}' -w 1 --pipeline.id #{SecureRandom.hex(8)}", { :settings => { + "legacy.monitoring.enabled" => true, "monitoring.enabled" => true, "monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "monitoring.collection.interval" => "1s", diff --git a/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb b/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb index 9b3c31057cb..e9b71d0b99c 100644 --- a/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb +++ b/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb @@ -85,6 +85,7 @@ def start_monitoring_logstash(config, prefix = "") end logstash_with_empty_default("bin/logstash -e '#{config}' -w 1", { :settings => { + "legacy.monitoring.enabled" => true, "#{mon_prefix}monitoring.enabled" => true, "#{mon_prefix}monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "#{mon_prefix}monitoring.collection.interval" => "1s", diff --git a/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb b/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb index 235c92de4e3..ee1ed2ec656 100644 --- a/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb +++ b/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb @@ -14,6 +14,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { + "legacy.monitoring.enabled" => true, "xpack.monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "xpack.monitoring.collection.interval" => "1s", "xpack.monitoring.elasticsearch.username" => "elastic", diff --git a/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb b/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb index 0213d675a63..cd6147e972c 100644 --- a/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb +++ b/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb @@ -16,6 +16,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { + "legacy.monitoring.enabled" => true, "xpack.monitoring.collection.interval" => "1s", "xpack.monitoring.elasticsearch.username" => "elastic", "xpack.monitoring.elasticsearch.password" => elastic_password diff --git a/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb b/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb index 1ed1ca84614..39749e94b02 100644 --- a/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb +++ b/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb @@ -14,6 +14,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { + "legacy.monitoring.enabled" => true, "xpack.monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "xpack.monitoring.collection.interval" => "1s", "queue.type" => "persisted", diff --git a/x-pack/qa/integration/support/helpers.rb b/x-pack/qa/integration/support/helpers.rb index a216d1cd8d4..d163a19e4aa 100644 --- a/x-pack/qa/integration/support/helpers.rb +++ b/x-pack/qa/integration/support/helpers.rb @@ -146,6 +146,7 @@ def logstash_command_append(cmd, argument, value) end def logstash(cmd, options = {}) + # logstash_with_empty_default(cmd, options, {"xpack.monitoring.enabled" => true, "legacy.monitoring.enabled" => true}) logstash_with_empty_default(cmd, options, {"xpack.monitoring.enabled" => true}) end From 2fefd51626bb39aef9c4a8f8d55a5ce70b2d8507 Mon Sep 17 00:00:00 2001 From: andsel Date: Fri, 18 Oct 2024 17:52:57 +0200 Subject: [PATCH 02/14] Removed commented line --- docs/static/settings-file.asciidoc | 2 +- x-pack/qa/integration/support/helpers.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/static/settings-file.asciidoc b/docs/static/settings-file.asciidoc index 5a4003572c7..f3a3d86cff3 100644 --- a/docs/static/settings-file.asciidoc +++ b/docs/static/settings-file.asciidoc @@ -366,7 +366,7 @@ separating each log lines per pipeline could be helpful in case you need to trou Currently defaults to `direct` but can be switched to `heap` to select Java heap space, which will become the default in the future. | `direct` Check out <> for more info. -| `monitoring.legacy.enabled` +| `legacy.monitoring.enabled` | Setting to explicitly enable <>. | `false` |======================================================================= diff --git a/x-pack/qa/integration/support/helpers.rb b/x-pack/qa/integration/support/helpers.rb index d163a19e4aa..a216d1cd8d4 100644 --- a/x-pack/qa/integration/support/helpers.rb +++ b/x-pack/qa/integration/support/helpers.rb @@ -146,7 +146,6 @@ def logstash_command_append(cmd, argument, value) end def logstash(cmd, options = {}) - # logstash_with_empty_default(cmd, options, {"xpack.monitoring.enabled" => true, "legacy.monitoring.enabled" => true}) logstash_with_empty_default(cmd, options, {"xpack.monitoring.enabled" => true}) end From 6bd75a6b459fc7aa11603f461a4be6a4a9bc0c50 Mon Sep 17 00:00:00 2001 From: andsel Date: Fri, 18 Oct 2024 17:58:34 +0200 Subject: [PATCH 03/14] [Docs] Added note to call the user that want to force the legacy monitoring. --- docs/static/monitoring/monitoring-internal-legacy.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/static/monitoring/monitoring-internal-legacy.asciidoc b/docs/static/monitoring/monitoring-internal-legacy.asciidoc index 7b1730baef0..c403b97290d 100644 --- a/docs/static/monitoring/monitoring-internal-legacy.asciidoc +++ b/docs/static/monitoring/monitoring-internal-legacy.asciidoc @@ -7,6 +7,8 @@ deprecated[7.9.0] +NOTE: Starting from version 9.0 legacy monitoring is disabled, to force it use the flag `legacy.monitoring.enabled`. + ==== Components for legacy collection Monitoring {ls} with legacy collection uses these components: From 8fd24d295e3c39fcb7398421658917e8127825a3 Mon Sep 17 00:00:00 2001 From: andsel Date: Mon, 21 Oct 2024 11:53:34 +0200 Subject: [PATCH 04/14] [test] Added test to cover monitoring to be enabled only when legacy.monitoring is overridden --- .../spec/monitoring/pipeline_register_hook_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb index 98723413e81..db6a7ac2dc3 100644 --- a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb +++ b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb @@ -24,6 +24,18 @@ } context 'validate monitoring settings' do + it "it's not enabled with just xpack.monitoring.enabled set to true" do + expect(subject.monitoring_enabled?(settings)).to be_falsey + end + + context 'when legacy.monitoring is overridden' do + let(:settings) { super().merge({"legacy.monitoring.enabled" => true}) } + + it "should be enabled" do + expect(subject.monitoring_enabled?(settings)).to be_truthy + end + end + it "work without any monitoring settings" do settings.set_value("xpack.monitoring.enabled", true) expect(subject.generate_pipeline_config(settings)).to be_truthy From df08f2cdb77e37a8cd8ba067abc0788bd7df3fc1 Mon Sep 17 00:00:00 2001 From: Andrea Selva Date: Tue, 22 Oct 2024 09:44:26 +0200 Subject: [PATCH 05/14] Apply suggestions from code review Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com> --- config/logstash.yml | 4 ++-- docs/static/settings-file.asciidoc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/logstash.yml b/config/logstash.yml index 5fbca366156..fd575afa01b 100644 --- a/config/logstash.yml +++ b/config/logstash.yml @@ -335,9 +335,9 @@ # pipeline.buffer.type: direct # # -# Flag to enable the legacy internal monitoring. +# Flag to allow the legacy internal monitoring. # Default is false -# legacy.monitoring.enabled false +# legacy.monitoring.enabled: false # # ------------ X-Pack Settings (not applicable for OSS build)-------------- # diff --git a/docs/static/settings-file.asciidoc b/docs/static/settings-file.asciidoc index f3a3d86cff3..f6db0e3b9d6 100644 --- a/docs/static/settings-file.asciidoc +++ b/docs/static/settings-file.asciidoc @@ -367,6 +367,6 @@ Currently defaults to `direct` but can be switched to `heap` to select Java heap | `direct` Check out <> for more info. | `legacy.monitoring.enabled` -| Setting to explicitly enable <>. +| Set to `true` to allow <>. | `false` |======================================================================= From fbe907d19784bc01b0a60907518bea1f33497407 Mon Sep 17 00:00:00 2001 From: andsel Date: Tue, 22 Oct 2024 10:03:00 +0200 Subject: [PATCH 06/14] Renamed 'legacy.monitoring.enabled' to 'allow.legacy.monitoring' --- config/logstash.yml | 2 +- docs/static/monitoring/monitoring-internal-legacy.asciidoc | 2 +- docs/static/settings-file.asciidoc | 2 +- logstash-core/lib/logstash/environment.rb | 2 +- x-pack/lib/monitoring/monitoring.rb | 4 ++-- x-pack/qa/integration/management/multiple_pipelines_spec.rb | 2 +- x-pack/qa/integration/monitoring/direct_shipping_spec.rb | 2 +- .../monitoring/es_documents_structure_validation_spec.rb | 2 +- .../qa/integration/monitoring/multiple_host_defined_spec.rb | 2 +- .../monitoring/no_ssl_create_monitoring_indexes_spec.rb | 2 +- .../monitoring/persisted_queue_is_enabled_spec.rb | 2 +- x-pack/spec/monitoring/pipeline_register_hook_spec.rb | 6 +++--- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/config/logstash.yml b/config/logstash.yml index fd575afa01b..c1b70222a0e 100644 --- a/config/logstash.yml +++ b/config/logstash.yml @@ -337,7 +337,7 @@ # # Flag to allow the legacy internal monitoring. # Default is false -# legacy.monitoring.enabled: false +# allow.legacy.monitoring: false # # ------------ X-Pack Settings (not applicable for OSS build)-------------- # diff --git a/docs/static/monitoring/monitoring-internal-legacy.asciidoc b/docs/static/monitoring/monitoring-internal-legacy.asciidoc index c403b97290d..580b5d0793c 100644 --- a/docs/static/monitoring/monitoring-internal-legacy.asciidoc +++ b/docs/static/monitoring/monitoring-internal-legacy.asciidoc @@ -7,7 +7,7 @@ deprecated[7.9.0] -NOTE: Starting from version 9.0 legacy monitoring is disabled, to force it use the flag `legacy.monitoring.enabled`. +NOTE: Starting from version 9.0 legacy monitoring is disabled, to force it use the flag `allow.legacy.monitoring`. ==== Components for legacy collection diff --git a/docs/static/settings-file.asciidoc b/docs/static/settings-file.asciidoc index f6db0e3b9d6..79bd7b8ec59 100644 --- a/docs/static/settings-file.asciidoc +++ b/docs/static/settings-file.asciidoc @@ -366,7 +366,7 @@ separating each log lines per pipeline could be helpful in case you need to trou Currently defaults to `direct` but can be switched to `heap` to select Java heap space, which will become the default in the future. | `direct` Check out <> for more info. -| `legacy.monitoring.enabled` +| `allow.legacy.monitoring` | Set to `true` to allow <>. | `false` |======================================================================= diff --git a/logstash-core/lib/logstash/environment.rb b/logstash-core/lib/logstash/environment.rb index 907b7d470ee..db24852323f 100644 --- a/logstash-core/lib/logstash/environment.rb +++ b/logstash-core/lib/logstash/environment.rb @@ -111,7 +111,7 @@ module Environment Setting::String.new("keystore.file", ::File.join(::File.join(LogStash::Environment::LOGSTASH_HOME, "config"), "logstash.keystore"), false), # will be populated on Setting::NullableString.new("monitoring.cluster_uuid"), Setting::String.new("pipeline.buffer.type", "direct", true, ["direct", "heap"]), - Setting::Boolean.new("legacy.monitoring.enabled", false) + Setting::Boolean.new("allow.legacy.monitoring", false) # post_process ].each {|setting| SETTINGS.register(setting) } diff --git a/x-pack/lib/monitoring/monitoring.rb b/x-pack/lib/monitoring/monitoring.rb index 3e914d29caf..535a63f19a0 100644 --- a/x-pack/lib/monitoring/monitoring.rb +++ b/x-pack/lib/monitoring/monitoring.rb @@ -155,9 +155,9 @@ def after_agent(runner) # For versions prior to 6.3 the default value of "xpack.monitoring.enabled" was true # For versions 6.3+ the default of "xpack.monitoring.enabled" is false. # To help keep passivity, assume that if "xpack.monitoring.elasticsearch.hosts" has been set that monitoring should be enabled. - # return true if legacy.monitoring.enabled=true and xpack.monitoring.enabled=true (explicitly) or xpack.monitoring.elasticsearch.hosts is configured + # return true if allow.legacy.monitoring=true and xpack.monitoring.enabled=true (explicitly) or xpack.monitoring.elasticsearch.hosts is configured def monitoring_enabled?(settings) - return false unless settings.get_value("legacy.monitoring.enabled") + return false unless settings.get_value("allow.legacy.monitoring") return settings.get_value("monitoring.enabled") if settings.set?("monitoring.enabled") return settings.get_value("xpack.monitoring.enabled") if settings.set?("xpack.monitoring.enabled") diff --git a/x-pack/qa/integration/management/multiple_pipelines_spec.rb b/x-pack/qa/integration/management/multiple_pipelines_spec.rb index 83efb88596f..c9b81cc204d 100644 --- a/x-pack/qa/integration/management/multiple_pipelines_spec.rb +++ b/x-pack/qa/integration/management/multiple_pipelines_spec.rb @@ -33,7 +33,7 @@ "xpack.management.elasticsearch.hosts" => ["http://localhost:9200"], "xpack.management.elasticsearch.username" => "elastic", "xpack.management.elasticsearch.password" => elastic_password, - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "xpack.monitoring.elasticsearch.username" => "elastic", "xpack.monitoring.elasticsearch.password" => elastic_password diff --git a/x-pack/qa/integration/monitoring/direct_shipping_spec.rb b/x-pack/qa/integration/monitoring/direct_shipping_spec.rb index 1be3ab9170c..2459b7fbec3 100644 --- a/x-pack/qa/integration/monitoring/direct_shipping_spec.rb +++ b/x-pack/qa/integration/monitoring/direct_shipping_spec.rb @@ -15,7 +15,7 @@ @logstash_service = logstash_with_empty_default("bin/logstash -e '#{config}' -w 1 --pipeline.id #{SecureRandom.hex(8)}", { :settings => { - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "monitoring.enabled" => true, "monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "monitoring.collection.interval" => "1s", diff --git a/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb b/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb index e9b71d0b99c..f5cb328c49d 100644 --- a/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb +++ b/x-pack/qa/integration/monitoring/es_documents_structure_validation_spec.rb @@ -85,7 +85,7 @@ def start_monitoring_logstash(config, prefix = "") end logstash_with_empty_default("bin/logstash -e '#{config}' -w 1", { :settings => { - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "#{mon_prefix}monitoring.enabled" => true, "#{mon_prefix}monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "#{mon_prefix}monitoring.collection.interval" => "1s", diff --git a/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb b/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb index ee1ed2ec656..820310b8735 100644 --- a/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb +++ b/x-pack/qa/integration/monitoring/multiple_host_defined_spec.rb @@ -14,7 +14,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "xpack.monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "xpack.monitoring.collection.interval" => "1s", "xpack.monitoring.elasticsearch.username" => "elastic", diff --git a/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb b/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb index cd6147e972c..87c329098e3 100644 --- a/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb +++ b/x-pack/qa/integration/monitoring/no_ssl_create_monitoring_indexes_spec.rb @@ -16,7 +16,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "xpack.monitoring.collection.interval" => "1s", "xpack.monitoring.elasticsearch.username" => "elastic", "xpack.monitoring.elasticsearch.password" => elastic_password diff --git a/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb b/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb index 39749e94b02..e3505c63b53 100644 --- a/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb +++ b/x-pack/qa/integration/monitoring/persisted_queue_is_enabled_spec.rb @@ -14,7 +14,7 @@ @logstash_service = logstash("bin/logstash -e '#{config}' -w 1", { :settings => { - "legacy.monitoring.enabled" => true, + "allow.legacy.monitoring" => true, "xpack.monitoring.elasticsearch.hosts" => ["http://localhost:9200", "http://localhost:9200"], "xpack.monitoring.collection.interval" => "1s", "queue.type" => "persisted", diff --git a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb index db6a7ac2dc3..471a7a8fc32 100644 --- a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb +++ b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb @@ -28,10 +28,10 @@ expect(subject.monitoring_enabled?(settings)).to be_falsey end - context 'when legacy.monitoring is overridden' do - let(:settings) { super().merge({"legacy.monitoring.enabled" => true}) } + context 'when legacy monitoring is allowed and is xpack monitoring is enabled' do + let(:settings) { super().merge({"allow.legacy.monitoring" => true}) } - it "should be enabled" do + it "then internal monitoring should be effectively enabled" do expect(subject.monitoring_enabled?(settings)).to be_truthy end end From 9929323c98d0c813b1e984e8b5f70a7a5bd9f04d Mon Sep 17 00:00:00 2001 From: andsel Date: Tue, 22 Oct 2024 10:21:22 +0200 Subject: [PATCH 07/14] Exposed 'allow.legacy.monitoring' from Docker image --- docker/data/logstash/env2yaml/env2yaml.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/data/logstash/env2yaml/env2yaml.go b/docker/data/logstash/env2yaml/env2yaml.go index c7b79266815..c1c41e8be3a 100644 --- a/docker/data/logstash/env2yaml/env2yaml.go +++ b/docker/data/logstash/env2yaml/env2yaml.go @@ -17,7 +17,6 @@ package main import ( "errors" "fmt" - "gopkg.in/yaml.v2" "io/ioutil" "log" "os" @@ -86,6 +85,7 @@ var validSettings = []string{ "api.auth.basic.password_policy.include.symbol", "allow_superuser", "monitoring.cluster_uuid", + "allow.legacy.monitoring", "xpack.monitoring.enabled", "xpack.monitoring.collection.interval", "xpack.monitoring.elasticsearch.hosts", From 014853fed87ee564de6a5808633b054421623a87 Mon Sep 17 00:00:00 2001 From: Andrea Selva Date: Tue, 22 Oct 2024 14:06:54 +0200 Subject: [PATCH 08/14] Apply suggestions from code review Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com> --- config/logstash.yml | 3 +-- docs/static/monitoring/monitoring-internal-legacy.asciidoc | 2 +- x-pack/spec/monitoring/pipeline_register_hook_spec.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/config/logstash.yml b/config/logstash.yml index c1b70222a0e..5950321b93f 100644 --- a/config/logstash.yml +++ b/config/logstash.yml @@ -335,8 +335,7 @@ # pipeline.buffer.type: direct # # -# Flag to allow the legacy internal monitoring. -# Default is false +# Flag to allow the legacy internal monitoring (default: false) # allow.legacy.monitoring: false # # ------------ X-Pack Settings (not applicable for OSS build)-------------- diff --git a/docs/static/monitoring/monitoring-internal-legacy.asciidoc b/docs/static/monitoring/monitoring-internal-legacy.asciidoc index 580b5d0793c..48cd89c54ed 100644 --- a/docs/static/monitoring/monitoring-internal-legacy.asciidoc +++ b/docs/static/monitoring/monitoring-internal-legacy.asciidoc @@ -7,7 +7,7 @@ deprecated[7.9.0] -NOTE: Starting from version 9.0 legacy monitoring is disabled, to force it use the flag `allow.legacy.monitoring`. +NOTE: Starting from version 9.0, legacy monitoring is deactivated and behind a feature flag. Set `allow.legacy.monitoring` to `true` to allow access to the feature. ==== Components for legacy collection diff --git a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb index 471a7a8fc32..57d9da2a7dc 100644 --- a/x-pack/spec/monitoring/pipeline_register_hook_spec.rb +++ b/x-pack/spec/monitoring/pipeline_register_hook_spec.rb @@ -28,7 +28,7 @@ expect(subject.monitoring_enabled?(settings)).to be_falsey end - context 'when legacy monitoring is allowed and is xpack monitoring is enabled' do + context 'when legacy monitoring is allowed and xpack monitoring is enabled' do let(:settings) { super().merge({"allow.legacy.monitoring" => true}) } it "then internal monitoring should be effectively enabled" do From 3e791f7dc4e55cd003d77f31dee17ebdbc67f94f Mon Sep 17 00:00:00 2001 From: andsel Date: Tue, 22 Oct 2024 14:14:32 +0200 Subject: [PATCH 09/14] Updated benchmark BK pipeline to explicitly allow legacy collection --- .buildkite/scripts/benchmark/config/logstash.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.buildkite/scripts/benchmark/config/logstash.yml b/.buildkite/scripts/benchmark/config/logstash.yml index 5b228a64edd..9f6d3a72c81 100644 --- a/.buildkite/scripts/benchmark/config/logstash.yml +++ b/.buildkite/scripts/benchmark/config/logstash.yml @@ -3,6 +3,7 @@ pipeline.workers: ${WORKER} pipeline.batch.size: ${BATCH_SIZE} queue.type: ${QTYPE} +allow.legacy:.monitoring: true xpack.monitoring.enabled: true xpack.monitoring.elasticsearch.username: ${MONITOR_ES_USER} xpack.monitoring.elasticsearch.password: ${MONITOR_ES_PW} From 485d37fe37719f27cddc6dae481815e4b93e0670 Mon Sep 17 00:00:00 2001 From: andsel Date: Tue, 22 Oct 2024 14:29:42 +0200 Subject: [PATCH 10/14] Logs a warning message when detects that legacy monitoring is enabled but not allowed. --- x-pack/lib/monitoring/monitoring.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x-pack/lib/monitoring/monitoring.rb b/x-pack/lib/monitoring/monitoring.rb index 535a63f19a0..989ee60157b 100644 --- a/x-pack/lib/monitoring/monitoring.rb +++ b/x-pack/lib/monitoring/monitoring.rb @@ -157,6 +157,7 @@ def after_agent(runner) # To help keep passivity, assume that if "xpack.monitoring.elasticsearch.hosts" has been set that monitoring should be enabled. # return true if allow.legacy.monitoring=true and xpack.monitoring.enabled=true (explicitly) or xpack.monitoring.elasticsearch.hosts is configured def monitoring_enabled?(settings) + log_warn_if_legacy_is_enabled_and_not_allowed(settings) return false unless settings.get_value("allow.legacy.monitoring") return settings.get_value("monitoring.enabled") if settings.set?("monitoring.enabled") return settings.get_value("xpack.monitoring.enabled") if settings.set?("xpack.monitoring.enabled") @@ -171,6 +172,15 @@ def monitoring_enabled?(settings) end end + def log_warn_if_legacy_is_enabled_and_not_allowed(settings) + allowed = settings.get_value("allow.legacy.monitoring") + legacy_monitoring_enabled = (settings.get_value("xpack.monitoring.enabled") || settings.get_value("monitoring.enabled")) + if !allowed && legacy_monitoring_enabled + logger.warn("Legacy internal monitoring is enabled (xpack.monitoring.enabled or monitoring.enabled is true) but not allowed. Please check your configuration and eventually set allow.legacy.monitoring") + end + end + private :log_warn_if_legacy_is_enabled_and_not_allowed + def setup_metrics_pipeline settings = LogStash::SETTINGS.clone From cb91379caad315c23e20e0a80349e05ea6ef3fbd Mon Sep 17 00:00:00 2001 From: Andrea Selva Date: Tue, 22 Oct 2024 15:40:00 +0200 Subject: [PATCH 11/14] Update x-pack/lib/monitoring/monitoring.rb Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com> --- x-pack/lib/monitoring/monitoring.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/lib/monitoring/monitoring.rb b/x-pack/lib/monitoring/monitoring.rb index 989ee60157b..ca6bf3b157c 100644 --- a/x-pack/lib/monitoring/monitoring.rb +++ b/x-pack/lib/monitoring/monitoring.rb @@ -176,7 +176,7 @@ def log_warn_if_legacy_is_enabled_and_not_allowed(settings) allowed = settings.get_value("allow.legacy.monitoring") legacy_monitoring_enabled = (settings.get_value("xpack.monitoring.enabled") || settings.get_value("monitoring.enabled")) if !allowed && legacy_monitoring_enabled - logger.warn("Legacy internal monitoring is enabled (xpack.monitoring.enabled or monitoring.enabled is true) but not allowed. Please check your configuration and eventually set allow.legacy.monitoring") + logger.warn("You have enabled legacy internal monitoring. However, starting from version 9.0, this feature is deactivated and behind a feature flag. Set `allow.legacy.monitoring` to `true` to allow access to the feature.) end end private :log_warn_if_legacy_is_enabled_and_not_allowed From 2e90f147c6eab93f0e5123ec9b2f4cb31cd0babe Mon Sep 17 00:00:00 2001 From: andsel Date: Wed, 23 Oct 2024 09:22:39 +0200 Subject: [PATCH 12/14] Fixed sunthax error --- x-pack/lib/monitoring/monitoring.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/lib/monitoring/monitoring.rb b/x-pack/lib/monitoring/monitoring.rb index ca6bf3b157c..240394da258 100644 --- a/x-pack/lib/monitoring/monitoring.rb +++ b/x-pack/lib/monitoring/monitoring.rb @@ -176,7 +176,7 @@ def log_warn_if_legacy_is_enabled_and_not_allowed(settings) allowed = settings.get_value("allow.legacy.monitoring") legacy_monitoring_enabled = (settings.get_value("xpack.monitoring.enabled") || settings.get_value("monitoring.enabled")) if !allowed && legacy_monitoring_enabled - logger.warn("You have enabled legacy internal monitoring. However, starting from version 9.0, this feature is deactivated and behind a feature flag. Set `allow.legacy.monitoring` to `true` to allow access to the feature.) + logger.warn("You have enabled legacy internal monitoring. However, starting from version 9.0, this feature is deactivated and behind a feature flag. Set `allow.legacy.monitoring` to `true` to allow access to the feature.") end end private :log_warn_if_legacy_is_enabled_and_not_allowed From 8abc7935c2ca1f272d5c9d2a722a4c7c42162ecb Mon Sep 17 00:00:00 2001 From: andsel Date: Wed, 23 Oct 2024 09:53:11 +0200 Subject: [PATCH 13/14] Added allow legacy flag to expose montoring section in API --- .../lib/logstash/api/commands/default_metadata.rb | 1 + .../logstash/api/commands/default_metadata_spec.rb | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/logstash-core/lib/logstash/api/commands/default_metadata.rb b/logstash-core/lib/logstash/api/commands/default_metadata.rb index 635e3e5f43a..ea3ae109e21 100644 --- a/logstash-core/lib/logstash/api/commands/default_metadata.rb +++ b/logstash-core/lib/logstash/api/commands/default_metadata.rb @@ -71,6 +71,7 @@ def http_address private def enabled_xpack_monitoring? + LogStash::SETTINGS.get_value("allow.legacy.monitoring") && LogStash::SETTINGS.registered?("xpack.monitoring.enabled") && LogStash::SETTINGS.get("xpack.monitoring.enabled") end diff --git a/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb b/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb index acdbc8a5091..66ddd78e9e1 100644 --- a/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb +++ b/logstash-core/spec/logstash/api/commands/default_metadata_spec.rb @@ -54,13 +54,22 @@ def registerIfNot(setting) ) end - it "check monitoring exist when monitoring is enabled" do + it "check monitoring section exist when legacy monitoring is enabled and allowed" do + LogStash::SETTINGS.set_value("allow.legacy.monitoring", true) LogStash::SETTINGS.set_value("xpack.monitoring.enabled", true) expect(report.keys).to include( :monitoring ) end + it "check monitoring section does not appear when legacy monitoring is not allowed but enabled" do + LogStash::SETTINGS.set_value("allow.legacy.monitoring", false) + LogStash::SETTINGS.set_value("xpack.monitoring.enabled", true) + expect(report.keys).not_to include( + :monitoring + ) + end + it "check monitoring does not appear when not enabled and nor cluster_uuid is defined" do LogStash::SETTINGS.set_value("xpack.monitoring.enabled", false) LogStash::SETTINGS.get_setting("monitoring.cluster_uuid").reset From 3d5ab00cb8fe95d86774b089e056cf5e6a1aeb1e Mon Sep 17 00:00:00 2001 From: andsel Date: Wed, 23 Oct 2024 14:35:56 +0200 Subject: [PATCH 14/14] Re-added yaml parsing lib that made the go build phase to fail --- docker/data/logstash/env2yaml/env2yaml.go | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/data/logstash/env2yaml/env2yaml.go b/docker/data/logstash/env2yaml/env2yaml.go index c1c41e8be3a..a3091f0554a 100644 --- a/docker/data/logstash/env2yaml/env2yaml.go +++ b/docker/data/logstash/env2yaml/env2yaml.go @@ -17,6 +17,7 @@ package main import ( "errors" "fmt" + "gopkg.in/yaml.v2" "io/ioutil" "log" "os"