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

[fleet] Add Integration Preference selector #114432

Merged

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Oct 11, 2021

Summary

This PR adds the Integration Preference selector to the Available Packages page.

Screen Shot 2021-10-12 at 8 46 56 PM

Oct-12-2021.20-37-57.mp4

It also updates the Storybook mocks to make the tests make sense.

@clintandrewhall clintandrewhall requested a review from a team as a code owner October 11, 2021 05:08
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@clintandrewhall clintandrewhall added auto-backport Deprecated - use backport:version if exact versions are needed Feature:Unified Integrations Unified Integrations view feature impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes review v7.16.0 v8.0.0 labels Oct 11, 2021
@clintandrewhall clintandrewhall changed the title [nit] Strongly-type Shipper and Category [fleet] Add Integration Preference selector Oct 11, 2021
@thomasneirynck
Copy link
Contributor

Discussed offline with @clintandrewhall and @joshdover . Before writing the actual filtering logic on the on-change event, should wait on #114429 to merge first, as that one makes changes to how cards/categories are handled client-side. Otherwise, we'd encounter merge conflicts on either end.

+1 on UX

@alexfrancoeur
Copy link

alexfrancoeur commented Oct 12, 2021

A question around logic came up for this work. I'll describe how I've been thinking about this logic, but will want @mostlyjason and @dborodyansky to confirm before we move forward with it.

Elastic Agent (recommended) is selected

Input Output
A beats integration is available with no agent equivalent Show beats integration
A beats integration is available with a beta or experimental agent equivalent Show beats integration
A beats integration is available with a GA agent equivalent Show agent integration
An agent integration is available with no equivalent beats integration Show agent integration

Beats is selected

Input Output
A beats integration is available with no agent equivalent Show beats integration
A beats integration is available with a beta or experimental agent equivalent Show beats integration
A beats integration is available with a GA agent equivalent Show beats integration
An agent integration is available with no equivalent beats integration Show agent integration

@clintandrewhall
Copy link
Contributor Author

clintandrewhall commented Oct 12, 2021

@alexfrancoeur Thanks for that table. I'm going to copy and restate it to be perfectly clear:

The logic for this change will be to hide cards. The implementation has all possibilities available, and we'll be filtering out some possibilities based on this radio button.

With that in mind, here's your table:

Elastic Agent (recommended) is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration
A beats integration is available with a GA agent equivalent Remove beats integration
An agent integration is available with no equivalent beats integration Remove beats integration

Beats is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration ... do we remove the agent integration?
A beats integration is available with a GA agent equivalent Keep beats integration ... do we remove the agent integration?
An agent integration is available with no equivalent beats integration Keep agent integration

Follow up: is there ever a time when everything is displayed? Should we consider a "display all" option?

@alexfrancoeur
Copy link

The logic for this change will be to hide cards. The implementation has all possibilities available, and we'll be filtering out some possibilities based on this radio button.

Yes, the logic would be to hide the cards. Based on the radio button, we'll be removing beats or agent cards.

Follow up: is there ever a time when everything is displayed? Should we consider a "display all" option?

We did explore this option early on but decided against it to avoid duplicative cards. The original purpose of this filter was to provide the user who knows they want beats, with a quick option to only show them.

I've updated the table with your language to map the logic I believe we want. @mostlyjason what do you think about the above answers and below logic?

Elastic Agent (recommended) is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration
A beats integration is available with a GA agent equivalent Remove beats integration
An agent integration is available with no equivalent beats integration Remove beats integration

Beats is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration and remove the agent integration
A beats integration is available with a GA agent equivalent Keep beats integration and remove the agent integration
An agent integration is available with no equivalent beats integration Keep agent integration

@alexfrancoeur
Copy link

alexfrancoeur commented Oct 12, 2021

It was a bit late last night when I updated that table, but I think this there is a clarifying point I missed when the Elastic Agent option is selected. Updating the table accordingly. The only real difference between the two is for the A beats integration is available with a GA agent equivalent state. I am pretty confident this is the behavior we agreed upon, but we'll want to wait for @mostlyjason to chime in this morning. In both of these scenarios, what's not easily discoverable is experimental agent integrations that have a beats equivalent - and that is by design.

Elastic Agent (recommended) is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration and remove the agent integration
A beats integration is available with a GA agent equivalent Remove beats integration
An agent integration is available with no equivalent beats integration Keep agent integration

Beats is selected

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration and remove the agent integration
A beats integration is available with a GA agent equivalent Keep beats integration and remove the agent integration
An agent integration is available with no equivalent beats integration Keep agent integration

@mostlyjason
Copy link
Contributor

Thanks @alexfrancoeur that table looks good to me!

@alexfrancoeur
Copy link

alexfrancoeur commented Oct 12, 2021

After some amazing collaboration and super fast decision making in slack, we have landed on the following logic.

There will now be three new filter options to replace the first two.

  • Recommended (default)
  • Elastic Agent only
  • Beats only

Each one of these options could benefit from a tooltip. I'm adding some example text to each option below, but could benefit from @gchaps and the teams thoughts and feedback on messaging.

Recommended (default) is selected

Tooltip

Filter on Elastic recommended integrations. Production ready integrations will be recommended over beta and experimental.

Logic

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration and remove the agent integration
A beats integration is available with a GA agent equivalent Remove beats integration
An agent integration is available with no equivalent beats integration Keep agent integration

Elastic Agent only is selected

Tooltip

Filter on all Elastic Agent integrations.

Logic

Input Output
A beats integration is available with no agent equivalent Remove beats integration
A beats integration is available with a beta or experimental agent equivalent Keep agent integration and remove the beats integration
A beats integration is available with a GA agent equivalent Keep agent integration and remove the beats integration
An agent integration is available with no equivalent beats integration Keep agent integration

Beats only is selected

Tooltip

Filter on all Beats integrations.

Logic

Input Output
A beats integration is available with no agent equivalent Keep beats integration
A beats integration is available with a beta or experimental agent equivalent Keep beats integration and remove the agent integration
A beats integration is available with a GA agent equivalent Keep beats integration and remove the agent integration
An agent integration is available with no equivalent beats integration Remove agent integration

@dborodyansky
Copy link
Contributor

Updated design prototype, though I would recommend a slight tweak. Tooltips for Elastic Agent and Beats as stated don't seem to offer any additional information, and we do offer a docs link to explain and compare the shippers already. Also "Filter on..." text seems unnecessary.

Proposed design change:

pref

@clintandrewhall
Copy link
Contributor Author

I'll make these changes right away.

@clintandrewhall clintandrewhall force-pushed the fleet/integration-preference branch from 87d5b6d to bbe866d Compare October 13, 2021 01:45
@clintandrewhall clintandrewhall force-pushed the fleet/integration-preference branch from bbe866d to dbaa8ab Compare October 13, 2021 01:47
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 578 579 +1

Async chunks

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

id before after diff
fleet 626.1KB 627.9KB +1.8KB

History

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

@clintandrewhall clintandrewhall enabled auto-merge (squash) October 13, 2021 03:52
@joshdover
Copy link
Contributor

Tested locally and the filtering functionality matches what we agreed upon above 👍

This doesn't currently persist the preference anywhere, which we can address as a follow up. @alexfrancoeur @mostlyjason do we consider persisting this setting to localStorage as a "must have" for this release?

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Tested functionality on the design side. LGTM. OK skipping persistence given our timing.

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Functionality and code LGTM

@clintandrewhall clintandrewhall merged commit 8068eec into elastic:master Oct 13, 2021
@mostlyjason
Copy link
Contributor

Yeah I think persistence could be a nice to have since users can accomplish their goal to filter with a few extra clicks.

I just tested the logic and I noticed that "j. Experimental, has Beats" shows up under the version control and web categories if I select "Elastic Agent only" mode but not under "Recommended" mode. I thought that was odd but not necessarily wrong. We should double check the categories once we've hooked it up to real data.

@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 114432

@alexfrancoeur
Copy link

Looks like there is an issue with the backport. @clintandrewhall is out. @joshdover or @thomasneirynck do you think this is something one of you could help with?

@snide
Copy link
Contributor

snide commented Oct 13, 2021

@alexfrancoeur @thomasneirynck @joshdover I got the backport.

@thomasneirynck
Copy link
Contributor

discussed offline with @snide . There are a few failing backports from yesterday, that are backing this up.

#114429 (comment) -> #114431

will investigate

jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 13, 2021
…ide-users-to-saving-ux

* 'master' of github.com:elastic/kibana: (133 commits)
  [DOCS] Indicate reports are a subscription feature (elastic#114653)
  Update namespace for indices (elastic#114612)
  [DOCS] Adds Logstash pipeline settings (elastic#114648)
  Bump EPR snapshot version used for tests (elastic#114529)
  [Security Solution] [Endpoint] Fleet summary card adjustments (elastic#114291)
  skip flaky suite (elastic#68400)
  [Visualizations] fix usage of optional dependencies (elastic#114286)
  [Security Solution] [Detections] Improves custom query rule upgrade test (elastic#114454)
  [fleet] Add Integration Preference selector (elastic#114432)
  [Reporting] Add new `data-render-error` attribute (elastic#114472)
  Replace EuiCodeEditor with CodeEditor in app-services code (elastic#114316)
  [data views] add getDefaultDataView method  (elastic#113891)
  [Security Solution] [Endpoint] Event filters uses the new card design (elastic#114126)
  [fleet] Tweak Header UI (elastic#114704)
  [APM] Filter on tx metrics for instance stats (elastic#114758)
  [APM] Fix typo in linting docs (elastic#114764)
  [Discover] Removing SavedObject usage for savedSearch (elastic#112983)
  [Fleet] Add Integration Policy Page Improvements (elastic#114556)
  [Lens] Keep the custom label when transitioning to/from Formula (elastic#114270)
  [Security Solution][Endpoint] Host Isolation API changes (elastic#113621)
  ...
@thomasneirynck
Copy link
Contributor

Working through the chain: backport of this one blocked by #114870. When that one merges, we should be able to backport this one cleanly.

@hop-dev
Copy link
Contributor

hop-dev commented Oct 26, 2021

Hi @clintandrewhall we have had this issue raised asking where we intend the Elastic agent and beats link to navigate to, I can't see this discussed anywhere or in the designs?

CC @dborodyansky @dedemorton maybe?

@dedemorton
Copy link
Contributor

@hop-dev A single link is going to be a bit awkward because beats and agent are currently documented in different “books. There are a couple of links that could work:

https://www.elastic.co/guide/en/fleet/7.16/beats-agent-comparison.html (Describes difference between agent and beats.)

https://www.elastic.co/guide/en/fleet/7.16/integrations.html

(I’m on PTO this week so please defer to @gchaps

@alexfrancoeur
Copy link

I believe the original intent was to link to https://www.elastic.co/guide/en/fleet/7.16/beats-agent-comparison.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Unified Integrations Unified Integrations view feature impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Fleet Team label for Observability Data Collection Fleet team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.