Skip to content

Commit

Permalink
[Discover] Add getRenderAppWrapper extension (elastic#197556)
Browse files Browse the repository at this point in the history
## Summary

This PR adds a `getRenderAppWrapper` extension to Discover to support
advanced state management use cases. It also includes new documentation
for the extension point, and overriding default profile implementations:


https://github.com/user-attachments/assets/70633cbb-1cfe-47fe-984e-ba8afb18fc90

To test, add the following config to `kibana.dev.yml`:
```yml
discover.experimental.enabledProfiles:
  [
    'example-root-profile',
    'example-solution-view-root-profile',
    'example-data-source-profile',
    'example-document-profile',
  ]
```

Then ingest demo logs and run this in dev tools:
```
POST _aliases
{
  "actions": [
    {
      "add": {
        "index": "kibana_sample_data_logs",
        "alias": "my-example-logs"
      }
    }
  ]
}
```

Flaky test runs:
- 🔴 x25:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7238
- 🔴 x25:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7289
- 🟢 x25:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7291
- x25:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7303

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
- [ ] This will appear in the **Release Notes** and follow the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Julia Rechkunova <[email protected]>
  • Loading branch information
davismcphee and jughosta authored Nov 6, 2024
1 parent d601e23 commit 4a95eec
Show file tree
Hide file tree
Showing 35 changed files with 824 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { LoadingIndicator } from '../../components/common/loading_indicator';
import { useDataView } from '../../hooks/use_data_view';
import type { ContextHistoryLocationState } from './services/locator';
import { useDiscoverServices } from '../../hooks/use_discover_services';
import { useRootProfile } from '../../context_awareness';

export interface ContextUrlParams {
dataViewId: string;
Expand Down Expand Up @@ -47,8 +48,8 @@ export function ContextAppRoute() {
const { dataViewId: encodedDataViewId, id } = useParams<ContextUrlParams>();
const dataViewId = decodeURIComponent(encodedDataViewId);
const anchorId = decodeURIComponent(id);

const { dataView, error } = useDataView({ index: locationState?.dataViewSpec || dataViewId });
const rootProfileState = useRootProfile();

if (error) {
return (
Expand All @@ -72,9 +73,13 @@ export function ContextAppRoute() {
);
}

if (!dataView) {
if (!dataView || rootProfileState.rootProfileLoading) {
return <LoadingIndicator />;
}

return <ContextApp anchorId={anchorId} dataView={dataView} referrer={locationState?.referrer} />;
return (
<rootProfileState.AppWrapper>
<ContextApp anchorId={anchorId} dataView={dataView} referrer={locationState?.referrer} />
</rootProfileState.AppWrapper>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { firstValueFrom, lastValueFrom } from 'rxjs';
import { lastValueFrom } from 'rxjs';
import { i18n } from '@kbn/i18n';
import { ISearchSource, EsQuerySortValue } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
Expand All @@ -29,11 +29,7 @@ export async function fetchAnchor(
anchorRow: DataTableRecord;
interceptedWarnings: SearchResponseWarning[];
}> {
const { core, profilesManager } = services;

const solutionNavId = await firstValueFrom(core.chrome.getActiveSolutionNavId$());
await profilesManager.resolveRootProfile({ solutionNavId });
await profilesManager.resolveDataSourceProfile({
await services.profilesManager.resolveDataSourceProfile({
dataSource: createDataSource({ dataView, query: undefined }),
dataView,
query: { query: '', language: 'kuery' },
Expand Down Expand Up @@ -68,7 +64,7 @@ export async function fetchAnchor(
});

return {
anchorRow: profilesManager.resolveDocumentProfile({
anchorRow: services.profilesManager.resolveDocumentProfile({
record: buildDataTableRecord(doc, dataView, true),
}),
interceptedWarnings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import React, { useCallback, useEffect } from 'react';
import { FormattedMessage } from '@kbn/i18n-react';
import { firstValueFrom } from 'rxjs';
import { EuiCallOut, EuiLink, EuiLoadingSpinner, EuiPage, EuiPageBody } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { ElasticRequestState } from '@kbn/unified-doc-viewer';
Expand All @@ -31,18 +30,16 @@ export interface DocProps extends EsDocSearchProps {
export function Doc(props: DocProps) {
const { dataView } = props;
const services = useDiscoverServices();
const { locator, chrome, docLinks, core, profilesManager } = services;
const { locator, chrome, docLinks, profilesManager } = services;
const indexExistsLink = docLinks.links.apis.indexExists;

const onBeforeFetch = useCallback(async () => {
const solutionNavId = await firstValueFrom(core.chrome.getActiveSolutionNavId$());
await profilesManager.resolveRootProfile({ solutionNavId });
await profilesManager.resolveDataSourceProfile({
dataSource: dataView?.id ? createDataViewDataSource({ dataViewId: dataView.id }) : undefined,
dataView,
query: { query: '', language: 'kuery' },
});
}, [profilesManager, core, dataView]);
}, [profilesManager, dataView]);

const onProcessRecord = useCallback(
(record: DataTableRecord) => {
Expand Down
11 changes: 9 additions & 2 deletions src/plugins/discover/public/application/doc/single_doc_route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { useDiscoverServices } from '../../hooks/use_discover_services';
import { DiscoverError } from '../../components/common/error_alert';
import { useDataView } from '../../hooks/use_data_view';
import { DocHistoryLocationState } from './locator';
import { useRootProfile } from '../../context_awareness';

export interface DocUrlParams {
dataViewId: string;
Expand Down Expand Up @@ -53,6 +54,8 @@ export const SingleDocRoute = () => {
index: locationState?.dataViewSpec || decodeURIComponent(dataViewId),
});

const rootProfileState = useRootProfile();

if (error) {
return (
<EuiEmptyPrompt
Expand All @@ -75,7 +78,7 @@ export const SingleDocRoute = () => {
);
}

if (!dataView) {
if (!dataView || rootProfileState.rootProfileLoading) {
return <LoadingIndicator />;
}

Expand All @@ -94,5 +97,9 @@ export const SingleDocRoute = () => {
);
}

return <Doc id={id} index={index} dataView={dataView} referrer={locationState?.referrer} />;
return (
<rootProfileState.AppWrapper>
<Doc id={id} index={index} dataView={dataView} referrer={locationState?.referrer} />
</rootProfileState.AppWrapper>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import React from 'react';
import React, { ReactNode } from 'react';
import { mountWithIntl } from '@kbn/test-jest-helpers';
import { waitFor } from '@testing-library/react';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
Expand Down Expand Up @@ -50,6 +50,7 @@ jest.mock('../../context_awareness', () => {
...originalModule,
useRootProfile: () => ({
rootProfileLoading: mockRootProfileLoading,
AppWrapper: ({ children }: { children: ReactNode }) => <>{children}</>,
}),
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,27 +345,26 @@ export function DiscoverMainRoute({
stateContainer,
]);

const { solutionNavId } = customizationContext;
const { rootProfileLoading } = useRootProfile({ solutionNavId });
const rootProfileState = useRootProfile();

if (error) {
return <DiscoverError error={error} />;
}

if (!customizationService || rootProfileLoading) {
if (!customizationService || rootProfileState.rootProfileLoading) {
return loadingIndicator;
}

return (
<DiscoverCustomizationProvider value={customizationService}>
<DiscoverMainProvider value={stateContainer}>
<>
<rootProfileState.AppWrapper>
<DiscoverTopNavInline
stateContainer={stateContainer}
hideNavMenuItems={loading || noDataState.showNoDataPage}
/>
{mainContent}
</>
</rootProfileState.AppWrapper>
</DiscoverMainProvider>
</DiscoverCustomizationProvider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const discoverContainerWrapperCss = css`
`;

const customizationContext: DiscoverCustomizationContext = {
solutionNavId: null,
displayMode: 'embedded',
inlineTopNav: {
enabled: false,
Expand Down
190 changes: 189 additions & 1 deletion src/plugins/discover/public/context_awareness/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Existing providers can be extended using the [`extendProfileProvider`](./profile

Example profile provider implementations are located in [`profile_providers/example`](./profile_providers/example).

## Example implementation
### Example implementation

```ts
/**
Expand Down Expand Up @@ -191,3 +191,191 @@ const createDataSourceProfileProviders = (providerServices: ProfileProviderServi
* to resolve the profile: `FROM my-example-logs`
*/
```

## React context and state management

In the Discover context awareness framework, pieces of Discover’s state are passed down explicitly to extension points as needed. This avoids leaking Discover internals – which may change – to consumer extension point implementations and allows us to be intentional about which pieces of state extension points have access to. This approach generally works well when extension points need access to things like the current ES|QL query or data view, time range, columns, etc. However, this does not provide a solution for consumers to manage custom shared state between their extension point implementations.

In cases where the state for an extension point implementation is local to that implementation, consumers can simply manage the state within the corresponding profile method or returned React component:

```tsx
// Extension point implementation definition
const getCellRenderers = (prev) => (params) => {
// Declare shared state within the profile method closure
const blueOrRed$ = new BehaviorSubject<'blue' | 'red'>('blue');

return {
...prev(params),
foo: function FooComponent() {
// It's still in scope and can be easily accessed...
const blueOrRed = useObservable(blueOrRed$, blueOrRed$.getValue());

return (
// ...and modified...
<button onClick={() => blueOrRed$.next(blueOrRed === 'blue' ? 'red' : 'blue')}>
Click to make bar {blueOrRed === 'blue' ? 'red' : 'blue'}
</button>
);
},
bar: function BarComponent() {
const blueOrRed = useObservable(blueOrRed$, blueOrRed$.getValue());

// ...and we can react to the changes
return <span style={{ color: blueOrRed }}>Look ma, I'm {blueOrRed}!</span>;
},
};
};
```

For more advanced use cases, such as when state needs to be shared across extension point implementations, we provide an extension point called `getRenderAppWrapper`. The app wrapper extension point allows consumers to wrap the Discover root in a custom wrapper component, such as a React context provider. With this approach consumers can handle things like integrating with a state management library, accessing custom services from within their extension point implementations, managing shared components such as flyouts, etc. in a React-friendly way and without needing to work around the context awareness framework:

```tsx
// The app wrapper extension point supports common patterns like React context
const flyoutContext = createContext({ setFlyoutOpen: (open: boolean) => {} });

// App wrapper implementations can only exist at the root level, and their lifecycle will match the Discover lifecycle
export const createSecurityRootProfileProvider = (): RootProfileProvider => ({
profileId: 'security-root-profile',
profile: {
// The app wrapper extension point implementation
getRenderAppWrapper: (PrevWrapper) =>
function AppWrapper({ children }) {
// Now we can declare state high up in the React tree
const [flyoutOpen, setFlyoutOpen] = useState(false);

return (
// Be sure to render the previous wrapper as well
<PrevWrapper>
// This is our wrapper -- it uses React context to give extension point implementations
access to the shared state
<flyoutContext.Provider value={{ setFlyoutOpen }}>
// Make sure to render `children`, which is the Discover app
{children}
// Now extension point implementations can interact with shared state managed higher
up in the tree
{flyoutOpen && (
<EuiFlyout onClose={() => setFlyoutOpen(false)}>
Check it out, I'm a flyout!
</EuiFlyout>
)}
</flyoutContext.Provider>
</PrevWrapper>
);
},
// Some other extension point implementation that depends on the shared state
getCellRenderers: (prev) => (params) => ({
...prev(params),
foo: function FooComponent() {
// Since the app wrapper implementation wrapped Discover with a React context provider, we can now access its values from within our extension point implementations
const { setFlyoutOpen } = useContext(flyoutContext);

return <button onClick={() => setFlyoutOpen(true)}>Click me to open a flyout!</button>;
},
}),
},
resolve: (params) => {
if (params.solutionNavId === SolutionType.Security) {
return {
isMatch: true,
context: { solutionType: SolutionType.Security },
};
}

return { isMatch: false };
},
});
```

## Overriding defaults

Discover ships with a set of common contextual profiles, shared across Solutions in Kibana (e.g. the current logs data source profile). The goal of these profiles is to provide Solution agnostic contextual features to help improve the default data exploration experience for various data types. They should be generally useful across user types and not be tailored to specific Solution workflows – for example, viewing logs should be a delightful experience regardless of whether it’s done within the Observability Solution, the Search Solution, or the classic on-prem experience.

We’re aiming to make these profiles generic enough that they don’t obstruct Solution workflows or create confusion, but there will always be some complexity around juggling the various Discover use cases. For situations where Solution teams are confident some common profile feature will not be helpful to their users or will create confusion, there is an option to override these defaults while keeping the remainder of the functionality for the target profile intact. To do so a Solution team would follow these steps:

- Create and register a Solution specific root profile provider, e.g. `SecurityRootProfileProvider`.
- Identify the contextual feature you want to override and the common profile provider it belongs to, e.g. the `getDocViewer` implementation in the common `LogsDataSourceProfileProvider`.
- Implement a Solution specific version of the profile provider that extends the common provider as its base (using the `extendProfileProvider` utility), and excludes the extension point implementations you don’t want, e.g. `SecurityLogsDataSourceProfileProvider`. Other than the excluded extension point implementations, the only required change is to update its `resolve` method to first check the `rootContext.solutionType` for the target solution type before executing the base provider’s `resolve` method. This will ensure the override profile only resolves for the specific Solution, and will fall back to the common profile in other Solutions.
- Register the Solution specific version of the profile provider in Discover, ensuring it precedes the common provider in the registration array. The ordering here is important since the Solution specific profile should attempt to resolve first, otherwise the common profile would be resolved instead.

This is how an example implementation would work in code:

```tsx
/**
* profile_providers/security/security_root_profile/profile.tsx
*/

// Create a solution specific root profile provider
export const createSecurityRootProfileProvider = (): RootProfileProvider => ({
profileId: 'security-root-profile',
profile: {},
resolve: (params) => {
if (params.solutionNavId === SolutionType.Security) {
return {
isMatch: true,
context: { solutionType: SolutionType.Security },
};
}

return { isMatch: false };
},
});

/**
* profile_providers/security/security_logs_data_source_profile/profile.tsx
*/

// Create a solution specific data source profile provider that extends a target base provider
export const createSecurityLogsDataSourceProfileProivder = (
logsDataSourceProfileProvider: DataSourceProfileProvider
): DataSourceProfileProvider =>
// Extend the base profile provider with `extendProfileProvider`
extendProfileProvider(logsDataSourceProfileProvider, {
profileId: 'security-logs-data-source-profile',
profile: {
// Completely remove a specific extension point implementation
getDocViewer: undefined,
// Modify the result of an existing extension point implementation
getCellRenderers: (prev) => (params) => {
// Retrieve and execute the base implementation
const baseImpl = logsDataSourceProfileProvider.profile.getCellRenderers?.(prev);
const baseRenderers = baseImpl?.(params);

// Return the modified result
return omit(baseRenderers, 'log.level');
},
},
// Customize the `resolve` implementation
resolve: (params) => {
// Only match this profile when in the target solution context
if (params.rootContext.solutionType !== SolutionType.Security) {
return { isMatch: false };
}

// Delegate to the base implementation
return logsDataSourceProfileProvider.resolve(params);
},
});

/**
* profile_providers/register_profile_providers.ts
*/

// Register root profile providers
const createRootProfileProviders = (providerServices: ProfileProviderServices) => [
// Register the solution specific root profile provider
createSecurityRootProfileProvider(),
];

// Register data source profile providers
const createDataSourceProfileProviders = (providerServices: ProfileProviderServices) => {
// Instantiate the data source profile provider base implementation
const logsDataSourceProfileProvider = createLogsDataSourceProfileProvider(providerServices);

return [
// Ensure the solution specific override is registered and resolved first
createSecurityLogsDataSourceProfileProivder(logsDataSourceProfileProvider),
// Then register the base implementation
logsDataSourceProfileProvider,
];
};
```
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
*/

export { useProfileAccessor } from './use_profile_accessor';
export { useRootProfile } from './use_root_profile';
export { useRootProfile, BaseAppWrapper } from './use_root_profile';
export { useAdditionalCellActions } from './use_additional_cell_actions';
Loading

0 comments on commit 4a95eec

Please sign in to comment.