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

[ES Query] Make ES Query rule created in Discover visible in Observability #170497

Closed
maryam-saeidi opened this issue Nov 3, 2023 · 17 comments · Fixed by #171364
Closed

[ES Query] Make ES Query rule created in Discover visible in Observability #170497

maryam-saeidi opened this issue Nov 3, 2023 · 17 comments · Fixed by #171364
Assignees
Labels
Team:obs-ux-management Observability Management User Experience Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.12.0

Comments

@maryam-saeidi
Copy link
Member

📝 Summary

When a user creates an ES query rule from the Discover view, this rule will not be visible in Observability. (Slack discussion)
We already brought this rule in Observability in this epic. We only need to set the consumer to one of the observability privileges, such as Logs or Metrics.

To solve this issue, we need to:

  1. Show the role visibility drop-down and use that value for the consumer field.
  1. Decide whether to set Logs as the default value.
    The benefit of setting it is to improve user experience. This field is required, and if the user hits the save button without scrolling to set this field, nothing happens.

✅ Acceptance Criteria

  • Make the ES Query rule created in Discover visible in Observability
@maryam-saeidi maryam-saeidi added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.12.0 labels Nov 3, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

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

@paulb-elastic paulb-elastic removed the Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" label Nov 14, 2023
@XavierM XavierM self-assigned this Nov 15, 2023
@XavierM XavierM moved this from Awaiting Triage to Up for grabs in AppEx: ResponseOps - Rules & Alerts Management Nov 15, 2023
@XavierM XavierM assigned Zacqary and unassigned XavierM Nov 15, 2023
@Zacqary Zacqary moved this from Up for grabs to In Progress in AppEx: ResponseOps - Rules & Alerts Management Nov 15, 2023
@Zacqary
Copy link
Contributor

Zacqary commented Nov 15, 2023

Screenshot 2023-11-15 at 4 06 20 PM

Enabling the scope dropdown in Discover reveals three authorized consumers, not just two.

Is this desired?

@maryam-saeidi
Copy link
Member Author

@vinaychandrasekhar @roshan-elastic What do you think about the above question? Personally, I think it is fine.

@Zacqary Is the logs option the default choice?

@Zacqary
Copy link
Contributor

Zacqary commented Nov 16, 2023

Still trying to figure out how to properly set a default choice. Currently the role visibility drops down if the alert form gets passed a consumer of alerts, so passing it a default of logs would end up just hiding the dropdown.

So it's going to take some refactoring, but yes, I will make sure Logs is the default

@Zacqary
Copy link
Contributor

Zacqary commented Nov 16, 2023

Figured out how to remove Stack Rules from the authorized consumer list.

@Zacqary
Copy link
Contributor

Zacqary commented Nov 21, 2023

Having trouble figuring out how to resolve some tests that are failing on permissions errors

Current tests for creating an ES Query rule in Discover only require the stackAlerts permission, and these tests fail when we set the default consumer to logs. Do we want to hide the rule creation flyout from Discover if the user doesn't have either logs or infrastructure permissions? Or should I add the Stack Rules consumer back to the authorized list, and use it as a secondary default?

Need input from @maryam-saeidi but also @elastic/kibana-data-discovery

@Zacqary
Copy link
Contributor

Zacqary commented Nov 21, 2023

We already brought this rule in Observability in this https://github.com/elastic/actionable-observability/issues/37.

To clarify, does this mean Observability now owns the Elasticsearch Query rule, and it's no longer a stack alert? I still see it listed in the bottom category when I try to create an alert from Stack Management, so I assumed it was still a stack alert, but if it's exclusively an O11y alert now then that changes things

@davismcphee
Copy link
Contributor

Current tests for creating an ES Query rule in Discover only require the stackAlerts permission, and these tests fail when we set the default consumer to logs. Do we want to hide the rule creation flyout from Discover if the user doesn't have either logs or infrastructure permissions? Or should I add the Stack Rules consumer back to the authorized list, and use it as a secondary default?

@Zacqary I'm not very familiar with this project or the consumers concept, but it would be a surprise to us on the Discover side if this functionality was removed from Discover since we weren't aware if this was something being considered. My guess is we'd want to keep the "Stack Rules" consumer in the dropdown if it's required for the existing experience to continue working.

Since Discover isn't logs or O11y specific (and I don't think the ES query rule is either), would it instead make sense to keep all options in the dropdown and leave it blank by default, similar to what's done in Stack Management -> Rules? That way the current experience would still be supported and users would just need to select the consumer themselves:
alert

@maryam-saeidi
Copy link
Member Author

To clarify, does this mean Observability now owns the Elasticsearch Query rule, and it's no longer a stack alert? I still see it listed in the bottom category when I try to create an alert from Stack Management, so I assumed it was still a stack alert, but if it's exclusively an O11y alert now then that changes things

@Zacqary It is available in the stack as well as observability, so we can create this rule from both places.

should I add the Stack Rules consumer back to the authorized list, and use it as a secondary default?

@davismcphee What do you think about the above-mentioned option? This request was originally related to not being able to see the ES Query rule in observability, and I assume in a lot of scenarios, users want to be able to see the rule both in observability and stack management (which is possible when you select the logs as the default option)

The reason that I suggest having a default value for this field is to make it easier for our users without needing to know what role visibility is (considering this is a required field), and at the same time, if the second default is Stack rules, we will keep the current behavior base on permissions as well. Do you see a downside to this option?

@roshan-elastic
Copy link

@vinaychandrasekhar @roshan-elastic What do you think about the above question? Personally, I think it is fine.

@Zacqary Is the logs option the default choice?

Thanks for looping me in here @maryam-saeidi...I don't have a great deal to offer on this one at the moment - I think @vinaychandrasekhar will be better informed to comment here.

@vinaychandrasekhar
Copy link

I don't have enough context here. For example, how does this ticket relate to bringing Discover / Logs Explorer into Observability (in Serverless for now, and in self-managed in future)? Would be better to discuss live.
cc @ruflin - FYI on this discussion.

@davismcphee
Copy link
Contributor

@maryam-saeidi Thanks for the context! Personally I don't seen an issue with what you're proposing, although it may be a questioned better answered by PMs tbh. I discussed it briefly wth the other engineers on my team last week, and the only concern raised was that it may surprise some users to find the rules they created from Discover in O11y if they were used to the old behaviour, but this is probably a minor concern overall. We have a team sync tomorrow with our PM, so I've added this to the agenda and will follow up afterward.

@ruflin
Copy link
Contributor

ruflin commented Nov 28, 2023

For example, how does this ticket relate to bringing Discover / Logs Explorer into Observability

My understanding currently is this is not related.

@maryam-saeidi
Copy link
Member Author

@davismcphee Thanks for the info, so, the only question is whether to have Logs as the default option or Stack Rules, right? It is more important to have a default value, and it made sense to me to have it as Logs.
It's a good idea to set it based on the value that your team decides after syncing with your PM.

@XavierM XavierM moved this from In Progress to In Review in AppEx: ResponseOps - Rules & Alerts Management Nov 29, 2023
@davismcphee
Copy link
Contributor

@maryam-saeidi Apologies for the delay, I'm on SDH duty this week and it's a busy one 🙂 We discussed this as a team and agreed it should be fine to set the default value to Logs considering it will continue working as it does today for existing users, aside from the fact the rules will also appear in O11y if they have the required privileges. I'm done for the day today, but will try to get a chance to review and test the PR tomorrow.

Zacqary added a commit that referenced this issue Dec 4, 2023
…71364)

## Summary

Closes #170497 

<img width="483" alt="Screenshot 2023-11-16 at 1 25 18 PM"
src="https://github.com/elastic/kibana/assets/1445834/4d974eab-9641-4618-b52a-2facf4c07667">

Adds scope dropdown to ES Query rules created from Discovery. If Logs or
Metrics are selected, rules created here will be visible in
Observability.

Also makes `Logs` the default consumer when creating a rule from either
Discovery and Observability.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:obs-ux-management Observability Management User Experience Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.12.0
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

9 participants