elastic · edsavage · Mar 6, 2025 · Mar 5, 2025 · Mar 6, 2025
diff --git a/build.gradle b/build.gradle
@@ -435,7 +435,7 @@ task buildDependencyReport(type: Exec) {
 // This gives us the flexibility to build in different
 // ways and still use the same upload code.
 task upload(type: UploadS3Task) {
-  bucket 'prelert-artifacts'
+  bucket='prelert-artifacts'
   // Only upload the platform-specific artifacts in this task
   def zipFileDir = fileTree("${buildDir}/distributions").matching {
     include "*-aarch64.zip", "*-x86_64.zip"
@@ -447,7 +447,7 @@ task upload(type: UploadS3Task) {
 }
 
 task uploadAll(type: UploadS3Task) {
-  bucket 'prelert-artifacts'
+  bucket='prelert-artifacts'
   // Upload ALL artifacts (including the dependency report) in this task
   def fileDir = fileTree("${buildDir}/distributions").matching {
     include "ml-cpp-${project.version}*.zip", "dependencies-${version}.csv"
@@ -462,7 +462,7 @@ task uberUpload(type: UploadS3Task, dependsOn: [buildUberZipFromDownloads,
                                                 buildDependenciesZipFromDownloads,
                                                 buildNoDependenciesZipFromDownloads,
                                                 buildDependencyReport]) {
-  bucket 'prelert-artifacts'
+  bucket='prelert-artifacts'
   upload buildUberZipFromDownloads.outputs.files.singleFile, "maven/${artifactGroupPath}/${artifactName}/${project.version}/${buildUberZipFromDownloads.outputs.files.singleFile.name}"
   upload buildDependenciesZipFromDownloads.outputs.files.singleFile, "maven/${artifactGroupPath}/${artifactName}/${project.version}/${buildDependenciesZipFromDownloads.outputs.files.singleFile.name}"
   upload buildNoDependenciesZipFromDownloads.outputs.files.singleFile, "maven/${artifactGroupPath}/${artifactName}/${project.version}/${buildNoDependenciesZipFromDownloads.outputs.files.singleFile.name}"

diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc
@@ -28,6 +28,12 @@
 
 //=== Regressions
 
+== {es} version 8.16.6
+
+=== Bug Fixes
+
+* Correct handling of config updates. (See {ml-pull}2821[#2821].)
+
 == {es} version 8.16.4
 
 === Bug Fixes

diff --git a/include/api/CAnomalyJob.h b/include/api/CAnomalyJob.h
@@ -37,6 +37,7 @@
 
 namespace CAnomalyJobTest {
 struct testParsePersistControlMessageArgs;
+struct testConfigUpdate;
 struct testOutputBucketResultsUntilGivenIncompleteInitialBucket;
 }
 
@@ -521,6 +522,7 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {
     core_t::TTime m_InitialLastFinalisedBucketEndTime{0};
 
     // Test case access
+    friend struct CAnomalyJobTest::testConfigUpdate;
     friend struct CAnomalyJobTest::testParsePersistControlMessageArgs;
 
     friend struct CAnomalyJobTest::testOutputBucketResultsUntilGivenIncompleteInitialBucket;

diff --git a/include/api/CAnomalyJobConfig.h b/include/api/CAnomalyJobConfig.h
@@ -239,7 +239,19 @@ class API_EXPORT CAnomalyJobConfig {
         }
 
         void initRuleFilters(const CDetectionRulesJsonParser::TStrPatternSetUMap& ruleFilters) {
-            m_RuleFilters = ruleFilters;
+            // Update or insert values that are in the new map - we never delete filters at this level.
+            // Note that we can't simply assign "m_RuleFilters = ruleFilters", as that would result in
+            // the pattern set objects being destroyed and, as they are referenced by the anomaly detector models,
+            // this is a bad thing.
+            for (const auto& kv : ruleFilters) {
+                CDetectionRulesJsonParser::TStrPatternSetUMap::iterator itr =
+                    m_RuleFilters.find(kv.first);
+                if (itr != m_RuleFilters.end()) {
+                    itr->second = kv.second;
+                } else {
+                    m_RuleFilters.insert(kv);
+                }
+            }
         }
 
         void initScheduledEvents(const TStrDetectionRulePrVec& scheduledEvents) {
@@ -249,19 +261,6 @@ class API_EXPORT CAnomalyJobConfig {
         //! Parse a JSON value representing an entire analysis config object.
         void parse(const json::value& json);
 
-        //! Return a JSON string representing the analysis config
-        const std::string& getAnalysisConfig();
-
-        //! Reparse the detector configuration object from within a stored
-        //! string representing the analysis config object.
-        //! This is necessary to correctly reinitialise scoped rule objects
-        //! folowing an update of the fiter rules configuration.
-        bool reparseDetectorsFromStoredConfig(const std::string& analysisConfig);
-
-        void setConfig(const std::string& analysisConfigString) {
-            m_AnalysisConfigString = analysisConfigString;
-        }
-
         core_t::TTime bucketSpan() const { return m_BucketSpan; }
 
         //! Return the size of the model prune window expressed as a whole number of seconds.

diff --git a/lib/api/CAnomalyJob.cc b/lib/api/CAnomalyJob.cc
@@ -474,8 +474,6 @@ void CAnomalyJob::updateConfig(const std::string& config) {
     if (configUpdater.update(config) == false) {
         LOG_ERROR(<< "Failed to update configuration");
     }
-    const std::string& analysisConfig = m_JobConfig.analysisConfig().getAnalysisConfig();
-    m_JobConfig.analysisConfig().reparseDetectorsFromStoredConfig(analysisConfig);
 }
 
 void CAnomalyJob::advanceTime(const std::string& time_) {

diff --git a/lib/api/CAnomalyJobConfig.cc b/lib/api/CAnomalyJobConfig.cc
@@ -589,7 +589,6 @@ bool CAnomalyJobConfig::parse(const std::string& jsonStr) {
 
         auto analysisConfig = parameters[ANALYSIS_CONFIG].jsonObject();
         if (analysisConfig != nullptr) {
-            m_AnalysisConfig.setConfig(toString(*analysisConfig));
             m_AnalysisConfig.parse(*analysisConfig);
         }
 
@@ -724,27 +723,6 @@ void CAnomalyJobConfig::CAnalysisConfig::parseDetectorsConfig(const json::value&
     }
 }
 
-const std::string& CAnomalyJobConfig::CAnalysisConfig::getAnalysisConfig() {
-    return m_AnalysisConfigString;
-}
-
-bool CAnomalyJobConfig::CAnalysisConfig::reparseDetectorsFromStoredConfig(const std::string& analysisConfig) {
-    json::value doc;
-    bool ok = core::CBoostJsonParser::parse(analysisConfig, doc);
-    if (ok == false) {
-        LOG_ERROR(<< "An error occurred while parsing anomaly job config from JSON: \""
-                  << analysisConfig << "\"");
-        return false;
-    }
-
-    auto parameters = ANALYSIS_CONFIG_READER.read(doc);
-    auto detectorsConfig = parameters[DETECTORS].jsonObject();
-    if (detectorsConfig != nullptr) {
-        this->parseDetectorsConfig(*detectorsConfig);
-    }
-    return true;
-}
-
 void CAnomalyJobConfig::CAnalysisConfig::parse(const json::value& analysisConfig) {
     auto parameters = ANALYSIS_CONFIG_READER.read(analysisConfig);
     // We choose to ignore any errors here parsing the time duration string as

diff --git a/lib/api/CConfigUpdater.cc b/lib/api/CConfigUpdater.cc
@@ -41,41 +41,53 @@ bool CConfigUpdater::update(const std::string& json) {
     }
 
     json::object obj = doc.as_object();
+    for (const auto& kv : obj) {
+        if (kv.key() == CAnomalyJobConfig::MODEL_PLOT_CONFIG) {
+            LOG_TRACE(<< "Updating model plot config");
 
-    if (obj.contains(CAnomalyJobConfig::MODEL_PLOT_CONFIG)) {
-        if (obj[CAnomalyJobConfig::MODEL_PLOT_CONFIG].is_object() == false) {
-            LOG_ERROR(<< "Input error: expected " << CAnomalyJobConfig::MODEL_PLOT_CONFIG
-                      << " to be JSON object but input was '" << json
-                      << "'. Please report this problem.");
-            return false;
-        }
-        const json::value& value = obj[CAnomalyJobConfig::MODEL_PLOT_CONFIG];
+            if (kv.value().is_object() == false) {
+                LOG_ERROR(<< "Input error: expected " << CAnomalyJobConfig::MODEL_PLOT_CONFIG
+                          << " to be JSON object but input was '" << json
+                          << "'. Please report this problem.");
+                return false;
+            }
 
-        m_JobConfig.modelPlotConfig().parse(value);
-        const ml::api::CAnomalyJobConfig::CModelPlotConfig& modelPlotConfig =
-            m_JobConfig.modelPlotConfig();
-        m_ModelConfig.configureModelPlot(modelPlotConfig.enabled(),
-                                         modelPlotConfig.annotationsEnabled(),
-                                         modelPlotConfig.terms());
-    } else if (obj.contains(CAnomalyJobConfig::FILTERS)) {
-        if (m_JobConfig.parseFilterConfig(json) == false) {
-            LOG_ERROR(<< "Failed to parse filter config update: " << json);
-            return false;
-        }
-        m_JobConfig.initRuleFilters();
-    } else if (obj.contains(CAnomalyJobConfig::EVENTS)) {
-        if (m_JobConfig.parseEventConfig(json) == false) {
-            LOG_ERROR(<< "Failed to parse events config update: " << json);
+            m_JobConfig.modelPlotConfig().parse(kv.value());
+            const ml::api::CAnomalyJobConfig::CModelPlotConfig& modelPlotConfig =
+                m_JobConfig.modelPlotConfig();
+            m_ModelConfig.configureModelPlot(modelPlotConfig.enabled(),
+                                             modelPlotConfig.annotationsEnabled(),
+                                             modelPlotConfig.terms());
+        } else if (kv.key() == CAnomalyJobConfig::FILTERS) {
+            LOG_TRACE(<< "Updating filters config");
+
+            if (m_JobConfig.parseFilterConfig(json) == false) {
+                LOG_ERROR(<< "Failed to parse filter config update: " << json);
+                return false;
+            }
+            LOG_TRACE(<< "Calling m_JobConfig.initRuleFilters");
+
+            m_JobConfig.initRuleFilters();
+
+            LOG_TRACE(<< "Done calling m_JobConfig.initRuleFilters");
+
+        } else if (kv.key() == CAnomalyJobConfig::EVENTS) {
+            LOG_TRACE(<< "Updating events config");
+
+            if (m_JobConfig.parseEventConfig(json) == false) {
+                LOG_ERROR(<< "Failed to parse events config update: " << json);
+                return false;
+            }
+            m_JobConfig.initScheduledEvents();
+        } else if (kv.key() == CAnomalyJobConfig::CAnalysisConfig::CDetectorConfig::DETECTOR_RULES) {
+            LOG_TRACE(<< "Updating detector rules config");
+            return m_JobConfig.analysisConfig().parseRulesUpdate(kv.value());
+        } else {
+            LOG_ERROR(<< "Unexpected JSON update message: " << json);
             return false;
         }
-        m_JobConfig.initScheduledEvents();
-    } else if (obj.contains(CAnomalyJobConfig::CAnalysisConfig::CDetectorConfig::DETECTOR_RULES)) {
-        return m_JobConfig.analysisConfig().parseRulesUpdate(
-            obj[CAnomalyJobConfig::CAnalysisConfig::CDetectorConfig::DETECTOR_RULES]);
-    } else {
-        LOG_ERROR(<< "Unexpected JSON update message: " << json);
-        return false;
     }
+
     return true;
 }
 }

diff --git a/lib/api/unittest/CAnomalyJobConfigTest.cc b/lib/api/unittest/CAnomalyJobConfigTest.cc
@@ -57,51 +57,6 @@ BOOST_AUTO_TEST_CASE(testIntervalStagger) {
     BOOST_REQUIRE_EQUAL(job3Config.intervalStagger(), job1Config.intervalStagger());
 }
 
-BOOST_AUTO_TEST_CASE(testReparseDetectorsFromStoredConfig) {
-    const std::string validAnomalyJobConfigWithCustomRuleFilter{
-        "{\"job_id\":\"mean_bytes_by_clientip\",\"job_type\":\"anomaly_detector\",\"job_version\":\"8.0.0\",\"create_time\":1604671135245,\"description\":\"mean bytes by clientip\","
-        "\"analysis_config\":{\"bucket_span\":\"3h\",\"detectors\":[{\"detector_description\":\"mean(bytes) by clientip\",\"function\":\"mean\",\"field_name\":\"bytes\",\"by_field_name\":\"clientip\","
-        "\"custom_rules\":[{\"actions\":[\"skip_result\"],\"scope\":{\"clientip\":{\"filter_id\":\"safe_ips\",\"filter_type\":\"include\"}},\"conditions\":[{\"applies_to\":\"actual\",\"operator\":\"lt\",\"value\":10.0}]}],"
-        "\"detector_index\":0}],\"influencers\":[\"clientip\"]},\"analysis_limits\":{\"model_memory_limit\":\"42mb\",\"categorization_examples_limit\":4},"
-        "\"data_description\":{\"time_field\":\"timestamp\",\"time_format\":\"epoch_ms\"},\"model_plot_config\":{\"enabled\":false,\"annotations_enabled\":false},"
-        "\"model_snapshot_retention_days\":10,\"daily_model_snapshot_retention_after_days\":1,\"results_index_name\":\"shared\",\"allow_lazy_open\":false}"};
-
-    // Expect parsing to succeed if the filter referenced by the custom rule can be found in the filter map.
-    const std::string filterConfigJson{"{\"filters\":[{\"filter_id\":\"safe_ips\",\"items\":[]}]}"};
-    ml::api::CAnomalyJobConfig jobConfig;
-    BOOST_TEST_REQUIRE(jobConfig.parseFilterConfig(filterConfigJson));
-
-    const std::string validScheduledEventsConfigJson{"{\"events\":["
-                                                     "]}"};
-
-    BOOST_TEST_REQUIRE(jobConfig.parseEventConfig(validScheduledEventsConfigJson));
-
-    jobConfig.analysisConfig().init(jobConfig.ruleFilters(), jobConfig.scheduledEvents());
-
-    BOOST_REQUIRE_MESSAGE(jobConfig.parse(validAnomalyJobConfigWithCustomRuleFilter),
-                          "Cannot parse JSON job config!");
-    BOOST_TEST_REQUIRE(jobConfig.isInitialized());
-
-    // Expect parsing to fail if the analysis config JSON string is invalid
-    const std::string inValidAnalysisConfigString{"{\"bucket_span\":\"1h\""};
-    BOOST_TEST_REQUIRE(!jobConfig.analysisConfig().reparseDetectorsFromStoredConfig(
-        inValidAnalysisConfigString));
-
-    // Expect parsing to fail if the filter referenced by the custom rule cannot be found
-    const std::string validAnalysisConfigStringWithUnknownFilter{
-        "{\"bucket_span\":\"1h\",\"detectors\":[{\"detector_description\":\"count over ip\",\"function\":\"count\",\"over_field_name\":\"ip\",\"custom_rules\":[{\"actions\":[\"skip_result\"],\"scope\":{\"ip\":{\"filter_id\":\"unknown_filter\",\"filter_type\":\"include\"}}}],\"detector_index\":0}],\"influencers\":[],\"model_prune_window\":\"30d\"}"};
-    BOOST_REQUIRE_EXCEPTION(
-        jobConfig.analysisConfig().reparseDetectorsFromStoredConfig(validAnalysisConfigStringWithUnknownFilter),
-        ml::api::CAnomalyJobConfigReader::CParseError,
-        [](ml::api::CAnomalyJobConfigReader::CParseError const&) { return true; });
-
-    // Expect parsing to succeed if the filter referenced by the custom rule is registered.
-    const std::string validAnalysisConfigString{
-        "{\"bucket_span\":\"1h\",\"detectors\":[{\"detector_description\":\"count over ip\",\"function\":\"count\",\"over_field_name\":\"ip\",\"custom_rules\":[{\"actions\":[\"skip_result\"],\"scope\":{\"ip\":{\"filter_id\":\"safe_ips\",\"filter_type\":\"include\"}}}],\"detector_index\":0}],\"influencers\":[],\"model_prune_window\":\"30d\"}"};
-    BOOST_TEST_REQUIRE(jobConfig.analysisConfig().reparseDetectorsFromStoredConfig(
-        validAnalysisConfigString));
-}
-
 BOOST_AUTO_TEST_CASE(testParse) {
 
     using TAnalysisConfig = ml::api::CAnomalyJobConfig::CAnalysisConfig;