elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.69k stars 8.12k forks source link

[Lens] [ES|QL] Refactoring column ids to be uuids #189044

Open mbondyra opened 1 month ago

mbondyra commented 1 month ago

When adding ES|QL support for Lens, we decided to derive the column IDs of Lens columns from the field names in the ES|QL query. Below is an example of a column for an ES|QL layer:

{
    "columnId": "BUCKET(@timestamp, 1 day)",
    "fieldName": "BUCKET(@timestamp, 1 day)",
    "meta": {
        "type": "date",
        "esType": "date"
    }
}

The columnId serves not just as an identifier for a dimension but also as a field name potentially used in other parts of the code. This can easily introduce bugs, some of which already happened or may occur in the future. Examples of these bugs can be found here.

Why It's Important to Use UUIDs in Lens Instead of Field-Based IDs

Lens was designed with the concept that column IDs should be interchangeable:

Using field names as IDs can be confusing for anyone analysing the saved object or writing code based on it. For instance, in the past, we referred to the ID to get information from the cache by matching it with the field name. However, this approach was buggy in some egdecases, as the ID may already refer to another field. Using meaningful IDs might encourage this buggy practice.

While not using UUIDs isn't technically a bug, it has caused several issues already and isn't a good practice, given how Lens is written.

Regarding compatibility, using UUIDs should work fine because an ID is just an identifier. For example, a 'bytes' column could have an ID 'memory' in the saved objects columns and still work. We aim to use UUIDs to break the unnecessary connection between field names and IDs, not to fix existing issues (which need to be addressed separately if there are any).

Screenshot 2024-07-24 at 11 03 27
elasticmachine commented 1 month ago

Pinging @elastic/kibana-visualizations (Team:Visualizations)