Skip to content

Enable TS command in non-snapshot builds#134768

Merged
dnhatn merged 20 commits intoelastic:mainfrom
dnhatn:enable-time-series
Sep 18, 2025
Merged

Enable TS command in non-snapshot builds#134768
dnhatn merged 20 commits intoelastic:mainfrom
dnhatn:enable-time-series

Conversation

@dnhatn
Copy link
Copy Markdown
Member

@dnhatn dnhatn commented Sep 16, 2025

This change enables the TS command in non-snapshot builds. We are also starting to run BWC tests for the TS command, including mixed-cluster and cross-cluster tests. Note that some validations for the TS command still need improvement, but it is fine to address these after this PR.

@dnhatn dnhatn added test-full-bwc Trigger full BWC version matrix tests test-release Trigger CI checks against release build labels Sep 16, 2025
@dnhatn dnhatn force-pushed the enable-time-series branch 2 times, most recently from d335d29 to 8494f86 Compare September 16, 2025 03:14
@github-actions
Copy link
Copy Markdown
Contributor

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

@dnhatn dnhatn requested a review from kkrik-es September 16, 2025 05:01
/**
* The metrics command
*/
@Deprecated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG, do the same? @limotova fyi.

Copy link
Copy Markdown
Member

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

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

:)

@dnhatn dnhatn requested a review from limotova September 16, 2025 06:06
Copy link
Copy Markdown
Contributor

@limotova limotova left a comment

Choose a reason for hiding this comment

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

LGTM! Just some questions


absent_over_time_of_aggregate_metric_double
required_capability: metrics_command
required_capability: absent_over_time
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we removing the absent_over_time capability here/replacing it with agg_metric but still keeping it everywhere else? (Same with the test absent_over_time_older_than_10d and similar present_over_time tests)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because aggregate_metric_double still requires snapshot builds, while TS is available in both snapshot and release builds, we need to skip these tests in release builds (since aggregate_metric_double is not yet supported).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think i didn't phrase my question well, i was more wondering why the absent_over_time capability was being only removed here and not anywhere else?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We should remove it everywhere, but this PR is getting too large. I will address this in a follow-up.

irate_of_aggregate_metric
required_capability: metrics_command
required_capability: ts_command_v0
required_capability: aggregate_metric_double
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this test uses aggregate_metric_double (I believe network.total_bytes_in is a counter_long)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed in 65ea9c5

rate_of_aggregate_metric
required_capability: metrics_command
required_capability: ts_command_v0
required_capability: aggregate_metric_double
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same for here, I don't think this uses aggregate_metric_double

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks, fixed in 65ea9c5

max_over_time
required_capability: metrics_command
required_capability: ts_command_v0
required_capability: max_over_time
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we keeping the max/min/etc_over_time capabilities here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, we should remove all capability except for ts_command_v0. I will do this in a follow-up.

dnhatn added a commit that referenced this pull request Sep 17, 2025
Some InlineStats tests failed in release builds (see #134768). 
We need to guard these tests with capability checks.
@dnhatn dnhatn added :StorageEngine/TSDB You know, for Metrics :Analytics/ES|QL AKA ESQL >non-issue ES|QL-ui Impacts ES|QL UI labels Sep 18, 2025
@dnhatn dnhatn marked this pull request as ready for review September 18, 2025 03:58
@dnhatn
Copy link
Copy Markdown
Member Author

dnhatn commented Sep 18, 2025

@kkrik-es @limotova Thanks for reviews!

@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine labels Sep 18, 2025
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/kibana-esql (ES|QL-ui)

@dnhatn dnhatn enabled auto-merge (squash) September 18, 2025 03:59
@dnhatn dnhatn disabled auto-merge September 18, 2025 03:59
@dnhatn dnhatn merged commit 38db50a into elastic:main Sep 18, 2025
28 of 32 checks passed
@dnhatn dnhatn deleted the enable-time-series branch September 18, 2025 03:59
gmjehovich pushed a commit to gmjehovich/elasticsearch that referenced this pull request Sep 18, 2025
Some InlineStats tests failed in release builds (see elastic#134768). 
We need to guard these tests with capability checks.
gmjehovich pushed a commit to gmjehovich/elasticsearch that referenced this pull request Sep 18, 2025
This change enables the TS command in non-snapshot builds. We are also 
starting to run BWC tests for the TS command, including mixed-cluster
and cross-cluster tests. Note that some validations for the TS command
still need improvement, but it is fine to address these after this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL ES|QL-ui Impacts ES|QL UI >non-issue :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:StorageEngine test-full-bwc Trigger full BWC version matrix tests test-release Trigger CI checks against release build v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants