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

[data views] moving more functionality server side #113440

Closed
Tracked by #166175
mattkime opened this issue Sep 29, 2021 · 6 comments
Closed
Tracked by #166175

[data views] moving more functionality server side #113440

mattkime opened this issue Sep 29, 2021 · 6 comments
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Icebox impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews)

Comments

@mattkime
Copy link
Contributor

Original issue - #90076

The IndexPatternsService is currently being called both client side and server side.

  • The IndexPatternsService should not be called by the index pattern server routes
  • The IndexPatternsService methods should be updated to be like the getFieldsForWildcard, which makes a server-side call to do the work. All the logic from IndexPatternsService should be moved to server-side file IndexPatternsFetcher.
  • The routes that reference client-side IndexPatternsService should be changed to reference the new methods in server-side IndexPatternsFetcher

our plan regarding server/client side index patterns goes mostly like this:

  • all calls to elasticsearch should be made from server side
  • as much as possible of actual code/logic should exist only on the server
  • server should expose public API (to be consumed by other plugins) as well as REST API (to be consumed by client side and other software outside of kibana when necesarry)
  • client side API should be just a wrapper around REST API mimicking the same interface we have on public interface on the server
@mattkime mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 Team:AppServicesSv labels Sep 29, 2021
@mattkime mattkime self-assigned this Sep 29, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Oct 4, 2021
@mattkime
Copy link
Contributor Author

mattkime commented Oct 5, 2021

@ppisljar I'm curious about your thoughts about this issue.

Roughly speaking, I think there’s two ways to create APIs. There’s the current way data views is implemented - a thin layer abstracts the saved object layer - vs creating a RESTful api thats consumed by the client. This is probably a topic that can be worked through separate from any thing data view specific.

Saved object layer wrapper

  • More code gets pushed to client (anything above the saved object wrapper)
  • Potentially more calls made from the client - minor impact
  • Full typescript safety
  • Makes use of _bulk_get REST api

RESTful

  • Using our own restfulAPI
  • Would probably need more private APIs
  • Might be a bit more work
  • less code in browser
  • client needs to provide type safety

  • Is there a way we can transition without full commitment one way or another?
  • How would this apply to the various data_view apis?

@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Oct 5, 2021
@ppisljar
Copy link
Member

ppisljar commented Oct 5, 2021

I don't understand really how this is blocking the index patterns everywhere project.

From my perspective:

  • we only expose pubic REST api's when its really needed as we end up maintaining those for a long time (actual user need)
  • if REST api's are made internal, they are just implementation detail
  • pushing more code to the server is not always good, we can quickly run into scaling issues.
  • we want our code to be easy to work with and we want to reduce code duplication to near 0. that can be done either way imo. Having /common code should work just as the RESTful approach should.

I would like to better understand how needed this is ?
I think there should be a way to transition gradually. Most of the data view APIs are already the same on client and on server. For some of the functionallity REST apis already exist. We could start rewriting the client to not use common code but use REST endpoints. But i would need to understand why we are doing this in the first place.

@vadimkibana
Copy link
Contributor

vadimkibana commented Oct 9, 2021

our plan regarding server/client side index patterns goes mostly like this:

  • all calls to elasticsearch should be made from server side
  • as much as possible of actual code/logic should exist only on the server
  • server should expose public API (to be consumed by other plugins) as well as REST API (to be consumed by client side and other software outside of kibana when necesarry)
  • client side API should be just a wrapper around REST API mimicking the same interface we have on public interface on the server

+1, I like the plan.


I cannot answer the "need" questions, but I could try explaining why it is a bad idea to use Saved Objects Service on the browser.

But i would need to understand why we are doing this in the first place.

  • Using Saved Objects Service from browser is like connecting to MySQL from browser. In my opinion it is a good practice in Kibana to stop using Saved Objects Service on the client side. It might have been a nice way to manage documents when Kibana just started and did not have a full fledged sever. But now, when saved objects are essentially the database of Kibana—the lowest layer of storage, where we do migrations and references—using Saved Objects Service on the client only creates unnecessary problems.
  • In my experience, code looks cleaner and easier to understand when the database layer is kept on the server. It becomes a classic pattern, where all database access is done on the server, and everyone else interacts through public HTTP API.
    • This leaves the room for the server-side implementation to implement the actual storage in whatever way it wants. (It can change the saved object type. It may store something as multiple saved objects, it can even store something not in a saved object, but somewhere else.)
  • Many things become impossible to do, when client code has direct access to the database. The fact that something is stored as a Saved Object should be an implementation detail. Instead, when using Saved Objects Service on the client it is not an implementation detail anymore, now you cannot easily use some other way to store data, or even simply change the type of a saved object. And this pattern only works with saved objects, because we have Saved Object Service implemented in browser, but other things don't have such a client in browser, say file storage.
  • Using Saved Objects Service only on the server encourages developers to create HTTP endpoints. (If some of the endpoints we don't want to make public, we have a mechanism to mark them as /internal.)
  • Programmable Kibana — Kibana would have been so much better if we had well defined public HTTP APIs instead of our customers and support using Saved Objects HTTP API to manipulate internal data.
  • The only thing that Saved Object Services on the server and browser have in common is their name. To write universal code, one needs to abstract away the Saved Object Service.
    • Core does not have /common folder where some common types for Saved Object Service would be defined. In fact, the server and browser Saved Object Services are quite different.
    • TypeScript types of saved objects are different, interfaces for the services themselves are different, too.
    • The server Saved Object Service is usually scoped to a request, whereas the browser one is always available—this alone creates substantial work to code around when you want to inject Saved Objects Service somewhere as a dependency.
  • Because anyone at any time can manipulate data in saved objects (which they don't own), we cannot even trust our own data when we read it from the database.
  • I believe, when we use Saved Object Service on the browser, we duplicate the business logic that works with those saved objects, which increases our frontend bundle size. That business logic could live just on the server in many cases.

P.S. I understand that connecting directly to database could be a nice modern solution when done well, think Firebase. But in Kibana I think it would be better to keep Saved Objects Service only on the server.

@mattkime
Copy link
Contributor Author

@vadimkibana At this point I put most of those benefits under 'marginal improvements' - they would be good to have but the current situation isn't blocking anything from happening.

@exalate-issue-sync exalate-issue-sync bot added loe:small Small Level of Effort and removed loe:medium Medium Level of Effort labels Dec 21, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Apr 7, 2022
@kertal kertal added bug Fixes for quality problems that affect the customer experience and removed bug Fixes for quality problems that affect the customer experience labels Nov 18, 2022
@petrklapka petrklapka added Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews) and removed Team:AppServicesSv labels Nov 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@kertal
Copy link
Member

kertal commented Sep 24, 2024

Closing this because it's not planned to be resolved in the foreseeable future. It will be tracked in our Icebox and will be re-opened if our priorities change. Feel free to re-open if you think it should be melted sooner.

@kertal kertal closed this as not planned Won't fix, can't repro, duplicate, stale Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Data Views Data Views code and UI - index patterns before 8.0 Icebox impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews)
Projects
None yet
Development

No branches or pull requests

6 participants