elastic / connectors

Source code for all Elastic connectors, developed by the Search team at Elastic, and home of our Python connector development framework
https://www.elastic.co/guide/en/enterprise-search/master/index.html
Other
59 stars 118 forks source link

Better feedback for failed sync rules #1523

Open llermaly opened 10 months ago

llermaly commented 10 months ago

Problem Description

When many rules fail, it is hard to identify which rule failed or why.

Proposed Solution

  1. Add more details about which rule failed
  2. Refer to the rule number, or highlight the rule that failed. Currently the error refers to an ID which is not present in the UI

Additional Context

CleanShot 2023-08-23 at 18 18 54

For example on this screenshot all the errors are triggering for rule #12 ?

danajuratoni commented 10 months ago

We should not use IDs in UI error messages that are not exposed in Kibana, as previously discussed.

Referencing the rule with relative numbering e.g. rule 2 and 4 conflict, should have been the go to approach since several versions now, marking this as a bug that should be fixed asap.

Ideally, we should be highlighting the affected rows (using IDs internally) and display only a user friendly/meaningful error message in Kibana. @efegurkan could you please check if the highlighting logic was implemented (I believe 8.8 was when we originally planned this for)

danajuratoni commented 10 months ago

cc: @artem-shelkovnikov

seanstory commented 10 months ago

This isn't going to make 8.10.0

timgrein commented 8 months ago

I've adjusted the effort to medium. This requires a small protocol change adding a order field. We could solve it in a hacky way setting the id to the order, but that would make it hard to debug it later as the order can be changed, but the id not.

DianaJourdan commented 8 months ago

@timgrein what is the remaining work on this bug? Just the Kibana header change? "additional Kibana change is needed to also adapt the heading of the error message in the UI."

seanstory commented 8 months ago

This requires a small protocol change adding a order field.

I don't think that's the case. Lists/arrays are ordered in JSON - you can use the index in the list/array to know what order the rule is in.

But regardless, the way I'd looked at addressing this (and still think it could be handled) is to just improve the to_string function for these rules, so that it's more clear to the user what rules are conflicting.

DianaJourdan commented 8 months ago

Agreed in the team sync that no protocol change is needed, but we will stick to an error message + highlight the rules that fail