-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Alert details page] Update source and status bar #194842
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
To update your PR or re-run it, just comment with: |
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.
I just have a question but otherwise lgtm. Tested locally and worked as expected.
if (groups && groups.length > 0) { | ||
alertSummary.push({ | ||
label: i18n.translate('xpack.observability.alertDetails.sourceBar.source', { | ||
defaultMessage: 'Source', | ||
}), | ||
value: ( | ||
<Groups groups={groups} timeRange={alertEnd ? timeRange : { ...timeRange, to: 'now' }} /> | ||
), | ||
}); | ||
} |
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.
Is that used?
@@ -213,16 +214,16 @@ export function AlertDetails() { | |||
isAlertDetailsEnabledPerApp(alertDetail.formatted, config) ? ( | |||
<> | |||
<EuiSpacer size="l" /> | |||
<AlertSummary alert={alertDetail.formatted} alertSummaryFields={summaryFields} /> | |||
<SourceBar alert={alertDetail.formatted} sources={sources} /> | |||
<AlertDetailContextualInsights alert={alertDetail} /> |
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.
probably need a spacer in between if the ai assistant is on
Closes #187110
Closes #187111
Closes #153834
Summary
This PR changes how we show source and alert status information, as shown below:
Also, for the APM Latency rule type, we now have a threshold component similar to other rules. For the SLO burn rate rule, the SLO link is added to the source list.