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

[Discover][Alerts] Fix Discover results when alert excludes matches from previous runs #176690

Merged
merged 4 commits into from Feb 14, 2024

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented Feb 12, 2024

Summary

In case if user creates a rule and enables "Exclude matches from previous runs", Discover link will now include a time filter to filter previous results out.

Screenshot 2024-02-12 at 14 02 18

For testing:

  • Open Discover with an index which has documents before and after current time (e.g. a freshly installed Kibana Sample Data Logs)
  • Create a new rule
    • Enable/disable "Exclude matches from previous runs" switch
    • Define an index connector with a link
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
  • Now navigate to Discover, create a data view for the connector index
  • Copy locator links from the appearing alerts and open Discover with them in another tab
Screenshot 2024-02-12 at 15 19 24

If "Exclude matches from previous runs" was enabled, then an additional filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in context_message field.

@jughosta jughosta added release_note:fix Feature:Alerting Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) backport:prev-minor Backport to the previous minor version (i.e. one version back from main) labels Feb 12, 2024
@jughosta jughosta self-assigned this Feb 12, 2024
@jughosta
Copy link
Contributor Author

/ci

@jughosta jughosta changed the title [Discover][Alerts] Fix time range start date when opening an alert in Discover (when matches are excluded from previous runs) [Discover][Alerts] Fix Discover results when alert excludes matches from previous runs Feb 12, 2024
@jughosta
Copy link
Contributor Author

/ci

@jughosta jughosta marked this pull request as ready for review February 12, 2024 15:18
@jughosta jughosta requested a review from a team as a code owner February 12, 2024 15:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@jughosta jughosta requested a review from a team February 12, 2024 15:18
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; works as expected

I'm wondering though whether we should enable this "behavior" in other ways. For instance, when I turn "exclude matches" off, but obviously still have a "time window" (FOR THE LAST ), shouldn't we use a filter that would only show the docs within that window? Currently that link doesn't filter on time ...

This will be very useful as-is though, as a lot of folks seem to use exclude previous matches but then are confused by the generated (or non-generated) alerts. I think this will help!

@jughosta
Copy link
Contributor Author

Hi @pmuellr,

Thanks for the review! Yes, indeed, it makes sense to show results only for the alert's time window. We are already doing it by passing timeRange.from and timeRange.to as params to Discover locator.


This sets the correct time range for the time picker control in Discover UI which automatically translates into a time range filter for ES request. Or it's not working for you?

For this PR I initially was going to replace dateStart with latestTimestamp in Discover locator params to narrow down the search (if "exclude matches" is on) but it would include results for latestTimestamp too as Discover uses gte. So, I changed the approach to add an extra filter with gt instead to fetch results within dateStart and dateEnd but only after latestTimestamp.

@pmuellr
Copy link
Member

pmuellr commented Feb 13, 2024

This sets the correct time range for the time picker control in Discover UI which automatically translates into a time range filter for ES request. Or it's not working for you?

Let me retry that - I saw a big difference in the number of docs, more than I was expecting. But it's also possible I was testing with a long window and short intervals so there WERE lots of docs.

@pmuellr
Copy link
Member

pmuellr commented Feb 13, 2024

Let me retry that - I saw a big difference in the number of docs, more than I was expecting. But it's also possible I was testing with a long window and short intervals so there WERE lots of docs.

Heh, ya, that was just me. I think the difference ended up confusing me, but I wasn't actually looking at the date values:

with exclude matches:

Screenshot 2024-02-13 at 08 42 27

without exclude matches:

Screenshot 2024-02-13 at 08 42 51

Both show the time window of the rule, which makes sense. When I saw the second one, I figured it actually extended out more to the left, basically listing ALL the matching docs. But, it's just showing the exact window of matching docs :-)

This PR certainly improves things!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jughosta

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and it works as expected, great work on this! And thanks a bunch for the testing instructions, they made it much easier for me to test.

I actually think using the GT filter in addition to the time filter is preferable since it shows the expected number of docs by default, but also allows users to disable the filter to see all matches if they want to. Thanks for fixing this and LGTM 👍

@jughosta jughosta merged commit 7e3a9f8 into elastic:main Feb 14, 2024
17 checks passed
@jughosta jughosta deleted the 148282-alert-start-time branch February 14, 2024 16:44
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2024
…rom previous runs (elastic#176690)

- Closes elastic#148282

## Summary

In case if user creates a rule and enables "Exclude matches from
previous runs", Discover link will now include a time filter to filter
previous results out.

<img width="500" alt="Screenshot 2024-02-12 at 14 02 18"
src="https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545">

For testing:
- Open Discover with an index which has documents before and after
current time (e.g. a freshly installed Kibana Sample Data Logs)
- Create a new rule
  - Enable/disable "Exclude matches from previous runs" switch
  - Define an index connector with a link
```
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
```
- Now navigate to Discover, create a data view for the connector index
- Copy locator links from the appearing alerts and open Discover with
them in another tab

<img width="300" alt="Screenshot 2024-02-12 at 15 19 24"
src="https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088">

If "Exclude matches from previous runs" was enabled, then an additional
filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in
`context_message` field.

(cherry picked from commit 7e3a9f8)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 14, 2024
…tches from previous runs (#176690) (#176931)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Discover][Alerts] Fix Discover results when alert excludes matches
from previous runs
(#176690)](#176690)

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

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

<!--BACKPORT [{"author":{"name":"Julia
Rechkunova","email":"julia.rechkunova@elastic.co"},"sourceCommit":{"committedDate":"2024-02-14T16:44:40Z","message":"[Discover][Alerts]
Fix Discover results when alert excludes matches from previous runs
(#176690)\n\n- Closes
#148282
Summary\r\n\r\nIn case if user creates a rule and enables \"Exclude
matches from\r\nprevious runs\", Discover link will now include a time
filter to filter\r\nprevious results out.\r\n\r\n<img width=\"500\"
alt=\"Screenshot 2024-02-12 at 14 02
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545\">\r\n\r\nFor
testing:\r\n- Open Discover with an index which has documents before and
after\r\ncurrent time (e.g. a freshly installed Kibana Sample Data
Logs)\r\n- Create a new rule \r\n - Enable/disable \"Exclude matches
from previous runs\" switch\r\n - Define an index connector with a
link\r\n```\r\n {\r\n \"rule_id\": \"\",\r\n \"rule_name\": \"\",\r\n
\"alert_id\": \"\",\r\n \"context_message\": \"\",\r\n \"link\":
\"\"\r\n}\r\n```\r\n- Now navigate to Discover, create a data view for
the connector index\r\n- Copy locator links from the appearing alerts
and open Discover with\r\nthem in another tab\r\n\r\n<img width=\"300\"
alt=\"Screenshot 2024-02-12 at 15 19
24\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088\">\r\n\r\n\r\nIf
\"Exclude matches from previous runs\" was enabled, then an
additional\r\nfilter will show up on Discover page for the locator
link.\r\n\r\nCheck that Discover total count is the same as the one
mentioned in\r\n`context_message`
field.","sha":"7e3a9f8fbe7c5513e4a4b74939593779d9ba4b24","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Alerting","Team:DataDiscovery","backport:prev-minor","v8.13.0"],"title":"[Discover][Alerts]
Fix Discover results when alert excludes matches from previous
runs","number":176690,"url":"#176690
Fix Discover results when alert excludes matches from previous runs
(#176690)\n\n- Closes
#148282
Summary\r\n\r\nIn case if user creates a rule and enables \"Exclude
matches from\r\nprevious runs\", Discover link will now include a time
filter to filter\r\nprevious results out.\r\n\r\n<img width=\"500\"
alt=\"Screenshot 2024-02-12 at 14 02
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545\">\r\n\r\nFor
testing:\r\n- Open Discover with an index which has documents before and
after\r\ncurrent time (e.g. a freshly installed Kibana Sample Data
Logs)\r\n- Create a new rule \r\n - Enable/disable \"Exclude matches
from previous runs\" switch\r\n - Define an index connector with a
link\r\n```\r\n {\r\n \"rule_id\": \"\",\r\n \"rule_name\": \"\",\r\n
\"alert_id\": \"\",\r\n \"context_message\": \"\",\r\n \"link\":
\"\"\r\n}\r\n```\r\n- Now navigate to Discover, create a data view for
the connector index\r\n- Copy locator links from the appearing alerts
and open Discover with\r\nthem in another tab\r\n\r\n<img width=\"300\"
alt=\"Screenshot 2024-02-12 at 15 19
24\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088\">\r\n\r\n\r\nIf
\"Exclude matches from previous runs\" was enabled, then an
additional\r\nfilter will show up on Discover page for the locator
link.\r\n\r\nCheck that Discover total count is the same as the one
mentioned in\r\n`context_message`
field.","sha":"7e3a9f8fbe7c5513e4a4b74939593779d9ba4b24"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"#176690
Fix Discover results when alert excludes matches from previous runs
(#176690)\n\n- Closes
#148282
Summary\r\n\r\nIn case if user creates a rule and enables \"Exclude
matches from\r\nprevious runs\", Discover link will now include a time
filter to filter\r\nprevious results out.\r\n\r\n<img width=\"500\"
alt=\"Screenshot 2024-02-12 at 14 02
18\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545\">\r\n\r\nFor
testing:\r\n- Open Discover with an index which has documents before and
after\r\ncurrent time (e.g. a freshly installed Kibana Sample Data
Logs)\r\n- Create a new rule \r\n - Enable/disable \"Exclude matches
from previous runs\" switch\r\n - Define an index connector with a
link\r\n```\r\n {\r\n \"rule_id\": \"\",\r\n \"rule_name\": \"\",\r\n
\"alert_id\": \"\",\r\n \"context_message\": \"\",\r\n \"link\":
\"\"\r\n}\r\n```\r\n- Now navigate to Discover, create a data view for
the connector index\r\n- Copy locator links from the appearing alerts
and open Discover with\r\nthem in another tab\r\n\r\n<img width=\"300\"
alt=\"Screenshot 2024-02-12 at 15 19
24\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088\">\r\n\r\n\r\nIf
\"Exclude matches from previous runs\" was enabled, then an
additional\r\nfilter will show up on Discover page for the locator
link.\r\n\r\nCheck that Discover total count is the same as the one
mentioned in\r\n`context_message`
field.","sha":"7e3a9f8fbe7c5513e4a4b74939593779d9ba4b24"}}]}]
BACKPORT-->

Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…rom previous runs (elastic#176690)

- Closes elastic#148282

## Summary

In case if user creates a rule and enables "Exclude matches from
previous runs", Discover link will now include a time filter to filter
previous results out.

<img width="500" alt="Screenshot 2024-02-12 at 14 02 18"
src="https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545">

For testing:
- Open Discover with an index which has documents before and after
current time (e.g. a freshly installed Kibana Sample Data Logs)
- Create a new rule 
  - Enable/disable "Exclude matches from previous runs" switch
  - Define an index connector with a link
```
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
```
- Now navigate to Discover, create a data view for the connector index
- Copy locator links from the appearing alerts and open Discover with
them in another tab

<img width="300" alt="Screenshot 2024-02-12 at 15 19 24"
src="https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088">


If "Exclude matches from previous runs" was enabled, then an additional
filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in
`context_message` field.
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…rom previous runs (elastic#176690)

- Closes elastic#148282

## Summary

In case if user creates a rule and enables "Exclude matches from
previous runs", Discover link will now include a time filter to filter
previous results out.

<img width="500" alt="Screenshot 2024-02-12 at 14 02 18"
src="https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545">

For testing:
- Open Discover with an index which has documents before and after
current time (e.g. a freshly installed Kibana Sample Data Logs)
- Create a new rule 
  - Enable/disable "Exclude matches from previous runs" switch
  - Define an index connector with a link
```
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
```
- Now navigate to Discover, create a data view for the connector index
- Copy locator links from the appearing alerts and open Discover with
them in another tab

<img width="300" alt="Screenshot 2024-02-12 at 15 19 24"
src="https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088">


If "Exclude matches from previous runs" was enabled, then an additional
filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in
`context_message` field.
awahab07 pushed a commit to awahab07/kibana that referenced this pull request Feb 19, 2024
…rom previous runs (elastic#176690)

- Closes elastic#148282

## Summary

In case if user creates a rule and enables "Exclude matches from
previous runs", Discover link will now include a time filter to filter
previous results out.

<img width="500" alt="Screenshot 2024-02-12 at 14 02 18"
src="https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545">

For testing:
- Open Discover with an index which has documents before and after
current time (e.g. a freshly installed Kibana Sample Data Logs)
- Create a new rule 
  - Enable/disable "Exclude matches from previous runs" switch
  - Define an index connector with a link
```
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
```
- Now navigate to Discover, create a data view for the connector index
- Copy locator links from the appearing alerts and open Discover with
them in another tab

<img width="300" alt="Screenshot 2024-02-12 at 15 19 24"
src="https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088">


If "Exclude matches from previous runs" was enabled, then an additional
filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in
`context_message` field.
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…rom previous runs (elastic#176690)

- Closes elastic#148282

## Summary

In case if user creates a rule and enables "Exclude matches from
previous runs", Discover link will now include a time filter to filter
previous results out.

<img width="500" alt="Screenshot 2024-02-12 at 14 02 18"
src="https://github.com/elastic/kibana/assets/1415710/89ae9bb1-5fe7-4366-a3db-6ed3b8ae7545">

For testing:
- Open Discover with an index which has documents before and after
current time (e.g. a freshly installed Kibana Sample Data Logs)
- Create a new rule 
  - Enable/disable "Exclude matches from previous runs" switch
  - Define an index connector with a link
```
 {
  "rule_id": "{{rule.id}}",
  "rule_name": "{{rule.name}}",
  "alert_id": "{{alert.id}}",
  "context_message": "{{context.message}}",
  "link": "{{context.link}}"
}
```
- Now navigate to Discover, create a data view for the connector index
- Copy locator links from the appearing alerts and open Discover with
them in another tab

<img width="300" alt="Screenshot 2024-02-12 at 15 19 24"
src="https://github.com/elastic/kibana/assets/1415710/0e5c3718-b16a-4360-a213-490479f85088">


If "Exclude matches from previous runs" was enabled, then an additional
filter will show up on Discover page for the locator link.

Check that Discover total count is the same as the one mentioned in
`context_message` field.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:Alerting release_note:fix Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) v8.12.2 v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover][Alerts] Exclude matches from previous runs
6 participants