-
Notifications
You must be signed in to change notification settings - Fork 8.5k
🌊 Show detected field types for classic streams enrichment #222579
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
🌊 Show detected field types for classic streams enrichment #222579
Conversation
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/streams --include-path /api/fleet --include-path /api/dashboards --include-path /api/saved_objects/_import --include-path /api/saved_objects/_export --include-path /api/maintenance_window --update'
| return { name, type: confirmedValidDetectedFields[name]?.type }; | ||
| const existingFieldCaps = Object.keys(streamFieldCaps[name] || {}); | ||
|
|
||
| const esType = existingFieldCaps.length > 0 ? existingFieldCaps[0] : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to double check, since we only query the field caps of the write index only one type can be defined per field ?
| const lastIndexMapping = await scopedClusterClient.asCurrentUser.indices.get({ | ||
| index: lastIndex.index_name, | ||
| }); | ||
| const [lastIndexMapping, lastIndexFieldCaps] = await Promise.all([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this is a wrongly naming I leftover from some previous work, but this is retrieving the last index, not its mappings only. Can you fix the naming around this retrieved value please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I might be wrong but doesn't the fieldCaps return the same field types as specified in the last index mappings, which is available already on the retrieved index? Or does it return a different result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field caps have it in normalized form - the same information should be in the mapping as well, but it's very messy (flattened fields, multiple layers of object fields and so on). The structure of field caps is much easier to parse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed
tonyghiani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it's a bit confusing from a user perspective to differentiate between esType and type, for the end user it should be transparent when we talk about a data type, otherwise it might raise questions around what data type a field really has.
For this work, I initially thought the esType (when available) would have been a fallback value for the unmapped fields type, not a parallel value. Can you explain the reasoing behing this choice?
The streams-level field type is a normalized thing which we control. The underlying esType can be much more and is not controlled by streams. I made it a separate thing to emphasize the difference (one is something that is controllable by streams, the other thing is a piece of meta data we explicitly don't control). Is your concern about the API/data structure or how it's shown as separate columns in the fields table? |
|
Thinking about it, I think it would be OK to make it a fallback of the |
…293/kibana into flash1293/field-types-for-classic
Much better IMO, we want to keep the UI as simple as possible for the user, and having multiple column for types might complicate it. A single |
|
@tonyghiani adjusted like this:
|
tonyghiani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Starting backport for target branches: 8.19 |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
|
…22579) Adds "detected fields" tab for classic streams enrichment editor <img width="1005" alt="Screenshot 2025-06-04 at 17 58 57" src="https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54" /> This PR adds the "detected fields" tab for classic streams by fetching the actual Elasticsearch field type from field caps and showing it along with the detected fields. This currently doesn't work for fields that are not mapped yet but would get added as part of the simulation (Elasticsearch feature request here: elastic/elasticsearch#128760 ). This adds a new column "Elasticsearch field type" to the schema editor table. For wired streams, this column is not relevant at all, but it can be helpful for classic streams to highlight the non-managed parts. --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 36699ad)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…22579) Adds "detected fields" tab for classic streams enrichment editor <img width="1005" alt="Screenshot 2025-06-04 at 17 58 57" src="https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54" /> This PR adds the "detected fields" tab for classic streams by fetching the actual Elasticsearch field type from field caps and showing it along with the detected fields. This currently doesn't work for fields that are not mapped yet but would get added as part of the simulation (Elasticsearch feature request here: elastic/elasticsearch#128760 ). This adds a new column "Elasticsearch field type" to the schema editor table. For wired streams, this column is not relevant at all, but it can be helpful for classic streams to highlight the non-managed parts. --------- Co-authored-by: kibanamachine <[email protected]>
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
1 similar comment
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…22579) Adds "detected fields" tab for classic streams enrichment editor <img width="1005" alt="Screenshot 2025-06-04 at 17 58 57" src="https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54" /> This PR adds the "detected fields" tab for classic streams by fetching the actual Elasticsearch field type from field caps and showing it along with the detected fields. This currently doesn't work for fields that are not mapped yet but would get added as part of the simulation (Elasticsearch feature request here: elastic/elasticsearch#128760 ). This adds a new column "Elasticsearch field type" to the schema editor table. For wired streams, this column is not relevant at all, but it can be helpful for classic streams to highlight the non-managed parts. --------- Co-authored-by: kibanamachine <[email protected]>
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
1 similar comment
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…22579) Adds "detected fields" tab for classic streams enrichment editor <img width="1005" alt="Screenshot 2025-06-04 at 17 58 57" src="https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54" /> This PR adds the "detected fields" tab for classic streams by fetching the actual Elasticsearch field type from field caps and showing it along with the detected fields. This currently doesn't work for fields that are not mapped yet but would get added as part of the simulation (Elasticsearch feature request here: elastic/elasticsearch#128760 ). This adds a new column "Elasticsearch field type" to the schema editor table. For wired streams, this column is not relevant at all, but it can be helpful for classic streams to highlight the non-managed parts. --------- Co-authored-by: kibanamachine <[email protected]>
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
…22579) (#222798) # Backport This will backport the following commits from `main` to `8.19`: - [🌊 Show detected field types for classic streams enrichment (#222579)](#222579) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Joe Reuter","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-06-05T11:45:32Z","message":"🌊 Show detected field types for classic streams enrichment (#222579)\n\nAdds \"detected fields\" tab for classic streams enrichment editor\n\n<img width=\"1005\" alt=\"Screenshot 2025-06-04 at 17 58 57\"\nsrc=\"https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54\"\n/>\n\nThis PR adds the \"detected fields\" tab for classic streams by fetching\nthe actual Elasticsearch field type from field caps and showing it along\nwith the detected fields. This currently doesn't work for fields that\nare not mapped yet but would get added as part of the simulation\n(Elasticsearch feature request here:\nhttps://github.com/elastic/elasticsearch/issues/128760 ).\n\nThis adds a new column \"Elasticsearch field type\" to the schema editor\ntable. For wired streams, this column is not relevant at all, but it can\nbe helpful for classic streams to highlight the non-managed parts.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"36699ad4ae5c2f9ba212e05e55c6ee9692a9b2e5","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:obs-ux-logs","backport:version","Feature:Streams","v9.1.0","v8.19.0"],"title":"🌊 Show detected field types for classic streams enrichment","number":222579,"url":"https://github.com/elastic/kibana/pull/222579","mergeCommit":{"message":"🌊 Show detected field types for classic streams enrichment (#222579)\n\nAdds \"detected fields\" tab for classic streams enrichment editor\n\n<img width=\"1005\" alt=\"Screenshot 2025-06-04 at 17 58 57\"\nsrc=\"https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54\"\n/>\n\nThis PR adds the \"detected fields\" tab for classic streams by fetching\nthe actual Elasticsearch field type from field caps and showing it along\nwith the detected fields. This currently doesn't work for fields that\nare not mapped yet but would get added as part of the simulation\n(Elasticsearch feature request here:\nhttps://github.com/elastic/elasticsearch/issues/128760 ).\n\nThis adds a new column \"Elasticsearch field type\" to the schema editor\ntable. For wired streams, this column is not relevant at all, but it can\nbe helpful for classic streams to highlight the non-managed parts.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"36699ad4ae5c2f9ba212e05e55c6ee9692a9b2e5"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/222579","number":222579,"mergeCommit":{"message":"🌊 Show detected field types for classic streams enrichment (#222579)\n\nAdds \"detected fields\" tab for classic streams enrichment editor\n\n<img width=\"1005\" alt=\"Screenshot 2025-06-04 at 17 58 57\"\nsrc=\"https://github.com/user-attachments/assets/3f3bc959-a27d-4e53-af96-153f0cd0fb54\"\n/>\n\nThis PR adds the \"detected fields\" tab for classic streams by fetching\nthe actual Elasticsearch field type from field caps and showing it along\nwith the detected fields. This currently doesn't work for fields that\nare not mapped yet but would get added as part of the simulation\n(Elasticsearch feature request here:\nhttps://github.com/elastic/elasticsearch/issues/128760 ).\n\nThis adds a new column \"Elasticsearch field type\" to the schema editor\ntable. For wired streams, this column is not relevant at all, but it can\nbe helpful for classic streams to highlight the non-managed parts.\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>","sha":"36699ad4ae5c2f9ba212e05e55c6ee9692a9b2e5"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Joe Reuter <[email protected]>


Adds "detected fields" tab for classic streams enrichment editor
This PR adds the "detected fields" tab for classic streams by fetching the actual Elasticsearch field type from field caps and showing it along with the detected fields. This currently doesn't work for fields that are not mapped yet but would get added as part of the simulation (Elasticsearch feature request here: elastic/elasticsearch#128760 ).
This adds a new column "Elasticsearch field type" to the schema editor table. For wired streams, this column is not relevant at all, but it can be helpful for classic streams to highlight the non-managed parts.