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

[Observability] [Alert] Update alertDetailsUrl to have link to alert detail pages #180889

Closed
benakansara opened this issue Apr 16, 2024 · 11 comments · Fixed by #193674
Closed

[Observability] [Alert] Update alertDetailsUrl to have link to alert detail pages #180889

benakansara opened this issue Apr 16, 2024 · 11 comments · Fixed by #193674
Assignees
Labels
Feature:Alerting Team:obs-ux-management Observability Management User Experience Team

Comments

@benakansara
Copy link
Contributor

benakansara commented Apr 16, 2024

With #179338, we have now baseline alert detail pages for all types of Observability alerts. We need to update alertDetailsUrl action variable to have link to alert detail page instead of /Alerts page for these rules:

  • Inventory rule
  • Metric threshold rule
  • Error count threshold rule
  • Failed transaction rate threshold rule
  • APM Anomaly rule

For the following two rules, we have a separate ticket: #193753

  • Elasticsearch query rule
  • Anomaly detection rule
@benakansara benakansara added Feature:Alerting Team:obs-ux-management Observability Management User Experience Team labels Apr 16, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@maryam-saeidi
Copy link
Member

IMO, it would be better to make this change after we have more useful information on the alert details page. In the current state, sending users to the /Alerts page is more useful as they can change the filter and see the other alerts during the same time range, without missing any information in comparison to the alert details page.

@jasonrhodes
Copy link
Member

jasonrhodes commented Apr 17, 2024

@maryam-saeidi I think this is a good point around usefulness but I wonder what a user expects if they link to something called {alertDetailsUrl}? I'd think they expect to go to that one alert's details only in that case, so maybe we can do what Bena suggests here so that the URL for this always takes you to that alert's "permalink", and introduce another context variable for "alertsPageUrl"?

@maryam-saeidi
Copy link
Member

@maryam-saeidi I think this is a good point around usefulness but I wonder what a user expects if they link to something called {alertDetailsUrl}? I'd think they expect to go to that one alert's details only in that case,

I agree. The ideal solution would be to have an alert details page containing all the necessary information but in the meantime, I wonder how we balance the trade-off between user expectations and the options that we can give our users.

An idea might be to have another tab on the alert details page with filtered alerts for that specific group or rule. In @maciejforcone 's design, we wanted to have these related alerts, so maybe we can start with showing other alerts for the same rule or the same group.

Another option would be to add a link to the alert details page to see this alert in the alert table or something like that if we don't want to go with the above approach.

so maybe we can do what Bena suggests here so that the URL for this always takes you to that alert's "permalink",

If we think users can investigate an alert with the current default page, sure, let's do that. My main point is to make sure wherever we send our users, we have some options for them to investigate the issue.

and introduce another context variable for "alertsPageUrl"?

I would suggest not adding another context variable and instead focusing on adding more useful information in a shared way to the alert details page. We previously had this idea of having only one link (one entry point) as the starting point for our users and then allowing them to navigate to other sections (like app, logs, alert table, ...).
This would make our life easier in maintenance as we don't need to think that a link is shared with our users and we need to make sure they are backward compatible, at the same time, it would be easier to release new features as part of the alert details page and create a process around alert investigation for our users.

@jasonrhodes
Copy link
Member

I would suggest not adding another context variable and instead focusing on adding more useful information in a shared way to the alert details page. We previously had this idea of having only one link (one entry point) as the starting point for our users and then allowing them to navigate to other sections

This makes sense to me, good point.

Another option would be to add a link to the alert details page to see this alert in the alert table or something like that if we don't want to go with the above approach.

I like this idea better than adding "related alerts" tab or similar just for right now, because I think that route will invite a lot more questions about what makes a "related" alert, that we won't be able to simply address. A link back to the full alerts page, in context, sounds like a great compromise for pushing users to our new alert details pages but also allowing them to navigate to other alerts in the same time range.

I think that will be especially interesting once we change the flyout to use the tabbed alert details page design, because that link can just close the flyout and make sure the page is open to the right params, probably. (We'll have a few things to work out if we actually go that way, so not in scope for now at all, just doing a bit of daydreaming :P )

@benakansara
Copy link
Contributor Author

I think showing alerts page ties to the idea of showing "related alerts" in alert details page. It would require more discussion to define what are related alerts - same time range, same source, related source, any other? I see this as a separate topic than the alertDetailsUrl.

Initially we added link to alerts page where we didn't have actual alert details page, which was a great idea to help users find the alert from long list of alerts on the page. Now we have alert details page, so it makes sense to redirect user directly to this page. We have affected entity info with links and all the metadata of the alert on the page, which from my point of view, would be more useful to know more about the alert itself before going to Alerts page to check other alerts triggered in the same time rage.

Also, user has to remove the filter from page, so it's not a out-of-box solution, since user has to figure out that they need to remove filter, change time range if needed, etc.

Another aspect is that if we don't update alertDetailsUrl for rules with baseline alert details page, user can still open alert details page from alerts UI. So it could be confusing that for some rules, we redirect user directly to alert details page, and for some rules, we redirect them to alerts page from where they can again open alert details page.

@benakansara
Copy link
Contributor Author

Do we need more refinement or is it ready to move it to Ready/Awaiting prioritization?

@maryam-saeidi
Copy link
Member

From my perspective, the scope of this ticket is clear, the only point of discussion left is the following item:

Mary: An idea might be to have another tab on the alert details page with filtered alerts for that specific group or rule. In @maciejforcone 's design, we wanted to have these related alerts, so maybe we can start with showing other alerts for the same rule or the same group.

Another option would be to add a link to the alert details page to see this alert in the alert table or something like that if we don't want to go with the above approach.

Jason: I like this idea better than adding "related alerts" tab or similar just for right now, because I think that route will invite a lot more questions about what makes a "related" alert, that we won't be able to simply address. A link back to the full alerts page, in context, sounds like a great compromise for pushing users to our new alert details pages but also allowing them to navigate to other alerts in the same time range.

If the idea sounds good, I can create a separate ticket for it.

@benakansara
Copy link
Contributor Author

sounds good to me!

@maryam-saeidi
Copy link
Member

@benakansara I created a PR to update the alertDetailsUrl context variables. I saw you also included the following 2 rules:

  • Elasticsearch query rule
  • Anomaly detection rule

I didn't see this context variable there, so I am considering creating a separate ticket for that addition if that is OK with you.

@benakansara
Copy link
Contributor Author

I didn't see this context variable there, so I am considering creating a separate ticket for that addition if that is OK with you.

@maryam-saeidi sounds good, thanks!

maryam-saeidi added a commit that referenced this issue Sep 24, 2024
…3674)

Closes #180889

## Summary

Update the alertDetailsUrl context variable for the following rules:

- Inventory rule
- Metric threshold rule
- Error count threshold rule
- Failed transaction rate threshold rule
- APM Anomaly rule
- Synthetics TLS rule
- Synthetics monitor status rule
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Sep 24, 2024
…stic#193674)

Closes elastic#180889

## Summary

Update the alertDetailsUrl context variable for the following rules:

- Inventory rule
- Metric threshold rule
- Error count threshold rule
- Failed transaction rate threshold rule
- APM Anomaly rule
- Synthetics TLS rule
- Synthetics monitor status rule

(cherry picked from commit 09082f2)
kibanamachine added a commit that referenced this issue Sep 24, 2024
#193674) (#193920)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Alert details page] Update alertDetailsUrl context variable URL
(#193674)](#193674)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maryam
Saeidi","email":"maryam.saeidi@elastic.co"},"sourceCommit":{"committedDate":"2024-09-24T20:02:14Z","message":"[Alert
details page] Update alertDetailsUrl context variable URL
(#193674)\n\nCloses #180889\r\n\r\n## Summary\r\n\r\nUpdate the
alertDetailsUrl context variable for the following rules:\r\n\r\n-
Inventory rule\r\n- Metric threshold rule\r\n- Error count threshold
rule\r\n- Failed transaction rate threshold rule\r\n- APM Anomaly
rule\r\n- Synthetics TLS rule\r\n- Synthetics monitor status
rule","sha":"09082f2d3e751776c8546e31cdad95db11cab8d5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","Team:obs-ux-management","apm:review"],"title":"[Alert
details page] Update alertDetailsUrl context variable
URL","number":193674,"url":"https://github.com/elastic/kibana/pull/193674","mergeCommit":{"message":"[Alert
details page] Update alertDetailsUrl context variable URL
(#193674)\n\nCloses #180889\r\n\r\n## Summary\r\n\r\nUpdate the
alertDetailsUrl context variable for the following rules:\r\n\r\n-
Inventory rule\r\n- Metric threshold rule\r\n- Error count threshold
rule\r\n- Failed transaction rate threshold rule\r\n- APM Anomaly
rule\r\n- Synthetics TLS rule\r\n- Synthetics monitor status
rule","sha":"09082f2d3e751776c8546e31cdad95db11cab8d5"}},"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/193674","number":193674,"mergeCommit":{"message":"[Alert
details page] Update alertDetailsUrl context variable URL
(#193674)\n\nCloses #180889\r\n\r\n## Summary\r\n\r\nUpdate the
alertDetailsUrl context variable for the following rules:\r\n\r\n-
Inventory rule\r\n- Metric threshold rule\r\n- Error count threshold
rule\r\n- Failed transaction rate threshold rule\r\n- APM Anomaly
rule\r\n- Synthetics TLS rule\r\n- Synthetics monitor status
rule","sha":"09082f2d3e751776c8546e31cdad95db11cab8d5"}}]}] BACKPORT-->

Co-authored-by: Maryam Saeidi <maryam.saeidi@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:obs-ux-management Observability Management User Experience Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants