Skip to content

Commit

Permalink
[Discover] Create Discover Shared plugin and features registry (#181952)
Browse files Browse the repository at this point in the history
## 📓 Summary

Closes #181528 
Closes #181976 

**N.B.** This work was initially reviewed on a separate PR at
tonyghiani#1.

This implementation aims to have a stateful layer that allows the
management of dependencies between Discover and other plugins, reducing
the need for a direct dependency.

Although the initial thought was to have a plugin to register features
for Discover, there might be other cases in future where we need to
prevent cyclic dependencies.
With this in mind, I created the plugin as a more generic solution to
hold stateful logic as a communication layer for Discover <-> Plugins.

## Discover Shared

Based on some recurring naming in the Kibana codebase, `discover_shared`
felt like the right place for owning these dependencies and exposing
Discover functionalities to the external world.
It is initially pretty simple and only exposes a registry for the
Discover features, but might be a good place to implement other upcoming
concepts related to Discover.

Also, this plugin should ideally never depend on other solution plugins
and keep its dependencies to a bare minimum of packages and core/data
services.

```mermaid
flowchart TD

A(Discover) -- Get --> E[DiscoverShared]
B(Logs Explorer) -- Set --> E[DiscoverShared]
C(Security) -- Set --> E[DiscoverShared]
D(Any app) -- Set --> E[DiscoverShared]
```

## DiscoverFeaturesService

This service initializes and exposes a strictly typed registry to allow
consumer apps to register additional features and Discover and retrieve
them.

The **README** file explains a real use case of when we'd need to use it
and how to do that step-by-step.

Although it introduces a more nested folder structure, I decided to
implement the service as a client-service and expose it through the
plugin lifecycle methods to provide the necessary flexibility we might
need:
- We don't know yet if any of the features we register will be done on
the setup/start steps, so having the registry available in both places
opens the door to any case we face.
- The service is client-only on purpose. My opinion is that if we ever
need to register features such as server services or anything else, it
should be scoped to a similar service dedicated for the server lifecycle
and its environment.
It should never be possible to register the ObsAIAssistant
presentational component from the server, as it should not be permitted
to register a server service in the client registry.
A server DiscoverFeaturesService is not required yet for any feature, so
I left it out to avoid overcomplicating the implementation.

## FeaturesRegistry

To have a strictly typed utility that suggests the available features on
a registry and adheres to a base contract, the registry exposed on the
DiscoverFeaturesService is an instance of the `FeaturesRegistry` class,
which implements the registration/retrieval logic such that:
- If a feature is already registered, is not possible to override it and
an error is thrown to notify the user of the collision.
- In case we need to react to registry changes, is possible to subscribe
to the registry or obtain it as an observable for more complex
scenarios.

The FeaturesRegistry already takes care of the required logic for the
registry, so that `DiscoverFeaturesService`is left with the
responsibility of instantiating/exposing an instance and provide the set
of allowed features.

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Davis McPhee <davismcphee@hotmail.com>
  • Loading branch information
4 people committed May 3, 2024
1 parent a6491ab commit 747ecea
Show file tree
Hide file tree
Showing 45 changed files with 550 additions and 369 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ examples/developer_examples @elastic/appex-sharedux
examples/discover_customization_examples @elastic/kibana-data-discovery
x-pack/plugins/discover_enhanced @elastic/kibana-data-discovery
src/plugins/discover @elastic/kibana-data-discovery
src/plugins/discover_shared @elastic/kibana-data-discovery @elastic/obs-ux-logs-team
packages/kbn-discover-utils @elastic/kibana-data-discovery
packages/kbn-doc-links @elastic/docs
packages/kbn-docs-utils @elastic/kibana-operations
Expand Down
4 changes: 4 additions & 0 deletions docs/developer/plugin-list.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ This API doesn't support angular, for registering angular dev tools, bootstrap a
|Contains the Discover application and the saved search embeddable.
|{kib-repo}blob/{branch}/src/plugins/discover_shared/README.md[discoverShared]
|A stateful layer to register shared features and provide an access point to discover without a direct dependency.
|{kib-repo}blob/{branch}/src/plugins/embeddable/README.md[embeddable]
|The Embeddables Plugin provides an opportunity to expose reusable interactive widgets that can be embedded outside the original plugin.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@
"@kbn/discover-customization-examples-plugin": "link:examples/discover_customization_examples",
"@kbn/discover-enhanced-plugin": "link:x-pack/plugins/discover_enhanced",
"@kbn/discover-plugin": "link:src/plugins/discover",
"@kbn/discover-shared-plugin": "link:src/plugins/discover_shared",
"@kbn/discover-utils": "link:packages/kbn-discover-utils",
"@kbn/doc-links": "link:packages/kbn-doc-links",
"@kbn/dom-drag-drop": "link:packages/kbn-dom-drag-drop",
Expand Down
3 changes: 2 additions & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pageLoadAssetSize:
devTools: 38637
discover: 99999
discoverEnhanced: 42730
discoverShared: 17111
embeddable: 87309
embeddableEnhanced: 22107
enterpriseSearch: 50858
Expand Down Expand Up @@ -143,7 +144,7 @@ pageLoadAssetSize:
snapshotRestore: 79032
spaces: 57868
stackAlerts: 58316
stackConnectors: 52131
stackConnectors: 67227
synthetics: 40958
telemetry: 51957
telemetryManagementSection: 38586
Expand Down
90 changes: 90 additions & 0 deletions src/plugins/discover_shared/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Discover Shared

A stateful layer to register shared features and provide an access point to discover without a direct dependency.

## Register new features

The plugin exposes a service to register features that can be opinionatedly used in Discover on both the setup and start lifecycle hooks.

Although this allows for greater flexibility, its purpose is not to customize Discover as a default choice but to be used as a solution to prevent cyclic dependency between plugins that interact with Discover.

To register a new feature, let's take a more practical case.

> _We want to introduce the LogsAIAssistant in the Discover flyout. Porting all the logic of the Observability AI Assistant into Discover is not an option, and we don't want Discover to directly depend on the AI Assistant codebase._
We can solve this case with some steps:

### Define a feature registration contract

First of all, we need to define an interface to which the plugin registering the AI Assistant and Discover can adhere.

The `DiscoverFeaturesService` already defines a union of available features and uses them to strictly type the exposed registry from the discover_shared plugin, so we can update it with the new feature:

```tsx
// src/plugins/discover_shared/public/services/discover_features/types.ts

export interface SecurityAIAssistantFeature {
id: 'security-ai-assistant';
render: (/* Update with deps required for this integration */) => React.ReactNode;
// Add any prop required for the feature
}

export interface ObservabilityLogsAIAssistantFeature {
id: 'observability-logs-ai-assistant';
render: (deps: {doc: DataTableRecord}) => React.ReactNode;
// Add any prop required for the feature
}

// This should be a union of all the available client features.
export type DiscoverFeature = SecurityAIAssistantFeature | ObservabilityLogsAIAssistantFeature;
```

### Discover consumes the registered feature

Once we have an interface for the feature, Discover can now retrieve it and use its content if is registered by any app in Kibana.

```tsx
// Somewhere in the unified doc viewer

function LogsOverviewAIAssistant ({ doc }) {
const { discoverShared } = getUnifiedDocViewerServices();

const logsAIAssistantFeature = discoverShared.features.registry.getById('observability-logs-ai-assistant')

if (logsAIAssistantFeature) {
return logsAIAssistantFeature.render({ doc })
}
}
```

### Register the feature

Having an interface for the feature and Discover consuming its definition, we are left with the registration part.

For our example, we'll go to the logs app that owns the LogsAIAssistant codebase and register the feature:

```tsx
// x-pack/plugins/observability_solution/logs_shared/public/plugin.ts

export class LogsSharedPlugin implements LogsSharedClientPluginClass {
// The rest of the plugin implementation is hidden for a cleaner example

public start(core: CoreStart, plugins: LogsSharedClientStartDeps) {
const { observabilityAIAssistant } = plugins;

const LogAIAssistant = createLogAIAssistant({ observabilityAIAssistant });

// Strict typing on the registry will let you know which features you can register
plugins.discoverShared.features.registry.register({
id: 'observability-logs-ai-assistant',
render: ({doc}) => <LogAIAssistant doc={doc}/>
})

return {
LogAIAssistant,
};
}
}
```

At this point, the feature should work correctly when registered and we have not created any direct dependency between the Discover and LogsShared apps.
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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.
*/

import { FeaturesRegistry } from './features_registry';

type TestFeature =
| { id: 'feature-id-1'; adHocProperty1?: string }
| { id: 'feature-id-2'; adHocProperty2?: string }
| { id: 'feature-id-3'; adHocProperty3?: string };

describe('FeaturesRegistry', () => {
describe('#register', () => {
test('should add a feature to the registry', () => {
const registry = new FeaturesRegistry<TestFeature>();

registry.register({ id: 'feature-id-1' });

expect(registry.getById('feature-id-1')).toBeDefined();
});

test('should throw an error when a feature is already registered by the given id', () => {
const registry = new FeaturesRegistry<TestFeature>();

registry.register({ id: 'feature-id-1' });

expect(() => registry.register({ id: 'feature-id-1' })).toThrow(
'FeaturesRegistry#register: feature with id "feature-id-1" already exists in the registry.'
);
});
});

describe('#getById', () => {
test('should retrieve a feature by its id', () => {
const registry = new FeaturesRegistry<TestFeature>();

registry.register({ id: 'feature-id-1', adHocProperty1: 'test' });
registry.register({ id: 'feature-id-2', adHocProperty2: 'test' });

expect(registry.getById('feature-id-1')).toEqual({
id: 'feature-id-1',
adHocProperty1: 'test',
});
});

test('should return undefined if there is no feature registered by the given id', () => {
const registry = new FeaturesRegistry<TestFeature>();

registry.register({ id: 'feature-id-1' });

expect(registry.getById('feature-id-2')).toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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.
*/
import { BaseFeature } from './types';

export class FeaturesRegistry<Feature extends BaseFeature = BaseFeature> {
private readonly features = new Map<Feature['id'], Feature>();

register(feature: Feature): void {
if (this.features.has(feature.id)) {
throw new Error(
`FeaturesRegistry#register: feature with id "${feature.id}" already exists in the registry.`
);
}

this.features.set(feature.id, feature);
}

getById<Id extends Feature['id']>(id: Id) {
return this.features.get(id) as Extract<Feature, { id: Id }> | undefined;
}
}
9 changes: 9 additions & 0 deletions src/plugins/discover_shared/common/features_registry/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 { FeaturesRegistry } from './features_registry';
11 changes: 11 additions & 0 deletions src/plugins/discover_shared/common/features_registry/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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 interface BaseFeature {
id: string;
}
9 changes: 9 additions & 0 deletions src/plugins/discover_shared/common/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 { FeaturesRegistry } from './features_registry';
18 changes: 18 additions & 0 deletions src/plugins/discover_shared/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* 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',
rootDir: '../../..',
roots: ['<rootDir>/src/plugins/discover_shared'],
coverageDirectory: '<rootDir>/target/kibana-coverage/jest/src/plugins/discover_shared',
coverageReporters: ['text', 'html'],
collectCoverageFrom: [
'<rootDir>/src/plugins/discover_shared/{common,public,server}/**/*.{js,ts,tsx}',
],
};
13 changes: 13 additions & 0 deletions src/plugins/discover_shared/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"type": "plugin",
"id": "@kbn/discover-shared-plugin",
"owner": ["@elastic/kibana-data-discovery", "@elastic/obs-ux-logs-team"],
"description": "A stateful layer to register shared features and provide an access point to discover without a direct dependency",
"plugin": {
"id": "discoverShared",
"server": false,
"browser": true,
"requiredPlugins": [],
"optionalPlugins": [],
},
}
20 changes: 20 additions & 0 deletions src/plugins/discover_shared/public/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* 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.
*/

import { DiscoverSharedPlugin } from './plugin';

export function plugin() {
return new DiscoverSharedPlugin();
}

export type { DiscoverSharedPublicSetup, DiscoverSharedPublicStart } from './types';
export type {
ObservabilityLogsAIAssistantFeatureRenderDeps,
ObservabilityLogsAIAssistantFeature,
DiscoverFeature,
} from './services/discover_features';
33 changes: 33 additions & 0 deletions src/plugins/discover_shared/public/mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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.
*/

import {
createDiscoverFeaturesServiceSetupMock,
createDiscoverFeaturesServiceStartMock,
} from './services/discover_features/discover_features_service.mock';
import { DiscoverSharedPublicSetup, DiscoverSharedPublicStart } from './types';

export type Setup = jest.Mocked<DiscoverSharedPublicSetup>;
export type Start = jest.Mocked<DiscoverSharedPublicStart>;

const createSetupContract = (): Setup => {
return {
features: createDiscoverFeaturesServiceSetupMock(),
};
};

const createStartContract = (): Start => {
return {
features: createDiscoverFeaturesServiceStartMock(),
};
};

export const discoverSharedPluginMock = {
createSetupContract,
createStartContract,
};
26 changes: 26 additions & 0 deletions src/plugins/discover_shared/public/plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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.
*/

import { DiscoverFeaturesService } from './services/discover_features';
import { DiscoverSharedPublicPlugin } from './types';

export class DiscoverSharedPlugin implements DiscoverSharedPublicPlugin {
private discoverFeaturesService: DiscoverFeaturesService = new DiscoverFeaturesService();

public setup() {
return {
features: this.discoverFeaturesService.setup(),
};
}

public start() {
return {
features: this.discoverFeaturesService.start(),
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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.
*/

import { FeaturesRegistry } from '../../../common';
import { DiscoverFeature } from './types';

const registry = new FeaturesRegistry<DiscoverFeature>();

export const createDiscoverFeaturesServiceSetupMock = () => ({ registry });
export const createDiscoverFeaturesServiceStartMock = () => ({ registry });

0 comments on commit 747ecea

Please sign in to comment.