[RAC] Populate Observability alerts table with data from alerts indices #96692

dgieselaar · 2021-04-09T10:24:58Z

Changes to the rule registry:

The rule registry now has a public plugin as well. It doesn’t do much other than creating a client-side root rule registry, and exposing it in its setup contract.
The scopedRuleRegistryClient now expects rule uuids to be passed into it, rather than fetching it by itself. I ran into issues with using a scoped saved objects client, and I couldn’t figure out how to get access to the alertsClient inside a rule executor, only in the context of a request. That means that the scopedRuleRegistryClient in a rule executor can also only read data from that rule. That is something that should be fixed at some point, to enable alerts on alerts.
The rule registry now attempts to update template and index mappings on startup. This is to make it easier for now to make changes to the mappings. We probably need a different strategy once it’s no longer experimental.
The scopedRuleRegistryClient now also has a getDynamicIndexPattern() that is used for the search bar. Probably autocompletion needs to be moved to the server entirely, because non-superusers will likely not have access to the alerts indices by default.

Observability/APM changes:

The APM rules now also store evaluation data (value & threshold).
A top alerts API endpoint is added in the Observability plugin.
Some code was moved and/or copied from the APM folder to allow the Observability plugin to use it.
The APM rule types now implement a format() function that takes an alert, and returns a reason/link that is used in the alerts table and flyout.

shahzad31 · 2021-04-12T05:44:25Z

x-pack/plugins/observability/server/routes/rules.ts

+const alertsDynamicIndexPatternRoute = createObservabilityServerRoute({
+  endpoint: 'GET /api/observability/rules/alerts/dynamic_index_pattern',
+  options: {
+    tags: [],
+  },
+  handler: async ({ ruleRegistry, context }) => {
+    const ruleRegistryClient = ruleRegistry.createScopedRuleRegistryClient({
+      context,
+    });
+
+    if (!ruleRegistryClient) {
+      throw Boom.failedDependency();
+    }
+
+    return ruleRegistryClient.getDynamicIndexPattern();
+  },
+});


you can use data.indexPatterns.getFieldsForWildcard on client, i don't think it make sense to create a separate route for it.

This call is done with the internal user. I assume that's not possible in the browser?

you are right, but will it make a different in this case? AFAIK, you can't assign individual permissions to index patterns. Kibana permissions are weird as far as data goes, they are useful for limiting UI features though.

I'm not sure what you mean with "assign individual permissions to index patterns"?

…-rule-apis

* Use `MemoryRouter` in the stories and `useHistory` in the component to get the history * Replace examples with ones from "real" data * Use `() => {}` instead of `jest.fn()` in mock registry data

…-rule-apis

…a into observability-rule-apis

smith

This looks good as-is.

We're going to need to do something about the bundle size. The rule registry plugin has extraPublicDirs: common so anything exported from common gets loaded on every page load. We might have to limit what's in common. Not sure where the APM increase is coming from. Haven't looked.

I suppose some increase in bundle size is acceptable since we're adding new features.

smith · 2021-04-12T13:08:35Z

x-pack/plugins/observability/public/rules/observability_rule_registry_mock.ts

+import { ObservabilityRuleRegistry } from '../plugin';
+
+const createRuleRegistryMock = () => ({
+  registerType: jest.fn(),


I think we're trying to use these in stories, where there is no jest global.

smith · 2021-04-12T15:25:05Z

x-pack/plugins/apm/server/lib/alerts/alerting_es_client.ts

+  logger: Logger;
+  params: TParams;
+}): Promise<ESSearchResponse<unknown, TParams>> {
+  // logger.debug(JSON.stringify(params, null, 2));


Remove this?

smith · 2021-04-12T15:25:14Z

x-pack/plugins/apm/server/lib/alerts/alerting_es_client.ts

  const response = await scopedClusterClient.asCurrentUser.search({
    ...params,
    ignore_unavailable: true,
  });

+  // logger.debug(JSON.stringify(response.body, null, 2));


Remove this?

smith · 2021-04-12T15:25:57Z

x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_alert_type.ts

@@ -149,6 +150,10 @@ export function registerTransactionDurationAlertType({
                  ? { [SERVICE_ENVIRONMENT]: environmentParsed.esFieldValue }
                  : {}),
                [TRANSACTION_TYPE]: alertParams.transactionType,
+                [PROCESSOR_EVENT]: ProcessorEvent.transaction,
+                'kibana.observability.evaluation.value': transactionDuration,


Should we have constants for these?

Yeah, I do miss the constants from APM. But I'll put this one on the TODO list, I need to look at how the mappings/types are generated anyway.

…-rule-apis

elasticmachine · 2021-04-14T15:08:18Z

Pinging @elastic/apm-ui (Team:apm)

dgieselaar · 2021-04-14T15:18:28Z

@elastic/kibana-operations I've created #97128 to look at the bundle size.

jbudz · 2021-04-14T16:28:25Z

packages/kbn-optimizer/limits.yml

@@ -61,6 +61,7 @@ pageLoadAssetSize:
  remoteClusters: 51327
  reporting: 183418
  rollup: 97204
+  ruleRegistry: 107971


Can start this with 100kb? It looks like it should still pass the limits check.

Sure thing 👍

…-rule-apis

kibanamachine · 2021-04-14T19:43:22Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1573	1574	+1
`observability`	314	334	+20
`ruleRegistry`	-	17	+17
total			+38

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	4.2MB	4.1MB	-54.5KB
`observability`	416.8KB	441.8KB	+25.1KB
total			-29.5KB

Page load bundle

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

id	before	after	diff
`apm`	22.2KB	53.5KB	+31.3KB
`observability`	35.1KB	39.0KB	+3.9KB
`ruleRegistry`	-	90.8KB	+90.8KB
total			+126.0KB

History

💚 Build #119566 succeeded 1b0c9c1
💔 Build #119536 failed ba9caea
💔 Build #119307 failed b578fa7
💔 Build #119193 failed 2832e18
💔 Build #118801 failed 6dcf396

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

shahzad31 · 2021-04-15T08:57:27Z

It more than doubles page load bundles for apm and i have seen the since RAC initiative , observability plugin page load bundle is also increasing bit by bit in each PR.

I worked hard on reducing at least these two bundles :D

can we please put an extra effort to make sure this can be avoided? Will really appreciate that.

dgieselaar · 2021-04-15T09:01:50Z

@shahzad31 #97128.

sorenlouv · 2021-04-15T12:41:42Z

x-pack/plugins/observability/common/utils/formatters/size.ts

+  }
+
+  return asBytes;
+};


I don't see this file as deleted. So it was just copied over from APM?

(I tried copying one over, but they're all somehow connected, so ended up copying the whole folder and assuming we will start using those from APM in the near future).

sorenlouv · 2021-04-15T12:43:16Z

x-pack/plugins/observability/public/hooks/use_fetcher.tsx

@@ -28,7 +28,7 @@ type InferResponseType<TReturn> = Exclude<TReturn, undefined> extends Promise<in
  : unknown;

 export function useFetcher<TReturn>(


Should we delete the useFetcher hook in APM to avoid the two drifting apart again?

Preferably yes! But it's kind of hard to share public code right now. Hoping we can tackle that later.

sorenlouv

Wow, huuuge PR! But overall lgtm.

dgieselaar · 2021-04-15T13:34:28Z

@shahzad31 @jbudz I've opened up a PR that hopefully reduces the page load bundle size somewhat again. I think the rule registry plugin is down to 10k and the APM one is down again as well (though it's still a small increase). #97251

spong

Rule registry changes LGTM! 👍

…es (elastic#96692) * Set up Observability rule APIs * Populate alerts table with data from API * Move field map types/utils to common * Format reason/link in alert type * Format reason/link in alert type * Fix issues with tsconfigs * Storybook cleanup for example alerts * Use `MemoryRouter` in the stories and `useHistory` in the component to get the history * Replace examples with ones from "real" data * Use `() => {}` instead of `jest.fn()` in mock registry data * Store/display evaluations, add active/recovered badge * Some more story fixes * Decode rule data with type from owning registry * Use transaction type/environment in link to app * Fix type issues * Fix API tests * Undo changes in task_runner.ts * Remove Mutable<> wrappers for field map * Remove logger.debug calls in alerting es client * Add API test for recovery of alerts * Revert changes to src/core/server/http/router * Use type imports where possible * Update limits * Set limit to 100kb Co-authored-by: Nathan L Smith <[email protected]>

…es (#96692) (#97399) * Set up Observability rule APIs * Populate alerts table with data from API * Move field map types/utils to common * Format reason/link in alert type * Format reason/link in alert type * Fix issues with tsconfigs * Storybook cleanup for example alerts * Use `MemoryRouter` in the stories and `useHistory` in the component to get the history * Replace examples with ones from "real" data * Use `() => {}` instead of `jest.fn()` in mock registry data * Store/display evaluations, add active/recovered badge * Some more story fixes * Decode rule data with type from owning registry * Use transaction type/environment in link to app * Fix type issues * Fix API tests * Undo changes in task_runner.ts * Remove Mutable<> wrappers for field map * Remove logger.debug calls in alerting es client * Add API test for recovery of alerts * Revert changes to src/core/server/http/router * Use type imports where possible * Update limits * Set limit to 100kb Co-authored-by: Nathan L Smith <[email protected]> Co-authored-by: Nathan L Smith <[email protected]>

yctercero · 2021-04-19T22:03:14Z

packages/kbn-optimizer/limits.yml

@@ -61,6 +61,7 @@ pageLoadAssetSize:
  remoteClusters: 51327
  reporting: 183418
  rollup: 97204
+  ruleRegistry: 100000


I know this is a bit late, but shouldn't this be near 0 since there's no actual UI yet?

There isn't any UI, but there is the rule registry setup/and start methods and the RuleRegistry class is exported. The bundle includes some io/fp-ts utils and the ECS field mapping, which takes up most of the space.

It's down to 10kb now.

dgieselaar added 2 commits April 9, 2021 12:12

Set up Observability rule APIs

2a2e9bc

Populate alerts table with data from API

4b585f7

dgieselaar added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes v7.13.0 Theme: rac label obsolete labels Apr 9, 2021

Move field map types/utils to common

4401d37

shahzad31 reviewed Apr 12, 2021

View reviewed changes

dgieselaar added 4 commits April 12, 2021 12:08

Format reason/link in alert type

383eb78

Format reason/link in alert type

0a60956

Merge branch 'master' of github.com:elastic/kibana into observability…

d95d3d8

…-rule-apis

Fix issues with tsconfigs

a30fc84

smith mentioned this pull request Apr 12, 2021

Populate observability alerts table with alert data #96566

Closed

smith and others added 11 commits April 12, 2021 10:11

Storybook cleanup for example alerts

e72e9b4

* Use `MemoryRouter` in the stories and `useHistory` in the component to get the history * Replace examples with ones from "real" data * Use `() => {}` instead of `jest.fn()` in mock registry data

Store/display evaluations, add active/recovered badge

f5bb6f9

Some more story fixes

6dcf396

Merge branch 'master' of github.com:elastic/kibana into observability…

54d851a

…-rule-apis

Decode rule data with type from owning registry

46cda90

Merge branch 'master' of github.com:elastic/kibana into observability…

ab29974

…-rule-apis

Use transaction type/environment in link to app

f6e7997

Fix type issues

c3f63c4

Merge branch 'master' of github.com:elastic/kibana into observability…

075a3c7

…-rule-apis

Merge branch 'observability-rule-apis' of github.com:dgieselaar/kiban…

2832e18

…a into observability-rule-apis

Fix API tests

b578fa7

smith approved these changes Apr 14, 2021

View reviewed changes

dgieselaar added 5 commits April 14, 2021 08:38

Merge branch 'master' of github.com:elastic/kibana into observability…

e72de87

…-rule-apis

Undo changes in task_runner.ts

6089beb

Remove Mutable<> wrappers for field map

728fc3f

Remove logger.debug calls in alerting es client

5c190a8

Add API test for recovery of alerts

d745a25

dgieselaar added 4 commits April 14, 2021 10:50

Merge branch 'master' of github.com:elastic/kibana into observability…

ba9caea

…-rule-apis

Revert changes to src/core/server/http/router

79a98b9

Use type imports where possible

4a5d7bb

Update limits

1b0c9c1

dgieselaar marked this pull request as ready for review April 14, 2021 15:08

dgieselaar requested review from a team as code owners April 14, 2021 15:08

dgieselaar requested a review from spong April 14, 2021 15:08

jbudz reviewed Apr 14, 2021

View reviewed changes

dgieselaar added 2 commits April 14, 2021 19:36

Set limit to 100kb

110a762

Merge branch 'master' of github.com:elastic/kibana into observability…

64b637c

…-rule-apis

smith mentioned this pull request Apr 14, 2021

Flyout and alerts table improvements #96976

Merged

sorenlouv reviewed Apr 15, 2021

View reviewed changes

sorenlouv approved these changes Apr 15, 2021

View reviewed changes

spong approved these changes Apr 15, 2021

View reviewed changes

jbudz approved these changes Apr 15, 2021

View reviewed changes

dgieselaar merged commit 5bb9eec into elastic:master Apr 15, 2021

dgieselaar deleted the observability-rule-apis branch April 15, 2021 16:25

dgieselaar mentioned this pull request Apr 18, 2021

[7.x] [RAC] Populate Observability alerts table with data from alerts indices (#96692) #97399

Merged

yctercero reviewed Apr 19, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC] Populate Observability alerts table with data from alerts indices #96692

[RAC] Populate Observability alerts table with data from alerts indices #96692

dgieselaar commented Apr 9, 2021 •

edited

Loading

shahzad31 Apr 12, 2021 •

edited

Loading

dgieselaar Apr 12, 2021

shahzad31 Apr 12, 2021

dgieselaar Apr 12, 2021

smith left a comment

smith Apr 12, 2021

smith Apr 12, 2021

smith Apr 12, 2021

smith Apr 12, 2021

dgieselaar Apr 14, 2021

elasticmachine commented Apr 14, 2021

dgieselaar commented Apr 14, 2021

jbudz Apr 14, 2021

dgieselaar Apr 14, 2021

kibanamachine commented Apr 14, 2021

shahzad31 commented Apr 15, 2021

dgieselaar commented Apr 15, 2021

sorenlouv Apr 15, 2021

dgieselaar Apr 15, 2021

dgieselaar Apr 15, 2021

sorenlouv Apr 15, 2021

dgieselaar Apr 15, 2021

sorenlouv left a comment

dgieselaar commented Apr 15, 2021 •

edited

Loading

spong left a comment

yctercero Apr 19, 2021

smith Apr 19, 2021

dgieselaar Apr 20, 2021

		@@ -28,7 +28,7 @@ type InferResponseType<TReturn> = Exclude<TReturn, undefined> extends Promise<in
		: unknown;

		export function useFetcher<TReturn>(

[RAC] Populate Observability alerts table with data from alerts indices #96692

[RAC] Populate Observability alerts table with data from alerts indices #96692

Conversation

dgieselaar commented Apr 9, 2021 • edited Loading

shahzad31 Apr 12, 2021 • 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

smith left a 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

elasticmachine commented Apr 14, 2021

dgieselaar commented Apr 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Apr 14, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

shahzad31 commented Apr 15, 2021

dgieselaar commented Apr 15, 2021

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

sorenlouv left a comment

Choose a reason for hiding this comment

dgieselaar commented Apr 15, 2021 • edited Loading

spong left a 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

dgieselaar commented Apr 9, 2021 •

edited

Loading

shahzad31 Apr 12, 2021 •

edited

Loading

dgieselaar commented Apr 15, 2021 •

edited

Loading