🌊 Streams: Classic streams field mappings #228415
Conversation
|
/ci |
…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'
…kibana into flash1293/classic-fields-impl
flash1293
added
release_note:skip
|
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
elastic-vault-github-plugin-prod
bot
requested review from
a team
as code owners
…kibana into flash1293/classic-fields-impl
|
The tests fails at the moment because the per datastream mappings API is not available on serverless yet. However, this is the main intended use of it, so we should do the review, also testing edge cases with different mappings and so on to see whether something needs to be changed on Elasticsearch level. |
| this.upsertWriteIndexOrRollover(upsert_write_index_or_rollover), | ||
| this.updateLifecycle(update_lifecycle), | ||
| this.updateDataStreamMappingsAndRollover(update_data_stream_mappings), |
There was a problem hiding this comment.
And this is safe to do at the same time because we expect updateDataStreamMappingsAndRollover to only apply to Unwired streams while upsertWriteIndexOrRollover is for Wired streams?
| | UpsertDotStreamsDocumentAction | ||
| | DeleteDotStreamsDocumentAction; | ||
| | DeleteDotStreamsDocumentAction | ||
| | UpdateDataStreamMappingsAction; |
There was a problem hiding this comment.
What does it look like if someone needs to remove an override, we just set the empty object instead?
There was a problem hiding this comment.
yes, or nothing - the way this new API works is different from the index _mappings API - it's not an update that's merged into the existing object, it's replacing.
| } | ||
| } | ||
|
|
||
| // necessary or not? |
There was a problem hiding this comment.
I think it's valuable.
If a bulk change attempts to change 10 Unwired streams, but one of them would fail, this check means we can avoid the work plus the rollback work.
Since this also has a dry_run option, I would imagine it to be not too expensive of a call?
For a single change, it doesn't safe us much but for larger changes it does, though we need data to say if we'll have many bulk changes.
| lifecycle: { inherit: {} }, | ||
| processing: [], | ||
| unwired: { | ||
| field_overrides: {}, |
There was a problem hiding this comment.
Is this the same as just omitting this property?
|
There's an issue with the simulation logic, working on fixing that... |
|
elastic/elasticsearch#132043 merged yesterday, so I'll try and nudge this along. |
…kibana into flash1293/classic-fields-impl
…kibana into flash1293/classic-fields-impl
There was a problem hiding this comment.
Given this another pass functionality wise after my changes to the branch and everything seems to be working as expected 👍
@flash1293 Next CI run should be green. Are you happy for this to be merged? Or would you prefer an additional review as I've also made changes here?
x-pack/platform/plugins/shared/streams/server/lib/streams/component_templates/logs_layer.ts
Outdated
Show resolved
Hide resolved
| ); | ||
| } | ||
|
|
||
| private async updateDataStreamMappingsAndRollover(actions: UpdateDataStreamMappingsAction[]) { |
There was a problem hiding this comment.
nit: not async, this just carries away the returned promise.
There was a problem hiding this comment.
true, but they are all like this - will leave it like that for now
...hared/streams_app/public/components/data_management/schema_editor/hooks/use_schema_fields.ts
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Async chunks
History
|
This PR adds the ability to map fields in classic streams. This fits really well into the existing flow - it's basically just enabling the schema editor for classic streams. Under the hood this is using the new data stream mappings API instead of changing component templates. Changes that are compatible with the write index mapping are applied directly, otherwise a rollover is performed automatically. Schema editor for a classic stream. Fields mapped via streams are marked as "mapped". Fields that are mapped via the underlying index template are shown as "unmanaged", with a type that comes from field caps (the tooltip is calling it out). <img width="1248" height="647" alt="Screenshot 2025-07-17 at 17 23 40" src="https://github.com/user-attachments/assets/c8e20652-6394-43e2-b119-b42aa78418a6" /> <img width="743" height="223" alt="Screenshot 2025-07-17 at 17 26 53" src="https://github.com/user-attachments/assets/e2fcb6f7-6e5d-46da-b2e5-1698b31b4a1d" /> Previously it was called "unmapped" - it's unmapped on the streams level, but not actually unmapped in the underlying data. I'm sure that will be confusing. I'm not sure whether "unmanaged" is good enough, happy to hear other opinions. In the processing view, the fields are shown in the same way <img width="1247" height="411" alt="Screenshot 2025-07-17 at 17 28 05" src="https://github.com/user-attachments/assets/cc251181-4bfd-44a4-952d-6ce2326f0159" /> It's possible that a mapping isn't compatible with what's defined in the template of a stream. In this case an toast error is shown when the user is trying to save (hopefully it explains well enough what's going on, it's basically what comes from Elasticsearch): <img width="399" height="314" alt="Screenshot 2025-07-17 at 17 46 58" src="https://github.com/user-attachments/assets/a7ec64c2-40dd-4045-b499-7d77a11c4a0a" /> ## Notes ### Discover I think the types were incomplete here - the boolean flag in question is definitely returned in these objects, that's why adjusted upstream. ### Simulation reset too early A little drive-by fix: If the saving of the processing changes failed, the simulation would have been reset already, which makes us loose the field mappings that are staged there. This PR moves the reset to the success state transition, so everything sticks around on failure so the user can fix and try again. ### Rollover logic If the mapping is changed, the data stream is rolled over. ### Error handling If setting the mapping on the data stream level doesn't work, we bail out. This should trigger a resync with elastic#227738 , which will bring the system into a valid state again. However, this is considered a 500 instead of a 400. By using the dry run in the validation phase we can catch it early, but not sure whether it's worth it... (it also takes longer of course). ### Sourcing fields Fetching the fields for the schema editor, it's now also sourcing them from the field caps along with the definition and unmapped fields API, to give as complete of a picture as possible. ### Field simulation Had to adjust the field simulation logic a bit, I think I actually simplified it successfully so it works the same for wired and classic streams, but worth taking another look. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]>
10 participants
This PR adds the ability to map fields in classic streams. This fits really well into the existing flow - it's basically just enabling the schema editor for classic streams. Under the hood this is using the new data stream mappings API instead of changing component templates. Changes that are compatible with the write index mapping are applied directly, otherwise a rollover is performed automatically.
Schema editor for a classic stream. Fields mapped via streams are marked as "mapped". Fields that are mapped via the underlying index template are shown as "unmanaged", with a type that comes from field caps (the tooltip is calling it out).
Previously it was called "unmapped" - it's unmapped on the streams level, but not actually unmapped in the underlying data. I'm sure that will be confusing. I'm not sure whether "unmanaged" is good enough, happy to hear other opinions.
In the processing view, the fields are shown in the same way
It's possible that a mapping isn't compatible with what's defined in the template of a stream. In this case an toast error is shown when the user is trying to save (hopefully it explains well enough what's going on, it's basically what comes from Elasticsearch):
Notes
Discover
I think the types were incomplete here - the boolean flag in question is definitely returned in these objects, that's why adjusted upstream.
Simulation reset too early
A little drive-by fix: If the saving of the processing changes failed, the simulation would have been reset already, which makes us loose the field mappings that are staged there. This PR moves the reset to the success state transition, so everything sticks around on failure so the user can fix and try again.
Rollover logic
If the mapping is changed, the data stream is rolled over.
Error handling
If setting the mapping on the data stream level doesn't work, we bail out. This should trigger a resync with #227738 , which will bring the system into a valid state again. However, this is considered a 500 instead of a 400. By using the dry run in the validation phase we can catch it early, but not sure whether it's worth it... (it also takes longer of course).
Sourcing fields
Fetching the fields for the schema editor, it's now also sourcing them from the field caps along with the definition and unmapped fields API, to give as complete of a picture as possible.
Field simulation
Had to adjust the field simulation logic a bit, I think I actually simplified it successfully so it works the same for wired and classic streams, but worth taking another look.