-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[IM] Address index templates feedback #71353
[IM] Address index templates feedback #71353
Conversation
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
💚 Build SucceededBuild metrics
To update your PR or re-run it, just comment with: |
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.
Code and screenshot LGTM! Didn't test locally.
…11y-overlay * 'master' of github.com:elastic/kibana: (33 commits) address index templates feedback (elastic#71353) Upgrade EUI to v26.3.1 (elastic#70243) [build] Creates Linux aarch64 archive (elastic#69165) [SIEM][Detection Engine] Fixes skipped tests (elastic#71347) [SIEM][Detection Engine][Lists] Adds read_privileges route for lists and list items [kbn/optimizer] implement "requiredBundles" property of KP plugins (elastic#70911) [Security Solution][Exceptions] - Exceptions modal pt 2 (elastic#70886) [ML] DF Analytics: stop status polling when job stopped (elastic#71159) [SIEM][CASE] IBM Resilient Connector (elastic#66385) jenkins_xpack_saved_objects_field_metrics.sh expects to be run from the KIBANA_DIR in CI Deduplication of entries and items before sending to endpoint (elastic#71297) [services/remote/webdriver] fix eslint error (elastic#71346) send slack notifications on visual baseline failures fix visual regression job (elastic#70999) [Ingest Manager] Add schema to usageCollector. (elastic#71219) [ftr] use typed chromeOptions object, adding TEST_BROWSER_BINARY_PATH (elastic#71279) [Ingest Manager] Fix limited packages incorrect response (elastic#71292) Support multiple features declaring same properties (elastic#71106) [Security_Solution][Resolver]Add beta badge to Resolver panel (elastic#71183) [DOCS] Clarify trial subscription levels (elastic#70636) ...
I do find it quite confusing for the user:
Index templates never create indices, so the sentence is confusing to me. @alisonelizabeth @cjcenizal |
* master: (78 commits) Bump lodash package version (elastic#71392) refactor: 💡 use allow-list in AppArch codebase (elastic#71400) improve bugfix 7198 test stability (elastic#71250) [Security Solution][Ingest Manager][Endpoint] Optional ingest manager (elastic#71198) [Metrics UI] Round metric threshold time buckets to nearest unit (elastic#71172) [Security Solution][Endpoint] Policy creation callback fixes + Improved error handling in user manifest loop (elastic#71269) [Security Solution] Allow to configure Event Renderers settings (elastic#69693) Fix a11y keyboard overlay (elastic#71214) [APM] UI text updates (elastic#71333) [Logs UI] Limit `extendDatemath` to valid ranges (elastic#71113) [SIEM] fix tooltip of notes (elastic#71342) address index templates feedback (elastic#71353) Upgrade EUI to v26.3.1 (elastic#70243) [build] Creates Linux aarch64 archive (elastic#69165) [SIEM][Detection Engine] Fixes skipped tests (elastic#71347) [SIEM][Detection Engine][Lists] Adds read_privileges route for lists and list items [kbn/optimizer] implement "requiredBundles" property of KP plugins (elastic#70911) [Security Solution][Exceptions] - Exceptions modal pt 2 (elastic#70886) [ML] DF Analytics: stop status polling when job stopped (elastic#71159) [SIEM][CASE] IBM Resilient Connector (elastic#66385) ...
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.
I left a suggestion regarding the text here. I think its important to emphasize that data stream templates are exclusive to data streams.
However, I don't know if we want to do that by comparing them to indices. Backing indices make that a little confusing.
@@ -62,20 +62,21 @@ function getFieldsMeta(esDocsBase: string) { | |||
description: ( | |||
<FormattedMessage | |||
id="xpack.idxMgmt.templateForm.stepLogistics.dataStreamDescription" | |||
defaultMessage="Wheter indices that match the index patterns should automatically create a data stream. {docsLink}" | |||
defaultMessage="Create a data stream instead of an index. {docsLink}" |
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.
How about:
Use the template for data streams and backing indices only.
There are a couple of problems with the current text:
- You're not creating a data stream. You're creating a template that is then used to create a data stream.
- A single template can be used to create multiple data streams.
I don't think instead of an index
is accurate either. The template is used to create the data stream and its backing indices.
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.
These are really good points! A couple things bother me and I wonder what you think of them.
One thing is the wording of “Use the template...” The way this is phrased, it sounds like the UI is telling the user to use the template to create data streams. I think this sounds like the UI is identifying some sort of restriction to this template. It leads me to wonder, “OK I have to use it to create data streams — how do I do that?” Is there a way to rephrased this to describe the template’s behavior? This is what I was trying to do (poorly and without much thought) with the “Create...” phrasing. Can we improve on that without losing the behavior aspect?
The other thing that niggles in the back of my head is the mention of backing indices. I don’t think we should expose this implementation detail in such a high-level UI. I can see us exposing it in index management (and we do) but in this UI it just seems like an unhelpful distraction. Am I missing something? Do you think it’s helpful here? With my original copy I was trying to draw a clearer distinction between the default index-oriented behavior and the new data stream behavior. Do you think that’s helpful to the user and if so, is there a way we can reword it to emphasize that distinction?
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.
Thanks @cjcenizal. Good points!
Would something like this work?
The template creates data streams only.
OR
The template is only used to create data streams.
(passive voice)
OR
The template is only used for data streams.
(passive voice)
That should clear up confusion re: imperative voice. And you're likely right re: backing indices. If users are interested in more, they can read up on it using the docs link. Thanks for iterating on this!
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.
I think these sound good, @jrodewig! Is there room for confusion with the word "only"? Is it clear what this implies? Or can we be explicit by contrasting this against the alternative behavior, e.g. "The template creates data streams instead of..."?
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 template creates data streams instead of an index.
works for me @cjcenizal. Thanks again!
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.
Thanks for working through this @cjcenizal and @jrodewig! I opened #71615 with the updated copy.
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.
I personally think there is still some confusion.
An index template does not create indices. As per the docs:
An index template is a way to tell Elasticsearch how to configure an index when it is created. Templates are configured prior to index creation and then when an index is created either manually or through indexing a document, the template settings are used as a basis for creating the index.
So the template does not create the index but is used as a set of configuration that is ready to be applied on newly created indices.
I think the confusion comes from the fact that we ask a user to ingest to an index to be able to create a data stream instead of having a dedicated API. And here we try to explain that indirection.
Maybe I got it wrong but: isn't the composable index template configuration applied to the backing indices?
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.
An index template does not create indices.
That correct. An index template does not create indices or data streams. The template is used to
create indices/data streams by configuring them.
A matching index template is required to create a data stream. This template is used to configure the stream's backing indices.
However, as CJ pointed out, backing indices are an implementation detail we don't want to raise here. It's simpler for users to think of the template as a way to create a data stream. This also allows us to contrast data streams with regular indices.
I think the confusion comes from the fact that we ask a user to ingest to an index to be able to create a data stream instead of having a dedicated API. And here we try to explain that indirection.
Ingest isn't required to create a data stream, but that is the most common use case.
You can create an empty data stream using the create data stream API:
https://www.elastic.co/guide/en/elasticsearch/reference/master/indices-create-data-stream.html
However, a matching index template is still required. Creating a data stream, even an empty one, also creates a backing index that's configured based on the template.
Maybe I got it wrong but: isn't the composable index template configuration applied to the backing indices?
That's correct. However, as CJ pointed out, backing indices are an implementation detail we don't want to raise at this level. If we abstract away backing indices, users can think of the template as something used to create data streams.
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.
Thanks for the explanation @jrodewig ! I didn't know about the PUT _data_stream/<name>
API.
I will get my head around it, but I think as a consumer I would have preferred a dedicated API to ingest into (and manage) this new data source.
POST _data_stream/_doc
POST _data_stream/_bulk
And here in the UI we would have simply said: "Use this template for data streams.".
This would have simplified a lot my mental model. I realize that this would have implied a lot of duplicated APIs (e.g. PUT _data_stream/my-data-stream/_mapping
). But being explicit sometimes helps.
This PR addresses #71065 (review) and #71065 (review).