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

[GS] Allows apps to register searchable keywords for Global Search #85686

Merged
merged 10 commits into from
Dec 18, 2020

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Dec 11, 2020

Resolves #76778

Summary

Allows applications to register keywords as search terms for Global Search.
Since keywords are not data, they are have been added to the App type as a meta property. This PR moves searchDeepLinks into the new meta property for the same reason (they are not data).

core.application.register({
  id: 'myApp',
  title: 'Translated title,
  mount,
  meta: {
     keywords: ['translated keyword1', 'translated keyword2']
	 searchDeepLinks: [
     	{ id: 'sub1', title: 'Sub1', path: '/sub1', meta: { keywords: ['subpath1'] } },
   	 ],
  }
})

Scoring by keywords implements the same algorithm as searching for applications by title and weights the scores in favor of matching by title (keywords score * 0.8). The weighting factor is an arbitrary constant.

This PR makes no assumptions on what keywords to assign to apps, rather leaving it up to application developers and solutions to choose their own.

How to test this

The unit tests for get_app_info, e.g. https://github.com/elastic/kibana/pull/85686/files#diff-5d4a779847b1ac5095589e30e25778e35a3350c09b4c6d9bdfcca0b22bc820fbR122 show how the keywords are added to an app and the unit tests for get_app_results, e.g. https://github.com/elastic/kibana/pull/85686/files#diff-3656691fb1cb333c33264ea83656f59878c49b13d7c3cdda8399ddbcc4e7d03eR113 show how scoring by keywords works.

To test this 'live', you'll need to add keywords or add a new plugin that has keywords and, optionally, deep links (see https://github.com/elastic/kibana/pull/85686/files#diff-efac386db0738f497e14a9772c5da15dde935e474ee2dc3b62fe5fca2e0e92d8R247 as an example), after pulling the code.

Example

Plugin: Uptime
Modified registration:

    core.application.register({
      id: PLUGIN.ID,
      euiIconType: 'logoObservability',
      order: 8400,
      title: PLUGIN.TITLE,
      category: DEFAULT_APP_CATEGORIES.observability,
      meta: {
        keywords: ['awake'], // --> add your keyword(s)
      },
      mount: async (params: AppMountParameters) => {
        const [coreStart, corePlugins] = await core.getStartServices();

        const { renderApp } = await import('./render_app');

        return renderApp(coreStart, plugins, corePlugins, params);
      },
    });

Run the code and search for 'awake'. This should show a direct link to 'Uptime'

Uptime_awake_keyword

Plugin: APM
Modified registration:

core.application.register({
      id: 'apm',
      title: 'APM',
      order: 8300,
      euiIconType: 'logoObservability',
      appRoute: '/app/apm',
      icon: 'plugins/apm/public/icon.svg',
      category: DEFAULT_APP_CATEGORIES.observability,
      meta: {
        keywords: ['application monitoring', 'performance'], // --> add your keyword(s)
      },

      async mount(params: AppMountParameters<unknown>) {
        ...
      },
    });

Searching for 'performance' or 'application monitoring' or any shorter version of these keywords will show a direct link to the 'APM' app:

APM_performance_keyword

Keywords in deep links work similarly.

Release Notes

Kibana's navigation search now implements keyword registration for applications, allowing you to search for applications by keywords as well as by title.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@TinaHeiligers TinaHeiligers added release_note:enhancement Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result v8.0.0 v7.12.0 labels Dec 11, 2020
@TinaHeiligers
Copy link
Contributor Author

@joshdover @pgayvallet This is still very much work in progress. I wanted to open a draft early so that we can discuss what we still need for this feature and what we decide to strip out (e.g. removing keywords from application deep links)

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic. I do think we should move the searchDeepLinks property into the meta object so that these two concepts are closer to one another in the API. We don't have to do that in this PR though if you'd prefer a follow up for that.

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great for now!

+1 for josh's proposal to move searchDeepLinks into the new meta property

@TinaHeiligers TinaHeiligers marked this pull request as ready for review December 17, 2020 17:30
@TinaHeiligers TinaHeiligers requested a review from a team December 17, 2020 17:30
@TinaHeiligers TinaHeiligers requested a review from a team as a code owner December 17, 2020 17:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits, but overall the implementation is great.

LGTM

Comment on lines 242 to 247
*
* @remarks
* Used to populate navigational search results (where available).
* Can be updated using the {@link App.updater$} observable. See {@link AppSubLink} for more details.
*
* @example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: was already present before this PR, but the link to AppSubLink is broken. I guess it should point to PublicAppSearchDeepLinkInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix it while waiting for the @elastic/kibana-app team's review.

Comment on lines +89 to +90
const scores = keywords.map((keyword) => {
return scoreAppByTerms(term, keyword);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: we could break/return earlier if a keyword's score is 100.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add that as an optional optimization strategy if/when plugin owners start adding their keywords. Right now, I don't think it's needed.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 47285 48045 +760

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 554.6KB 555.2KB +605.0B
globalSearchProviders 8.8KB 9.9KB +1.1KB
management 22.2KB 23.3KB +1.0KB
total +2.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KibanaApp code changes LGTM, didn't test locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result release_note:enhancement Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GS] Register aliases or keywords for navigational search
6 participants