-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Integrations UI] Add support for custom asset definitions in Integration assets tab #103554
[Integrations UI] Add support for custom asset definitions in Integration assets tab #103554
Conversation
Pinging @elastic/fleet (Team:Fleet) |
Pinging @elastic/uptime (Team:uptime) |
x-pack/plugins/fleet/public/lazy_custom_logs_assets_component.tsx
Outdated
Show resolved
Hide resolved
...plugins/fleet/public/applications/integrations/sections/epm/screens/detail/assets/assets.tsx
Outdated
Show resolved
Hide resolved
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.
Great start, I spotted a few issues in my first run with it.
...management/pages/policy/view/ingest_manager_integration/endpoint_custom_assets_extension.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/uptime/public/components/fleet_package/synthetics_custom_assets_extension.tsx
Outdated
Show resolved
Hide resolved
Pinging @elastic/apm-ui (Team:apm) |
x-pack/plugins/apm/public/components/fleet_integration/apm_custom_assets_extension.tsx
Outdated
Show resolved
Hide resolved
...t/public/applications/integrations/sections/epm/screens/detail/policies/package_policies.tsx
Outdated
Show resolved
Hide resolved
@nchaulet @jen-huang - Want to try and gather some thoughts on these two tasks with APM here, as I'm not sure how to proceed.
The idea here is to allow the APM app to hook into Fleet via a UI extension that replaces the final step of the agent enrollment flyout, so we can direct the user back into the APM flow. See this screenshot from the original issue here: The problem is, these We pass in the content for the Lines 101 to 129 in 7ec04e1
So, my initial thought was to swap out this argument based on the presence of a UI Extension, but this only controls the content underneath the title and step UI, e.g. So, what we really need is a way to replace the entire kibana/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx Lines 111 to 113 in 7ec04e1
kibana/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx Lines 127 to 147 in 7ec04e1
I don't think we have a way to allow a UI Extension to export arbitrary data instead of a React component, so I think this is a bit of a blocker. Do either of you have any potential input here? |
export interface AgentEnrollmentFlyoutFinalStepExtension { | ||
package: string; | ||
view: 'agent-enrollment-flyout'; | ||
component: LazyExoticComponent<AgentEnrollmentFlyoutFinalStepExtensionComponent>; |
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.
what if you adjusted this interface to accept a title
field that you can then use to modify the step title?
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.
This definitely makes sense, then the apps that want to hook into this functionality will simply need to export something like stepProps
in the extension data to control what we pass into EuiSteps
.
The next question I have here is how to handle this change in contract in our useUiExtensionHook
, which currently expects there to be a component
in each of these extension interfaces:
kibana/x-pack/plugins/fleet/public/hooks/use_ui_extension.ts
Lines 20 to 37 in de19795
export const useUIExtension = <V extends UIExtensionPoint['view'] = UIExtensionPoint['view']>( | |
packageName: UIExtensionPoint['package'], | |
view: V | |
): NarrowExtensionPoint<V>['component'] | undefined => { | |
const registeredExtensions = useContext(UIExtensionsContext); | |
if (!registeredExtensions) { | |
throw new Error('useUIExtension called outside of UIExtensionsContext'); | |
} | |
const extension = registeredExtensions?.[packageName]?.[view]; | |
if (extension) { | |
// FIXME:PT Revisit ignore below and see if TS error can be addressed | |
// @ts-ignore | |
return extension.component; | |
} | |
}; |
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 the solution I'm going to work towards will be to simply include a new hook useAgentEnrollmentFlyoutExtension
to capture this special case for now.
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.
@kpollich I think we can have a component
and a title
in our AgentEnrollmentFlyoutFinalStepExtension
type and you can build the step later when you retrieve the extension with that, I am wondering if the useUIExtension
should return the whole extension instead of just the component too.
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 just pushed up e4fb321 which adds a separate hook that does what you're mentioning. We could instead refactor useUIExtension
to return the entire extension object and then update all our usages of useUIExtensiion
(only a few places) to do something like
const extension = useUIExtension(...)
// ...
if (extension) {
<extension.Component />
}
Is that what you're thinking here?
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.
Yes it was I was thinking, and in the case of the step do something like this (it's a little less coupled to the steps component)
const extension = useUIExtension(...)
// ...
if (extension) {
const step = { title: extension.title, children: (<extension.Component />)}
}
|
||
export function getApmEnrollmentFlyoutStepProps(): AgentEnrollmentFlyoutFinalStepExtension['stepProps'] { | ||
// TODO: Figure out how to get `http.basePath` here | ||
const installApmAgentLink = '/app/home#/tutorial/apm'; |
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 can I get Kibana's basePath
value here without using a React hook?
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.
You would have to pass it in as an argument.
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 it's a good argument to change the extension to have a title and a component, you will be able to use hooks in the component, and I am sure in the future it will prevent some headache to be able to use hooks here too. what do you think?
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.
Yeah that should work here. I have the refactor title + Component in the extension working locally. I will try pulling in hooks to the component tree rendered here to get at this data.
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.
Looks like this is still WIP. Can you switch it to draft?
x-pack/plugins/apm/public/components/fleet_integration/apm_custom_assets_extension.tsx
Outdated
Show resolved
Hide resolved
|
||
export function getApmEnrollmentFlyoutStepProps(): AgentEnrollmentFlyoutFinalStepExtension['stepProps'] { | ||
// TODO: Figure out how to get `http.basePath` here | ||
const installApmAgentLink = '/app/home#/tutorial/apm'; |
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.
You would have to pass it in as an argument.
x-pack/plugins/apm/public/components/fleet_integration/apm_enrollment_flyout_extension.tsx
Show resolved
Hide resolved
...s/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx
Show resolved
Hide resolved
...t/public/applications/integrations/sections/epm/screens/detail/policies/package_policies.tsx
Show resolved
Hide resolved
- Remove default filter for log stream url - Fix missing basePath prepend on asset urls - Expand accordion by default on assetless integrations
...management/pages/policy/view/ingest_manager_integration/endpoint_custom_assets_extension.tsx
Outdated
Show resolved
Hide resolved
Left a couple comments of things to change, but looks good otherwise! Will 👍 after changes for the @elastic/security-onboarding-and-lifecycle-mgt team |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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.
Seems to be working for me. I had a few suggestions but nothing blocking.
@cauemarcondes has been doing some work on Fleet for APM and tutorials so you might want to check with him next week to make sure there aren't any conflicts with work he's done.
x-pack/plugins/apm/public/components/fleet_integration/apm_enrollment_flyout_extension.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_enrollment_flyout.test.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/fleet/public/components/agent_enrollment_flyout/types.ts
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
History
To update your PR or re-run it, just comment with: cc @kpollich |
…tion assets tab (elastic#103554) * Add UI extension logic for assets + set up custom log views * Add endpoint security UI extension * Add synthetics ui extension * Address PR feedback - Remove default filter for log stream url - Fix missing basePath prepend on asset urls - Expand accordion by default on assetless integrations * Fix type errors * Add initial APM extension setup * Fix missing ExtensionWrapper for enrollment extension * Fix custom logs asset extension * Fix type errors * Add new hook for enrollment flyout ui extensions * Address PR review + refactor UI extension usage for flyout * Update limits.yml via script * Fix type errors * Add tests for custom assets UI extensions * Update tests for flyout * Remove unused import * Fix type errors in ui extension tests * Skip view data tests and link to issue * Use RedirectAppLinks + fix synthetics link * Use constants for app ID's where possible * Revert limits.yml * Fix lazy imports for custom asset components * Update endpoint custom assets link + description * Add translation for custom assets UI * Address PR review in APM Co-authored-by: Kibana Machine <[email protected]>
…tion assets tab (elastic#103554) * Add UI extension logic for assets + set up custom log views * Add endpoint security UI extension * Add synthetics ui extension * Address PR feedback - Remove default filter for log stream url - Fix missing basePath prepend on asset urls - Expand accordion by default on assetless integrations * Fix type errors * Add initial APM extension setup * Fix missing ExtensionWrapper for enrollment extension * Fix custom logs asset extension * Fix type errors * Add new hook for enrollment flyout ui extensions * Address PR review + refactor UI extension usage for flyout * Update limits.yml via script * Fix type errors * Add tests for custom assets UI extensions * Update tests for flyout * Remove unused import * Fix type errors in ui extension tests * Skip view data tests and link to issue * Use RedirectAppLinks + fix synthetics link * Use constants for app ID's where possible * Revert limits.yml * Fix lazy imports for custom asset components * Update endpoint custom assets link + description * Add translation for custom assets UI * Address PR review in APM Co-authored-by: Kibana Machine <[email protected]>
…tion assets tab (#103554) (#104230) * Add UI extension logic for assets + set up custom log views * Add endpoint security UI extension * Add synthetics ui extension * Address PR feedback - Remove default filter for log stream url - Fix missing basePath prepend on asset urls - Expand accordion by default on assetless integrations * Fix type errors * Add initial APM extension setup * Fix missing ExtensionWrapper for enrollment extension * Fix custom logs asset extension * Fix type errors * Add new hook for enrollment flyout ui extensions * Address PR review + refactor UI extension usage for flyout * Update limits.yml via script * Fix type errors * Add tests for custom assets UI extensions * Update tests for flyout * Remove unused import * Fix type errors in ui extension tests * Skip view data tests and link to issue * Use RedirectAppLinks + fix synthetics link * Use constants for app ID's where possible * Revert limits.yml * Fix lazy imports for custom asset components * Update endpoint custom assets link + description * Add translation for custom assets UI * Address PR review in APM Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kyle Pollich <[email protected]>
…tion assets tab (#103554) (#104229) * Add UI extension logic for assets + set up custom log views * Add endpoint security UI extension * Add synthetics ui extension * Address PR feedback - Remove default filter for log stream url - Fix missing basePath prepend on asset urls - Expand accordion by default on assetless integrations * Fix type errors * Add initial APM extension setup * Fix missing ExtensionWrapper for enrollment extension * Fix custom logs asset extension * Fix type errors * Add new hook for enrollment flyout ui extensions * Address PR review + refactor UI extension usage for flyout * Update limits.yml via script * Fix type errors * Add tests for custom assets UI extensions * Update tests for flyout * Remove unused import * Fix type errors in ui extension tests * Skip view data tests and link to issue * Use RedirectAppLinks + fix synthetics link * Use constants for app ID's where possible * Revert limits.yml * Fix lazy imports for custom asset components * Update endpoint custom assets link + description * Add translation for custom assets UI * Address PR review in APM Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kyle Pollich <[email protected]>
Hi @EricDavisX We have validated this PR and executed related testcases under below test run on 7.13 BC3 kibana staging cloud build and found them working fine. Custom asset definitions in Integration assets tab Test run Please let us know if anything is missing. Thanks |
Hi, question if ingest pipeline assets are included in the UI? Is there anywhere I can track newer work? This issue linked from this comment but if there's something newer I'd love to follow it |
Summary
Adds support for UI extensions that customize the "Assets" tab on the integration details page to support packages that don't export assets.
Also sets up UI extensions for existing "assetless" integrations.
Closes #102967
Screenshots
Custom Logs
Endpoint
APM
Synthetics