Initial implementation of report-builder interface

[Swiddis]

Description

Well, this is turning out quite long.

The fundamental idea of this PR is to add a lot more context to error messages. For example, take this bad query:

source = big5
| eval range_bucket = case(
   `metrics.size` < -10, 'range_1',
   `metrics.size` >= -10 and `metrics.size` < 10, 'range_2',
   `metrics.size` >= 10 and `metrics.size` < 100, 'range_3',
   `metrics.size` >= 100 and `metrics.size` < 1000, 'range_4',
   `metrics.size` >= 1000 and `metrics.size` < 2000, 'range_5',
   `metrics.size` >= 2000, 'range_6')
| stats count() by range_bucket, span(`@timestamp`, 1h) as auto_span
| sort + range_bucket, + auto_spa

Now there's more context, provided at various layers:

{
  "error": {
    "context": {
      "stage": "analyzing",
      "stage_description": "Parsing and validating the query"
    },
    "details": "Field [nonex] not found.",
    "location": [
      "while preparing and validating the query plan"
    ],
    "code": "FIELD_NOT_FOUND",
    "type": "IllegalArgumentException"
  },
  "status": 400
}

Or, the humble 'index not found':

{
  "error": {
    "context": {
      "stage": "analyzing",
      "index_name": "big5_nonex",
      "stage_description": "Parsing and validating the query"
    },
    "details": "no such index [big5_nonex]",
    "location": [
      "while preparing and validating the query plan",
      "while fetching index mappings"
    ],
    "code": "INDEX_NOT_FOUND",
    "type": "IndexNotFoundException"
  },
  "status": 404
}

The doctest deltas have a few concrete cases where I went through and did light touch-ups. Improving any particular error takes O(2 minutes) to wrap in a report with context, and it'll integrate with any higher-up wrapping to give the completed report at the end.

A lot of this PR in terms of line count is just patching specific places where tests are asserting errors behave in specific ways. The core of the PR is:

• ErrorReport and StageErrorHandler, which adds utilities for incrementally building detailed errors
• QueryService where we hook up the outer shell of the stage handlers, and
• RestPPLQueryAction where we handle and present the errors.

Simple tests are in error_reports.yml, as well as doctest and various other scattered tests. (There's also lots of tests that have no diff, but did catch bugs in earlier versions of this PR.)

The enhancements aren't super complete in this PR as the emphasis is scaffolding (and the integration work + fixing tests is already quite significant), for the most part the changes are backwards compatible in normal cases. The type, details, and status fields are in the same spots and by default pull from the underlying exception. I'll be adding more thorough probes for specific error cases in followups.

Related Issues

Meta: #5261
Closes #4919

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

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]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 6f8d67a.

Path	Line	Severity	Description
integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceEnabledIT.java	154	low	Debug logging statements (System.err.println) printing full HTTP response bodies were added to a test helper method. While in test code and not exfiltrating data externally, these could inadvertently log sensitive response content (tokens, credentials) to stderr in CI/CD pipelines. Likely a development artifact left in unintentionally.
core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java	434	low	Internal query plan content is extracted from exception messages and placed into structured error responses returned to clients (via the 'plan' context key). This could expose internal execution plan details to end users. The behavior is intentional for debugging but represents information disclosure of internal system structure.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 0 | Medium: 0 | Low: 2

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

Failed to generate code suggestions for PR

[Swiddis]

The error enrichment logic extracts the raw SQL query plan string from internal exception messages and...

In context, this is pulling details out of a potentially very fuzzy Calcite error. The error logic without any of this ultimately returns the plan with a message resembling "Failed to prepare plan: [blob]." The plan is provided as context as the innermost error might be weird and only apply to one node, e.g. CalciteEnumerableNestedAggregate having a problem means nothing without access to the plan containing said aggregate. In OSD this plan probably won't be presented to users directly.

Any details about our internal implementation can be easily scrutinized by navigating to https://github.com/opensearch-project/sql in a recent version of Netscape Navigator.

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[github-actions]

Failed to generate code suggestions for PR

[Swiddis]

self-suggestion: These conditions can be simplified by using error codes at a lower level

The checks here just become report.getCode() == ErrorCode.ILLEGAL_AGGREGATE_PUSHDOWN and report.getCode() == ErrorCode.INVALID_WINDOW_BIN or something, and we can remove the helpers. Can add this if it comes up or is in the way of other changes.

I mostly bring it up to highlight why I'm introducing codes in the first place: They can let us specify much more specific scenarios than generic exceptions do. I just didn't fully integrate it here in the interest of time & keeping the PR as small as I could get it.

[anasalkouz]

Not sure if understand the improvements: what is the error experience before and after this PR?

[Swiddis]

You can see some concrete examples by looking at the diffs for doctests that check for errors https://github.com/opensearch-project/sql/pull/5266/changes#diff-601c336135ceed5ca58ff75bf3783afd2722610e3b5583160476fc9378b34cd6

[dai-chen]

In meta issue, we mention the "The core problem is that misbehaving queries are typically hard to debug". So the improvements like location are mostly for developer? Probably need some clarification for the goal.