-
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
[Alerting] Display Action Group in Alert Details #82645
[Alerting] Display Action Group in Alert Details #82645
Conversation
…status in alert details view
…ing/action-group-alert-details
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
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, just left a few thoughts... 👍
@@ -452,14 +534,17 @@ export class EventsFactory { | |||
return this; | |||
} | |||
|
|||
addActiveInstance(instanceId: string): EventsFactory { | |||
addActiveInstance(instanceId: string, actionGroupId: string | undefined): EventsFactory { |
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.
When might an action not have an action group? 🤔
Just wondering if we need the undefined
here...
Or is this for when we resolve?
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 added the undefined
in order to support the test 'alert with currently active instance with no action group in event log'
, which ensures we can handle event log entries from prior to this PR which won't have the action_group_id
field.
...triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx
Outdated
Show resolved
Hide resolved
...triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx
Outdated
Show resolved
Hide resolved
…ing/action-group-alert-details
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!
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! I did make one comment about a code change to avoid having to use !
because it scares me :-). Also a comment about opening an issue on how to deal with "action group changes" while the alert is active.
@@ -18,6 +18,10 @@ exports.EcsKibanaExtensionsMappings = { | |||
type: 'keyword', | |||
ignore_above: 1024, | |||
}, | |||
action_group_id: { |
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.
cool - these changes to the event log mappings/schema gen tools look good, hope it was straight-forward, please update the relevant docs (READMEs?) if something was unclear
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.
Yep! The schema generation was straightfoward.
.addActiveInstance('instance-1', 'action group A') | ||
.advanceTime(10000) | ||
.addExecute() | ||
.addActiveInstance('instance-1', 'action group B') |
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.
This makes me wonder what kind of events we should be generating when an instance "switches" action groups. My first thought was it should probably send a resolved-instance
with the old action group, and a new-instance
with the new instance group, in between these active-instance
events. Which implies sending the action groups on new-instance
and resolved-instance
as well.
But not sure. Perhaps it would be better to leave new-instance
and resolved-instance
as is, and maybe have a new event active-action-group-changed
or such.
I think we'll need to think about this a little bit - create a new issue? ie "what events should be logged when an alert switches action groups".
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.
Created this issue: #82792
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
* Adding action group id to event log. Showing action group as part of status in alert details view * Simplifying getting action group id * Cleanup * Adding unit tests * Updating functional tests * Updating test * Fix types check * Updating test * PR fixes * PR fixes
* Adding action group id to event log. Showing action group as part of status in alert details view * Simplifying getting action group id * Cleanup * Adding unit tests * Updating functional tests * Updating test * Fix types check * Updating test * PR fixes * PR fixes
Resolves #82275
Summary
kibana.alerting.action_group_id
and updated message to include action group id. Event log entry looks like:Status
field based on this commentIf no
action_group_id
is available in the event log data, it will default to showing the default action group nameChecklist
Delete any items that are not applicable to this PR.