-
Notifications
You must be signed in to change notification settings - Fork 916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Proposal] Data Explorer/Discover 2.0 #4165
Comments
Hey @ashwin-pc, thanks for the proposal! Couple of things
|
Somewhat related; I know there is some security concerns with AngularJS but do we have specific user requirements we're working backwards from? Or some guiding beliefs that are informing this full redesign? We might want to include those to make it easier to provide feedback. For example, I think the unification of Event Explorer and Discover is one of those reasons, but I'm not sure this proposal address the overlaps and gaps between the two and how this will solve that problem. Right now, the background only focuses on the security concerns and doesn't explain why we'd make all the other changes as well. |
Will the core concept of the data explorer build around index patterns still? |
Should we include functional tests in the requirements? such as adding new functional tests, modifying the old discover functional tests for discover feature in data explorer, and also removing the irrelevant functional tests etc |
Some quick and relatively minor feedback:
|
Feedback from the initial review:
|
Not really, upgrading discover take care of deangular tasks as well since we wont carry over any angular components. Part of disciver is already mgrated away from AngularJS so we can use those as is, and we need to only focus on the components that arent.
There are intentionally few guardrails here for now since it makes migrating existing applications onto Data Explorer easier. We can revisit the guardrails in future once we have more data about how this app is being used. There is enough isolation between views so changes in one should not affect the other, but the views we have today are sufficiently different that abstracting away more features into Data Explorer makes the integration more difficult. Also since Data explorer has so few abstractions, i dont think that any view will need to modify it to make their feature work, and that was the intention.
Its at the tenant level, and will be present in advanced settings. We plan on removing the toggle once the deangularization work is complete and i will update the proposal to call that out since its pretty lite on that right now.
Not really, there is only one question, and that is about how we want to handle the other views that discover supports (surrounding documents and single doc views). Right now i'm just looking for general feedback on the approach and if there are any things that i missed in my proposal.
Thats a good point, yes combining the event explorer view with discover is one such reason, but I didnt mention that explicitly here since Event Explorer is not a part of the base OSD repo and for developers who are using the minimal distribution, may never encounter that view. Thats why i focused on VisBuilder which is another such view that we want to be able to toggle between to explore the data. I also kept the description of view intentionally ambiguous since i want to make it possible for any view (e.g. Event Explorer) to easily integrate with Data Explorer regardless of their underlying architecture. That being said i will expand the overview section to highlight the current gaps with our different views |
Tests should be a part of any feature that is built so i didnt want to call that out as an explicit requirement here |
Its not a new term and is infact borrowed from Visualize. This is the property Visualize uses to list all the compatible visualization saved objects in the visualize listing view.
Yeah, detailed designs for some of the components will follow, this proposal just focusses on the high level design of the project |
Another path would be to share some of the user-centric reasoning behind this large undertaking. Modernizing is important but trying to understand why this work specifically. |
@ashwin-pc For some of the routing challenges of supporting old and new simultaneously, have you looked at https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/url_forwarding/README.md#L0-L1 ? |
@ashwin-pc this looks very promising ...
|
Hey @YANG-DB can you help me understand what you mean by |
similarly the data-source can have the next notions: [ The labels technique is common practice for reflecting metadata |
@ashwin-pc Thanks for this writeup. A lot of work... just to document this UX repositioning and describe the impact. Upon first read, it sounds like de-Angularization is being used as a primary motivation for the enhancement. I'm concerned that this is "burying the lead" on why we're moving in this new direction for UX on discovery. de-Angularization is important, yet it is technically a stand-alone effort and can be accomplished with no UX/UI impact. Seems like it's convenient timing, maybe opportunistic timing; we should be focusing the UX position on how and why it is better for the user, without concern over how it will be implemented, and why it is more-timely now than later. |
@pjfitzgibbons you are right in stating that deangularization can be achieved without any UI/UX impact and was infact the recommended approach at first. However when it was discussed whether we should spend a whole lot of time first migrating our angular components to react, just to rewrite the whole thing again as a Data Explorer tool, it seems like throwaway effort which is why the two projects were combined. This then introduced a wrinkle that the deangularization had to happen by EOY since it was a security risk that we had committed to remove by then. This mean that to meet the deadline without refactoring very different applications (Log Explorer and Discover) into one, a data explorer skeleton that allowed two independent applications to run at the same time had to be developed. That is the design you see here. |
This ^^ is the bit I was getting at. I feel declaring de-Ng as a "requirement" to the design effort of DataExplorer is misguided. I feel also, and more importantly, that it is valid and important to indicate the org-level commitment above, and how that agreement (de-Ng) has side-effect impacts on the ultimate design. TLDR; It's ok to "And also this accomplishes an Org goal to remove security risk of Angular library". It's not ok to say "This, because Angular" |
Orthogonal to this, I would like to remove the dependency around index patterns. I think there's some work here, but I'd like to rethink index pattern as a concept. An index pattern really is a dynamic data schema, or an aggregation schema. It also gives us some advance features like run-time fields, scripted fields, etc. At the end of the day, we should invest in the ability to compose a query using other query languages, query indexes directly (and potentially be able to simply read the index mapping API to provide run-time schemas, which could be saved as a saved object for ease of exploration down the road). This will also give us more flexibility to correlate alerts, detectors, and other features from across other plugins. |
Overview
Data Explorer project represents a strategic effort to address the security concerns associated with the legacy Discover component while delivering an enhanced and versatile data exploration experience for users. Data Explorer serves a dual purpose. Firstly, it aims to upgrade the existing Discover plugin to use React. Secondly, it seeks to become a comprehensive platform that caters to all data exploration needs.
All the existing features of Discover will seamlessly transition into Data Explorer. Additionally, this upgraded platform will introduce new capabilities, such as exploratory data analysis through aggregations and visualizations, as well as support for various query languages. The integration of these features within Data Explorer aims to provide a unified and efficient environment for data exploration tasks.
Background
The Discover component has been a longstanding and crucial part of OSD OpenSearch Dashboards. However, it was built using Angular version 1.0, which poses security concerns, requiring its removal by the end of the year Ref. Data exploration in OpenSearch has also been confusing with many different applications such as Discover, Visbuilder, Event Analytics all solving similar problems. Data explorer aims to solve both problems at once by offering a single platform where all data exploration views can exist simultaneously while also using it first as a target for the Discover deangularization effort.
Requirements
Architecture
For this feature the following changes will be made to the plugins:
data_explorer
: This is a new plugin introduced to act as the view container for the different sub applications for data exploration.discover
: The discover 2.0 plugin that is a combination of existing non angular code from discover_legacy, vis_builder and other parts of OSD that is useful and new components where existing components do not existvis_builder
: Migrates its views from a separate app to a view insidedata_explorer
discover_legacy
: The existingdiscover
plugin will move todiscover_legacy
and all associated routes will be renamed asdiscover_legacy
. While sharing, the URL will still referencediscover
.Division of responsibility
Data explorer simply acts as a shell for other exploration views, so here is how the responsibilities are divided between Data Explorer and its views.
Data Explorer: Data explorer is responsible for 4 primary features.
Views: Each of the views on the other had are responsible for:
The goal here is that by minimizing the surface area for Data Explorer specific changes, each application can migrate their existing view relatively easily onto data explorer.
Wireframe
View Registry
The Data Explorer exposes a view registry that allows apps to register themselves as views within Data Explorer. Each view object has the following properties:
Data model:
id
: The id of the view. This is used to identify the view in the view switcher and in the URLtitle
: The title of the view. This is used to display the view in the view switchericon
: The icon of the view. This is used to display the view in the view switcherui
: This is an object that contains the UI components for the view.panel
: This is the component that is rendered in the panel when the view is selectedworkspace
: This is the component that is rendered in the workspace when the view is selecteddefaults
: This is the default state for the view. This is used to initialize the state when the view is selectedreducer
: This is the reducer for the view. This is used to update the state of the view when actions are dispatcheddefaultPath
: This is the default path for the view. This is used to redirect the user to the view when they select it in the view switcherextention
: This is an object that contains the extension for the view. This is used to register the embeddable for the viewtype
: This is the type of the embeddabletoList
: This is a function that converts the saved object into a view list item. This is used to display each saved object when wants to load a supported saved objectshouldShow
: This is an optional function that is used to determine if the view should be shown in the view switcher. This is useful for views that are not compatible with the current data source. OpMigrating existing applications
Data explorer will likely be a place where existing application can migrate their data exploration views. This means that Data Explorer should also take into account how an application can do this without disrupting the users workflow. Such a migration is already needed for Discover and VisBuilder so I assume that we will need to do do this again for other views.
Migrating the actual application is pretty straight forward. Since Data Explorer is only responsible for 3 things, Data source, state management and view registry, the application can simply register itself as a view and then start using the Data Explorer state management and data source. The only thing that the application needs to do is to migrate its existing state management to the Data Explorer state management and reference the datasource passed down by Data Explorer instead of its own. The application needs to also modify its routes to use the Data Explorer routes and UI to match the panel and workspace components UI.
Routing
Routing is important for Data Explorer especially when we do not want to break backwards compatibility with existing applications. For this reason, Data Explorer will have its own routes that are prefixed with
/data_explorer
and the existing routes for each view is responsible for redirecting to the new routes. For example, the existing discover route will redirect to/data_explorer/discover
and the existing vis builder route will redirect to/data_explorer/vis_builder
by their respective plugins. This way, the existing routes can still be used and the new routes can be used for the new views.Discover 2.0 - Deangularize
With the above architecture in mind, here is how the existing discover plugin will be migrated to the new architecture.
Research issue: #4130
Mock wireframe
Toggle
Toggling between the old and new views is a bit more complicated than a standard migration since in this view we need to support both the old and new plugins at the same time. To do this we implement the routing strategy as mentioned before with one change, Data Explorer's router also checks to see if using the legacy plugin is turned on or not. If it is, it routes all discover traffic to
discover_legacy
, else it sends it off to the discover view registered to it. Once the migration is complete we simply remove this check.Migrating Features
a. The selected fields section will be a net new feature that will also have a drag and drop UX pattern
b. The Available fields section will be built on top of a similar section from VisBuilder
c. Field summary will also migrate from VisBuilder with the added ability to create a visualization from the view directly.
Utilities that are either not tied to the UI or are already migrated like the JSON doc viewer will be migrated as is to the new experience.
Open questions
The text was updated successfully, but these errors were encountered: