[RFC] Global search API #64284
[RFC] Global search API #64284
Conversation
pgayvallet
added
Feature:New Platform
RFC
release_note:skip
|
Pinging @elastic/kibana-platform (Team:Platform) |
rfcs/text/0011_global_search.md
Outdated
| publicUrl, | ||
| // note: this is actually wrong. We would need the `requestScopePath` here | ||
| // as this currently returns `${this.serverBasePath}${requestScopePath}` and the basePath | ||
| // is already included in `publicAddress` | ||
| serverContract.basePath.get(request), | ||
| path |
There was a problem hiding this comment.
We will need something like basePath.getRequestScopePath for that. However the implementation is trivial as the basePath service is still aggregating the basePath with the extra path in ${this.serverBasePath}${requestScopePath}. @restrry do you see any issue in adding this API?
There was a problem hiding this comment.
One thing I do not see addressed is the number of results that should be returned by providers and how that factors into the merging of results & timeouts. I would expect that consumers of the find API should be able to provide a number of results to be returned.
I don't think we need to address paging at this time, but we could do that in the future. Thinking ahead a bit, would it make sense for paging to be implemented using an async generator? That gives us the ability to pull information from data sources, but it may complicate things with the proposed Observable-based API.
|
The RFC has been updated according to all the resolved conversations, and is now ready for a second pass. I updated the issue description to add the list of unfinished discussions and their links. Also, I would like to timebox the period until the RFC is put into final comment stage to one week. If there are any fundamental issues with this proposal, please raise before Thursday, May 7th. |
created #65222 to discuss the so result provider implem |
There was a problem hiding this comment.
LGTM, the only part I have doubts about it the result limiting and pagination.
How many search results should each provider return? Let's say each provider returns 5 results and we have 10 providers, there will be 50 results in total. If each provider sends over 10 results and there are 25 providers, there will be 250 results in total. I guess it is fine for typeahead, maybe there should be some note in docs about that, or some hard limit enforced by implementation.
But going forward, as I understand from the design mock linked in this RFC, the idea in the future is to also use this service for search results:
In that case I think we do need limits and pagination, as imagine a customer has 1,000 visualizations and each visualization has "staging" or "production" text in its title, then when user searches for, say "production", about 500 visualizations will match that phrase.
| * When a `find` request is aborted, the service will stop emitting any new result to the consumer anyway, but | ||
| * this can (and should) be used to cancel any pending asynchronous task and complete the result observable. | ||
| */ | ||
| aborted$: Observable<void>; |
There was a problem hiding this comment.
In data and expressions plugins we use AbortController API for handling abortion signalling. It stores the state boolean whether request was aborted and one can attach event listeners to listen for change in that state.
There was a problem hiding this comment.
If you stick with observable, maybe it is worth making it BehaviorSubject or observable that emits value immediately on subscription if request was aborted, as user might immediately abort a request they started.
There was a problem hiding this comment.
Observables already have the concept of being "aborted" baked in. When a consumer unsubscribes it signals to the producer that it no longer cares about the result. This doesn't mean the producer has to abort any async work, but this is the default for many API's e.g. ajax.getJSON
If a consumer wants to make cancellation more explicit they can implement their own aborted$ subject and use it to unsubscribe / cancel async requests:
import { ajax } from 'rxjs/ajax';
const aborted$ = new Subject();
ajax.getJSON(`/api/users/${action.payload}`).pipe(takeUntil(aborted$)).subscribe(/* do something */)
// Will cancel the XHR request if it's still pending
aborted$.next();There was a problem hiding this comment.
So, does this mean there will be two ways to abort the search: (1) through unsubscription; (2) through aborted$ observable?
There was a problem hiding this comment.
We are quite inconsistent regarding cancelation in core.
on the client-side http.fetch uses the signal parameter which is an AbortSignal
on the server-side, KibanaRequest has the events.aborted$ property which is an Observable with a replay effect.
We can leverage observable unsubscription, but that would be yet another way to perform cancelation. Also it would imho be less 'explicit' for result providers that they should perform their cleanup at cancelation (aka This doesn't mean the producer has to abort any async work, but this is the default for many API')
I still think i'd prefer using an explicit cancelation mechanism. Now, I don't have a very strong opinion between a signal or an observable.
If you stick with observable, maybe it is worth making it BehaviorSubject or observable that emits value immediately on subscription if request was aborted
consumer should provide a cold observable and/or with a replay effect if they think there is a risk for the abort signal to be already emitted. This is afaik an implementation detail from the consumer's side. Using a Replay/BehaviorSubject is indeed an option for them, and the Observable type we are expecting would allow them to do so.
| very easily be forgotten, making a given app non searchable | ||
| - client-side only plugins would need to add a server-side part to their plugin just to register their application on | ||
| the server side | ||
|
|
There was a problem hiding this comment.
Another hack-ish solution: we can send back to the server all registered on the client-side apps. 🤔 However, it requires that a user lands on a page at least once before calling GS API.
There was a problem hiding this comment.
Maybe we could hack the chromium instance in Reporting to do this for us? Sounds kinda gross though 😬
There was a problem hiding this comment.
Main limitation (even with the reporting plugin hack) is that we would have a static list of apps fetched once for all users, which mean that we wouldn't handle apps only registered for specific users/permissions, or things like that.
TBH, I would stick to the client-side providers until we find a clean solution to access apps from the server-side
|
I included the changes for all the remaining discussions and resolved them. This RFC is now ready for the final review stage. |
|
It seems like everything has been resolved or addressed. Unless objections are raised, this will be merged on Monday, 18th. |
* master: (24 commits) [APM] agent config 'profiling_inferred_spans_min_duration' default value is '0ms' but the min value in the field is '1ms' (elastic#66886) [Canvas] Fix flaky custom element functional tests (elastic#65908) Fix IE specific flexbox min-height issue (elastic#66555) [Discover] Unskip doc link functional test (elastic#66884) Index pattern management to Kibana platform (elastic#65026) Warning and link to support matrix for IE11 (elastic#66512) [Reporting] Consolidate Server Type Defs, move some out of Legacy (elastic#66144) [SIEM] [Maps] Fixes Network Map empty tooltip (elastic#66828) [Endpoint] Encode the index of the alert in the id response (elastic#66919) [services/testSubjects] reduce retry usage, add waitForEnabled (elastic#66538) [DOCS] Identifies cloud settings for APM (elastic#66935) [SIEM][CASE] Fix configuration's page user experience (elastic#66029) Resolver: Display node 75% view submenus (elastic#64121) [SIEM] Cases] Capture timeline click and open timeline in case view (elastic#66327) [APM] Lowercase agent names so icons work (elastic#66824) [dev/cli] add support for --no-cache (elastic#66837) [Ingest Manager] Better handling of package installation problems (elastic#66541) [ML] Enhances api docs for modules endpoints (elastic#66738) dont hide errors (elastic#66764) [RFC] Global search API (elastic#64284) ...
Summary
RFC for #61657
Unresolved discussions:
publicAddresshttp configuration [RFC] Global search API #64284 (comment)GS.findAPI returns aggregated or batched results: [RFC] Global search API #64284 (comment)urlfield returned by result providers [RFC] Global search API #64284 (comment)coreor in a plugin [RFC] Global search API #64284 (comment)