search only custom result index on detector detail page

[ylwu-amzn]

Signed-off-by: Yaliang Wu ylwu@amazon.com

Description

Only query custom result index for detector detail page. Query both default and custom AD result index on dashboard and detector list page.

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[ohltyler]

So looks like this adds a param to the following redux functions:
getDetectorResults()
searchResults()
getDetectorLiveResults()

which ends up making calls to the following server-side functions:
searchResults()
getDetectors() (which sets the new param as false to guarantee results for all detectors)
getAnomalyResults()

Any reason why the logic for determining whether onlyQueryCustomResultIndex should be true/false can't be determined within the server-side functions alone? From my understanding, on client-side, passing resultIndex = ALL_CUSTOM_AD_RESULT_INDICES always corresponds to onlyQueryCustomResultIndex = false, and vice versa. So couldn't the server-side functions determine onlyQueryCustomResultIndex by checking the value of resultIndex.startsWith(CUSTOM_AD_RESULT_INDEX_PREFIX)? Seems this could minimize the overall code changes, and not add any extra params which have to be passed from client-side -> server-side

[amitgalitz]

Is this change only for detectors that themselves have a custom result index? Not across the board for every detector detail page right?

[ylwu-amzn]

So looks like this adds a param to the following redux functions: getDetectorResults() searchResults() getDetectorLiveResults()

which ends up making calls to the following server-side functions: searchResults() getDetectors() (which sets the new param as false to guarantee results for all detectors) getAnomalyResults()

Any reason why the logic for determining whether onlyQueryCustomResultIndex should be true/false can't be determined within the server-side functions alone? From my understanding, on client-side, passing resultIndex = ALL_CUSTOM_AD_RESULT_INDICES always corresponds to onlyQueryCustomResultIndex = false, and vice versa. So couldn't the server-side functions determine onlyQueryCustomResultIndex by checking the value of resultIndex.startsWith(CUSTOM_AD_RESULT_INDEX_PREFIX)? Seems this could minimize the overall code changes, and not add any extra params which have to be passed from client-side -> server-side

Yeah, we can do that. Here I expose the parameter to make it more flexible for client side. If we want to query only ALL_CUSTOM_AD_RESULT_INDICES in future like showing how many anomalies in custom result indices, we can just pass true to server-side function. And this also reminds other engineers who don't know the backend implementation details that they should consider whether just query custom result index is enough.

[ylwu-amzn]

Is this change only for detectors that themselves have a custom result index? Not across the board for every detector detail page right?

Yes, if detector is not using custom result index, we just query default result index. This change is mainly to improve performance for detector using custom result index by not querying default result index.

[ohltyler]

So looks like this adds a param to the following redux functions: getDetectorResults() searchResults() getDetectorLiveResults()
which ends up making calls to the following server-side functions: searchResults() getDetectors() (which sets the new param as false to guarantee results for all detectors) getAnomalyResults()
Any reason why the logic for determining whether onlyQueryCustomResultIndex should be true/false can't be determined within the server-side functions alone? From my understanding, on client-side, passing resultIndex = ALL_CUSTOM_AD_RESULT_INDICES always corresponds to onlyQueryCustomResultIndex = false, and vice versa. So couldn't the server-side functions determine onlyQueryCustomResultIndex by checking the value of resultIndex.startsWith(CUSTOM_AD_RESULT_INDEX_PREFIX)? Seems this could minimize the overall code changes, and not add any extra params which have to be passed from client-side -> server-side

Yeah, we can do that. Here I expose the parameter to make it more flexible for client side. If we want to query only ALL_CUSTOM_AD_RESULT_INDICES in future like showing how many anomalies in custom result indices, we can just pass true to server-side function. And this also reminds other engineers who don't know the backend implementation details that they should consider whether just query custom result index is enough.

Got it, yeah I'm ok with this since it does add more flexibility, and easier to change in the future. Just hesitant to add more parameters since some components already have such a long list, and overall prefer to abstract out the details to be determined on server-side and/or downstream functions if possible.

Can you confirm the functionality is working fine for non-custom and custom detectors? If so I'm ok to approve.

[ylwu-amzn]

So looks like this adds a param to the following redux functions: getDetectorResults() searchResults() getDetectorLiveResults()
which ends up making calls to the following server-side functions: searchResults() getDetectors() (which sets the new param as false to guarantee results for all detectors) getAnomalyResults()
Any reason why the logic for determining whether onlyQueryCustomResultIndex should be true/false can't be determined within the server-side functions alone? From my understanding, on client-side, passing resultIndex = ALL_CUSTOM_AD_RESULT_INDICES always corresponds to onlyQueryCustomResultIndex = false, and vice versa. So couldn't the server-side functions determine onlyQueryCustomResultIndex by checking the value of resultIndex.startsWith(CUSTOM_AD_RESULT_INDEX_PREFIX)? Seems this could minimize the overall code changes, and not add any extra params which have to be passed from client-side -> server-side

Yeah, we can do that. Here I expose the parameter to make it more flexible for client side. If we want to query only ALL_CUSTOM_AD_RESULT_INDICES in future like showing how many anomalies in custom result indices, we can just pass true to server-side function. And this also reminds other engineers who don't know the backend implementation details that they should consider whether just query custom result index is enough.

Got it, yeah I'm ok with this since it does add more flexibility, and easier to change in the future. Just hesitant to add more parameters since some components already have such a long list, and overall prefer to abstract out the details to be determined on server-side and/or downstream functions if possible.

Can you confirm the functionality is working fine for non-custom and custom detectors? If so I'm ok to approve.

Yeah, I have deployed this PR to our test cluster. All detectors look good. You can also help verify on the test cluster.

[ohltyler]

LGTM, thanks for the change!

[amitgalitz]

LGTM, spent some time understanding change and Tyler's questions/comments above helped a lot. Thanks for adding this!