-
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
[Monitoring] Fix alert status for nodes listing view #101941
Conversation
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
Pinging @elastic/stack-monitoring (Team:Monitoring) |
@elasticmachine merge upstream |
@@ -137,7 +137,7 @@ const getColumns = (showCgroupMetricsElasticsearch, setupMode, clusterUuid, aler | |||
<AlertsStatus | |||
showBadge={true} | |||
alerts={alerts} | |||
stateFilter={(state) => (state.nodeId || state.nodeUuid) === node.resolver.uuid} | |||
stateFilter={(state) => (state.nodeId || state.nodeUuid) === node.resolver} |
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 catch, this definitely looks like it was just a bug from a previous PR.
One thing, though: I can't tell if that filter is doing what we want or not? Are we falling back to state.nodeUuid
only if state.nodeId
isn't present? At first I thought we might be trying to compare either/or to node.resolver
, i.e. we meant to do (state.nodeId === node.resolver) || (state.nodeUuid === node.resolver)
so it threw me off.
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.
Yes, the former. In my local environment state.nodeUuid
is always undefined.
@elasticmachine merge upstream |
jenkins, test this (restarting due to jenkins upgrade) |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @neptunian |
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, thanks
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Resolves: #101898 where alerts for Nodes are always "clear".
For some reason in #89410, the comparison function was changed to compare node.id from the firing alerts state to node.resolver.uuid instead of node.id from the list of nodes in the cluster. As far as I can see there is no
uuid
returned and the rest of the page usesnode.resolver
. The snapshot tests also check for resolver and not resolver.uuid.After this change, after generating alerts for a node you should see them appear in node listing:
Indices alerts should also function as normal.