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

[RAC] [Metrics UI] Include group name in the reason message #109610

Closed
mgiota opened this issue Aug 23, 2021 · 5 comments · Fixed by #115171
Closed

[RAC] [Metrics UI] Include group name in the reason message #109610

mgiota opened this issue Aug 23, 2021 · 5 comments · Fixed by #115171
Assignees
Labels
Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Theme: rac label obsolete v7.16.0

Comments

@mgiota
Copy link
Contributor

mgiota commented Aug 23, 2021

📝 Sumamary

In the following table it looks like each alert appears twice. It turns out that these alerts refer to different hosts, but that is not apparent from the reason message.

Screenshot 2021-08-23 at 12 47 16

✔️ Acceptance criteria

  • the alert group name is included in the reason, e.g. as a prefix my-host: ... or my-container: ...

Notes

The grouping field is used as the alert id. This is already implemented for the log threshold id, which can serve as a blueprint here:

export const getReasonMessageForGroupedCountAlert = (
actualCount: number,
expectedCount: number,
comparator: Comparator,
groupName: string
) =>
i18n.translate('xpack.infra.logs.alerting.threshold.groupedCountAlertReasonDescription', {
defaultMessage:
'{groupName}: {actualCount, plural, one {{actualCount} log entry} other {{actualCount} log entries} } ({translatedComparator} {expectedCount}) match the conditions.',
values: {
actualCount,
expectedCount,
groupName,
translatedComparator: ComparatorToi18nMap[comparator],
},
});

const alertInstance = alertInstanceFactory(
UNGROUPED_FACTORY_KEY,
getReasonMessageForUngroupedCountAlert(documentCount, count.value, count.comparator),
documentCount,
count.value
);

@mgiota mgiota added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Aug 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@mgiota mgiota added the Theme: rac label obsolete label Aug 23, 2021
@mgiota mgiota self-assigned this Aug 24, 2021
@paulb-elastic
Copy link
Contributor

Discussed in weekly meeting, this can be closed

@jasonrhodes jasonrhodes reopened this Sep 21, 2021
@jasonrhodes jasonrhodes changed the title [RAC][Observability] Investigate possibility of alerts being triggered/shown twice [RAC][Observability] Consider indexing host name and other ECS fields for Metrics Inventory alerts Sep 21, 2021
@jasonrhodes
Copy link
Member

@mgiota / @weltenwort can you work together to rewrite the description of this ticket (or log a new one and re-close this one) to reflect what we intend to do here for now? I know we want to index some fields—do we have a short list of the ones to start with that we could index in ECS format for now? Or do we want to push ECS field indexing to a future release?

@weltenwort weltenwort changed the title [RAC][Observability] Consider indexing host name and other ECS fields for Metrics Inventory alerts [RAC] [Metrics UI] Include group name in the reason message Oct 12, 2021
@weltenwort
Copy link
Member

weltenwort commented Oct 12, 2021

ℹ️ I updated the description so it focuses on the reason message for now

@mgiota
Copy link
Contributor Author

mgiota commented Oct 18, 2021

@weltenwort Can you check my comment here #115171 (comment)? I changed the reason format and moved the group at the end. Any objections on that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Theme: rac label obsolete v7.16.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants