Add interfaces for layering data on Visualizations #3108

ohltyler · 2022-12-19T23:07:28Z

Description

This PR adds the interfaces related to layering data on top of a visualization. These are persisted in the new vis_augmenter plugin. Also defines the expressions function definition and expressions type for the new vis layers (details on the expression function design in the related issue)

Note that currently the only implemented VisLayer is PointInTimeEventsVisLayer. Details on this design, as well as future possible VisLayer implementations, can be found in the related issue.

Issues Resolved

Closes #2882
Closes #2894

Check List

ohltyler · 2022-12-19T23:08:14Z

Keeping as draft until #3107 has been reviewed & merged; I will then rebase this PR

codecov-commenter · 2022-12-20T01:28:36Z

Codecov Report

Merging #3108 (25276e4) into feature/feature-anywhere (ba6f9eb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                    Coverage Diff                    @@
##           feature/feature-anywhere    #3108   +/-   ##
=========================================================
  Coverage                     66.67%   66.67%           
=========================================================
  Files                          3219     3220    +1     
  Lines                         61450    61454    +4     
  Branches                       9417     9417           
=========================================================
+ Hits                          40969    40973    +4     
  Misses                        18234    18234           
  Partials                       2247     2247

Impacted Files	Coverage Δ
src/plugins/vis_augmenter/common/types.ts	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

src/plugins/plugin_integration/public/types.ts

src/plugins/vis_augmenter/common/types.ts

joshuarrrr · 2022-12-23T21:13:35Z

src/plugins/vis_augmenter/common/types.ts

+// used to determine what vis layer's interface is being implemented.
+// currently PointInTimeEventsLayer is the only interface extending VisLayer
+export const isPointInTimeEventsVisLayer = (obj: any) => {
+  return 'events' in obj;
+};


How are you imagining this to be used? It seems like it would be simpler to have a static type string on the VisLayer interface that you can specify when defining a particular type of VisLayer, and then just access as a value. This type of convenient method may still be helpful, but it's actually implemented as more a capability check.

Agreed, existing soln seemed hacky. Updated to use enums in latest commits. Also added a general isValidVisLayer fn that can be ran when verifying the new saved object types are valid or not, based on expression function type

src/plugins/expressions/common/expression_types/specs/vis_layers.ts

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

joshuarrrr

Looks great, thanks for the improvements and the tests!

joshuarrrr · 2022-12-29T02:16:55Z

src/plugins/vis_augmenter/common/types.ts

-// used to determine what vis layer's interface is being implemented.
-// currently PointInTimeEventsLayer is the only interface extending VisLayer
 export const isPointInTimeEventsVisLayer = (obj: any) => {
-  return 'events' in obj;
+  return obj?.type === VisLayerTypes.PointInTimeEvents;
+};
+
+export const isValidVisLayer = (obj: any) => {
+  return obj?.type in VisLayerTypes;
 };


nice, this seems much more clear to me.

manasvinibs · 2022-12-29T22:35:09Z

Do we know why cypress tests are failing here or is it expected?
Also I see WhiteSource security check failing as 7 new vulnerabilities being flagged with this change. Does it require rebase with the main to include fixes ?

joshuarrrr · 2022-12-29T22:40:50Z

Do we know why cypress tests are failing here or is it expected? Also I see WhiteSource security check failing as 7 new vulnerabilities being flagged with this change. Does it require rebase with the main to include fixes ?

Yes, cypress test failures are to be expected for feature branches right now, based on the way those workflows are configured. I also think we can ignore whitesource checks when merging to the feature branch target - that's something we can make sure of as we rebase the feature branch and when we merge it into main.

ohltyler · 2023-01-05T21:09:59Z

@joshuarrrr #3107 will need to be merged to 2.x before this backport PR can be opened

joshuarrrr · 2023-01-05T21:47:50Z

This shouldn't be backported - sorry, forgot we were working on a feature branch here.

…t#3108) * Add vis layer interfaces Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

ohltyler requested a review from a team as a code owner December 19, 2022 23:07

ohltyler marked this pull request as draft December 19, 2022 23:07

ohltyler changed the title ~~Add vis layers~~ Add interface for layering data on Visualizations Dec 19, 2022

ohltyler mentioned this pull request Dec 19, 2022

Add a new augment-vis saved object type #3109

Merged

8 tasks

ohltyler changed the base branch from main to feature/feature-anywhere December 19, 2022 23:35

ohltyler force-pushed the add-vis-layers branch from c22ecd2 to c961d3f Compare December 20, 2022 17:47

ohltyler mentioned this pull request Dec 22, 2022

Add a new vis_augmenter plugin #3107

Merged

8 tasks

ohltyler changed the title ~~Add interface for layering data on Visualizations~~ Add interfaces for layering data on Visualizations Dec 22, 2022

ohltyler force-pushed the add-vis-layers branch 2 times, most recently from 342bc10 to d42e4fe Compare December 22, 2022 19:26

Add vis layer interfaces

9e1c51b

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

ohltyler force-pushed the add-vis-layers branch from c8e9ac3 to 9e1c51b Compare December 22, 2022 19:32

ohltyler added the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Dec 22, 2022

ohltyler requested review from ashwin-pc, joshuarrrr and kristenTian December 22, 2022 19:34

ohltyler marked this pull request as ready for review December 22, 2022 19:34

Minor cleanup

4ff4451

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

ohltyler mentioned this pull request Dec 23, 2022

[Feature Anywhere] Enable plugins to augment visualizations with additional data and context #2880

Closed

31 tasks

joshuarrrr reviewed Dec 23, 2022

View reviewed changes

ohltyler added 2 commits December 23, 2022 16:28

Move new expressions type to vis_augmenter; clean up vis layer types

d7bb8bd

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

Remove leftover expression types after moving

25276e4

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

joshuarrrr approved these changes Dec 29, 2022

View reviewed changes

joshuarrrr added the v2.5.0 'Issues and PRs related to version v2.5.0' label Dec 29, 2022

manasvinibs approved these changes Dec 29, 2022

View reviewed changes

joshuarrrr merged commit e78ccbf into opensearch-project:feature/feature-anywhere Dec 30, 2022

This was referenced Dec 30, 2022

[Feature Anywhere] Add a vis_layers expression function type #2894

Closed

[Feature Anywhere] Add a generic interface for overlaying data on a Visualization #2882

Closed

joshuarrrr added the backport 2.x label Jan 5, 2023

joshuarrrr removed backport 2.x v2.5.0 'Issues and PRs related to version v2.5.0' labels Jan 5, 2023

opensearch-project deleted a comment from opensearch-trigger-bot bot Jan 5, 2023

ohltyler added a commit to ohltyler/OpenSearch-Dashboards that referenced this pull request Feb 1, 2023

Add interfaces for layering data on Visualizations (opensearch-projec…

0aa03ec

…t#3108) * Add vis layer interfaces Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

ohltyler added a commit to ohltyler/OpenSearch-Dashboards that referenced this pull request Feb 1, 2023

Add interfaces for layering data on Visualizations (opensearch-projec…

c52f901

…t#3108) * Add vis layer interfaces Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add interfaces for layering data on Visualizations #3108

Add interfaces for layering data on Visualizations #3108

ohltyler commented Dec 19, 2022 •

edited

Loading

ohltyler commented Dec 19, 2022

codecov-commenter commented Dec 20, 2022 •

edited

Loading

joshuarrrr Dec 23, 2022

ohltyler Dec 24, 2022 •

edited

Loading

joshuarrrr left a comment

joshuarrrr Dec 29, 2022

manasvinibs commented Dec 29, 2022

joshuarrrr commented Dec 29, 2022

ohltyler commented Jan 5, 2023

joshuarrrr commented Jan 5, 2023

Add interfaces for layering data on Visualizations #3108

Add interfaces for layering data on Visualizations #3108

Conversation

ohltyler commented Dec 19, 2022 • edited Loading

Description

Issues Resolved

Check List

ohltyler commented Dec 19, 2022

codecov-commenter commented Dec 20, 2022 • edited Loading

Codecov Report

joshuarrrr Dec 23, 2022

Choose a reason for hiding this comment

ohltyler Dec 24, 2022 • edited Loading

Choose a reason for hiding this comment

joshuarrrr left a comment

Choose a reason for hiding this comment

joshuarrrr Dec 29, 2022

Choose a reason for hiding this comment

manasvinibs commented Dec 29, 2022

joshuarrrr commented Dec 29, 2022

ohltyler commented Jan 5, 2023

joshuarrrr commented Jan 5, 2023

ohltyler commented Dec 19, 2022 •

edited

Loading

codecov-commenter commented Dec 20, 2022 •

edited

Loading

ohltyler Dec 24, 2022 •

edited

Loading