From 793a2460561c63059a926b533670cbc8201fcd4d Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 16 Apr 2025 19:38:52 +0200 Subject: [PATCH 1/3] ES|QL: make telemetry more strict --- .../xpack/esql/telemetry/FeatureMetric.java | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java index 3a36f5b0d7c04..74cb3be206478 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java @@ -7,23 +7,38 @@ package org.elasticsearch.xpack.esql.telemetry; +import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; +import org.elasticsearch.xpack.esql.plan.logical.ChangePoint; +import org.elasticsearch.xpack.esql.plan.logical.Dedup; import org.elasticsearch.xpack.esql.plan.logical.Dissect; import org.elasticsearch.xpack.esql.plan.logical.Drop; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.EsRelation; import org.elasticsearch.xpack.esql.plan.logical.Eval; import org.elasticsearch.xpack.esql.plan.logical.Filter; +import org.elasticsearch.xpack.esql.plan.logical.Fork; import org.elasticsearch.xpack.esql.plan.logical.Grok; +import org.elasticsearch.xpack.esql.plan.logical.InlineStats; +import org.elasticsearch.xpack.esql.plan.logical.Insist; import org.elasticsearch.xpack.esql.plan.logical.Keep; +import org.elasticsearch.xpack.esql.plan.logical.Limit; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.MvExpand; import org.elasticsearch.xpack.esql.plan.logical.OrderBy; +import org.elasticsearch.xpack.esql.plan.logical.Project; import org.elasticsearch.xpack.esql.plan.logical.Rename; import org.elasticsearch.xpack.esql.plan.logical.Row; +import org.elasticsearch.xpack.esql.plan.logical.RrfScoreEval; +import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; +import org.elasticsearch.xpack.esql.plan.logical.inference.Completion; +import org.elasticsearch.xpack.esql.plan.logical.inference.Rerank; +import org.elasticsearch.xpack.esql.plan.logical.join.LookupJoin; +import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject; import org.elasticsearch.xpack.esql.plan.logical.show.ShowInfo; import java.util.BitSet; +import java.util.List; import java.util.Locale; import java.util.function.Predicate; @@ -42,7 +57,26 @@ public enum FeatureMetric { FROM(EsRelation.class::isInstance), DROP(Drop.class::isInstance), KEEP(Keep.class::isInstance), - RENAME(Rename.class::isInstance); + RENAME(Rename.class::isInstance), + LOOKUP_JOIN(LookupJoin.class::isInstance), + CHANGE_POINT(ChangePoint.class::isInstance), + INLINESTATS(InlineStats.class::isInstance), + RERANK(Rerank.class::isInstance), + DEDUP(Dedup.class::isInstance), + INSIST(Insist.class::isInstance), + FORK(Fork.class::isInstance), + RRF(RrfScoreEval.class::isInstance), + COMPLETION(Completion.class::isInstance); + + /** + * List here plans we want to exclude from telemetry + */ + private static final List> excluded = List.of( + UnresolvedRelation.class, + EsqlProject.class, + Project.class, + Limit.class // LIMIT is managed in another way, see above + ); private Predicate planCheck; @@ -61,6 +95,13 @@ public static void set(LogicalPlan plan, BitSet bitset) { return; } } + if (explicitlyExcluded(plan) == false) { + throw new EsqlIllegalArgumentException("Command not mapped for telemetry [{}]", plan.getClass().getSimpleName()); + } + } + + private static boolean explicitlyExcluded(LogicalPlan plan) { + return excluded.stream().anyMatch(x -> x.isInstance(plan)); } public static boolean set(LogicalPlan plan, BitSet bitset, FeatureMetric metric) { From e6c1e01576cec1a8d43d653f652f9226acb0eab6 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 16 Apr 2025 21:56:10 +0200 Subject: [PATCH 2/3] Lookup --- .../org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java index 74cb3be206478..69682c0d0bb0f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/telemetry/FeatureMetric.java @@ -24,6 +24,7 @@ import org.elasticsearch.xpack.esql.plan.logical.Keep; import org.elasticsearch.xpack.esql.plan.logical.Limit; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.elasticsearch.xpack.esql.plan.logical.Lookup; import org.elasticsearch.xpack.esql.plan.logical.MvExpand; import org.elasticsearch.xpack.esql.plan.logical.OrderBy; import org.elasticsearch.xpack.esql.plan.logical.Project; @@ -59,6 +60,7 @@ public enum FeatureMetric { KEEP(Keep.class::isInstance), RENAME(Rename.class::isInstance), LOOKUP_JOIN(LookupJoin.class::isInstance), + LOOKUP(Lookup.class::isInstance), CHANGE_POINT(ChangePoint.class::isInstance), INLINESTATS(InlineStats.class::isInstance), RERANK(Rerank.class::isInstance), From 0a639ee61d559b71e061afec513a33c4e80b54df Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 17 Apr 2025 09:38:40 +0200 Subject: [PATCH 3/3] Tests --- .../rest-api-spec/test/esql/60_usage.yml | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml index 838ec22a2d8b1..6f4bc45c018b3 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/60_usage.yml @@ -39,7 +39,7 @@ setup: - do: {xpack.usage: {}} - match: { esql.available: true } - match: { esql.enabled: true } - - length: { esql.features: 15 } + - length: { esql.features: 25 } - set: {esql.features.dissect: dissect_counter} - set: {esql.features.drop: drop_counter} - set: {esql.features.eval: eval_counter} @@ -55,6 +55,16 @@ setup: - set: {esql.features.sort: sort_counter} - set: {esql.features.stats: stats_counter} - set: {esql.features.where: where_counter} + - set: {esql.features.lookup_join: lookup_join_counter} + - set: {esql.features.lookup: lookup_counter} + - set: {esql.features.change_point: change_point_counter} + - set: {esql.features.inlinestats: inlinestats_counter} + - set: {esql.features.rerank: rerank_counter} + - set: {esql.features.dedup: dedup_counter} + - set: {esql.features.insist: insist_counter} + - set: {esql.features.fork: fork_counter} + - set: {esql.features.rrf: rrf_counter} + - set: {esql.features.completion: completion_counter} - length: { esql.queries: 3 } - set: {esql.queries.rest.total: rest_total_counter} - set: {esql.queries.rest.failed: rest_failed_counter} @@ -88,6 +98,16 @@ setup: - gt: {esql.features.sort: $sort_counter} - gt: {esql.features.stats: $stats_counter} - gt: {esql.features.where: $where_counter} + - match: {esql.features.lookup_join: $lookup_join_counter} + - match: {esql.features.lookup: $lookup_counter} + - match: {esql.features.change_point: $change_point_counter} + - match: {esql.features.inlinestats: $inlinestats_counter} + - match: {esql.features.rerank: $rerank_counter} + - match: {esql.features.dedup: $dedup_counter} + - match: {esql.features.insist: $insist_counter} + - match: {esql.features.fork: $fork_counter} + - match: {esql.features.rrf: $rrf_counter} + - match: {esql.features.completion: $completion_counter} - gt: {esql.queries.rest.total: $rest_total_counter} - match: {esql.queries.rest.failed: $rest_failed_counter} - match: {esql.queries.kibana.total: $kibana_total_counter} @@ -117,7 +137,7 @@ setup: - do: {xpack.usage: {}} - match: { esql.available: true } - match: { esql.enabled: true } - - length: { esql.features: 15 } + - length: { esql.features: 25 } - set: {esql.features.dissect: dissect_counter} - set: {esql.features.drop: drop_counter} - set: {esql.features.eval: eval_counter} @@ -133,6 +153,16 @@ setup: - set: {esql.features.sort: sort_counter} - set: {esql.features.stats: stats_counter} - set: {esql.features.where: where_counter} + - set: {esql.features.lookup_join: lookup_join_counter} + - set: {esql.features.lookup: lookup_counter} + - set: {esql.features.change_point: change_point_counter} + - set: {esql.features.inlinestats: inlinestats_counter} + - set: {esql.features.rerank: rerank_counter} + - set: {esql.features.dedup: dedup_counter} + - set: {esql.features.insist: insist_counter} + - set: {esql.features.fork: fork_counter} + - set: {esql.features.rrf: rrf_counter} + - set: {esql.features.completion: completion_counter} - length: { esql.queries: 3 } - set: {esql.queries.rest.total: rest_total_counter} - set: {esql.queries.rest.failed: rest_failed_counter} @@ -166,6 +196,16 @@ setup: - gt: {esql.features.sort: $sort_counter} - gt: {esql.features.stats: $stats_counter} - gt: {esql.features.where: $where_counter} + - match: {esql.features.lookup_join: $lookup_join_counter} + - match: {esql.features.lookup: $lookup_counter} + - match: {esql.features.change_point: $change_point_counter} + - match: {esql.features.inlinestats: $inlinestats_counter} + - match: {esql.features.rerank: $rerank_counter} + - match: {esql.features.dedup: $dedup_counter} + - match: {esql.features.insist: $insist_counter} + - match: {esql.features.fork: $fork_counter} + - match: {esql.features.rrf: $rrf_counter} + - match: {esql.features.completion: $completion_counter} - gt: {esql.queries.rest.total: $rest_total_counter} - match: {esql.queries.rest.failed: $rest_failed_counter} - match: {esql.queries.kibana.total: $kibana_total_counter}