-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Metric threshold] Fix the condition not showing up in the metric threshold flyout #192736
[Metric threshold] Fix the condition not showing up in the metric threshold flyout #192736
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
@@ -45,7 +45,7 @@ export const useSourceFetcher = ({ sourceId }: { sourceId: string }) => { | |||
method: 'GET', | |||
} | |||
); | |||
telemetry.reportPerformanceMetricEvent( | |||
telemetry?.reportPerformanceMetricEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main fix: apparently telemetry dependency is not available in observability plugin, but it is provided in the infra plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @crespocarlos for helping with the investigation!
const testSubjects = getService('testSubjects'); | ||
const kibanaServer = getService('kibanaServer'); | ||
|
||
describe('Metric threshold rule', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inspired by @dominiqueclarke 's PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with one nit comment
@@ -200,7 +200,7 @@ export const ExpressionRow = ({ | |||
|
|||
return ( | |||
<> | |||
<EuiFlexGroup gutterSize="xs"> | |||
<EuiFlexGroup gutterSize="xs" data-test-subj="expressionRow"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It may be good to give a slightly more descriptive name to this test ID; it's unique to Kibana at the moment but maybe metricThresholdExpressionRow
or something along those lines would be better. Not a show-stopper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, fixed in c9146b6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
…eshold flyout (elastic#192736) Closes elastic#192439 ## Summary This PR reverts this [line](https://github.com/elastic/kibana/pull/191948/files#diff-2dd82a791bba3d995e9e6b35d4a973053f166351cc6025a5cd1d24dc789766aeR48) in a previous [PR](elastic#191948) that caused an issue in loading the metric threshold flyout on the observability alerts page. | Before ❌ | After ✅ | |---|---| |![Image](https://github.com/user-attachments/assets/3c0b8812-8cd9-4769-bd20-ab10f559009b)|![Image](https://github.com/user-attachments/assets/9823e691-ce18-4c00-8748-ce5797a19943)| I also added a small test that fails before this fix. (cherry picked from commit c304b34)
…eshold flyout (elastic#192736) Closes elastic#192439 ## Summary This PR reverts this [line](https://github.com/elastic/kibana/pull/191948/files#diff-2dd82a791bba3d995e9e6b35d4a973053f166351cc6025a5cd1d24dc789766aeR48) in a previous [PR](elastic#191948) that caused an issue in loading the metric threshold flyout on the observability alerts page. | Before ❌ | After ✅ | |---|---| |![Image](https://github.com/user-attachments/assets/3c0b8812-8cd9-4769-bd20-ab10f559009b)|![Image](https://github.com/user-attachments/assets/9823e691-ce18-4c00-8748-ce5797a19943)| I also added a small test that fails before this fix. (cherry picked from commit c304b34)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ic threshold flyout (#192736) (#192825) # Backport This will backport the following commits from `main` to `8.x`: - [[Metric threshold] Fix the condition not showing up in the metric threshold flyout (#192736)](#192736) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maryam Saeidi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-13T09:10:15Z","message":"[Metric threshold] Fix the condition not showing up in the metric threshold flyout (#192736)\n\nCloses #192439\r\n\r\n## Summary\r\n\r\nThis PR reverts this\r\n[line](https://github.com/elastic/kibana/pull/191948/files#diff-2dd82a791bba3d995e9e6b35d4a973053f166351cc6025a5cd1d24dc789766aeR48)\r\nin a previous [PR](#191948) that\r\ncaused an issue in loading the metric threshold flyout on the\r\nobservability alerts page.\r\n\r\n| Before ❌ | After ✅ |\r\n|---|---|\r\n\r\n|![Image](https://github.com/user-attachments/assets/3c0b8812-8cd9-4769-bd20-ab10f559009b)|![Image](https://github.com/user-attachments/assets/9823e691-ce18-4c00-8748-ce5797a19943)|\r\n\r\nI also added a small test that fails before this fix.","sha":"c304b34e0edd90dedcb67dff10da6472d4a823c0","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","backport:prev-major","ci:project-deploy-observability"],"title":"[Metric threshold] Fix the condition not showing up in the metric threshold flyout","number":192736,"url":"https://github.com/elastic/kibana/pull/192736","mergeCommit":{"message":"[Metric threshold] Fix the condition not showing up in the metric threshold flyout (#192736)\n\nCloses #192439\r\n\r\n## Summary\r\n\r\nThis PR reverts this\r\n[line](https://github.com/elastic/kibana/pull/191948/files#diff-2dd82a791bba3d995e9e6b35d4a973053f166351cc6025a5cd1d24dc789766aeR48)\r\nin a previous [PR](#191948) that\r\ncaused an issue in loading the metric threshold flyout on the\r\nobservability alerts page.\r\n\r\n| Before ❌ | After ✅ |\r\n|---|---|\r\n\r\n|![Image](https://github.com/user-attachments/assets/3c0b8812-8cd9-4769-bd20-ab10f559009b)|![Image](https://github.com/user-attachments/assets/9823e691-ce18-4c00-8748-ce5797a19943)|\r\n\r\nI also added a small test that fails before this fix.","sha":"c304b34e0edd90dedcb67dff10da6472d4a823c0"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192736","number":192736,"mergeCommit":{"message":"[Metric threshold] Fix the condition not showing up in the metric threshold flyout (#192736)\n\nCloses #192439\r\n\r\n## Summary\r\n\r\nThis PR reverts this\r\n[line](https://github.com/elastic/kibana/pull/191948/files#diff-2dd82a791bba3d995e9e6b35d4a973053f166351cc6025a5cd1d24dc789766aeR48)\r\nin a previous [PR](#191948) that\r\ncaused an issue in loading the metric threshold flyout on the\r\nobservability alerts page.\r\n\r\n| Before ❌ | After ✅ |\r\n|---|---|\r\n\r\n|![Image](https://github.com/user-attachments/assets/3c0b8812-8cd9-4769-bd20-ab10f559009b)|![Image](https://github.com/user-attachments/assets/9823e691-ce18-4c00-8748-ce5797a19943)|\r\n\r\nI also added a small test that fails before this fix.","sha":"c304b34e0edd90dedcb67dff10da6472d4a823c0"}}]}] BACKPORT--> Co-authored-by: Maryam Saeidi <[email protected]>
Closes #192439
Summary
This PR reverts this line in a previous PR that caused an issue in loading the metric threshold flyout on the observability alerts page.
I also added a small test that failed before this fix.
To run server
To run the test