Support wildcard cluster names in anomaly detection data source indices #1612

bakert-96 · 2025-11-12T17:33:03Z

Description

Updates to the validation of time field and categorical field to query all remote clusters when a wildcard cluster prefix is specified.

Related Issues

This is related to the proposal opened here #1606

Check List

Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@Inject

… detection jobs - Updated time field and category field validation methods to query all remote indices when wildcard specified for cluster name - Added @Inject annotation for TransportService in multiple action handlers Signed-off-by: Tim Baker <baker.timothy96@gmail.com>

codecov · 2025-11-12T19:00:27Z

Codecov Report

❌ Patch coverage is 27.63158% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.07%. Comparing base (a6c1916) to head (34445ef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../rest/handler/AbstractTimeSeriesActionHandler.java	26.66%	49 Missing and 6 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1612      +/-   ##
============================================
- Coverage     81.31%   81.07%   -0.25%     
+ Complexity     6144     6139       -5     
============================================
  Files           542      542              
  Lines         24998    25065      +67     
  Branches       2543     2557      +14     
============================================
- Hits          20328    20321       -7     
- Misses         3394     3461      +67     
- Partials       1276     1283       +7

Flag	Coverage Δ
plugin	`81.07% <27.63%> (-0.25%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../handler/ValidateAnomalyDetectorActionHandler.java	`100.00% <ø> (ø)`
...nsport/ValidateAnomalyDetectorTransportAction.java	`100.00% <ø> (ø)`
.../rest/handler/ValidateForecasterActionHandler.java	`100.00% <ø> (ø)`
...t/transport/ValidateForecasterTransportAction.java	`100.00% <ø> (ø)`
...ies/transport/BaseDeleteConfigTransportAction.java	`82.88% <ø> (ø)`
...es/transport/BaseEntityProfileTransportAction.java	`77.58% <ø> (ø)`
...series/transport/BaseGetConfigTransportAction.java	`86.17% <ø> (ø)`
...h/timeseries/transport/BaseJobTransportAction.java	`88.09% <ø> (ø)`
...s/transport/BaseValidateConfigTransportAction.java	`73.11% <100.00%> (+0.29%)`	⬆️
...ensearch/timeseries/transport/ResultProcessor.java	`83.95% <ø> (+0.80%)`	⬆️
... and 1 more

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bakert-96 · 2025-11-13T17:15:23Z

I will get a changelog item and tests added 👌

amitgalitz · 2025-11-24T16:44:00Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java


-    protected void validateTimeField(boolean indexingDryRun, ActionListener<T> listener) {
+    protected void validateTimeFieldInAllClusters(boolean indexingDryRun, ActionListener<T> listener) {
+        List<String> wildcardClusterIndices = config.getIndices().stream().filter(idx -> idx.startsWith("*:")).toList();


There's duplicate code in validateTimeFieldInAllClusters and validateCategoricalFieldsInAllClusters, I suggest we extract to a method to handle some of this validation, if you want to have different validation issue types you can pass that in as variables.

amitgalitz · 2025-11-24T16:45:52Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+        for (String remote : remoteClusters) {
+            List<String> remoteIndices = wildcardClusterIndices
+                .stream()
+                .map(idx -> idx.substring(2)) // remove "*:" prefix


nit:
here if a user passes this to indices:
"indices": [ "*:" ],
then we have empty indices being passed, we can probably filter or validate earlier here so error message makes more sense then: Exceptions: [Fail to get the index mapping of []

amitgalitz · 2025-11-24T16:46:31Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

 import org.opensearch.timeseries.util.RestHandlerUtils;
 import org.opensearch.timeseries.util.SecurityClientUtil;
-import org.opensearch.transport.TransportService;
+import org.opensearch.transport.*;


expand wildcard imports here, we try to have all imports be specific (for example:

import org.opensearch.timeseries.util.ParseUtils; import org.opensearch.timeseries.util.RestHandlerUtils; import org.opensearch.timeseries.util.SecurityClientUtil;

amitgalitz · 2025-11-24T16:52:50Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+                listener
+                    .onFailure(
+                        new ValidationException(
+                            "No indices specified for time field validation.",


Currently i know that we are inside time field validation but for this error specifically its more so that no indices are specific for the given configuration, this can be put as a ValidationIssueType.INDICES as the error here isn't necessarily for time field. Same comment can be made on similar code in checking of categorical field. if we move the code to a different method we can relook at the error messages we have.

amitgalitz · 2025-11-24T16:57:05Z

Thank you for making this change!
Added some comments on first review, try to add some UT/IT for the change. Additionally one more improvement that can be made here or in future PRs is that we can have the cluster wildcard support prefix, so we can look for something like cluster*, right now it would look for cluster with the name cluster* but we can expand that to all clusters under that pattern like cluster1, cluster200 and etc.

kaituo · 2025-11-29T01:04:08Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+        if (!nonWildcardClusterIndices.isEmpty()) {
+            HashMap<String, List<String>> nonWildcardIndicesMap = CrossClusterConfigUtils
+                .separateClusterIndexes(nonWildcardClusterIndices, clusterService);
+            clusterIndicesMap.putAll(nonWildcardIndicesMap);


When merging wildcard and non-wildcard cluster indices, the putAll() call overwrites entries from the wildcard loop if both target the same cluster. For example, if a user specifies both *:index1 and remote1:index2, only index2 will be validated for remote1. The indices should be combined, not replaced. This affects both time field and categorical field validation.

kaituo · 2025-11-29T01:08:32Z

src/main/java/org/opensearch/ad/rest/handler/ValidateAnomalyDetectorActionHandler.java

 import org.opensearch.timeseries.model.Config;
 import org.opensearch.timeseries.transport.ValidateConfigResponse;
 import org.opensearch.timeseries.util.SecurityClientUtil;
+import org.opensearch.transport.*;


don't use wildcard import

kaituo · 2025-11-29T01:08:53Z

src/main/java/org/opensearch/forecast/rest/handler/ValidateForecasterActionHandler.java

 import org.opensearch.timeseries.model.Config;
 import org.opensearch.timeseries.transport.ValidateConfigResponse;
 import org.opensearch.timeseries.util.SecurityClientUtil;
+import org.opensearch.transport.*;


don't use wildcard import

kaituo · 2025-11-29T01:29:27Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+        // For each remote cluster, add all indices matching the wildcard pattern
+        for (String remote : remoteClusters) {
+            List<String> remoteIndices = wildcardClusterIndices
+                .stream()
+                .map(idx -> idx.substring(2)) // remove "*:" prefix
+                .toList();
+
+            if (!remoteIndices.isEmpty()) {
+                clusterIndicesMap.put(remote, remoteIndices);
+            }
+        }


Because the wildcardClusterIndices list doesn’t depend on the loop variable (remote), the same list of stripped index names is recomputed on every iteration. How about:

List remoteIndices = wildcardClusterIndices.stream()
.map(idx -> idx.substring(2))
.toList();
for (String remote : remoteClusters) {
clusterIndicesMap.put(remote, remoteIndices);
}

kaituo · 2025-11-29T01:32:08Z

src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+        RemoteClusterService remoteClusterService = transportService.getRemoteClusterService();
+        Set<String> remoteClusters = remoteClusterService.getRegisteredRemoteClusterNames();


Can we wrap the call in a try/catch where we throw listener.onFailure in case of exception?

kaituo · 2025-11-29T05:36:58Z

src/main/java/org/opensearch/timeseries/transport/BaseGetConfigTransportAction.java

 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.CheckedConsumer;
+import org.opensearch.common.inject.*;


remove since you don't need Inject

kaituo · 2025-11-29T05:37:08Z

src/main/java/org/opensearch/timeseries/transport/BaseJobTransportAction.java

    private final Clock clock;
    private final Class<ConfigType> configTypeClass;

+    @Inject


kaituo · 2025-11-29T05:37:11Z

src/main/java/org/opensearch/timeseries/transport/BaseJobTransportAction.java

 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.cluster.service.ClusterService;
+import org.opensearch.common.inject.*;


kaituo · 2025-11-29T05:37:50Z

src/main/java/org/opensearch/timeseries/transport/ResultProcessor.java


    protected boolean runOnce;

+    @Inject


kaituo · 2025-11-29T05:37:55Z

src/main/java/org/opensearch/timeseries/transport/ResultProcessor.java

 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.service.ClusterService;
+import org.opensearch.common.inject.*;


bakert-96 requested review from VijayanB, amitgalitz, dbwiddis, jackiehanyang, jmazanec15, jngz-es, joshpalis, kaituo, ohltyler, owaiskazi19, saratvemulapalli, sean-zheng-amazon, vamshin and ylwu-amzn as code owners November 12, 2025 17:33

opensearch-trigger-bot bot added the backport 2.x label Nov 12, 2025

bakert-96 force-pushed the feature/support-cluster-wildcard branch from 2788eb3 to 4aa1195 Compare November 12, 2025 17:40

bakert-96 force-pushed the feature/support-cluster-wildcard branch from 4aa1195 to 34445ef Compare November 12, 2025 17:50

amitgalitz reviewed Nov 24, 2025

View reviewed changes

kaituo reviewed Nov 29, 2025

View reviewed changes

		RemoteClusterService remoteClusterService = transportService.getRemoteClusterService();
		Set<String> remoteClusters = remoteClusterService.getRegisteredRemoteClusterNames();

Support wildcard cluster names in anomaly detection data source indices #1612

Are you sure you want to change the base?

Support wildcard cluster names in anomaly detection data source indices #1612

Uh oh!

Conversation

bakert-96 commented Nov 12, 2025

Description

Related Issues

Check List

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bakert-96 commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amitgalitz commented Nov 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 12, 2025 •

edited

Loading