[Backport 2.x] [MDS] Support for Timeline #6493
Conversation
![opensearch-trigger-bot[bot]](https://avatars.githubusercontent.com/u/80134844?s=60&v=4){kind=small}
* Add MDS support to Timeline Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor to function and add unit tests Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix typo in args Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor build request to pass unit tests Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Add to CHANGELOG Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Refactor error messages + address comments Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix ut Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Change to data source feature Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Fix UT Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> * Address comments Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> --------- Signed-off-by: Huy Nguyen <73027756+huyaboo@users.noreply.github.com> (cherry picked from commit 73e5d78) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
opensearch-trigger-bot
bot
added
autocut
opensearch-trigger-bot
bot
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT and
xinruiba
as code owners
|
is this one targeting 2.14. or 2.15 ? seen the 2.15 tag but if we merged into 2.x now it would be released out as part of 2.14 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #6493 +/- ##
==========================================
+ Coverage 31.75% 31.85% +0.10%
==========================================
Files 2220 2223 +3
Lines 44686 44743 +57
Branches 6951 6960 +9
==========================================
+ Hits 14190 14253 +63
+ Misses 29839 29830 -9
- Partials 657 660 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@huyaboo I think it's for 2.14, can you confirm? |
| throw new Error('To query from multiple data sources, first enable the data source feature'); | ||
| } | ||
|
|
||
| const dataSources = await client.find<DataSourceAttributes>({ |
There was a problem hiding this comment.
I don't think we should need to query the cluster every time to get the source. If I put an auto refresh interval then it seems like this will hit the cluster twice per request. I think we should caching these data sources like index patterns. If the data source exists in that cache then we can just pull that, if not then the service will pull the data sources.
| return undefined; | ||
| } | ||
|
|
||
| if (!getDataSourceEnabled().enabled) { |
There was a problem hiding this comment.
I can imagine a world where saved objects were imported from a cluster that enabled MDS to a new domain that didn't enable it. If the timeline vis was added to a dashboard now it will fail.
Do we want to consider that if it's not enabled it just queries to the local cluster?
There was a problem hiding this comment.
Existing behavior with Timeline is that any invalid params are caught and a toast error is displayed, meaning before this merge, if a user types in data_source_name, a toast would be thrown anyway because this was not a named argument. This means that any syntax error or invalid param can and should be caught. If the visualization just queries a local cluster, then it can be confusing behavior especially since this change enables Timeline to query both local cluster and MDS queries within one visualization.
zhyuanqi
added
bug
Backport 73e5d78 from #6385.