Introduced monitoring mode for query string query max length.

[lukasz-soszynski-eliatra]

Description

The PR introduces "query string query" monitoring mode. The mode is by default disabled. If the system administrator enables the mode (using the configuration property search.query.max_query_string_length_monitor_only), then the OS executes queries that exceed the max length limit and logs an additional warning message.

The PR is related to the discussion

Related Issues

#19491 (comment)

Check List

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

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.

[github-actions]

❌ Gradle check result for 377eca4: FAILURE

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?

[kkhatua]

Tagging @cwperks , @jainankitk for a review.

[cwperks]

@kkhatua I'm not really in favor of one-off settings like this. The max-length setting introduced in #19491 is already changeable dynamically.

What do you think about making any responses that exceed the configured max length to be actionable? i.e. the response should not just be that the request fails, but state the reason for failure and how to adjust the max length setting.

This is from one of the tests:

assertThat(e.getDetailedMessage(), containsString("Query string length exceeds max allowed length 10"));

Maybe we can specify the setting name in that message?

[kumargu]

@kkhatua I'm not really in favor of one-off settings like this. The max-length setting introduced in #19491 is already changeable dynamically.

What do you think about making any responses that exceed the configured max length to be actionable? i.e. the response should not just be that the request fails, but state the reason for failure and how to adjust the max length setting.

This is from one of the tests:

assertThat(e.getDetailedMessage(), containsString("Query string length exceeds max allowed length 10"));

Maybe we can specify the setting name in that message?

yeah having a failure reason would be useful.

[kkhatua]

Agree on providing a failure reason with the settings to modify, similar to other settings.
However, the monitoring mode ensures this is an optional breaking change.

Tagging @jainankitk to review. Would be good to have this for 3.3 release, but it's not a must.

[cwperks]

Fixed the conflicts due to the CHANGELOG being cleared. The content of this change looks good to me. I was comparing this more to XContentConstraint where there is a max enforced in the Jackson library itself. Similar to Lucene BoolQuery with 1024 max clause count:

• XContentConstraint on main: https://github.com/opensearch-project/OpenSearch/blob/main/libs/x-content/src/main/java/org/opensearch/common/xcontent/XContentContraints.java
• XContentConstraint on 2.19: https://github.com/opensearch-project/OpenSearch/blob/2.19/libs/x-content/src/main/java/org/opensearch/common/xcontent/XContentContraints.java

[github-actions]

✅ Gradle check result for 25b7541: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.09%. Comparing base (006a53f) to head (25b7541).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...pensearch/index/search/QueryStringQueryParser.java	33.33%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #19539      +/-   ##
============================================
+ Coverage     73.05%   73.09%   +0.04%     
+ Complexity    70627    70608      -19     
============================================
  Files          5723     5723              
  Lines        323489   323502      +13     
  Branches      46851    46852       +1     
============================================
+ Hits         236311   236468     +157     
+ Misses        68174    67959     -215     
- Partials      19004    19075      +71     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.