Skip to content
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.

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

ESQL: Improve command resolution in telemetry #115992

Open
costin opened this issue Oct 30, 2024 · 2 comments
Open

ESQL: Improve command resolution in telemetry #115992

costin opened this issue Oct 30, 2024 · 2 comments
Assignees
Labels
:Analytics/ES|QL AKA ESQL >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@costin
Copy link
Member

costin commented Oct 30, 2024

Description

The ESQL telemetry infrastructure expects each LogicalPlan to map uniquely to a command, (through LogicalPlan#commandName()) an assumption that can't be generalized.
Instead the mapping between the plan and the commands needs to be moved to a separate class / registry / map.
P.S. Furthermore, the telemetry might in time be moved to occur at parsing time to track queries that don't pass the grammar, before the logical AST is fully assembled.

Example: FROM index | INLINESTATS c = count() should count FROM and INLINESTATS but ends up counting FROM, INLINESTATS and STATS (due to the normalized plan).

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 30, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@luigidellaquila
Copy link
Contributor

@costin I gave it a try when I implemented APM telemetry, and I ended up using commandName() and making it dynamic (see what we do for METRICS)
Even if we move the logic in a separate class, we'll still have to store some information inside the plan, that is the only thing we have after parsing.

P.S. Furthermore, the telemetry might in time be moved to occur at parsing time to track queries that don't pass the grammar, before the logical AST is fully assembled.

That's the best thing to do for sure, but it's not straight forward. If we have to invest on this thing though, IMHO this is the right approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

No branches or pull requests

3 participants