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

[Spacetime] tRPC #138529

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

[Spacetime] tRPC #138529

wants to merge 11 commits into from

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Aug 10, 2022

Summary

This is a minimal example for how (t)RPC can be included in our stack. The following areas are tested:

  • How simple is it to setup new endpoints for plugin developers? (also, how much time does it save)
  • How complicated is it to use with current HTTP functionality?
  • Can it be used to get around certain scenarios where we have circular dependencies between plugins?

Assumptions

  1. Not all HTTP endpoints need to be versioned, public and RESTful - it's simply too much overhead
  2. Writing, maintaining and running tests is something we should try to reduce where possible
  3. We want to have a distinction between internal and external endpoints, RPC could be only internal
  4. We need a way to get around circular dependencies between plugins (this should largely be addressable by making everything a package, but this RPC-style solution could be a more immediate solution)

Notes and observations

  • The biggest benefit as far as I can tell is being able to quickly get data from server-side to front-end while not needing any end-to-end tests 🚀 (this checking is done by the type system)
    • The net effect here is being able to move much quicker and removing test burden from developers and CI
    • Error cases are handled and mapped to known shapes by useQuery
    • useQuery is very well integrated with types (endpoints and args are fully type checked)
  • Use of the declarative API for creating RPC routes at plugin-level provides a set of types that can be shared between a plugins public and server side code. This interface can also be shared between plugins which means client frontends can call server-side procs of other plugins. The dependency is "documented" in the code.
  • Did not finish a complete core-exposed version of the trpc client, this can be done based on the examples in examples/plugin_a and examples/plugin_b.
  • This should have no affect on backwards-compatible architecture (BWA) since everything exposed via the plugin contracts over RPC
  • The RPC server is a single endpoint that will accept procedure names, these need to be properly namespaced. For example: pluginA.myProc. This could be enforced at the core service level
  • It is possible to generate swagger doc for the RPC endpoints: https://github.com/jlalmes/trpc-openapi

Propsed work

  • Based on these findings, I believe there is a strong value proposition for improving DX and speed of developing new features involving server-to-client communication
  • ...

Bonus: batching with useQuery included

Screenshot 2022-08-11 at 17 31 57

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens jloleysens force-pushed the jl/space-time-trpc branch 2 times, most recently from d10f29b to e73de4c Compare August 22, 2022 11:11
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 23, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #5 / does not fail on "setup" if there are unused paths detected
  • [job] [logs] Jest Tests #5 / injects legacy dependency to context#setup()
  • [job] [logs] Jest Tests #5 / runs services on "start"
  • [job] [logs] Jest Tests #5 / sets up services on "setup"
  • [job] [logs] Jest Tests #5 / stops services on "stop"

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
apm 14 13 -1
observability 6 5 -1
total -2

ESLint disabled line counts

id before after diff
apm 81 78 -3
enterpriseSearch 13 11 -2
observability 44 43 -1
synthetics 57 51 -6
ux 10 9 -1
total -13

Total ESLint disabled count

id before after diff
apm 95 91 -4
enterpriseSearch 13 11 -2
observability 50 48 -2
synthetics 63 57 -6
ux 13 12 -1
total -15

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants