Skip to content
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

[FEATURE] Dashboards List Extensions #1866

Closed
pjfitzgibbons opened this issue Jul 8, 2022 · 11 comments · Fixed by #3090
Closed

[FEATURE] Dashboards List Extensions #1866

pjfitzgibbons opened this issue Jul 8, 2022 · 11 comments · Fixed by #3090
Assignees
Labels
enhancement New feature or request roadmap v2.7.0

Comments

@pjfitzgibbons
Copy link
Contributor

pjfitzgibbons commented Jul 8, 2022

Is your feature request related to a problem? Please describe.

As a Plugin Developer/Implementer, I would like to be able to save my "dashboard like" items in SavedObjects and have them be included in "Dashbaords / Dashboards" listing.

Describe the solution you'd like

Audio starts at 0:05. Unmute to listen.

Dashboard-integration-demo2.mov

Option : Save "items" to be displayed by Dashboard-List into SavedObjects (Recommended)
With this option, Plugins that desire to have their object items included in the Core Dashboards List would utilize the OSD-Core SavedObjects store and API, ensuring the plugins SavedObject type is unique to all other plugins.
The Plugin would then register it's object type to the OSD-Core Dashboards component, as exposed to the DepsStart object on Plugin interface's start() callback.

  • Pros

    • Minimal change to OSD-Core - only to allow plugins to register their “dashboard type”
    • All current features and benchmark performance maintained
  • Cons

    • Plugin developers must determine how to manage storing objects in SavedObjects.
      • This represents architectural requirements to integrate, and may involve extensive changes for some implementers.
  • Design

    • The registerDashboardProvider call back’s type definition :
        registerDashboardProvider: RegisterDashboardProviderFn = (provider: {
            id: string;  // this is the plugin's id, like 'my-plugin'
            savedObjectType: string;
            editUrlFn: (id: string)  => string;
            viewUrlFn: (id: string) => string;
            deleteUrlFn: (id: string) => string;
            createLinkText: string | JSX.Element;
            createUrl: string;
          }) => void;
      
  • Design Ideas (presented by others)

    • Consider callbacks saved in SavedObject definition ? (J. Romero)

      • This would be up to a plugin developer to use. This design requires no special use of SavedObjects apart from that described in OpenSearch online and in-code documentation.
    • Migrations

      • Use a “Migrate now” button - manual migration - indicate this is required for Dashboard Integration.
        • This might be a consideration of a plugin that is storing it's items in an index NOT SavedObjects (.kibana). Otherwise, standard SavedObjects usage is applied, which includes migrations: {...} on the SavedObjects type declaration.
  • Design Todo / Required Research

    • Security

      • Need to know how to migrate prev. Observabilty roles into OSD-core SavedObject roles.
        • See Dave Lago / Peter Nied for list of POC to meet
        • This design does not functionally change the access model of DashboardsList / SavedObject API, so no additional app-sec is necessary.
    • Object Store

Option : DashboardList Providers registry with data-fetch callbacks per plugin (Rejected)
Dashboards List widget (part of ./plugins/dashboards module) will be enhanced to include a registry of plugin "list providers" that will be queried and their results aggregated to the main displayed list.

  • Pros

    • Plugin data-store remains independent of OSD-Core
    • This feature, when enabled, “meets the customer where they are” - a plugin’s existing data storage mechanism remains unchanged to integrate with this feature.
  • Cons

    • Network data-transmission of benchmark test - 25,000 items - increases linearly with data count. At 25,000 records, transmission time as much as 6 sec.
    • Search functionality brought “to the browser”, and utilizes Regex for search. This is a change in CX functionality, as some customers expect the pre-existing Full Elastic Query that was by default enabled in EUI SearchBox component (due to query passed into OS index backend engine)

Both Cons represent hard-requirements for this feature. In order to integrate with OSD-Core, any plugin must store objects in .kibana index via StoredObjects API/library.

  • Design
    • A registerListProvider function will be returned on DashboardPlugin.setup(), making it available to other plugins when "dashboards" is referenced from CoreSetup input params.

    • In preparation for this modification, the Dashboard-List widget, currently implemented as an Angular Directive, will be re-written into a React Component. The extension modifications will then be built upon the newly written React Component.
      The <dashboard-list ...> directive and it's props will be matched exactly in the new implementation.

Describe alternatives you've considered

Additional context

Related PRs
Opensearch-Dashboards - Dashboard List Integration #3090

Dashboards-Observability - [Feature] Dashboards-List integrations and Left-Nav Updates #151

@pjfitzgibbons pjfitzgibbons added the enhancement New feature or request label Jul 8, 2022
@joshuarrrr
Copy link
Member

joshuarrrr commented Aug 2, 2022

I strongly favor Option 3 here, particularly because this is the solution already implemented by the visualizations app for this exact use case: to include the saved objects of other plugins that register themselves as "aliases" of a visualization saved object. I'd rather first keep the behavior consistent across the two core apps and then later refactor or re-architect them together.

As an example, here's what it looks like to register a visualization alias (in the public/plugin setup method):

visualizations.registerAlias({
      name: PLUGIN_ID,
      title: PLUGIN_NAME,
      description: DESCRIPTION,
      icon: ICON,
      aliasApp: PLUGIN_ID,
      aliasPath: '#/',
      appExtensions: {
        visualizations: {
          docTypes: [PLUGIN_ID],
          toListItem: ({ id, attributes }) => ({
            description: attributes?.description,
            editApp: PLUGIN_ID,
            editUrl: `${EDIT_PATH}/${encodeURIComponent(id)}`,
            icon: ICON,
            id,
            savedObjectType: SAVED_OBJECT,
            title: attributes?.title,
            typeTitle: PLUGIN_NAME,
          }),
        },
      },
    });

I'm not sure I follow the concern about adding new saved object types - migrations would not be necessary in that case.

@elfisher
Copy link

From the gif it looks like this would link out to a different page based on the type. Is there a reason for this? Could we be consistent since they are all dashboards?

@anirudha
Copy link

anirudha commented Sep 2, 2022

we could structure the crums / accordingly to maintain consistency

@pjfitzgibbons
Copy link
Contributor Author

@elfisher @anirudha I believe the intention of this feature is to allow plugins to expose their objects onto the "front page" of OSD, which would be the Dashboards List. We are not recommending or requiring the plugin author modify their plugin to appear as-if it were part of OSD-Core proper; instead we allow the plugin to maintain it's own breadcrumb scheme as they wish. The integrated OSD Dashboards List then indicates the plugin-type of the listing entry, and the link to that entry brings the user to that plugin's own environment.

Plugins currently are self-contained within their own private page-hierarchy and breadcrumb scheme. We do not break that wall with this feature.

@pjfitzgibbons
Copy link
Contributor Author

I strongly favor Option 3 here, particularly because this is the solution already implemented by the visualizations app for this exact use case: to include the saved objects of other plugins that register themselves as "aliases" of a visualization saved object. I'd rather first keep the behavior consistent across the two core apps and then later refactor or re-architect them together.

I'm not sure I follow the concern about adding new saved object types - migrations would not be necessary in that case.

@joshuarrrr The migrations issue would be this : current Plugin's objects are pre-existing in the system. This feature, along with a plugin's update to utilize this feature, would require the plugin to "migrate" existing objects to create the vis-alias records. This ultimately creates two-sources-of-truth to the Plugin for their objects. I doubt we could cleanly assume a one-time-migration would be correct or complete for any Plugin.

The chosen solution, as written, turns the source-of-truth toward the plugin author - any time the Dashboards List is rendered, the object queries any registered plugins for their list items - the Dashboards List regards each plugin as its own source-of-truth and behaves appropriately with the given list-result from each plugin.

This solution has its flexibility in decoupling the plugin's list-items from the OSD-Core and from the OS backend. Each Plugin can determine on its own how and where to store its metadata about the integrated list-objects.

@ashwin-pc
Copy link
Member

@pjfitzgibbons @joshuarrrr I think you are both talking about the same feature. visualizations.registerAlias is similar to what @pjfitzgibbons is proposing here. except that he is proposing that we inrtoduce two new interfaces from dashboards:

  1. service to register list providers (Since plugins will have their own lists)
  2. service to register new create dashboard providers

@pjfitzgibbons I think this makes sense functionally, however do we have mocks for how this should look since as you said this affects the "front page" of Dashboards. If not @KrooshalUX @kgcreative do you think this UX/flow makes sense? I wonder if we should have some sort of grouping/filtering ability too? since this could introduce a lot of new list items when combined.

@pjfitzgibbons
Copy link
Contributor Author

@ashwin-pc The mock-up we have is the animated gif at the description of (this) issue.

The dashboard list is by default paged, and the filter() method on Dashboard(legacy) Ng component has been updated to include the Type and Description columns, so Search is more like an "uber search" and quickly filters the list to desired items.

I'd like to hear if you feel there are pre-existing features for Dashboards List that I've missed?
If you're considering future-looking features as a response to this mod, I would like to hear how they might be grouped into P0/P1. We are wanting to target 2.5 for this change and the release schedule clock is running.

@ashwin-pc
Copy link
Member

ashwin-pc commented Dec 20, 2022

@pjfitzgibbons I dont think you have missed any feature. Just wanted to make sure that someone from the dashboard ux has seen this mock and has no concerns with it. We've had a few projects get quite far into development only to realize that it fundamentally broke some assumptions about how the Dashboards UX was supposed to function, so I just want to make sure we caught that (if any) UX concerns earlier.

And yes, and forward looking features can definitely be incorporated as P1. My callout about categorization was because we are going to increase the number of list items with this change which will have an impact on the users UX. Just want to make sure we thought about it.

@anirudha anirudha added v2.5.0 'Issues and PRs related to version v2.5.0' roadmap labels Jan 6, 2023
@joshuarrrr joshuarrrr added v2.6.0 and removed v2.5.0 'Issues and PRs related to version v2.5.0' labels Jan 9, 2023
@joshuarrrr joshuarrrr added v2.6.0 and removed v2.6.0 labels Jan 12, 2023
@zengyan-amazon
Copy link
Member

I'd also like this feature to be throughly reviewed by UX and product as well. @kgcreative @ahopp

this current experience makes user feel trace analytics and OSD dashboards are separate applications, e.g. the link to a different app location, and the different dashboards creation experience make them feels complete different and separate. thus having trace analytics listed in the Dashboards list, and integrate the create button makes the experience weird to me.

I think we should consider making the integration and experience more integrated and seamless.

@kgcreative
Copy link
Member

I've reviewed and am aligned with this as a first step. I agree that we need better integration in the fullness of time, however, this is the first step for bringing these (and other) applications in. We will likely see a bit more variance before these all start aligning in a more seamless way

@pjfitzgibbons pjfitzgibbons changed the title [FEATURE] Dashboards List Extensibility & Integrate Observability Apps/Panels/EventAnalytics [FEATURE] Dashboards List Extensions Feb 6, 2023
@vagimeli
Copy link
Contributor

@pjfitzgibbons Can we sync on this issue so that I can better understand the documentation need, use case, and audience?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request roadmap v2.7.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants