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] Show equivalent API requests #139658

Merged
merged 13 commits into from
Sep 15, 2022

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Aug 29, 2022

Summary

That PR add a new experimental feature to Fleet to show equivalent API request to some UI actions, this can be enabled by adding the following to your Kibana config file, the feature is enabled by default but it will be easy to revert if we do not want to ship it.

We use the shared component ViewApiRequestFlyout (used by stack management) to show the request, this allow to copy the requests and open them in the devtools, it's similar from what it's done in the stack managment plugin see bellow

Screen Shot 2022-08-30 at 10 44 57 AM

UI Changes

Screen Shot 2022-09-13 at 3 32 08 PM

Screen Shot 2022-09-13 at 3 31 58 PM

This is the same for create/edit agent policy and package policy

@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team labels Aug 29, 2022
@nchaulet nchaulet self-assigned this Aug 29, 2022
@nchaulet nchaulet added the ci:no-auto-commit Disable auto-committing changes on CI label Aug 29, 2022
@nchaulet nchaulet force-pushed the feature-fleet-show-devtools-request branch 3 times, most recently from e4b66f3 to 7ed62d4 Compare August 30, 2022 14:28
@nchaulet
Copy link
Member Author

@dborodyansky I am curious to have your thoughts on the bottom bar with an extra button

Screen Shot 2022-08-30 at 10 43 03 AM

@nchaulet nchaulet force-pushed the feature-fleet-show-devtools-request branch from 7ed62d4 to 33804e8 Compare August 30, 2022 15:38
@dborodyansky
Copy link
Contributor

Thanks @nchaulet. Is there a document where I can learn more about the product value and rationale for this feature? I'd like to understand the context around UI proximity to the main action on these screens vs. somewhere more subtle if this action is experimental and less anticipated.

On a minor note, we might consider "View API requests" or "Preview API requests" for the label (cc: @dedemorton). Also, spacing around the new button looks a bit tight. Happy to meet and chat about this.

@nchaulet
Copy link
Member Author

Thanks @nchaulet. Is there a document where I can learn more about the product value and rationale for this feature? I'd like to understand the context around UI proximity to the main action on these screens vs. somewhere more subtle if this action is experimental and less anticipated.

No there is no document about that, I am working on that as part of ON week and experimenting with this, I think it could be a good addition in our effort to making Fleet APIs ready for beta #123150

@nchaulet nchaulet marked this pull request as ready for review September 13, 2022 19:37
@nchaulet nchaulet requested a review from a team as a code owner September 13, 2022 19:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet requested a review from kpollich September 13, 2022 19:38

import { FleetErrorType } from './types';

export class IngestManagerError extends Error {
Copy link
Member Author

Choose a reason for hiding this comment

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

I add to move a server side service here, I think it make sense to have errors define here no?


import type { FleetErrorType } from './types';

export class IngestManagerError extends Error {
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be great to rename this error to FleetError, though that might touch many files

Copy link
Member Author

Choose a reason for hiding this comment

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

I will try to come with a separate PR for that (just after that one to avoid conflict) but with typescript this should be pretty easy to rename :)

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM, added a few nits on wording.

@nchaulet nchaulet enabled auto-merge (squash) September 15, 2022 14:40
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet merged commit 6e6b10a into elastic:main Sep 15, 2022
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #48 / alerting api integration security and spaces enabled Alerts - Group 1 alerts disable space_1_all_with_restricted_fixture at space1 should still be able to disable alert when AAD is broken
  • [job] [logs] FTR Configs #13 / endpoint Response Actions Responder from timeline "before all" hook for "should show Responder from alert in a timeline"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 722 728 +6

Async chunks

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

id before after diff
fleet 879.8KB 884.1KB +4.3KB

Page load bundle

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

id before after diff
fleet 107.4KB 107.5KB +95.0B
Unknown metric groups

ESLint disabled in files

id before after diff
fleet 7 8 +1

Total ESLint disabled count

id before after diff
fleet 68 69 +1

History

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

cc @nchaulet

@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 15, 2022
@joshdover joshdover added the QA:Needs Validation Issue needs to be validated by QA label Sep 21, 2022
@ghost
Copy link

ghost commented Sep 27, 2022

Hi Team

  • We have validated this feature on latest 8.5 BC1 Kibana cloud environment and found it working fine.

Build Details:
BUILD: 56595
COMMIT: 0d8de4d

Further we have created 05 testcases for this feature after reviewing under our Fleet Test plan.

Thanks

@ghost ghost added QA:Validated Issue has been validated by QA QA:Needs Validation Issue needs to be validated by QA and removed QA:Needs Validation Issue needs to be validated by QA QA:Validated Issue has been validated by QA labels Sep 27, 2022
@ghost
Copy link

ghost commented Oct 4, 2022

Hi Team,

We have executed 05 testcases for this feature under our Fleet Test run at Fleet 8.5.0-BC2 Feature test plan and found that it's working fine.

Build details:

Version: 8.5.0 BC2
Build: 56806
Commit: dc769f45a5a6dafb0a8c8f0c0cabcced4df45e11

Hence, marking this as QA:Validated.
Thanks!

@ghost ghost added QA:Validated Issue has been validated by QA and removed QA:Needs Validation Issue needs to be validated by QA labels Oct 4, 2022
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 QA:Validated Issue has been validated by QA release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants