Skip to content

Commit

Permalink
[Logs+] Create an integration while on-boarding logs (#163219)
Browse files Browse the repository at this point in the history
## Summary

This closes #161960, a basic
integration will now be created whilst onboarding logs (though the
custom logs flow).

This implements the *initial* version of this work, and does not include
things like adding a dataset to an existing integration.

## UI / UX

General:

![Screenshot 2023-08-07 at 15 20
21](https://github.com/elastic/kibana/assets/471693/3ca4e300-41c3-4554-a095-0f3dcf9e9523)

Naming conflict errors:

![Screenshot 2023-08-11 at 13 34
45](https://github.com/elastic/kibana/assets/471693/2a138eac-73e2-4cc9-b1e8-56c586b852ee)

![Screenshot 2023-08-11 at 13 34
59](https://github.com/elastic/kibana/assets/471693/6e651de9-debd-46aa-a3d5-2b6eb4e3bb4f)

Lack of permissions error:

![Screenshot 2023-08-09 at 17 10
35](https://github.com/elastic/kibana/assets/471693/d47b40c8-fe4a-4b86-abf8-d8fda51515fd)

General errors:

![Screenshot 2023-08-07 at 16 49
40](https://github.com/elastic/kibana/assets/471693/346c28d0-ec3e-4f7e-ae16-3f1adf440c21)

Success callout on the next panel:

![Screenshot 2023-08-07 at 17 20
45](https://github.com/elastic/kibana/assets/471693/03e78e45-871b-4224-9999-5b3d7e2ccdf0)

Delete previous flow (happens in the background):


![delete_process](https://github.com/elastic/kibana/assets/471693/44c18793-9df7-4228-b351-5668f098e138)


## Pointers for reviewers  / next steps

- This PR also creates a new package for the `useTrackedPromise` hook,
as this is used in several places and I didn't want to just duplicate it
again (I haven't replaced other current uses in this PR, but will as a
followup).

- `useFetcher` was avoided as A) it's very tightly coupled with the
observability onboarding server route repository (and `callApi` is
scoped to this) and I wanted to call an "external" API in Fleet and B) I
wanted explicit control over when the request is dispatched (not on
mount), and whilst this can sort of be achieved by not returning a
promise from the callback it gets quite messy. I also wanted more
granular error handling control.

- Moving forward I think we'll need to enhance the state management of
the plugin. We'll want to add the ability to "add to existing
integration" and this is going to make the state more complex (even with
chunks of this functionality likely moved to it's own package). I did
actually have the Wizard state moved in to a constate container at one
point (as a starter) but I reverted this commit to make the changeset
less intrusive. It's for this same reason that, for now, I haven't
focussed too closely on extracting things like generating the friendly
error messages etc as we'll likely want to extract some of the "create
integration" hooks / UI in to a standalone package so they can be used
elsewhere (not just onboarding). There are also quite a few `
eslint-disable-next-line react-hooks/exhaustive-deps` rules in the
plugin at the moment due to the references not being stable, we could
improve that at the same time as any state changes.

- You can technically navigate directly to
`/fox/app/observabilityOnboarding/customLogs/installElasticAgent`, but
no state is stored in the URL, so nothing is rehydrated resulting in a
very empty configuration. I'm not entirely sure this is a behaviour we
want, but for now I've just made the callout conditional on state
existing (so coming from the previous panel).

- The Fleet custom integrations API now throws a 409 (conflict) when
using a name that already exists.

## Testing

- Head to `/app/observabilityOnboarding` to trigger the onboarding flow
- Select "Stream log files"
- When hitting "continue" an integration should be created in the
background (check the network requests for
`api/fleet/epm/custom_integrations`)
- When continuing (to install shipper), then going back **and** making
changes to your integration options, when clicking continue again there
should be a network request that deletes the previously created
integration (to clean things up). This should be seamless to the user.
- You should not be able to use a name that already exists (for an
existing custom integration)
- General errors (like permission issues, asset installation issues)
should display at the bottom
- When you hit the next panel (install shipper) there should be a
success callout that also contains the name of the integration that was
created

## In progress

~Two changes still in progress, but they don't need to hold up the
review (8.10 coming soon 👀):~

- ~To have a friendlier error for permissions issues (not just
"forbidden")~
- ~Fleet API integration test for the naming collision~

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
Kerry350 and kibanamachine authored Aug 11, 2023
1 parent 4de6111 commit 00ffe1d
Show file tree
Hide file tree
Showing 20 changed files with 1,055 additions and 106 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ src/plugins/url_forwarding @elastic/kibana-visualizations
packages/kbn-url-state @elastic/security-threat-hunting-investigations
src/plugins/usage_collection @elastic/kibana-core
test/plugin_functional/plugins/usage_collection @elastic/kibana-core
packages/kbn-use-tracked-promise @elastic/infra-monitoring-ui
packages/kbn-user-profile-components @elastic/kibana-security
examples/user_profile_examples @elastic/kibana-security
x-pack/test/security_api_integration/plugins/user_profiles_consumer @elastic/kibana-security
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@
"@kbn/url-state": "link:packages/kbn-url-state",
"@kbn/usage-collection-plugin": "link:src/plugins/usage_collection",
"@kbn/usage-collection-test-plugin": "link:test/plugin_functional/plugins/usage_collection",
"@kbn/use-tracked-promise": "link:packages/kbn-use-tracked-promise",
"@kbn/user-profile-components": "link:packages/kbn-user-profile-components",
"@kbn/user-profile-examples-plugin": "link:examples/user_profile_examples",
"@kbn/user-profiles-consumer-plugin": "link:x-pack/test/security_api_integration/plugins/user_profiles_consumer",
Expand Down
62 changes: 62 additions & 0 deletions packages/kbn-use-tracked-promise/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# @kbn/use-tracked-promise

/**
* This hook manages a Promise factory and can create new Promises from it. The
* state of these Promises is tracked and they can be canceled when superseded
* to avoid race conditions.
*
* ```
* const [requestState, performRequest] = useTrackedPromise(
* {
* cancelPreviousOn: 'resolution',
* createPromise: async (url: string) => {
* return await fetchSomething(url)
* },
* onResolve: response => {
* setSomeState(response.data);
* },
* onReject: response => {
* setSomeError(response);
* },
* },
* [fetchSomething]
* );
* ```
*
* The `onResolve` and `onReject` handlers are registered separately, because
* the hook will inject a rejection when in case of a canellation. The
* `cancelPreviousOn` attribute can be used to indicate when the preceding
* pending promises should be canceled:
*
* 'never': No preceding promises will be canceled.
*
* 'creation': Any preceding promises will be canceled as soon as a new one is
* created.
*
* 'settlement': Any preceding promise will be canceled when a newer promise is
* resolved or rejected.
*
* 'resolution': Any preceding promise will be canceled when a newer promise is
* resolved.
*
* 'rejection': Any preceding promise will be canceled when a newer promise is
* rejected.
*
* Any pending promises will be canceled when the component using the hook is
* unmounted, but their status will not be tracked to avoid React warnings
* about memory leaks.
*
* The last argument is a normal React hook dependency list that indicates
* under which conditions a new reference to the configuration object should be
* used.
*
* The `onResolve`, `onReject` and possible uncatched errors are only triggered
* if the underlying component is mounted. To ensure they always trigger (i.e.
* if the promise is called in a `useLayoutEffect`) use the `triggerOrThrow`
* attribute:
*
* 'whenMounted': (default) they are called only if the component is mounted.
*
* 'always': they always call. The consumer is then responsible of ensuring no
* side effects happen if the underlying component is not mounted.
*/
9 changes: 9 additions & 0 deletions packages/kbn-use-tracked-promise/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { useTrackedPromise } from './use_tracked_promise';
13 changes: 13 additions & 0 deletions packages/kbn-use-tracked-promise/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_node',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-use-tracked-promise'],
};
5 changes: 5 additions & 0 deletions packages/kbn-use-tracked-promise/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "shared-common",
"id": "@kbn/use-tracked-promise",
"owner": "@elastic/infra-monitoring-ui"
}
6 changes: 6 additions & 0 deletions packages/kbn-use-tracked-promise/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@kbn/use-tracked-promise",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
17 changes: 17 additions & 0 deletions packages/kbn-use-tracked-promise/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"outDir": "target/types",
"types": [
"jest",
"node"
]
},
"include": [
"**/*.ts",
],
"exclude": [
"target/**/*"
],
"kbn_references": []
}
Loading

0 comments on commit 00ffe1d

Please sign in to comment.