[Discover] Improve warn feedback when data view has changed after rule creation #144908

dimaanj · 2022-11-09T16:05:49Z

Summary

This PR reduces cases Data view changed toast will appearance. It appears only on changing one of the following parameters.

kibana/src/plugins/discover/public/application/view_alert/view_alert_route.tsx

Line 23 in 317db09

const { title, timeFieldName, sourceFilters, runtimeFieldMap } = dataViewSpec;

Test notes

Create an output index and data view test

PUT test
{
    "settings" : {
        "number_of_shards" : 1
    },
    "mappings" : {
        "properties" : {
            "rule_id" : { "type" : "text" },
            "rule_name" : { "type" : "text" },
            "alert_id" : { "type" : "text" },
            "context_message": { "type" : "text" }
        }
    }
}

Create alerts connector using test index
Open esQuery alert in KQL or Lucene mode or just using Discover Alerts button.
Specify the following params: IS ABOVE: 1, FOR THE LAST: 30 min.
Try execute it by clicking Test query. It should match some results.
When choosing connector, use the following config

{
    "rule_id": "{{rule.id}}",
    "rule_name": "{{rule.name}}",
    "alert_id": "{{alert.id}}",
    "context_message": "{{context.message}}"
}

Create the alert
On management, change one of data view settings which might affect the results (see full list above). For example add new source field filter.
Find the alert document by browsing test data view in Discover
Follow the link from the document triggered by alert
See warning toast which notifies about DataView change.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…-warn-feedback-after-data-view-change # Conflicts: # src/plugins/discover/public/application/view_alert/view_alert_route.tsx

elasticmachine · 2022-11-16T10:46:25Z

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

ymao1 · 2022-11-16T13:19:56Z

Can you provide verification steps? I created a data view and then created a Discover rule with that data view. Then I changed a field in the data view to set a custom label and the next rule run navigated to the provided link and saw no warning toast. Maybe that's not the type of data view change this triggers off of?

pmuellr · 2022-11-16T16:38:41Z

Seems like this PR also includes a fix to get the space id in the generated link. Was that intended to be here? I think I'd prefer it to be a separate issue, but not a biggie. I think there is either an issue or maybe SDH open for that? If so, could we link it here for future reference - I found a Slack reference to the issue, but not an actual issue, so chasing down from there.

dimaanj · 2022-11-17T10:09:29Z

@elasticmachine merge upstream

dimaanj · 2022-11-17T12:10:41Z

Can you provide verification steps?

Sure, added to description.

dimaanj · 2022-11-17T12:12:49Z

Seems like this PR also includes a fix to get the space id in the generated link. Was that intended to be here?

Not really, could you share a link to the issue with space id in generated link?

ymao1 · 2022-11-17T13:33:30Z

x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts

@@ -27,8 +33,14 @@ export async function fetchSearchSourceQuery(

  const initialSearchSource = await searchSourceClient.create(params.searchConfiguration);

+  const index = initialSearchSource.getField('index') as DataView;
+  if (!isTimeBasedDataView(index)) {


Is there another issue to prevent rules from being created with non time based data views? Typically we like to prevent situations where users can create rules that error with every execution.

Yes, there is another issue for this case #135806

ymao1

Response Ops changes LGTM. Left a comment about preventing users from creating rules where the data view is not time based. This can be addressed in a separate issue but I think it's important to address.

mattkime · 2022-11-17T14:47:44Z

src/plugins/discover/public/application/view_alert/view_alert_route.tsx

-const getCurrentChecksum = (params: SearchThresholdAlertParams) =>
-  sha256.create().update(JSON.stringify(params)).hex();
+const getDataViewParamsChecksum = (dataViewSpec: DataViewSpec) => {
+  const { title, timeFieldName, sourceFilters, runtimeFieldMap } = dataViewSpec;


I don't think changing the source filters changes the output.

Actually we can filter out some field, then it will be unavailable on Discover. It first was found here.

Yes, but it won't alter the output of the alert.

Yes, but here we are deciding whether to show Data View has changed toast message. When he/she opens the alert results and data view has changed in the meantime, we detect it by comparing checksum.

@dimaanj

When he/she opens the alert results and data view has changed in the meantime, we detect it by comparing checksum.

Yes, I understand that, but maybe there's something else I'm missing. Can you describe the steps a user might take where a change to source filters deserves this toast?

Sure, I've updated description

mattkime · 2022-11-18T20:42:01Z

Actually, there's another case that we might want to consider handling.

sourceFilters brought to my attention that there are two types of changes that can occur. Alterations to the triggering of alerts and alterations to the display of the subsequent data. sourceFilters affect display but not trigger. Field formats are similar.

As best I can tell, this won't pick up changes to scripted fields.

Also, it should be noted that its possible for the field mapping to change which won't be caught either.

dimaanj · 2022-11-21T08:35:10Z

@elasticmachine merge upstream

kibana-ci · 2022-11-21T09:32:04Z

💚 Build Succeeded

Buildkite Build
Commit: bf17668

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`discover`	414.3KB	414.6KB	+234.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	109	115	+6
`securitySolution`	442	448	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	67	73	+6
`osquery`	110	117	+7
`securitySolution`	519	525	+6
total			+21

History

💚 Build #88763 succeeded 317db09
💛 Build #88713 was flaky 5139706
💚 Build #88334 succeeded 6e929ae
💔 Build #88001 failed 746a286
💔 Build #86706 failed c76129b

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

cc @dimaanj

dimaanj · 2022-11-21T14:33:54Z

sourceFilters affect display but not trigger. Field formats are similar.

I do agree that sourceFilters and field formats does not affect the results, but display. Correct me if I am wrong, you would suggest to leave only title and timeFieldName to catch data view changes, right?

mattkime · 2022-11-22T03:21:41Z

@dimaanj I'm not sure which fields are relevant to this, thats a product question. I merely thought it was worth summarizing all of them.

dimaanj · 2022-11-29T11:43:37Z

Closing it, since #146403 created instead

[Discover] improve data view changed warning

c76129b

dimaanj added Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.6.0 labels Nov 9, 2022

dimaanj self-assigned this Nov 9, 2022

dimaanj mentioned this pull request Nov 9, 2022

[Discover] Improve warn feedback when data view has changed after rule creation #144392

Closed

dimaanj added 2 commits November 15, 2022 16:51

Merge branch 'main' of https://github.com/elastic/kibana into improve…

746a286

…-warn-feedback-after-data-view-change # Conflicts: # src/plugins/discover/public/application/view_alert/view_alert_route.tsx

[Discover] update test

6e929ae

dimaanj marked this pull request as ready for review November 16, 2022 10:46

dimaanj requested review from a team as code owners November 16, 2022 10:46

kibanamachine and others added 2 commits November 17, 2022 05:09

Merge branch 'main' into improve-warn-feedback-after-data-view-change

5139706

[Discover] guarantee properties order within json.stringify

317db09

dimaanj added auto-backport Deprecated - use backport:version if exact versions are needed v8.7.0 labels Nov 17, 2022

ymao1 reviewed Nov 17, 2022

View reviewed changes

ymao1 approved these changes Nov 17, 2022

View reviewed changes

mattkime reviewed Nov 17, 2022

View reviewed changes

Merge branch 'main' into improve-warn-feedback-after-data-view-change

bf17668

dimaanj closed this Nov 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Improve warn feedback when data view has changed after rule creation #144908

[Discover] Improve warn feedback when data view has changed after rule creation #144908

dimaanj commented Nov 9, 2022 •

edited

Loading

elasticmachine commented Nov 16, 2022

ymao1 commented Nov 16, 2022

pmuellr commented Nov 16, 2022

dimaanj commented Nov 17, 2022

dimaanj commented Nov 17, 2022 •

edited

Loading

dimaanj commented Nov 17, 2022

ymao1 Nov 17, 2022

dimaanj Nov 18, 2022

ymao1 left a comment

mattkime Nov 17, 2022

dimaanj Nov 18, 2022

mattkime Nov 18, 2022

dimaanj Nov 18, 2022

mattkime Nov 18, 2022

dimaanj Nov 18, 2022

mattkime commented Nov 18, 2022

dimaanj commented Nov 21, 2022

kibana-ci commented Nov 21, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

dimaanj commented Nov 21, 2022 •

edited

Loading

mattkime commented Nov 22, 2022

dimaanj commented Nov 29, 2022

[Discover] Improve warn feedback when data view has changed after rule creation #144908

[Discover] Improve warn feedback when data view has changed after rule creation #144908

Conversation

dimaanj commented Nov 9, 2022 • edited Loading

Summary

Test notes

Checklist

elasticmachine commented Nov 16, 2022

ymao1 commented Nov 16, 2022

pmuellr commented Nov 16, 2022

dimaanj commented Nov 17, 2022

dimaanj commented Nov 17, 2022 • edited Loading

dimaanj commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ymao1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattkime commented Nov 18, 2022

dimaanj commented Nov 21, 2022

kibana-ci commented Nov 21, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

dimaanj commented Nov 21, 2022 • edited Loading

mattkime commented Nov 22, 2022

dimaanj commented Nov 29, 2022

dimaanj commented Nov 9, 2022 •

edited

Loading

dimaanj commented Nov 17, 2022 •

edited

Loading

dimaanj commented Nov 21, 2022 •

edited

Loading