[RAC] Link inventory alerts to the right inventory view #113553

afgomez · 2021-09-30T16:03:00Z

For @elastic/metrics-ui, what would be the right way to generate the URL for the inventory view?

elasticmachine · 2021-10-11T09:39:13Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

afgomez · 2021-10-11T09:40:30Z

x-pack/plugins/infra/public/pages/link_to/redirect_to_inventory.tsx

+import { parse } from 'query-string';
+import { Redirect, RouteComponentProps } from 'react-router-dom';
+
+// FIXME what would be the right way to build this query string?


If we cannot find a good way in this PR we can address this later.

If I recall correctly Kibana provides a service already to create good URLs? Maybe it's worth leaving this as is and wait until that service is used in Metrics to update this code

You would need to use the useKibana hook to get ahold of the application service like this:

const kibana = useKibana<InfraClientSetupDeps>(); const link = kibana.services.application?.getUrlForApp('infra', { path: `/inventory${inventoryQueryString}` });

As for the QUERY_STRING_TEMPLATE what you're doing is probably fine BUT if we wanted to make sure the Rison is serialized correctly, we could use the Rison library to encode the objects (like you did for the custom metric).

I'm more concerned about what happens if/when the parameters in the URL change. It feels I'm duplicating things here and it's just a matter of time until it gets changed somewhere else and this breaks.

Is there something in metrics to generate valid URLs?

Mmm, apparently the <Redirect /> component where this link is used is handling already the app prefix. When I use getUrlForApp the end result is /guk/app/metrics/guk/app/infra/inventory (when using infra as an app).

I think moving forward it's better to leave the code as it is now (revisiting how the query string is constructed). What do you think @simianhacker?

Sounds good.

afgomez · 2021-10-11T09:41:38Z

...er/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts


 const condition = schema.object({
  threshold: schema.arrayOf(schema.number()),
-  comparator: oneOfLiterals(Object.values(Comparator)),
-  timeUnit: schema.string(),
+  comparator: oneOfLiterals(Object.values(Comparator)) as Type<Comparator>,


All these type assertions are necessary to make the kibana schema play nicely with the io-ts generated types.

simianhacker

Everything looks good to me. I added some inline comments about using getUrlForApp to generate the URLs.

simianhacker · 2021-10-12T17:45:49Z

x-pack/plugins/infra/public/pages/link_to/redirect_to_inventory.tsx

+import { parse } from 'query-string';
+import { Redirect, RouteComponentProps } from 'react-router-dom';
+
+// FIXME what would be the right way to build this query string?


You would need to use the useKibana hook to get ahold of the application service like this:

const kibana = useKibana<InfraClientSetupDeps>(); const link = kibana.services.application?.getUrlForApp('infra', { path: `/inventory${inventoryQueryString}` });

As for the QUERY_STRING_TEMPLATE what you're doing is probably fine BUT if we wanted to make sure the Rison is serialized correctly, we could use the Rison library to encode the objects (like you did for the custom metric).

simianhacker · 2021-10-12T17:49:44Z

x-pack/plugins/infra/public/alerting/inventory/rule_data_formatters.ts

-  const link = '/app/metrics/inventory'; // TODO https://github.com/elastic/kibana/issues/106497
+  const ruleParams = parseRuleParams(fields[ALERT_RULE_PARAMS]);
+
+  let link = '/app/metrics/link-to/inventory?';


We should probably use getUrlForApp here as well.

This is not used within a React context so calling useKibana is not possible. I'm looking how easy is to inject the kibana services and it's not that straightforward.

The link is correctly prefixed when used (the prepend() call in the linked code), so I wonder if that's good enough.

shakes fist at sky "Damn you hooks!"

afgomez · 2021-10-14T14:17:56Z

@elasticmachine merge upstream

simianhacker

  _      _____ _______ __  __ 
 | |    / ____|__   __|  \/  |
 | |   | |  __   | |  | \  / |
 | |   | | |_ |  | |  | |\/| |
 | |___| |__| |  | |  | |  | |
 |______\_____|  |_|  |_|  |_|

afgomez · 2021-10-18T07:57:23Z

@elasticmachine merge upstream

kibanamachine · 2021-10-18T10:19:54Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	964	965	+1

Async chunks

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

id	before	after	diff
`infra`	938.5KB	939.3KB	+819.0B

Page load bundle

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

id	before	after	diff
`infra`	90.2KB	90.6KB	+451.0B

History

💚 Build #160947 succeeded e744466
💚 Build #159393 succeeded 269f813
💔 Build #159369 failed 6da0f212e4509df38473256629b21a9bf8af23b7
💔 Build #158885 failed 3a49cd870b2b44f080b10b97e968e54f5c8e5636
💔 Build #158873 failed a857bf43219c303f7baddfc5c120c0cba1dd15d5

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

Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-10-18T10:23:00Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…-link-to-kibana-app * 'master' of github.com:elastic/kibana: (287 commits) [Security Solution][Endpoint] Change `trustedAppByPolicyEnabled` flag to `true` by default (elastic#115264) [APM] generator: support error events and application metrics (elastic#115311) [kibanaUtils] Don't import full `semver` client side (elastic#114986) [RAC] Link inventory alerts to the right inventory view (elastic#113553) [Uptime] Added uptime query inspector panel (elastic#115170) [Osquery] Add packs (elastic#107345) [App Search] Allow for query parameter to indicate ingestion mechanism for new engines (elastic#115188) [Alerting] Active alerts do not recover after re-enabling a rule (elastic#111671) skip flaky tests. elastic#115308, elastic#115313 [Breaking] Remove deprecated `enabled` settings from plugins. (elastic#113495) skip flaky suite. elastic#107057 skip flaky tests. elastic#89052, elastic#113418, elastic#115304 skip flaky test. elastic#113892 Bump node to 16.11.1 (elastic#110684) [Security Solution] Restores Alerts table local storage persistence and the Remove Column action (elastic#114742) skip flaky suite. elastic#115130 one line remove assert (elastic#115127) Fixes migration bug where I was deleting attributes (elastic#115098) [Security Solutions] Fixes the newer notification system throttle resets and enabling immediate execution on first detection of a signal (elastic#114214) [build] Dockerfile update (elastic#115237) ... # Conflicts: # x-pack/plugins/reporting/public/management/__snapshots__/report_listing.test.tsx.snap

kibanamachine · 2021-10-20T10:23:22Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…15332) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Alejandro Fernández Gómez <[email protected]>

afgomez mentioned this pull request Oct 5, 2021

[RAC][Metrics UI] Link inventory threshold alerts to a specific timestamp in the metrics app #111380

Closed

afgomez force-pushed the 106497-inventory-alert-link-to-app branch 4 times, most recently from a857bf4 to 3a49cd8 Compare October 7, 2021 12:45

Alejandro Fernández Gómez added 4 commits October 11, 2021 10:53

Link inventory alerts to the right inventory view

f792832

Add a link-to route to inventory

556ca4e

Tweak InventoryMetricThresholdParams and use it when parsing the params

c1d0cf3

Reconciliate schema and io-ts types

269f813

afgomez force-pushed the 106497-inventory-alert-link-to-app branch from 6da0f21 to 269f813 Compare October 11, 2021 08:53

afgomez marked this pull request as ready for review October 11, 2021 09:39

afgomez requested a review from a team as a code owner October 11, 2021 09:39

afgomez commented Oct 11, 2021

View reviewed changes

simianhacker self-requested a review October 12, 2021 16:21

simianhacker requested changes Oct 12, 2021

View reviewed changes

afgomez requested a review from simianhacker October 14, 2021 11:46

Merge branch 'master' into 106497-inventory-alert-link-to-app

e744466

simianhacker approved these changes Oct 15, 2021

View reviewed changes

Merge branch 'master' into 106497-inventory-alert-link-to-app

638999b

afgomez enabled auto-merge (squash) October 18, 2021 07:58

afgomez merged commit 27c7c6f into elastic:master Oct 18, 2021

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2021

[RAC] Link inventory alerts to the right inventory view (elastic#113553)

b7ce472

Co-authored-by: Kibana Machine <[email protected]>

kibanamachine mentioned this pull request Oct 18, 2021

[7.x] [RAC] Link inventory alerts to the right inventory view (#113553) #115332

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 20, 2021

kibanamachine added a commit that referenced this pull request Oct 20, 2021

[RAC] Link inventory alerts to the right inventory view (#113553) (#1…

1649567

…15332) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Alejandro Fernández Gómez <[email protected]>

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC] Link inventory alerts to the right inventory view #113553

[RAC] Link inventory alerts to the right inventory view #113553

afgomez commented Sep 30, 2021 •

edited

Loading

elasticmachine commented Oct 11, 2021

afgomez Oct 11, 2021

simianhacker Oct 12, 2021

afgomez Oct 14, 2021 •

edited

Loading

afgomez Oct 14, 2021

simianhacker Oct 15, 2021

afgomez Oct 11, 2021

simianhacker left a comment

simianhacker Oct 12, 2021

simianhacker Oct 12, 2021

afgomez Oct 14, 2021

simianhacker Oct 14, 2021

afgomez commented Oct 14, 2021

simianhacker left a comment

afgomez commented Oct 18, 2021

kibanamachine commented Oct 18, 2021

kibanamachine commented Oct 18, 2021

kibanamachine commented Oct 20, 2021

[RAC] Link inventory alerts to the right inventory view #113553

[RAC] Link inventory alerts to the right inventory view #113553

Conversation

afgomez commented Sep 30, 2021 • edited Loading

elasticmachine commented Oct 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afgomez Oct 14, 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

simianhacker 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

afgomez commented Oct 14, 2021

simianhacker left a comment

Choose a reason for hiding this comment

afgomez commented Oct 18, 2021

kibanamachine commented Oct 18, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

kibanamachine commented Oct 18, 2021

💚 Backport successful

kibanamachine commented Oct 20, 2021

afgomez commented Sep 30, 2021 •

edited

Loading

afgomez Oct 14, 2021 •

edited

Loading