[Workload Management] Add rule schema for workload management #17238

ruai0511 · 2025-02-03T22:47:57Z

Description

This PR introduces the schema for Rule object used in the workload management feature. A single rule will only have one feature but could have multiple attributes.
For the example below, the feature is query_group, and corresponds to the tag dev_query_group_id

{
    "_id": "fwhbuib397u4o03=="
    "attribute_1": ["logs123"],
    "attribute_2": ["xxxx", "yyyy],
    "query_group": "dev_query_group_id",
    "updated_at": "01-10-2025T21:23:456Z"
}

Related issues:
RFC: #16797
#16813

Check List

Functionality includes testing.
API changes companion pull request created, if applicable.
Public documentation issue/PR created, if applicable.

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

github-actions · 2025-02-04T00:02:16Z

✅ Gradle check result for f6a4a28: SUCCESS

codecov · 2025-02-04T00:03:13Z

Codecov Report

Attention: Patch coverage is 82.55034% with 26 lines in your changes missing coverage. Please review.

Project coverage is 72.42%. Comparing base (5b2692f) to head (e152d95).

Files with missing lines	Patch %	Lines
...src/main/java/org/opensearch/autotagging/Rule.java	82.55%	10 Missing and 16 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##               main   #17238    +/-   ##
==========================================
  Coverage     72.41%   72.42%            
- Complexity    65531    65577    +46     
==========================================
  Files          5291     5292     +1     
  Lines        304335   304484   +149     
  Branches      44181    44211    +30     
==========================================
+ Hits         220395   220513   +118     
- Misses        65838    65903    +65     
+ Partials      18102    18068    -34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md

server/src/main/java/org/opensearch/wlm/Rule.java

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

github-actions · 2025-02-07T06:39:18Z

❕ Gradle check result for bb8d06c: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

jainankitk · 2025-02-11T07:25:04Z

server/src/main/java/org/opensearch/wlm/Rule.java

+ *     "_id": "fwehf8302582mglfio349==",
+ *     "index_pattern": ["logs123", "user*"],


Should we also have customer friendly - name/description fields for a rule?

Yeah i do think we can add a name field for better user readability. Especially when they get all the rules in the system, and only looking at the id and attributes could be overwhelming.

jainankitk · 2025-02-11T07:30:39Z

server/src/main/java/org/opensearch/wlm/Rule.java

+        QUERY_GROUP("query_group", Set.of(RuleAttribute.INDEX_PATTERN));
+
+        private final String name;
+        private final Set<RuleAttribute> allowedAttributes;


This design of using same rules for multiple features becomes really tricky if they have different allowed attribute values. Having more attributes present than the ones that apply to a rule can be very confusing and if we are not allowing that, the possible attributes is the intersection of allowed attributes for all the features.

jainankitk · 2025-02-11T07:31:56Z

server/src/main/java/org/opensearch/wlm/Rule.java

+ *     "query_group": "dev_query_group_id",
+ *     "updated_at": "01-10-2025T21:23:21.456Z"
+ * }


@ruai0511 / @kaushalmahi12 - Can we add some more examples of how these rules will look if we have multiple features and multiple attributes tomorrow? Especially when the allowed attributes for those features are not common

A single Rule entity will always map to a single feature. Hence each feature will define their set of allowed attributes.

For example since the slow logs are defined at index level. It doesn't make sense for s.ow log feature to consider index_pattern as attribute. It can define next layer of attributes to provide more specific details about the slow logs,

Yeah a single rule will only have one feature but could have multiple attributes.
For example,

{ "_id": "fwhbuib397u4o03==" "attribute_1": ["logs123", "user*"], "attribute_2": ["logs123", "user*"], "query_group": "dev_query_group_id", "updated_at": "01-10-2025T21:23:456Z" }

Here the feature is query_group, and corresponds to the tag dev_query_group_id

kaushalmahi12 · 2025-02-12T01:32:30Z

server/src/main/java/org/opensearch/wlm/Rule.java

We should move this file under a new package e,g; auto_tagging since the construct is meant to be used in generic context but the structure hints a specific use.

kaushalmahi12 · 2025-02-12T19:51:27Z

server/src/main/java/org/opensearch/wlm/Rule.java

+     * This RuleAttribute enum contains the attribute names for a rule.
+     * @opensearch.experimental
+     */
+    public enum RuleAttribute {


I think we should rename this enum to Attribute since it is already under Rule class and should be referred to as Rule.Attribute.

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

github-actions · 2025-02-13T00:03:18Z

✅ Gradle check result for e152d95: SUCCESS

kaushalmahi12 · 2025-02-13T19:08:44Z

LGTM! apart from the name field I think we should rename it to description

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

github-actions · 2025-02-13T23:37:29Z

❌ Gradle check result for aa1f8b7: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

jainankitk

Overall, I feel we should move the enum Feature into enum FeatureType, and introduce Feature class that includes FeatureType, label (essentially nothing but value for feature type), and attributeMap (as allowed attributes are governed by FeatureType). Rule itself being generic in nature should be agnostic of coupling itself with those validations, etc.

jainankitk · 2025-02-14T01:11:06Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+    private final Map<Attribute, Set<String>> attributeMap;
+    private final Feature feature;
+    private final String description;
+    private final String label;


label is corresponding to the feature right? Does it make sense to include as part of Feature?

jainankitk · 2025-02-14T01:14:31Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+        out.writeMap(attributeMap, Attribute::writeTo, StreamOutput::writeStringCollection);
+        out.writeString(label);
+        out.writeString(updatedAt);
+        out.writeString(feature.getName());


This should be delegated to Feature class

jainankitk · 2025-02-14T01:17:20Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+        for (Map.Entry<Attribute, Set<String>> entry : attributeMap.entrySet()) {
+            builder.array(entry.getKey().getName(), entry.getValue().toArray(new String[0]));
+        }
+        builder.field(feature.getName(), label);


should be delegated to the feature class

sgup432 · 2025-02-17T21:39:42Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+     * @opensearch.experimental
+     */
+    public enum Attribute {
+        INDEX_PATTERN("index_pattern");


What happens if an external plugin like queryInsights or other want to use this schema and eventually this Attribute class?
Will they add another enum for their usecase? That doesn't seem right. As core should be agnostic of plugin level logic.

Same for Feature class above.

sgup432 · 2025-02-17T22:19:33Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+        String label,
+        String updatedAt,
+        Feature feature,
+        ValidationException validationException


Passing validationException as a method param doesn't seem a very nice way to me.

Why not do below?

public static Optional<ValidationException> validateRuleInputs() {}

Here you just pass an optional exception in case u encounter any issues. The caller can additionally check if any exceptions are present or not, and accordingly handle it.

Same for below method.

In addition to this, instead of passing all these params, should we create an internal object which can be passed instead? And same can be used at Rule class level above?

I think creating a Validator class with all of the attributes which needs to be validated as member variables will be more suitable.

we can create something like following

static class RuleValidator { String description, Map<Attribute, Set<String>> attributeMap; String label; String updatedAt; Feature feature; .... Optional<ValidationException> validate() { validateAttributes(); validateFeature(); .... } ... }

I put it as a method param to reuse the same ValidationException object and throw all the errors together after all the checks. If we use public static Optional<ValidationException> validateRuleInputs() {} then the internal error-checking functions like validateAttributeMap can't reuse the ValidationException object

sgup432 · 2025-02-17T22:22:08Z

server/src/main/java/org/opensearch/autotagging/Rule.java

+     * This enum enumerates the features that can use the Rule Based Auto-tagging
+     * @opensearch.experimental
+     */
+    public enum Feature {


[NIT] Better to create a different file for this IMO. Same for below Attribute

kaushalmahi12 · 2025-02-17T22:54:43Z

Overall, I feel we should move the enum Feature into enum FeatureType, and introduce Feature class that includes FeatureType, label (essentially nothing but value for feature type), and attributeMap (as allowed attributes are governed by FeatureType). Rule itself being generic in nature should be agnostic of coupling itself with those validations, etc.

I might be missing some context here but this seems unnecessary at this point. The feature here is used only by the rule entity.
Why I believe creating a separate class doesn't make sense.

Class itself means a blueprint for something to create similar types of things. Here we will still duplicate the FeatureType a lot. attributes are a again a static list of constants for a feature
Feature here is a constant and it doesn't necessarily needs to carry the label for itself since label is rule's property.

I agree with @sgup432 comments, Feature and Feature supported attributes are specific to plugin.

ruai0511 force-pushed the rule-schema-PR branch from d2ec3f2 to bc12186 Compare February 3, 2025 22:58

add rule schema for workload management

f6a4a28

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

ruai0511 force-pushed the rule-schema-PR branch from bc12186 to f6a4a28 Compare February 3, 2025 23:01

kaushalmahi12 reviewed Feb 4, 2025

View reviewed changes

ruai0511 added the backport 2.x Backport to 2.x branch label Feb 7, 2025

modify rule structure based on comment

bb8d06c

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

ruai0511 force-pushed the rule-schema-PR branch from 094c95d to bb8d06c Compare February 7, 2025 05:41

opensearch-ci-bot mentioned this pull request Feb 7, 2025

[AUTOCUT] Gradle Check Flaky Test Report for MinimumClusterManagerNodesIT #14289

Open

kaushalmahi12 approved these changes Feb 10, 2025

View reviewed changes

jainankitk reviewed Feb 11, 2025

View reviewed changes

This was referenced Feb 12, 2025

[rule based autotagging] Add Create Rule API Logic #17335

Open

[rule based autotagging] Add Get Rule API Logic #17336

Open

kaushalmahi12 reviewed Feb 12, 2025

View reviewed changes

ruai0511 added 3 commits February 12, 2025 14:50

update based on comments

b32ce69

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

Merge branch 'main' into rule-schema-PR

f46caf8

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

add javadoc for autotagging directory

e152d95

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

kaushalmahi12 approved these changes Feb 13, 2025

View reviewed changes

change field name from 'name' to 'description'

aa1f8b7

Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>

opensearch-ci-bot mentioned this pull request Feb 14, 2025

[AUTOCUT] Gradle Check Flaky Test Report for ConcurrentSeqNoVersioningIT #17271

Open

kaushalmahi12 mentioned this pull request Feb 14, 2025

[Rule based auto tagging] Add in-memory rule processing service #17365

Open

3 tasks

jainankitk reviewed Feb 17, 2025

View reviewed changes

sgup432 reviewed Feb 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workload Management] Add rule schema for workload management #17238

[Workload Management] Add rule schema for workload management #17238

ruai0511 commented Feb 3, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025

codecov bot commented Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025

jainankitk Feb 11, 2025

ruai0511 Feb 12, 2025

jainankitk Feb 11, 2025

jainankitk Feb 11, 2025

kaushalmahi12 Feb 12, 2025

ruai0511 Feb 12, 2025

kaushalmahi12 Feb 12, 2025

kaushalmahi12 Feb 12, 2025

github-actions bot commented Feb 13, 2025

kaushalmahi12 commented Feb 13, 2025

github-actions bot commented Feb 13, 2025

jainankitk left a comment

jainankitk Feb 14, 2025

jainankitk Feb 14, 2025

jainankitk Feb 14, 2025

sgup432 Feb 17, 2025 •

edited

Loading

sgup432 Feb 17, 2025

sgup432 Feb 17, 2025

kaushalmahi12 Feb 17, 2025

ruai0511 Feb 18, 2025 •

edited

Loading

sgup432 Feb 17, 2025

kaushalmahi12 commented Feb 17, 2025

		* "_id": "fwehf8302582mglfio349==",
		* "index_pattern": ["logs123", "user*"],

[Workload Management] Add rule schema for workload management #17238

Are you sure you want to change the base?

[Workload Management] Add rule schema for workload management #17238

Conversation

ruai0511 commented Feb 3, 2025 • edited Loading

Description

Check List

github-actions bot commented Feb 4, 2025

codecov bot commented Feb 4, 2025 • edited Loading

Codecov Report

github-actions bot commented Feb 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 13, 2025

kaushalmahi12 commented Feb 13, 2025

github-actions bot commented Feb 13, 2025

jainankitk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgup432 Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruai0511 Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaushalmahi12 commented Feb 17, 2025

ruai0511 commented Feb 3, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading

sgup432 Feb 17, 2025 •

edited

Loading

ruai0511 Feb 18, 2025 •

edited

Loading