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

🌊 Streams: Show data retention on stream #204125

Merged
merged 20 commits into from
Jan 9, 2025

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Dec 12, 2024

Show data retention on streams

In case of a policy, the name of the policy is shown (badge is clickable and leads to the edit page of the policy):
Screenshot 2024-12-12 at 20 57 36

In case of DLM, the effect retention is shown:
Screenshot 2024-12-12 at 20 58 42

This is just the display piece, editing retention will be added later on.

This PR adjusts the base streams data stream settings to use a localized data stream retention configuration to make it compatible with serverless.

@flash1293 flash1293 added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.18.0 Feature:Streams This is the label for the Streams Project labels Dec 12, 2024
@flash1293 flash1293 marked this pull request as ready for review December 19, 2024 12:21
@flash1293 flash1293 requested review from a team as code owners December 19, 2024 12:21
@patpscal
Copy link

Wonderful @flash1293 - just one detail now that we are with this piece, can we make it so we use the <EuiBadge color="hollow" ? thanks a lot!

@flash1293
Copy link
Contributor Author

Thanks @patpscal , fixed that

@tonyghiani tonyghiani self-requested a review December 19, 2024 15:39
Comment on lines +9 to +12
export interface IlmLocatorParams extends SerializableRecord {
page: 'policies_list' | 'policy_edit' | 'policy_create';
policyName?: string;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: should this go into src/platform/packages/shared/deeplinks/management as other many other apps hold there the locator params? That would let you revert the visibility change for index-lifecycle-management plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's the right place - it looks like this is just for the top level app routing, not for specific locators within these apps.

As not all management apps are owned by the same team, I would like to keep this in a place owned by stack management as they will probably change the locator and the app itself at the same time.

But happy to go with what @elastic/kibana-management prefers here. If you want to go with a more elaborate solution I can also duplicate and inline the type for now and leave a todo, as it's more of a side quest of this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, the comment was not meant to be a blocker but to avoid switching visibility on a plugin that would then be mistakenly located in the new dir structure. I'll defer to what the Kibana team feels is best for this case, not a blocker on my end.

Copy link
Contributor

@mattkime mattkime Dec 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to avoid changing the visibility of this plugin although at the moment I'm not sure of the best way to do that. Is the locator API a good option?


After a bit more research, I think moving the plugin is the right call. Its probably best to use the locator API but I haven't fully wrapped my head around it.

If the plugin needs to be moved, that could happen in its own PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think there is an easier way - check the latest version of this. There is a shared package under x-pack/platform/packages/shared/index-lifecycle-management already, I moved the type over there.

Copy link
Contributor

@tonyghiani tonyghiani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewise LGTM, there is a minor comment already discussed.

@flash1293 flash1293 requested a review from a team as a code owner December 20, 2024 15:15
…93/kibana into flash1293/streams-data-retention
Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codeowner changes lgtm

@dgieselaar
Copy link
Member

@elasticmachine merge upstream

@tonyghiani
Copy link
Contributor

@flash1293 the change to move the type under x-pack/platform/packages/shared/index-lifecycle-management LGTM, seems the tests are failing cause the lifecycle property is required in the response, but the fixtures used in those tests don't include it, updating those fixtures should fix them

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexLifecycleManagement 241 242 +1
streamsApp 200 203 +3
synthetics 976 977 +1
total +5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/index-lifecycle-management-common-shared 73 76 +3
@kbn/streams-schema 86 88 +2
indexLifecycleManagement 4 0 -4
total +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
streamsApp 190.6KB 191.5KB +922.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
indexLifecycleManagement 27.5KB 27.5KB -51.0B
Unknown metric groups

API count

id before after diff
@kbn/index-lifecycle-management-common-shared 75 78 +3
@kbn/streams-schema 86 88 +2
indexLifecycleManagement 4 0 -4
total +1

History

@flash1293 flash1293 merged commit 3515a0f into elastic:main Jan 9, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12686159610

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- 🌊 Stream overview page (#204079)
- [Streams] Make root stream selectively immutable (#205609)
- [Streams] Dashboard linking (#204309)

Manual backport

To create the backport manually run:

node scripts/backport --pr 204125

Questions ?

Please refer to the Backport tool documentation

Zacqary pushed a commit to Zacqary/kibana that referenced this pull request Jan 9, 2025
Show data retention on streams

In case of a policy, the name of the policy is shown (badge is clickable
and leads to the edit page of the policy):
<img width="524" alt="Screenshot 2024-12-12 at 20 57 36"
src="https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43"
/>

In case of DLM, the effect retention is shown:
<img width="532" alt="Screenshot 2024-12-12 at 20 58 42"
src="https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5"
/>

This is just the display piece, editing retention will be added later
on.

This PR adjusts the base streams data stream settings to use a localized
data stream retention configuration to make it compatible with
serverless.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
@flash1293 flash1293 added backport:version Backport to applied version labels and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jan 13, 2025
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12743131680

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 13, 2025
Show data retention on streams

In case of a policy, the name of the policy is shown (badge is clickable
and leads to the edit page of the policy):
<img width="524" alt="Screenshot 2024-12-12 at 20 57 36"
src="https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43"
/>

In case of DLM, the effect retention is shown:
<img width="532" alt="Screenshot 2024-12-12 at 20 58 42"
src="https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5"
/>

This is just the display piece, editing retention will be added later
on.

This PR adjusts the base streams data stream settings to use a localized
data stream retention configuration to make it compatible with
serverless.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 3515a0f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 13, 2025
# Backport

This will backport the following commits from `main` to `8.x`:
- [🌊 Streams: Show data retention on stream
(#204125)](#204125)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Joe
Reuter","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-09T08:25:19Z","message":"🌊
Streams: Show data retention on stream (#204125)\n\nShow data retention
on streams\r\n\r\nIn case of a policy, the name of the policy is shown
(badge is clickable\r\nand leads to the edit page of the
policy):\r\n<img width=\"524\" alt=\"Screenshot 2024-12-12 at 20 57
36\"\r\nsrc=\"https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43\"\r\n/>\r\n\r\nIn
case of DLM, the effect retention is shown:\r\n<img width=\"532\"
alt=\"Screenshot 2024-12-12 at 20 58
42\"\r\nsrc=\"https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5\"\r\n/>\r\n\r\nThis
is just the display piece, editing retention will be added
later\r\non.\r\n\r\nThis PR adjusts the base streams data stream
settings to use a localized\r\ndata stream retention configuration to
make it compatible
with\r\nserverless.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3515a0f7b8958b11a9065696ac154b56446f7294","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:version","v8.18.0","Feature:Streams"],"title":"🌊
Streams: Show data retention on
stream","number":204125,"url":"https://github.com/elastic/kibana/pull/204125","mergeCommit":{"message":"🌊
Streams: Show data retention on stream (#204125)\n\nShow data retention
on streams\r\n\r\nIn case of a policy, the name of the policy is shown
(badge is clickable\r\nand leads to the edit page of the
policy):\r\n<img width=\"524\" alt=\"Screenshot 2024-12-12 at 20 57
36\"\r\nsrc=\"https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43\"\r\n/>\r\n\r\nIn
case of DLM, the effect retention is shown:\r\n<img width=\"532\"
alt=\"Screenshot 2024-12-12 at 20 58
42\"\r\nsrc=\"https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5\"\r\n/>\r\n\r\nThis
is just the display piece, editing retention will be added
later\r\non.\r\n\r\nThis PR adjusts the base streams data stream
settings to use a localized\r\ndata stream retention configuration to
make it compatible
with\r\nserverless.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3515a0f7b8958b11a9065696ac154b56446f7294"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204125","number":204125,"mergeCommit":{"message":"🌊
Streams: Show data retention on stream (#204125)\n\nShow data retention
on streams\r\n\r\nIn case of a policy, the name of the policy is shown
(badge is clickable\r\nand leads to the edit page of the
policy):\r\n<img width=\"524\" alt=\"Screenshot 2024-12-12 at 20 57
36\"\r\nsrc=\"https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43\"\r\n/>\r\n\r\nIn
case of DLM, the effect retention is shown:\r\n<img width=\"532\"
alt=\"Screenshot 2024-12-12 at 20 58
42\"\r\nsrc=\"https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5\"\r\n/>\r\n\r\nThis
is just the display piece, editing retention will be added
later\r\non.\r\n\r\nThis PR adjusts the base streams data stream
settings to use a localized\r\ndata stream retention configuration to
make it compatible
with\r\nserverless.\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"3515a0f7b8958b11a9065696ac154b56446f7294"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Joe Reuter <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
Show data retention on streams

In case of a policy, the name of the policy is shown (badge is clickable
and leads to the edit page of the policy):
<img width="524" alt="Screenshot 2024-12-12 at 20 57 36"
src="https://github.com/user-attachments/assets/2664b45b-2473-49c4-b1d6-dccb8fe48d43"
/>

In case of DLM, the effect retention is shown:
<img width="532" alt="Screenshot 2024-12-12 at 20 58 42"
src="https://github.com/user-attachments/assets/07ca8086-75e2-45f8-9d71-17bd0a76ebe5"
/>

This is just the display piece, editing retention will be added later
on.

This PR adjusts the base streams data stream settings to use a localized
data stream retention configuration to make it compatible with
serverless.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Streams This is the label for the Streams Project release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants