[Streams] Create Import from other Stream flow#275948
Conversation
There was a problem hiding this comment.
One reliability concern on the ILM policy fetch effect (potential infinite refetch loop when the cluster has no ILM policies or the request fails) — details inline. Otherwise the flow is well-structured and the pure helpers have good unit coverage.
Generated by Claude Reviewer for issue #275948 · 101.7 AIC · ⌖ 10 AIC · ⊞ 3.9K
03783b7 to
cd71610
Compare
cd71610 to
1f7234a
Compare
ElenaStoeva
left a comment
There was a problem hiding this comment.
Great work overall @SoniaSanzV! I tested locally and the UI works well. I have a few concerns/questions:
- On
logs.import-standard-target, importinglogs.import-tsdb-source-ilm-downsampleshows the warning “downsampling steps from the selected ILM policy will be excluded,” but after applying, the target is assignedlogs-import-ilm-downsample-policyunchanged. That policy still containswarm.actions.downsample, and ES reports it in use by the target stream's backing index.
Code-wise,getImportedLifecycle()always returns{ ilm: { policy } }for ILM sources regardless oftargetIsTimeSeries, so there is no exclusion step for ILM policies. We could either block/down-rank ILM policies with downsampling for standard targets, or change the warning/copy to make clear the policy is applied as-is and may be invalid for non-TSDB streams.
Screen.Recording.2026-07-13.at.23.01.54.mov
- After I apply an import, I see the value in the Lifecycle summary section flickering:
Screen.Recording.2026-07-13.at.18.35.15.mov
-
Can you clarify how you create the role for testing with a user that shouldn't see the import action? I created a user that can manage ilm but not failure store, but this doesn't display the stream details at all, so maybe I'm missing something.
-
I see there is a backport label for version 9.5.0 but the FF was last week so not sure if we can merge this for 9.5 - was this the plan?
|
Thank you for the feedback @ElenaStoeva!!
this is a wording issue. The policy won't be modified when applying, but the downsampling won't have any effect on that Stream. I'll consult with docs a better copy. But this callout was introduced before with the component itself so i'll handle that outside this PR
This is an issue that already existed and that we need to address, but it is not within the scope of this PR. I'm going to try to add this to this PR to improve transitions in general #277149
I used: PUT _security/role/streams_lifecycle_no_failure_store
{
"cluster": [
"monitor",
"read_ilm",
"manage_index_templates",
"manage_ingest_pipelines",
"manage_pipeline",
"read_pipeline",
"monitor_text_structure"
],
"indices": [
{
"names": ["logs*"],
"privileges": [
"read",
"monitor",
"view_index_metadata",
"manage_data_stream_lifecycle",
"manage_ilm"
]
}
]
}
POST _security/user/streams_test_user
{
"password": "changeme",
"roles": ["streams_lifecycle_no_failure_store", "kibana_admin"]"
}I also made some privileged improvement here c546c95
Yes, this was meant to go into |
ElenaStoeva
left a comment
There was a problem hiding this comment.
Thanks for addressing my feedback @SoniaSanzV! Updating the ILM downsampling warning copy sounds good to me - I'll leave this up to you to update in this PR or in a follow-up PR if you want to merge this first.
Regarding the backport label, let's first check if there are more BC's for 9.5.0 and if it's okay to merge this as is, given that this is a feature PR and usually we should only merge fixes after FF.
Code changes lgtm. Just left a nit about one of the tests but approving to unblock the PR.
| import type { IlmPolicyForFlyout } from '@kbn/data-lifecycle-phases'; | ||
| import { getImportedLifecycle, sourceHasDownsampling } from './get_imported_lifecycle'; | ||
|
|
||
| describe('getImportedLifecycle', () => { |
There was a problem hiding this comment.
nit: Could we add a focused test that documents the intended ILM behavior for non-time-series targets? The DSL path has coverage for dropping downsample when targetIsTimeSeries is false, but the ILM path intentionally keeps only the policy reference unchanged. A test name like “keeps ILM policy references unchanged for non-time-series targets” would make that distinction explicit and protect the copy/behavior contract.
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
cc @SoniaSanzV |
|
Starting backport for target branches: 9.5 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
3 similar comments
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Starting backport for target branches: 9.5 |
|
Starting backport for target branches: 9.5 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
1 similar comment
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Closes #267913
Summary
read_failure_storeare excluded from import candidates.Test plan
Run this setup in Dev Tools as an admin user:
Then:
logs.import-tsdb-target.logs.import-tsdb-source-downsampleand verify the import option shows the downsampling summary.logs.import-tsdb-source-ilm-downsample, inspect the ILM policy, and verify the policy includes a downsample action.logs-import-ilm-downsample-policy.logs.import-standard-target, import fromlogs.import-tsdb-source-downsample, and verify downsampling is omitted for the non-time-series target.manage_failure_store; verifyImport from another streamis not shown.Evidence
Screen.Recording.2026-07-10.at.12.50.01.mov
Release note
Adds the ability to import another stream's lifecycle configuration from the Streams data lifecycle tab.