Skip to content

Conversation

@eric-forte-elastic
Copy link
Contributor

@eric-forte-elastic eric-forte-elastic commented Dec 9, 2025

Pull Request

Issue link(s):

Resolves #5440

Related to #5439

Summary - What I changed

I added a schema test to enforce adding metadata fields to the keep portion of non-aggregate ES|QL queries.

Note unit tests will continue to fail until the rules are updated to match the new keep requirement.

How To Test

Use view rule on an ES|QL rule that does not have the metadata fields _id, _version, _index in its keep line.

E.g.

keep_testing

Checklist

  • Added a label for the type of pr: bug, enhancement, schema, maintenance, Rule: New, Rule: Deprecation, Rule: Tuning, Hunt: New, or Hunt: Tuning so guidelines can be generated
  • Added the meta:rapid-merge label if planning to merge within 24 hours
  • Secret and sensitive material has been managed correctly
  • Automated testing was updated or added to match the most common scenarios
  • Documentation and comments were added for features that require explanation

Contributor checklist

@eric-forte-elastic eric-forte-elastic self-assigned this Dec 9, 2025
@eric-forte-elastic eric-forte-elastic added test-suite unit and other testing components python Internal python for the repository labels Dec 9, 2025
@eric-forte-elastic eric-forte-elastic linked an issue Dec 9, 2025 that may be closed by this pull request
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

Enhancement - Guidelines

These guidelines serve as a reminder set of considerations when addressing adding a new schema feature to the code.

Documentation and Context

  • Describe the feature enhancement in detail (alternative solutions, description of the solution, etc.) if not already documented in an issue.
  • Include additional context or screenshots.
  • Ensure the enhancement includes necessary updates to the documentation and versioning.

Code Standards and Practices

  • Code follows established design patterns within the repo and avoids duplication.
  • Ensure that the code is modular and reusable where applicable.

Testing

  • New unit tests have been added to cover the enhancement.
  • Existing unit tests have been updated to reflect the changes.
  • Provide evidence of testing and validating the enhancement (e.g., test logs, screenshots).
  • Validate that any rules affected by the enhancement are correctly updated.
  • Ensure that performance is not negatively impacted by the changes.
  • Verify that any release artifacts are properly generated and tested.
  • Conducted system testing, including fleet, import, and create APIs (e.g., run make test-cli, make test-remote-cli, make test-hunting-cli)

Additional Schema Related Checks

  • Verify that the enhancement works across all relevant environments (e.g., different OS versions).
  • Link to the relevant Kibana PR or issue provided
  • Test export/import flow:
    • Exported detection rule(s) from Kibana to showcase the feature(s)
    • Converted the exported ndjson file(s) to toml in the detection-rules repo
    • Re-exported the toml rule(s) to ndjson and re-imported into Kibana
  • Updated necessary unit tests to accommodate the feature
  • Incorporated a comprehensive test rule in unit tests for full schema coverage
  • Applied min_compat restrictions to limit the feature to a specified minimum stack version
  • Executed all unit tests locally with a test toml rule to confirm passing
  • Included Kibana PR implementer as an optional reviewer for insights on the feature
  • Implemented requisite downgrade functionality
  • Cross-referenced the feature with product documentation for consistency
  • Confirm that the proper version label is applied to the PR patch, minor, major.

@eric-forte-elastic
Copy link
Contributor Author

eric-forte-elastic commented Dec 17, 2025

If #5433 merges and we are fine with keep * then this PR needs to be updated to allow for keep * as well.

Updated: Complete

Copy link
Contributor

@Mikaayenson Mikaayenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe clarify in docs/tests that this is specifically about ensuring those metadata fields are in the output row (i.e., in keep), not just in METADATA

)

# Ensure that keep clause includes metadata fields on non-aggregate queries
aggregate_pattern = re.compile(r"\bstats\b.*\bby\b", re.IGNORECASE | re.DOTALL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this catch things like | stats count(*) (no by)?

# Match | followed by optional whitespace/newlines and then 'keep'
keep_pattern = re.compile(r"\|\s*keep\b", re.IGNORECASE | re.DOTALL)
if not keep_pattern.search(query_lower):
keep_pattern = re.compile(r"\|\s*keep\b\s+([^\|]+)", re.IGNORECASE | re.DOTALL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work with things like keep Esql.* / keep aws.*?

raise EsqlSemanticError(
f"Rule: {data['name']} contains a keep clause without"
f" metadata fields '_id', '_version', and '_index' ->"
f" Add '_id, _version', '_index' to the keep command."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
f" Add '_id, _version', '_index' to the keep command."
f" Add '_id', _version', '_index' to the keep command."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport: auto patch python Internal python for the repository schema test-suite unit and other testing components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Test for Keep Metadata

3 participants