[investigate] Copy changes from POC #187936

kdelemme · 2024-07-10T07:51:40Z

Summary

Copy investigate/ plugin changes from #183293 into this PR.
Will follow up with the new plugin investigate_app/

Nothing to test here, it's just a public plugin exposing some hook and registry. Not used yet

obltmachine · 2024-07-10T07:51:56Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-07-10T08:01:21Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

…fix'

dgieselaar · 2024-07-11T11:21:18Z

x-pack/plugins/observability_solution/investigate/public/plugin.tsx

+      register: (callback) => {
+        const registrationPromise = Promise.race([
+          callback(this.widgetRegistry.registerWidget),
+          new Promise<void>((resolve, reject) => {
+            setTimeout(() => {
+              reject(new Error('Timed out running registration function'));
+            }, 30000);
+          }),
+        ]).catch((error) => {
+          this.logger.error(
+            new Error('Encountered an error during widget registration', { cause: error })
+          );
+          return Promise.resolve();
+        });
+
+        this.registrationPromises.push(registrationPromise);
+      },
    };


There is an opportunity here for a race condition: register is asynchronous, but getWidgetDefinitions is not. In practice you will probably never run into it, but it might become an issue for tests. You could consider making an Observable out of getWidgetDefinitions but there are some implications for code downstream.

mgiota · 2024-07-15T11:51:29Z

x-pack/plugins/observability_solution/investigate/public/index.ts

@@ -20,15 +20,17 @@ import type {
 export type { InvestigatePublicSetup, InvestigatePublicStart, OnWidgetAdd, WidgetRenderAPI };

 export {
-  type InvestigateTimeline,
+  type Investigation,


I am wondering why we export these types from both public and common folders? I was looking into the consumer side where it is imported from and in some places we import it from common '@kbn/investigate-plugin/common';, where as in others from public '@kbn/investigate-plugin/public';

nit: Maybe we can keep only one place where we export/import from, unless there is a restriction I am not aware of.

this is for other plugins to import it (without extra config, other plugins cannot import from /common). the difference that you see might be based on type imports vs code imports.

mgiota · 2024-07-15T12:28:24Z

x-pack/plugins/observability_solution/investigate/common/types.ts

+export interface Investigation {
+  id: string;
+  '@timestamp': number;
+  user: AuthenticatedUser;


Once we have the concept of more users participating in one investigation, do we consider having here an array of authenticated users? Or possibly we could have an extra field invitedUsers. This is definitely not part of version 1, but I am trying to think about what we might introduce in the future that could shape the Investigation interface.

mgiota · 2024-07-15T12:37:43Z

...bservability_solution/investigate/public/hooks/use_investigation/create_new_investigation.ts

+      defaultMessage: 'New investigation',
+    }),
+    revision: revisionId,
+    revisions: [revision],


Do I understand correctly that we add to the revisions array only the latest revision? I thought we might want to have a list of all revisions. Do I miss something here?

there is an Investigation, and an InvestigationRevision, the former contains the latter. (eg. investigation.revisions)

mgiota · 2024-07-15T12:51:25Z

x-pack/plugins/observability_solution/investigate/public/hooks/use_investigate_widget.tsx

+> {
+  onWidgetAdd: (create: InvestigateWidgetCreate) => Promise<void>;
+  blocks: {
+    publish: (blocks: WorkflowBlock[]) => UnregisterBlocksFunction;


Just weird that a publish function returns an unregisterBlocksFunction.

Maybe not as part of this PR, but let's think more about WorkflowBlock interface and map it to the mock up designs.

it's so you can use it in a useEffect hook

mgiota · 2024-07-15T13:07:54Z

x-pack/plugins/observability_solution/investigate/public/hooks/use_investigate_widget.tsx

+
+const InvestigateWidgetApiContext = createContext<UseInvestigateWidgetApi | undefined>(undefined);
+
+export const InvestigateWidgetApiContextProvider = InvestigateWidgetApiContext.Provider;


@dgieselaar Is this actually used anywhere? I was looking into the inital PR of how it is being used, but I didn't find any use of it, other than being exported here. If it is not being used, then we could remove it.

it isn't, but I did discuss it with Kevin & Bena: I think context in this case is better than prop drilling, so I would use this API instead

mgiota

Look like investigate plugin changes were properly copied from the initial POC to this new PR. I just left a few clarification questions.

elasticmachine · 2024-07-16T14:06:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 5333a8c
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-187936-5333a8c96901
Observability Deployment

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`investigate`	12	20	+8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`investigate`	95	134	+39

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`investigate`	4	6	+2

Page load bundle

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

id	before	after	diff
`investigate`	2.8KB	13.0KB	+10.2KB

Unknown metric groups

API count

id	before	after	diff
`investigate`	95	134	+39

ESLint disabled line counts

id	before	after	diff
`investigate`	0	1	+1

Total ESLint disabled count

id	before	after	diff
`investigate`	2	3	+1

History

💚 Build #220611 succeeded 229b616
💚 Build #220552 succeeded d50432d

cc @kdelemme

Copy investigate plugin changes from poc PR

3f92198

kdelemme marked this pull request as ready for review July 10, 2024 07:56

kdelemme requested a review from a team as a code owner July 10, 2024 07:56

kdelemme self-assigned this Jul 10, 2024

kdelemme added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.0 labels Jul 10, 2024

Change codeowner

d628ee4

botelastic bot added the ci:project-deploy-observability Create an Observability project label Jul 10, 2024

kibanamachine and others added 3 commits July 10, 2024 08:53

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

37022d8

…fix'

[CI] Auto-commit changed files from 'node scripts/generate codeowners'

3eaf5d7

Change codeowner

fe0ae81

kdelemme requested a review from a team as a code owner July 10, 2024 09:23

kibanamachine and others added 2 commits July 10, 2024 09:37

[CI] Auto-commit changed files from 'node scripts/generate codeowners'

d50432d

Merge branch 'main' into investigate-ui

229b616

kdelemme requested a review from a team as a code owner July 10, 2024 12:50

kdelemme removed request for a team July 10, 2024 12:50

dgieselaar reviewed Jul 11, 2024

View reviewed changes

This was referenced Jul 11, 2024

Investigate UI 2 kdelemme/kibana#1

Closed

chore(investigate): Add investigate-app plugin from poc #188122

Merged

mgiota self-requested a review July 15, 2024 09:39

mgiota reviewed Jul 15, 2024

View reviewed changes

mgiota approved these changes Jul 15, 2024

View reviewed changes

Merge branch 'main' into investigate-ui

5333a8c

kdelemme merged commit 4dca5f5 into elastic:main Jul 16, 2024
26 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Jul 16, 2024

mgiota mentioned this pull request Jul 17, 2024

[RCA] Create investigation detail page (V1) #187286

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[investigate] Copy changes from POC #187936

[investigate] Copy changes from POC #187936

kdelemme commented Jul 10, 2024 •

edited

Loading

obltmachine commented Jul 10, 2024

elasticmachine commented Jul 10, 2024

dgieselaar Jul 11, 2024

mgiota Jul 15, 2024 •

edited

Loading

dgieselaar Jul 15, 2024

mgiota Jul 15, 2024

mgiota Jul 15, 2024

dgieselaar Jul 15, 2024

mgiota Jul 15, 2024

dgieselaar Jul 15, 2024

mgiota Jul 15, 2024

dgieselaar Jul 15, 2024

mgiota left a comment •

edited

Loading

elasticmachine commented Jul 16, 2024 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count


		const InvestigateWidgetApiContext = createContext<UseInvestigateWidgetApi \| undefined>(undefined);

		export const InvestigateWidgetApiContextProvider = InvestigateWidgetApiContext.Provider;

[investigate] Copy changes from POC #187936

[investigate] Copy changes from POC #187936

Conversation

kdelemme commented Jul 10, 2024 • edited Loading

Summary

obltmachine commented Jul 10, 2024

🤖 GitHub comments

elasticmachine commented Jul 10, 2024

Choose a reason for hiding this comment

mgiota Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgiota left a comment • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Jul 16, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Public APIs missing comments

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

kdelemme commented Jul 10, 2024 •

edited

Loading

mgiota Jul 15, 2024 •

edited

Loading

mgiota left a comment •

edited

Loading

elasticmachine commented Jul 16, 2024 •

edited

Loading