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

Add Managed label to data streams and a view switch for the table #83049

Merged
merged 11 commits into from
Nov 18, 2020

Conversation

yuliacech
Copy link
Contributor

@yuliacech yuliacech commented Nov 10, 2020

Summary

This PR adds a 'Managed' label to data streams managed by Ingest Manager. There is also a new 'Include managed' switch, that is enabled by default. When disabled, managed data streams are hidden.

Screenshot

Screenshot 2020-11-19 at 17 47 03

gif

managed

How to test
  1. start Kibana with yarn start --xpack.fleet.enabled=true --no-base-path
  2. start ES with yarn es snapshot -E xpack.security.authc.api_key.enabled=true
  3. Download an Elastic agent for your OS from here and unpack the archive to a folder
  4. In Kibana go to Fleet -> Agents and click the button 'Create user and enable central management'
  5. After that click 'Add agent' button and in the flyout look for the enrolment token in section 3. (but don't copy the command, it doesn't work)
  6. In the folder where you unpacked downloaded Elastic agent, run ./elastic-agent enroll http://localhost:5601 $ENROLMENT_TOKEN --insecure
  7. In the folder where you unpacked downloaded Elastic agent, run ./elastic-agent run and keep this terminal tab open.
  8. You should see data streams in Fleet and in Index Management.

Checklist

Delete any items that are not applicable to this PR.

Release Note

In the Index Management app, data streams managed by Fleet can now be easily identified by a 'Managed' label.

@yuliacech yuliacech added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11 v8.0.0 release_note:enhancement labels Nov 10, 2020
@yuliacech yuliacech marked this pull request as ready for review November 10, 2020 17:20
@yuliacech yuliacech requested a review from a team as a code owner November 10, 2020 17:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@yuliacech
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great work @yuliacech! TIL how to set up Ingest Manager 😄.

Tested locally and everything worked as expected. I left a few comments, but nothing blocking.


export const filterDataStreams = (dataStreams: DataStream[], managed: boolean): DataStream[] => {
return dataStreams.filter((dataStream: DataStream) =>
managed ? isManagedByIngestManager(dataStream) : !isManagedByIngestManager(dataStream)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little hard for me to parse. If I'm understanding correctly, it looks like managed is only ever going to be false.

const filteredDataStreams = isIncludeManagedChecked
      ? dataStreams
      : filterDataStreams(dataStreams, isIncludeManagedChecked);

If that's correct, could the filter be simplified to this?

return dataStreams.filter((dataStream: DataStream) => isManagedByIngestManager(dataStream) === false)

>
{name}
</EuiLink>
<Fragment>
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) you can use shorthand <></> for fragments.

<EuiFlexItem grow={false}>
<EuiSwitch
label={i18n.translate('xpack.idxMgmt.dataStreamListControls.viewManagedSwitchLabel', {
defaultMessage: 'View managed',
Copy link
Contributor

Choose a reason for hiding this comment

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

Screen Shot 2020-11-12 at 1 03 02 PM

WDYT about changing this to "Include managed" to align with the other toggle?

{isManagedByIngestManager(dataStream) ? (
<Fragment>
&nbsp;
<EuiBadge color="hollow">
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding a tooltip or title to the badge to explain that "managed" means managed by Ingest Manager, specifically? Or maybe add this to the switch?

@spalger spalger added v7.11.0 and removed v7.11 labels Nov 12, 2020
@yuliacech
Copy link
Contributor Author

@elasticmachine merge upstream

@yuliacech
Copy link
Contributor Author

Thank you for the review, @alisonelizabeth ! I renamed the switch and added some tooltips as you suggested :)

@yuliacech yuliacech requested a review from esdocs November 16, 2020 14:14
@yuliacech
Copy link
Contributor Author

Hello @esdocs , could you please have a look at this PR's wording? I would appreciate some feedback on tooltips I added to the "Managed" label and to the "Include managed" switch (see screenshots in PR description). I'm not sure if I should use "Ingest Manager" or "Fleet" since the plugin name's change.

@jrodewig jrodewig requested review from jrodewig and removed request for esdocs November 17, 2020 18:09
Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

Thanks for reaching out about this PR @yuliacech. At the least, I think we'll need to replace Ingest Manager with Fleet throughout. However, I also think the current copy places too much emphasis on "manage" and may confuse ILM users.

What we're really trying to communicate here is that these are internal system data streams that the user generally doesn't need to view or touch. We may not even need to point out that they're managed by Fleet.

EDIT: I was mistaken about the purpose of these managed streams. I'll submit another review.

I left some related suggestions for the copy. I'd also like to get @dedemorton's approval before merging. She's been writing the Fleet docs and is most familiar with the current terms.

@jrodewig jrodewig requested a review from dedemorton November 17, 2020 19:08
@cjcenizal
Copy link
Contributor

@jrodewig I'm a little confused by this:

What we're really trying to communicate here is that these are internal system data streams that the user generally doesn't need to view or touch. We may not even need to point out that they're managed by Fleet.

My understanding of the term "system" is informed by my understanding of our original dot-prefixed system indices, like .kibana, .watches, and .security, which are being replaced by the formalized Elasticsearch system indices. These indices match your description of "internal" data that the user shouldn't be concerned with. Our Stack is responsible for managing them, not the user.

However, my understanding of Fleet data streams is that the user is concerned with them. These are created by the user when the user adds a new integration via Fleet, they store data ingested by the user's Elastic Agent, and the user will depend on visualizations and dashboards that consume this data. If the user wants to finesse storage costs, they'll fiddle with SLM and ILM to back up this data and move it to cheaper storage. Given this definition of Fleet's data streams, it seems useful to differentiate them from true system indices (and system data streams, once those exist).

So I'm sensing a gap between the way we're seeing things, and I'm wondering if I'm mistaken in my understanding of things. Can you help me understand your perspective so we can bridge the gap?

@jrodewig
Copy link
Contributor

jrodewig commented Nov 17, 2020

Hey @cjcenizal! Thanks for commenting. I'm likely wrong here. My main confusion arose from the -default streams vs streams like metric-system. But peaking at the data in those streams, you seem to be right. I'm a relative newbie to Elastic Agent, so my fault for not reaching out before submitting a review. Apologies.

I'll resolve my suggestions and submit a new review. However, I'll leave my original comments up for context. Thanks again!

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

LGTM. Apologies for my earlier misunderstanding. I left some terminology-related suggestions, but I'd still like approval from @dedemorton before merging. Thanks!

@yuliacech
Copy link
Contributor Author

@elasticmachine merge upstream

kibanamachine and others added 4 commits November 18, 2020 06:00
…me/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>
…me/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>
…me/data_stream_list/data_stream_table/data_stream_table.tsx

Co-authored-by: James Rodewig <[email protected]>
Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

Text strings LGTM.

@yuliacech yuliacech merged commit 47d6612 into elastic:master Nov 18, 2020
yuliacech added a commit to yuliacech/kibana that referenced this pull request Nov 19, 2020
…astic#83049)

* Add Managed label to data streams and a view switch for the table

* Fix i18n errors

* Updated some wording and made filter function easier (managed data streams)

* Update x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts

Co-authored-by: Alison Goryachev <[email protected]>

* Renamed view to include (managed data streams)

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx

Co-authored-by: James Rodewig <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
yuliacech added a commit that referenced this pull request Nov 19, 2020
…3049) (#83778)

* Add Managed label to data streams and a view switch for the table

* Fix i18n errors

* Updated some wording and made filter function easier (managed data streams)

* Update x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts

Co-authored-by: Alison Goryachev <[email protected]>

* Renamed view to include (managed data streams)

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx

Co-authored-by: James Rodewig <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>
Co-authored-by: James Rodewig <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 19, 2020
* master: (60 commits)
  Forward any registry cache-control header for files (elastic#83680)
  Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)"
  [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722)
  Make expectSnapshot available in all functional test runs (elastic#82932)
  Skip failing cypress test
  Increase bulk request timeout during esArchiver load (elastic#83657)
  [data.search] Server-side background session service (elastic#81099)
  [maps] convert VectorStyleEditor to TS (elastic#83582)
  Revert "[App Search] Engine overview layout stub (elastic#83504)"
  Adding documentation for global action configuration options (elastic#83557)
  [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596)
  chore(NA): update lmdb store to v0.8.15 (elastic#83726)
  [App Search] Engine overview layout stub (elastic#83504)
  [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714)
  [Enterprise Search] Rename React Router helpers (elastic#83718)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 19, 2020
* master: (60 commits)
  Forward any registry cache-control header for files (elastic#83680)
  Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)"
  [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722)
  Make expectSnapshot available in all functional test runs (elastic#82932)
  Skip failing cypress test
  Increase bulk request timeout during esArchiver load (elastic#83657)
  [data.search] Server-side background session service (elastic#81099)
  [maps] convert VectorStyleEditor to TS (elastic#83582)
  Revert "[App Search] Engine overview layout stub (elastic#83504)"
  Adding documentation for global action configuration options (elastic#83557)
  [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596)
  chore(NA): update lmdb store to v0.8.15 (elastic#83726)
  [App Search] Engine overview layout stub (elastic#83504)
  [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714)
  [Enterprise Search] Rename React Router helpers (elastic#83718)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  ...
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 19, 2020
… into add-logs-to-node-details

* 'add-logs-to-node-details' of github.com:phillipb/kibana: (87 commits)
  [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463)
  Updating code-owners to use new core/app-services team names (elastic#83731)
  Add Managed label to data streams and a view switch for the table (elastic#83049)
  [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871)
  fix(NA): search examples kibana version declaration (elastic#83182)
  Fixed console error, which appears when saving changes in Edit Alert flyout (elastic#83610)
  [Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)
  Not resetting server log level if level is defined (elastic#83651)
  disable incremenetal build for legacy tsconfig.json (elastic#82986)
  [Workplace Search] Migrate SourceLogic from ent-search (elastic#83593)
  [Workplace Search] Port Box changes from ent-search (elastic#83675)
  [APM] Improve router types (elastic#83620)
  Bump flat to v4.1.1 (elastic#83647)
  Bump y18n@5 to v5.0.5 (elastic#83644)
  Bump jsonpointer to v4.1.0 (elastic#83641)
  Bump is-my-json-valid to v2.20.5 (elastic#83642)
  [Telemetry] Move Monitoring collection strategy to a collector (elastic#82638)
  Update typescript eslint to v4.8 (elastic#83520)
  [ML] Persist URL state for Anomaly detection jobs using metric function (elastic#83507)
  [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (elastic#83216)
  ...
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 19, 2020
…astic#83049)

* Add Managed label to data streams and a view switch for the table

* Fix i18n errors

* Updated some wording and made filter function easier (managed data streams)

* Update x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts

Co-authored-by: Alison Goryachev <[email protected]>

* Renamed view to include (managed data streams)

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_list.tsx

Co-authored-by: James Rodewig <[email protected]>

* Update x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx

Co-authored-by: James Rodewig <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Alison Goryachev <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
indexManagement 511 512 +1

Async chunks

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

id before after diff
indexManagement 1.5MB 1.5MB +2.7KB

Page load bundle

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

id before after diff
indexManagement 114.1KB 114.2KB +41.0B

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Index Management Index and index templates UI release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants