Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Infrastructure UI][Rules] Fix viewInAppUrl for custom metrics for Inventory Threshold Rule #134114

Conversation

simianhacker
Copy link
Member

@simianhacker simianhacker commented Jun 9, 2022

Summary

This PR fixes #134110 by fixing a faulty if condition in the link creation function.

Checklist

@simianhacker simianhacker added v8.4.0 v8.3.1 Feature:Metrics UI Metrics UI feature Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" release_note:fix labels Jun 9, 2022
const criteriaCustomMetricId = fields[`${ALERT_RULE_PARAMETERS}.criteria.customMetric.id`][0];
if (criteriaCustomMetricId !== 'alert-custom-metric') {
if (criteriaMetric === 'custom') {
const criteriaCustomMetricId = fields[`${ALERT_RULE_PARAMETERS}.criteria.customMetric.id`][0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simianhacker This change looks reasonable to me. I am just wondering how it used to work in the first place. Here's the PR that introduced this change https://github.com/elastic/kibana/pull/113553/files#diff-173c0f4e56c53ef08415cbd7e2a1773b2bd84596e5c366be4fb55507d33265fdR2. I'll take a closer look

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why it was written that way, probably just a misunderstanding of how the custom metric stuff works.

@simianhacker simianhacker marked this pull request as ready for review June 13, 2022 14:53
@simianhacker simianhacker requested a review from a team as a code owner June 13, 2022 14:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

@simianhacker
Copy link
Member Author

@elasticmachine merge upstream

@simianhacker simianhacker enabled auto-merge (squash) June 13, 2022 14:54
@klacabane klacabane self-requested a review June 16, 2022 15:13
@klacabane
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@klacabane klacabane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested on an inventory rule with system.process.cpu.total.value custom metric - lgtm

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
infra 83.9KB 83.9KB -13.0B

History

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

@simianhacker simianhacker merged commit a020f05 into elastic:main Jun 16, 2022
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 20, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

7 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

6 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 134114 locally

@smith smith added auto-backport Deprecated: Automatically backport this PR after it's merged and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Jul 8, 2022
kibanamachine pushed a commit that referenced this pull request Jul 8, 2022
…ventory Threshold Rule (#134114)

* [Infrastructure UI][Rules] Fix viewInAppUrl for custom metrics for Inventory Threshold Rule

* Adding test for generating link, moving custom field inside if statement

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit a020f05)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jul 8, 2022
…ventory Threshold Rule (#134114) (#136042)

* [Infrastructure UI][Rules] Fix viewInAppUrl for custom metrics for Inventory Threshold Rule

* Adding test for generating link, moving custom field inside if statement

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit a020f05)

Co-authored-by: Chris Cowan <chris@chriscowan.us>
@simianhacker simianhacker deleted the issue-134110-fix-broken-inventory-view-in-app-url branch April 17, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged Feature:Metrics UI Metrics UI feature release_note:fix Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.3.1 v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infrastructure UI][Rules] The viewInAppURL for custom metrics for the Inventory Threshold Rule is broken.
7 participants