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

[ES|QL] Support _query versioning #179641

Closed
stratoula opened this issue Mar 28, 2024 · 17 comments
Closed

[ES|QL] Support _query versioning #179641

stratoula opened this issue Mar 28, 2024 · 17 comments
Assignees
Labels
discuss Feature:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Meta Team:ESQL ES|QL related features in Kibana

Comments

@stratoula
Copy link
Contributor

stratoula commented Mar 28, 2024

Describe the feature:

To support backwards compatibility, the ES team is going to add mandatory ES|QL version in the _query api. This version is not going to change very often, it is going to change only when a breaking change is being introduced (for example a command changes from comA to comB but not when a new command is being introduced.

The changes are mostly:

  • syntax changes are not expected often but if they happen, new features are not going to be supported in old versions. This means that our editor should always try to be on the latest version.
  • It is mostly about the output of a format. So you dont return double but you return boolean (hypothetical scenario). This is the most common scenario

This complicates things for kibana for multiple reasons:

  • The client side validation is based on the ES parser and lexer. It follows the ES parser and lexer so it will always be on the latest version (or try to be).
  • On a dashboard level, when the user is creating an ES|QL panel in version 1, we can always store this version on the panel level. But what happens if the user wants to upgrade the panel to run on the latest version? For example, they read the documentation and they see that the function x returns the data in double output and not in boolean as the version 1 and they want it. Or if they have a query with from logs | keep field1 and the keep changes to project? If the editor is in version 2, then the query will appear as invalid. The latter won't be very common but can happen
  • The different components that support ES|QL should be in the same version.
    • For example in Discover:
      • a user is expecting to be able to use the latest syntax (and version)
      • this syntax should be compatible with the api that Lens uses to create the histogram for example. On a syntax change (uncommon scenario but still) you want the Discover query and the Lens query to be in sync otherwise one of them will fail
      • You cant go to Discover, run the query, see different output and then go to Lens or ML or ... and see other output
    • On obs assistant:
      • the assistant works with ES documentation. Which version should they follow? you cant have a generated query using version 2 while Lens is using version 1 for example because if version 2 comes with a syntax change, the Lens panel will be broken. They need to be in sync

Solutions

  1. Kibana has a utility which stores the version. All the consumers are using this utility and when a breaking change comes we need to gather all consumers and decide the process for upgrade. This comes with caveats of course:
    • on syntax changes the applications should run migrations to upgrade to the latest version
    • on output changes we don't need migrations but if the version is being automatically upgraded the users might see different examples from what they used to. Is it ok?
    • what if ES removes a command and there is no equivalent? I guess that this is an uncommon scenario but still adding this on the loop
  2. The version is handled per application. Which means that the version is stored in the SOs (Discover, Dashboard etc). For "old" SOs we fetch the data using the version stored in the SO otherwise we use the latest version (the one that the ES|QL editor supports). This opens a can of worms. What if the customer wants to update the old panel from version 1 to version 2? What if the query is incompatible with the editor's version (they will see the query as invalid which is not the end of the world but is definitely a bad UX

Note

With the version: snapshot we can test the unreleased latest version

@stratoula stratoula added discuss Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana labels Mar 28, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@stratoula stratoula added the impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. label Mar 28, 2024
@drewdaemon
Copy link
Contributor

on syntax changes the applications should run migrations to upgrade to the latest version

Can we know that there will always be a migration path? Renaming a command is one thing. What about if the ES team actually removes something without replacing the functionality?

@stratoula
Copy link
Contributor Author

Can we know that there will always be a migration path? Renaming a command is one thing. What about if the ES team actually removes something without replacing the functionality?

This is another case, a most dangerous one. I a adding this to the description. Thanx Drew

@stratoula stratoula added the Meta label Apr 2, 2024
@jughosta
Copy link
Contributor

jughosta commented Apr 3, 2024

We (at least in Analytics) usually use AggregateQuery type and define/store a query in the following format:

{
  esql: "from logs* | limit 100"
}

In SOs we save this object too. What if we extend it with a version prop?

{
  esql: "from logs* | limit 100",
  version: "x.x.x"
}

This way during a fetch we can always pass the saved version along with esql string to ES (for example, when fetching data for dashboard panels or even when fetching data for saved searches).

If user edits the query, we can override the version with the current one regardless of what version was there before. In some cases users have to modify the query anyway to comply with the validation.

For Kibana to know the latest supported version, we would need to introduce a new API to fetch it from ES, no?

@stratoula
Copy link
Contributor Author

If user edits the query, we can override the version with the current one regardless of what version was there before. In some cases users have to modify the query anyway to comply with the validation.

There are cases that the user might not want to overwrite:

  • the output is so different which messes up with their chart
  • the syntax has changed significantly and the user might not want to re-write this manually
  • ....

Also it is very messy to have a dashboard with ES|QL panels of different versions. The ES|QL editor will always be in the latest version so it will complain for older versions (in case of syntax incompatible versions)

At some point we need to make a decision of course, we can't be perfect for all cases but I hope that this version won't change very often

For Kibana to know the latest supported version, we would need to introduce a new API to fetch it from ES, no?

We are not thinking of fetching it from ES at some point but having a utility at kibana. We are still on discussions about this.

@drewdaemon
Copy link
Contributor

The ES|QL editor will always be in the latest version so it will complain for older versions (in case of syntax incompatible versions)

I guess... we could version our validation engine...

@stratoula
Copy link
Contributor Author

I guess... we could version our validation engine...

We could but how many versions of the parser / lexer are we going to support? 🤷‍♀️

But still, the editor is not a big problem, the worst thing it can happen is that the validator is going to complain. We can disable it for versions < latest so no problem at all

@stratoula
Copy link
Contributor Author

Follow up here: We are going to have an api to retrieve the versions elastic/elasticsearch#107069

@rudolf
Copy link
Contributor

rudolf commented Apr 4, 2024

It seems to me that the root problem here is that our UI does not make the version explicit. All these problems exist for any ESQL API consumer but the ES team solved it by asking for an explicit version in the API request.

If in the UI an ESQL query is always accompanied by a version then it would be safe for a user to copy a version 1 query from a lens and paste it into discover, because discover will know to interpret this as a v1 query. I'm not sure what's the best way to display this, but I can imagine something like a v1 bubble in front of the query. If you press the copy to clipboard button you'd get something like what Julia recommended {esql: "from logs* | limit 100", version: "x.x.x"} so that any application it's pasted in knows how to correctly interpret it and send it to the _query API.

But this is really just a workaround for the fact that an ESQL query on it's own is not enough for Elasticsearch to give you a stable response, a query without a version is not a complete backwards compatible query. Why don't we expose the version directly in the query syntax:

  • v1:from logs | keep field1
  • v2:from logs | project field1

The version could be optional and default to the latest version. So from logs | keep field1 will be valid syntax if v1 is the latest version. If ever v2 gets introduced our migration will change all unversioned ESQL queries to prepend an explicit version to them so they become v1:from logs | keep field1 and keep on working.

This has the added benefit of when you copy and paste a query from somewhere on the internet it will keep working even after a breaking query change because the version was explicit.

Hopefully we'll never need a v2 for at least the next 10 years and everyone just happily writes unversioned queries forever. But in the unlikely event, we have an elegant way to introduce a breaking change without confusing users by hiding the version.

@stratoula
Copy link
Contributor Author

The version could be optional and default to the latest version.

We discussed this with the ES team but they want to make it mandatory. I don't think that this is very important tbh

If ever v2 gets introduced our migration will change all unversioned ESQL queries to prepend an explicit version to them so they become v1:from logs | keep field1 and keep on working

I am not sure about the version being part of the query, this will mean that we are having a different lexer / parser from ES and we don't want this. The version being visible in the UI, yes absolutely. I have also suggested that. The migration can update the query or add the version in a property, agreed 👍 In case of the latter it would be really weird in a dashboard though. Are you going to have a mix of v1 and v2 charts? If the user wants to upgrade to v2?

@rayafratkina
Copy link
Contributor

FWIW, I think we have to go with solution 1 that Stratoula proposed in order to provide a reasonable user experience - we need to be using the same version of ESQL across all of Kibana so user can use the same queries in different applications.

@afharo
Copy link
Member

afharo commented Apr 4, 2024

I think the best UX is to always upgrade the queries for the user (migrations).

However, I see a few downsides that make me lean towards exposing the version in the UI:

  1. What Rudolf mentioned about someone finding a query online (or an old internal document of their company) and reusing it. If we don't expose the concept of versions, users will be confused because they don't work, and can't tell why. Can we autoupgrade on the fly? How can we differentiate an old query from a bad query if it's not with the version?
  2. Migrations are easy if they are just about renaming the method, but, aside from removals (let's hope this never happens), there may be other sorts of replacements like:
    • Having to provide a different set of parameters
    • ES requires certain operations to always be performed at the end of the pipe
    • The introduction of a new parameter to deal with optional fields (?.), and if not used, the query will crash

I'm a bit scared that if we commit to autoupgrade users' queries, we might be adding more headaches to ourselves.

N.B.: If we decide not to auto-upgrade, we'll need to add a check in Upgrade Assistant to warn users when they upgrade to a version of the stack that no-longer supports their ES|QL version.

Speaking of version upgrades... how would deprecated versions work in Serverless? Should we follow a "Google" approach? Send an email about a version being deleted in 6 months (maybe some additional reminders), and hope that the user will migrate in time?

@davismcphee
Copy link
Contributor

I agree that trying to automatically upgrade queries for users sounds like a big headache with potentially little reward if we don't expect to break their queries often. I'm not very close to current ES|QL work, but @jughosta's suggestion seems like it makes the most sense to me.

If we store ES|QL queries in an object like { "esql": "...", "version": 1 } when we persist them, we would know what version to pass to the ES|QL endpoint when fetching data. I'm not sure it would be a big concern if for example multiple Dashboard panels were using different ES|QL versions unless a user tries to edit one, would it?

I would think at the point when a user goes to edit an outdated query, we could show an indicator in the editor that their query is outdated and validation is disabled, with an option to update the editor to the latest version. This might cause syntax errors or behaviour changes for them, but I feel like that would be the case when trying to run outdated code in any language that introduced breaking changes. We could also link to or embed relevant documentation in the editor to help guide them.

And if we really feel like some sort of automation is valuable, we could offer something like an in editor migration assistant to help update the query to the latest version (maybe an AI assistant that understands both ES|QL versions could help here?). Pasting code from outside sources would still be an issue since it would always try to run in the latest version in the editor, but this also feels like a situation you could run into with any language that has breaking changes.

This approach becomes an issue when we try to programatically modify a user provided query, but then maybe it's on us to version our utilities that do this for compatibility.

stratoula added a commit that referenced this issue Apr 9, 2024
## Summary

Due to #179641

ES is going to add a mandatory field `version`. (it is optional for now
but they want to make it required for the GA).

The version will change in backwards incompatible changes:
- syntax changes
- functions / commands output changes

This is the first PR which is adding the first version manually for now.
In #179641 we are exploring how
to deal with versioning in kibana. We are going to offer a solution in
8.15. For now we are adding the version hardcoded (otherwise when the
field is marked required kibana _query requests will fail.

👮‍♀️ If you are aware of any other application which uses the _query api
and is not updated in this PR please let me know

---------

Co-authored-by: kibanamachine <[email protected]>
@stratoula
Copy link
Contributor Author

stratoula commented Apr 10, 2024

Update: The version has been hardcoded everywhere in kibana we are using the _query api #180248

@stratoula
Copy link
Contributor Author

thank you all for your feedback so far. I can see that some of you prefer my bullet 1 and others the bullet 2, which indicates the complexity of this problem. I am writing a document which explains better each path forward with the cons and the pros but @davismcphee about the bullet 2 which is what you are also suggesting. It is definitely the solution where you don't introduce breaking changes to the customer or you don't run migrations but it comes with great UI complexity for all the apps that want to save ES|QL related SOs. I will give some examples of the things we need to take care:

  • This complicates the experience a lot, each application which saves a saved query should care about providing an upgrade option to their users and a UI to support this.
  • Technically is a much larger task than the ones mentioned above.
  • It needs design input for each application.
  • Creating a utility to automatically upgrade the query to newer versions can be tricky.
  • It can be confusing for the users. Why do I have queries in 3 different versions in my dashboard?
  • Copying and pasting an old query to another component which supports a newer version might not work
  • Saving ES|QL queries in the SOs (reusability) need to come with a version making this problem even more complex.
  • As we can’t support many versions of parsers / lexers, client side validation should be disabled for the older versions
  • Which version of documentation should the editor support? If the query is in version 1 I should display the documentation in version 1. We can’t have different variations of the docs though. It will be very confusing for the users. We can always hide the inline documentation for old versions of course but is this a good UX?

@stratoula
Copy link
Contributor Author

We decided to go with the simplest solution which means that we will use the same version in kibana everywhere. We want kibana to follow ES and we want to have the same version everywhere. ES is not going to introduce breaking changes often and they will try their best to have compatible queries along different versions. The philosophy is: We will do our best to not create breaking changes to the users.

So far all the consumers of the _query are sharing the same version thanks to a reusable constant introduced in 8.14.

We could improve the consumers' experience by providing easy tools to run their queries. So far we have:

  • The search strategy used by maps, ML team
  • The expressions esql function, used by Lens, Discover and the editor

We could:
Create a simple api for the consumers who want to run ES|QL without the search strategy or expressions. An example can be:

const runESQLQuery = (query: string) => {
 return await (
    await core
       ).elasticsearch.client.asCurrentUser.transport.request({
        method: 'POST',
        path: '_query',
        body: {
          query,
          version: ESQL_LATEST_VERSION,
    },
 });
}

The only dependency is the core plugin which is an essential dependency for all plugins and nothing more.

  • Remove the expressions dependency from the editor and use the api mentioned above. To achieve this we need to pass the time and the time field name to the function above.
  • Consider using the same to Discover, we will need to pass the filters too but it will remove the expressions dependency
  • Leave Lens as it is, the entire architecture depends on expressions so we can’t change it now

stratoula added a commit that referenced this issue May 1, 2024
## Summary

Adds a new utility for the users who want to retrieve the columns from a
query without expressions but using the search strategy.

This is the first utility to add for fetching ES|QL data without
expressions. This is only for columns but we can extend for fetching the
entire table instead. The latter will be part of
#179641 (comment)

---------

Co-authored-by: kibanamachine <[email protected]>
@stratoula stratoula added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. and removed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels May 13, 2024
@stratoula stratoula self-assigned this May 21, 2024
@stratoula
Copy link
Contributor Author

We decided to remove the version option on the _query API as part of 8.14 while still GA-ing ES|QL in 8.14. With this in mind I am closing this issue as is not relevant any more.

I will proceed with removing the version across kibana

stratoula added a commit that referenced this issue May 23, 2024
## Summary

Part of #179641

Adds another run query helper based on the search strategy in our
esql-utils package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:ES|QL ES|QL related features in Kibana impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Meta Team:ESQL ES|QL related features in Kibana
Projects
None yet
Development

No branches or pull requests

8 participants