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

[Index Management] Support data retention on Data Streams tab #165263

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Aug 30, 2023

This PR adds support for viewing the configured data retention value per data stream on the Data Streams tab in Index Management.

On serverless, a data stream can be managed by data stream lifecycle.

On stateful, a data stream can be managed by data stream lifecycle or ILM, or both.

Fixes #165134
Fixes #152720
Fixes #154259
Fixes #165143

Note: Skipping release note for now, however, we will want to add one once the remaining work is complete (meta: #154256)

How to test

  1. Create a data stream managed via data stream lifecycle (docs). The configured data retention should appear on the list view and details flyout.
  2. [Stateful only] Create a data stream managed via ILM (docs). There should be no data retention value. On the details flyout, there should be a link to the ILM policy.
  3. [Stateful only] Create a data stream managed by ILM and data stream lifecycle. The configured data retention should appear on the list view and details flyout. The ILM policy name should also appear on the details flyout.

Screenshots

Screenshot 2023-09-06 at 3 49 54 PM Screenshot 2023-09-06 at 12 52 03 PM Screenshot 2023-09-06 at 12 52 10 PM Screenshot 2023-09-06 at 12 52 15 PM

@jacksonbroshear
Copy link

jacksonbroshear commented Sep 6, 2023 via email

@@ -71,7 +71,6 @@ export const DataStreamTable: React.FunctionComponent<Props> = ({
render: (health: DataStream['health']) => {
return <DataHealth health={health} />;
},
width: '100px',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was causing some weird spacing issues in the table.

interface TimestampFieldFromEs {
name: string;
}

type TimestampField = TimestampFieldFromEs;

interface MetaFromEs {
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 refactored some of the server-side code to leverage the types from @elastic/elasticsearch rather than our own custom interfaces

@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@alisonelizabeth alisonelizabeth marked this pull request as ready for review September 7, 2023 20:05
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner September 7, 2023 20:05
@alisonelizabeth alisonelizabeth added release_note:skip Skip the PR/issue when compiling release notes Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Sep 7, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@alisonelizabeth alisonelizabeth added the ui-copy Review of UI copy with docs team is recommended label Sep 7, 2023
Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on data retention, @alisonelizabeth!
Tested locally and the information is displayed as expected, code changes LGTM too 👍

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

Indices appears twice in the flyout.

Screenshot 2023-09-12 at 2 00 51 PM

<EuiToolTip
content={i18n.translate('xpack.idxMgmt.dataStreamList.table.dataRetentionColumnTooltip', {
defaultMessage:
'The minimum amount of time the data stream will be stored for. Only applicable for data streams managed by data stream lifecycle.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Some suggestions:

The amount of time that a data stream retains its data before it is automatically deleted. Only applies to data streams managed by a data stream lifecycle policy.

The amount of time to keep the data indexed in the data stream. Only applies for data streams managed by a data stream lifecycle.

The amount of time to keep the data stream. Only applies for data streams managed by a data stream lifecycle.

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've learned this value may also not apply to all the data depending on the configuration. From the ES team:

Meaning, they switched from one to the other at some point, so for example: backing indices 1, 2 and 3 are managed by ILM, and backing indices 4 and 5 and any future ones are managed by data stream lifecycle.

Do you think we could work this in as well?

@alisonelizabeth
Copy link
Contributor Author

@gchaps I've addressed most of your feedback in f48949b. Let me know what you think about the final copy for the tooltip in the table. Here's the latest screenshot.

Screenshot 2023-09-14 at 11 41 11 AM

@tylerperk
Copy link

Let me know what you think about the final copy for the tooltip in the table. Here's the latest screenshot.

Screenshot 2023-09-14 at 11 41 11 AM

@alisonelizabeth Something that comes up is how well ILM and now DSL enable timely deletion for compliance reasons (GDPR etc). We don't delete the data exactly after 7 days (in the above example) so we should be careful to not give that impression. I wonder if we could improve the first sentence a little as the first time I read it I felt a bit like it implies a guarantee which we know won't be satisfied. It is more like "the minimum amount of time data will be available before it is automatically deleted" or "data will be be kept at least this long before it is automatically deleted" or ?

@gchaps
Copy link
Contributor

gchaps commented Sep 14, 2023

Here's a suggestion based on @tylerperk's comment:

Data will be be kept at least this long before it is automatically deleted. Only applies to data streams managed by a data stream lifecycle. This value might not apply to all data if the data stream also has an index lifecycle policy.

@alisonelizabeth
Copy link
Contributor Author

Thanks @gchaps! I updated the text in 264952a.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

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
indexManagement 189 181 -8

Async chunks

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

id before after diff
indexManagement 536.8KB 538.1KB +1.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
indexManagement 4 3 -1
Unknown metric groups

API count

id before after diff
indexManagement 194 186 -8

ESLint disabled line counts

id before after diff
indexManagement 14 13 -1

Total ESLint disabled count

id before after diff
indexManagement 18 17 -1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@alisonelizabeth alisonelizabeth merged commit be01217 into elastic:main Sep 18, 2023
@alisonelizabeth alisonelizabeth deleted the index_management/data_stream_lifecycle branch September 18, 2023 16:37
@kibanamachine kibanamachine added v8.11.0 backport:skip This commit does not require backporting labels Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Index Management Index and index templates UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more ui-copy Review of UI copy with docs team is recommended v8.11.0
Projects
None yet
8 participants