Skip to content

Add telemetry for TS command #135471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 27, 2025
Merged

Add telemetry for TS command #135471

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ebf430b
Add telemetry for `TS` command
kkrik-es Sep 25, 2025
4addd91
fix from review
kkrik-es Sep 26, 2025
83236aa
Merge branch 'main' into tsdb/ts-telemetry
kkrik-es Sep 26, 2025
4e05862
Merge branch 'main' into tsdb/ts-telemetry
kkrik-es Sep 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,6 @@ tests:
- class: org.elasticsearch.xpack.search.CrossClusterAsyncSearchIT
method: testCancelViaExpirationOnRemoteResultsWithMinimizeRoundtrips
issue: https://github.com/elastic/elasticsearch/issues/127302
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
method: test {p0=esql/60_usage/*}
issue: https://github.com/elastic/elasticsearch/issues/133449
- class: org.elasticsearch.xpack.esql.action.CrossClusterQueryWithFiltersIT
method: testFilterWithUnavailableRemote
issue: https://github.com/elastic/elasticsearch/issues/133450
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.xpack.esql.plan.logical.Rename;
import org.elasticsearch.xpack.esql.plan.logical.Row;
import org.elasticsearch.xpack.esql.plan.logical.Sample;
import org.elasticsearch.xpack.esql.plan.logical.TimeSeriesAggregate;
import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation;
import org.elasticsearch.xpack.esql.plan.logical.fuse.Fuse;
import org.elasticsearch.xpack.esql.plan.logical.fuse.FuseScoreEval;
Expand Down Expand Up @@ -60,6 +61,7 @@ public enum FeatureMetric {
SHOW(ShowInfo.class::isInstance),
ROW(Row.class::isInstance),
FROM(EsRelation.class::isInstance),
TS(TimeSeriesAggregate.class::isInstance),
DROP(Drop.class::isInstance),
KEEP(Keep.class::isInstance),
RENAME(Rename.class::isInstance),
Expand Down
54 changes: 54 additions & 0 deletions ...lugin/esql/src/test/java/org/elasticsearch/xpack/esql/telemetry/VerifierMetricsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import static org.elasticsearch.xpack.esql.telemetry.FeatureMetric.SHOW;
import static org.elasticsearch.xpack.esql.telemetry.FeatureMetric.SORT;
import static org.elasticsearch.xpack.esql.telemetry.FeatureMetric.STATS;
import static org.elasticsearch.xpack.esql.telemetry.FeatureMetric.TS;
import static org.elasticsearch.xpack.esql.telemetry.FeatureMetric.WHERE;
import static org.elasticsearch.xpack.esql.telemetry.Metrics.FEATURES_PREFIX;
import static org.elasticsearch.xpack.esql.telemetry.Metrics.FUNC_PREFIX;
Expand All @@ -63,6 +64,7 @@ public void testDissectQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -86,6 +88,7 @@ public void testEvalQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -109,6 +112,7 @@ public void testGrokQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -132,6 +136,7 @@ public void testLimitQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -154,6 +159,7 @@ public void testSortQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -176,6 +182,7 @@ public void testStatsQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -199,6 +206,7 @@ public void testWhereQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -221,6 +229,7 @@ public void testTwoWhereQuery() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -263,6 +272,7 @@ public void testTwoQueriesExecuted() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(2L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -349,6 +359,7 @@ public void testEnrich() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(1L, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -381,6 +392,7 @@ public void testMvExpand() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(1L, keep(c));
assertEquals(0, rename(c));
Expand All @@ -403,6 +415,7 @@ public void testShowInfo() {
assertEquals(1L, show(c));
assertEquals(0, row(c));
assertEquals(0, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -426,6 +439,7 @@ public void testRow() {
assertEquals(0, show(c));
assertEquals(1L, row(c));
assertEquals(0, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -448,6 +462,7 @@ public void testDropAndRename() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(1L, drop(c));
assertEquals(0, keep(c));
assertEquals(1L, rename(c));
Expand Down Expand Up @@ -476,6 +491,7 @@ public void testKeep() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(1L, keep(c));
assertEquals(0, rename(c));
Expand All @@ -502,6 +518,7 @@ public void testCategorize() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(1L, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -530,6 +547,7 @@ public void testInlineStatsStandalone() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -558,6 +576,7 @@ public void testInlineStatsWithOtherStats() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -585,6 +604,7 @@ public void testBinaryPlanAfterStats() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand Down Expand Up @@ -617,6 +637,7 @@ public void testBinaryPlanAfterStatsExpressionJoin() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(1L, rename(c));
Expand Down Expand Up @@ -645,6 +666,7 @@ public void testBinaryPlanAfterInlineStats() {
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(0, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
Expand All @@ -654,6 +676,34 @@ public void testBinaryPlanAfterInlineStats() {
assertEquals(1, function("max", c));
}

public void testTimeSeriesAggregate() {
assumeTrue("TS required", EsqlCapabilities.Cap.TS_COMMAND_V0.isEnabled());
Counters c = esql("""
TS metrics
Copy link
Contributor

@luigidellaquila luigidellaquila Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Does it also work for a query without stats?
Eg. will this return ts:1, keep:1, from:0?

TS metrics | KEEP @timestamp

I didn't test it, but if I remember well it gets planned as a normal FROM, so I expect it to return from:1, ts:0

Could you please add a test and verify?

If it's how I suspect, a fix could be problematic, as after analysis we lose the information we stored in the UnresolvedRelation to distinguish the two situations.

cc @astefan this is another case that would require the refactoring we discussed some time ago, ie. move the stats calculation to parse time, and unify it with APM telemetry

Copy link
Contributor

@luigidellaquila luigidellaquila Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, an easy fix could be to rely on indexMode in EsRelation, ie.

FROM(x -> x instanceof EsRelation r && r.indexMode() != IndexMode.TIME_SERIES),
TS(x -> x instanceof EsRelation r && r.indexMode() == IndexMode.TIME_SERIES),

Copy link
Contributor Author

@kkrik-es kkrik-es Sep 26, 2025

Choose a reason for hiding this comment

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

No that's not accurate, FROM is also applicable to time-series indices. In general, TS makes sense when combined with STATS, otherwise it doesn't offer much and it's actually more restrictive than it should. In that respect, I think it's ok if we don't track TS metrics | KEEP @timestamp as TS but rather as FROM.

Copy link
Contributor

@luigidellaquila luigidellaquila Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at the code, I see UnresolvedRelation is built with IndexMode.STANDARD from FROM and with IndexMode.TIME_SERIES for TS

Then we build the EsRelation starting from the UnresolvedRelation, using the same index mode.

At optimization time things can change, but here we are still at verification phase, so we should have the original information.

Am I missing something?

Copy link
Contributor Author

@kkrik-es kkrik-es Sep 26, 2025

Choose a reason for hiding this comment

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

Hm actually you're right, this seems to work! @dnhatn to double-check it too.

| STATS sum(avg_over_time(salary))""");
assertEquals(0, dissect(c));
assertEquals(0, eval(c));
assertEquals(0, grok(c));
assertEquals(0, limit(c));
assertEquals(0, sort(c));
assertEquals(1L, stats(c));
assertEquals(0, where(c));
assertEquals(0, enrich(c));
assertEquals(0, mvExpand(c));
assertEquals(0, show(c));
assertEquals(0, row(c));
assertEquals(1L, from(c));
assertEquals(1L, ts(c));
assertEquals(0, drop(c));
assertEquals(0, keep(c));
assertEquals(0, rename(c));
assertEquals(0, inlineStats(c));
assertEquals(0, lookupJoinOnFields(c));
assertEquals(0, lookupJoinOnExpression(c));
assertEquals(1, function("sum", c));
assertEquals(1, function("avg_over_time", c));
}

private long dissect(Counters c) {
return c.get(FEATURES_PREFIX + DISSECT);
}
Expand Down Expand Up @@ -702,6 +752,10 @@ private long from(Counters c) {
return c.get(FEATURES_PREFIX + FROM);
}

private long ts(Counters c) {
return c.get(FEATURES_PREFIX + TS);
}

private long drop(Counters c) {
return c.get(FEATURES_PREFIX + DROP);
}
Expand Down
Loading