Add ESQL telemetry collection by smalyshev · Pull Request #117282 · elastic/elasticsearch

smalyshev · 2024-11-21T20:10:23Z

Adds _esql section to ccs metrics at _cluster/stats. Only available when any ES|QL queries have been performed.

elasticsearchmachine · 2024-11-21T21:34:56Z

Hi @smalyshev, I've created a changelog YAML for you.

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java

quux00

Minor suggestions. Looks good!

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodeResponse.java

quux00 · 2024-11-25T21:51:13Z

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodeResponse.java

        return ccsMetrics;
    }

+    public CCSTelemetrySnapshot getEsqlMetrics() {


Suggested renamings:

getCcsMetrics -> getSearchCcsMetrics
getEsqlMetrics -> getEsqlCcsMetrics

quux00 · 2024-11-25T21:51:46Z

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsResponse.java

    final String clusterUUID;
    private final Map<String, RemoteClusterStats> remoteClustersStats;

+    public static final String CCS_TELEMETRY_FIELD_NAME = "_search";


nit: rename to SEARCH_TELEMETRY_FIELD_NAME?

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java

elasticsearchmachine · 2024-12-23T22:41:12Z

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

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java

server/src/main/java/org/elasticsearch/usage/UsageService.java

quux00

Good progress! Some suggested changes/questions left.

docs/reference/cluster/stats.asciidoc

server/src/main/java/org/elasticsearch/TransportVersions.java

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/CCSTelemetrySnapshot.java

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/CCSUsage.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java

quux00

Left a few optional nits. Nice work on this! Approved.

quux00 · 2025-01-02T17:07:15Z

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/CCSUsageTelemetry.java

    private final Map<String, LongAdder> clientCounts;
    private final Map<String, PerClusterCCSTelemetry> byRemoteCluster;
+    // Should we calculate separate metrics per MRT?
+    private boolean useMRT = true;


nit: I think the standard in Elasticsearch (based on my code reading at least) is to not define the default here, but rather to do it in the constructors.

So now the no-arg constructor should just call this(true) and move all the initializations from the no-arg constructor to the new public CCSUsageTelemetry(boolean useMRT) constructor.

quux00 · 2025-01-02T17:08:39Z

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/ClusterStatsNodeResponse.java

@@ -32,6 +32,7 @@ public class ClusterStatsNodeResponse extends BaseNodeResponse {
    private final SearchUsageStats searchUsageStats;
    private final RepositoryUsageStats repositoryUsageStats;
    private final CCSTelemetrySnapshot ccsMetrics;


nit: can we rename this to searchMetrics or searchCcsMetrics and esqlMetrics to esqlCcsMetrics. Otherwise these names are potentially confusing?

marciw

Small drive-by style/clarity comments for stats.asciidoc only

marciw · 2025-01-02T17:54:00Z

docs/reference/cluster/stats.asciidoc

+
 ======
+`_esql`:::
+(object) Contains the information about the ES|QL <<esql-cross-clusters,{ccs}>> usage in the cluster.


Suggested change

(object) Contains the information about the ES|QL <<esql-cross-clusters,{ccs}>> usage in the cluster.

(object) Contains information about <<esql-cross-clusters,{esql} {ccs}>> usage.

Note: you can keep "in the cluster" if you like, but it somewhat impedes readability and is also already covered by the top-level ccs description.

Also, if you like these edits, we should apply them to the _search object description too (lines 1393-94 in this PR), which is probably where you got the phrasing in the first place. 🙂 Happy to do that in a separate commit; just let me know.

fyi here's how this renders locally for me

@smalyshev Whoops, this PR was closed while I was working on my brief docs review. ~~Please disregard and I'll make this changes separately.~~

Edit: I'll apply my changes to #119474

smalyshev · 2025-01-02T18:23:28Z

Oops accidentally closed it, reopening as #119474

smalyshev · 2025-01-02T18:27:42Z

Try to reopen this

elasticsearchmachine added the v9.0.0 label Nov 21, 2024

smalyshev added :Analytics/ES|QL AKA ESQL v8.18.0 >enhancement labels Nov 21, 2024

smalyshev force-pushed the esql-telemetry branch 2 times, most recently from 30f6ac5 to f193a4d Compare November 22, 2024 22:25

smalyshev commented Nov 22, 2024

View reviewed changes

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java Outdated Show resolved Hide resolved

smalyshev commented Nov 22, 2024

View reviewed changes

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/execution/PlanExecutor.java Outdated Show resolved Hide resolved

quux00 reviewed Nov 25, 2024

View reviewed changes

smalyshev force-pushed the esql-telemetry branch 4 times, most recently from fccea7f to 72bdb43 Compare December 14, 2024 04:22

smalyshev changed the title ~~WIP: Add ESQL telemetry collection~~ Dec 23, 2024

smalyshev added the auto-backport Automatically create backport pull requests when merged label Dec 23, 2024

smalyshev marked this pull request as ready for review December 23, 2024 22:40

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Dec 23, 2024

smalyshev requested a review from quux00 December 26, 2024 20:57

pawankartik-elastic reviewed Dec 27, 2024

View reviewed changes

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java Outdated Show resolved Hide resolved

pawankartik-elastic reviewed Dec 27, 2024

View reviewed changes

.../plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java Outdated Show resolved Hide resolved

pawankartik-elastic reviewed Dec 27, 2024

View reviewed changes

server/src/main/java/org/elasticsearch/usage/UsageService.java Outdated Show resolved Hide resolved

quux00 reviewed Dec 27, 2024

View reviewed changes

quux00 approved these changes Jan 2, 2025

View reviewed changes

smalyshev closed this Jan 2, 2025

smalyshev force-pushed the esql-telemetry branch from ef59b3d to 0ca7517 Compare January 2, 2025 18:18

marciw reviewed Jan 2, 2025

View reviewed changes

marciw mentioned this pull request Jan 2, 2025

Add ESQL telemetry collection #119474

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ESQL telemetry collection#117282

Add ESQL telemetry collection#117282
smalyshev wants to merge 0 commit intoelastic:mainfrom
smalyshev:esql-telemetry

smalyshev commented Nov 21, 2024 •

edited

Loading

elasticsearchmachine commented Nov 21, 2024

Uh oh!

Uh oh!

quux00 left a comment

Uh oh!

quux00 Nov 25, 2024

quux00 Nov 25, 2024

Uh oh!

Uh oh!

elasticsearchmachine commented Dec 23, 2024

Uh oh!

Uh oh!

Uh oh!

quux00 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

quux00 left a comment

quux00 Jan 2, 2025

quux00 Jan 2, 2025

marciw left a comment

marciw Jan 2, 2025

marciw Jan 2, 2025

marciw Jan 2, 2025

marciw Jan 2, 2025 •

edited

Loading

smalyshev commented Jan 2, 2025

smalyshev commented Jan 2, 2025

Labels

5 participants

	(object) Contains the information about the ES\|QL <<esql-cross-clusters,{ccs}>> usage in the cluster.
	(object) Contains information about <<esql-cross-clusters,{esql} {ccs}>> usage.

Conversation

smalyshev commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Nov 21, 2024

Uh oh!

Uh oh!

quux00 left a comment

Choose a reason for hiding this comment

Uh oh!

quux00 Nov 25, 2024

Choose a reason for hiding this comment

quux00 Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Dec 23, 2024

Uh oh!

Uh oh!

Uh oh!

quux00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

quux00 left a comment

Choose a reason for hiding this comment

quux00 Jan 2, 2025

Choose a reason for hiding this comment

quux00 Jan 2, 2025

Choose a reason for hiding this comment

marciw left a comment

Choose a reason for hiding this comment

marciw Jan 2, 2025

Choose a reason for hiding this comment

marciw Jan 2, 2025

Choose a reason for hiding this comment

marciw Jan 2, 2025

Choose a reason for hiding this comment

marciw Jan 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

smalyshev commented Jan 2, 2025

smalyshev commented Jan 2, 2025

Labels

5 participants

smalyshev commented Nov 21, 2024 •

edited

Loading

marciw Jan 2, 2025 •

edited

Loading