[RFC] Bazel #92758

tylersmalley · 2021-02-25T06:24:34Z

Request for comment for migrating Kibana to Bazel. Requests close March 9th, 2021

Rendered: https://github.com/tylersmalley/kibana/blob/rfc-bazel/rfcs/text/0015_bazel.md

#10908

elasticmachine · 2021-02-25T06:42:31Z

Pinging @elastic/kibana-operations (Team:Operations)

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

rfcs/text/0015_bazel.md

Co-authored-by: Jonathan Budzenski <jon@budzenski.me>

rfcs/text/0015_bazel.md

Co-authored-by: Jonathan Budzenski <jon@budzenski.me>

spalger

Overall looks great, just a little note about running bazel and package builds in "parallel" via kbn/pm

rfcs/text/0015_bazel.md

Co-authored-by: Spencer <email@spalger.com>

mistic

Aside from my above comment on a typo I've found, LGTM

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

rfcs/text/0015_bazel.md

mshustov · 2021-03-04T17:14:38Z

rfcs/text/0015_bazel.md

+
+In this phase, we will be migrating each of the 135 plugins over to being built and unit tested using Bazel. During this time, the legacy systems will stay in place and run in parallel with Bazel. Once all plugins have been migrated, we can decommission the legacy systems.
+
+The `BUILD.bazel` files will look similar to that of packages, there will be a target for `web`, `server`, and `jest`. Just like packages, as the Jest unit tests are migrated, they will need to be removed from the root `jest.config.js` file as described in the Unit Testing section.


Does it mean we are going to have different configuration files for every Environnement? tsconfig.server.js, tsconfig.web.js, tsconfig.jest.js, etc.

It only means we will have rules to build web and server separately as we currently have plugins who use different compilers for each target. The configuration part is up to us. Regarding jest target it will rely on the previous builds for web and server but it just means we will have a target to run tests all at once for a given package \cc @tylersmalley

Ideally, we only have a single tsconfig.js file so we can use its output as the source of multiple targets. However, if we find we need or want separate configurations for different actions we do have that flexibility.

rfcs/text/0015_bazel.md

joshdover

Questions:

Will we integrate FTR with this? If so, what new things will we be able to do with e2e tests?
Can we have nested Bazel packages? If so, are there any caveats to be aware of with this and JavaScript?
How will we integrate the output of Bazel's webpack build with the Kibana server? For instance, right now we have the optimizer_mixin which exposes the routes for webpack outputs and will wait for this process to complete before attempting to serve these files. How will we do this when Bazel is building these webpack outputs?

rfcs/text/0015_bazel.md

joshdover · 2021-03-08T16:22:33Z

rfcs/text/0015_bazel.md

+
+The `BUILD.bazel` files will look similar to that of packages, there will be a target for `web`, `server`, and `jest`. Just like packages, as the Jest unit tests are migrated, they will need to be removed from the root `jest.config.js` file as described in the Unit Testing section.
+
+Plugins are built in a sandbox, so they will no longer be able to use relative imports from one another. For Typescript, relative imports will be replaced with a path reference to the `bazel/bin`.


Can we add an example of what this would look like in code? I think how these imports work is an important aspect of how well this tool will fit into our repo. It may also impact how we consider implementing #71566 and enforce encapsulation between plugins & domains.

I agree that we should elaborate on this change and discuss it with a wider audience. A dedicated e-mail to kibana-contributors@ would be great.

Started #94034 to attempt to get a consensus on this. Will pass it around to a few folks before blasting kibana-contributors.

Thanks, @tylersmalley.

rfcs/text/0015_bazel.md

Co-authored-by: Josh Dover <1813008+joshdover@users.noreply.github.com>

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2021-03-09T15:52:04Z

@joshdover

Will we integrate FTR with this? If so, what new things will we be able to do with e2e tests?

This is something I have been giving thought to, but there are no concrete plans yet. Conceptually, we could entirely replace the FTR runner with Bazel to run Elasticsearch, Kibana, and the tests. There is an old video of Dropbox discussing this https://www.youtube.com/watch?v=muvU1DYrY0w

Can we have nested Bazel packages? If so, are there any caveats to be aware of with this and JavaScript?

None that I am aware of. - @mistic?

How will we integrate the output of Bazel's webpack build with the Kibana server? For instance, right now we have the optimizer_mixin which exposes the routes for webpack outputs and will wait for this process to complete before attempting to serve these files. How will we do this when Bazel is building these webpack outputs?

I think this is an important part that should be included in the RFC, will work on adding it now and will ping you when it's complete.

mistic · 2021-03-09T15:57:57Z

Can we have nested Bazel packages? If so, are there any caveats to be aware of with this and JavaScript?

On Bazel language there is nested packages all over the place. But probably you are thinking about other cases. Can you give a concrete example to see if we can spot any caveats? At my first thoughts I think there won't be any special ones. \cc @joshdover

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

rfcs/text/0015_bazel.md

Co-authored-by: Spencer <email@spalger.com>

tylersmalley · 2021-03-09T22:10:49Z

@joshdover, added a note regarding the handling updates in development: https://github.com/elastic/kibana/pull/92758/files#diff-d3251298ab06494dfe78a21d16139be95192295555d9a3115799a81a30415478R79

mshustov · 2021-03-10T08:03:05Z

rfcs/text/0015_bazel.md

+Building TypeScript reference definitions and using `@babel/register` will be negated by using the TypeScript compiler directly instead of using Babel. Currently, we use Babel for code generation and `tsc` for type check and type declaration output. Additionally, the TypeScript implementation in [rules_nodejs](https://bazelbuild.github.io/rules_nodejs/TypeScript.html) for Bazel handles incremental builds, resulting in faster re-builds.
+
+In addition to the benefits of building code, there are also benefits regarding running unit tests. Developers currently need to understand what unit tests to run to validate changes or rely on waiting for CI, which has a long feedback loop. Since Bazel knows the dependency tree, it will only run unit tests for a package or plugin modified or dependent on those modifications. This optimization helps developers and will significantly reduce the amount of work CI needs to do. On CI, unit tests take 35 minutes to complete, where the average single Jest project takes just twenty seconds.
+


Unit tests are our least problem. We can use the migration to bazel as an opportunity to finally start functional tests separation by domain structure. @elastic/kibana-operations Do you anticipate any problems with this? We can use Core as a guinea pig.

I see no problem with this, and I plan to work towards moving functional tests to have a plugin owner where they will reside. There are a lot of benefits to this, including clear ownership for functional tests to assist with tooling and how we report failures. In addition, it would help get us on a path to reducing the number of functional tests we run by default on a pull request. Bazel provides the ability to query the action graph to understand what packages are affected by a change. We can use this, and additional configuration on the plugins to determine which functional tests should be run by default while allowing the PR to request additional tests to be run.

rudolf · 2021-03-10T10:24:49Z

rfcs/text/0015_bazel.md

+
+Developers currently use `yarn test:jest` to efficiently run tests in a given directory without remembering the command or path. This command will continue to work as it does today, but will begin running tests through Bazel for packages or plugins which have been migrated.
+
+CI will have an additional job to run `bazel test //…:jest`. This will run unit tests for any package or plugin modified or dependent on modifications since the last successful CI run on that branch.


Can you explain "dependent on modifications"

Core (as well as other plugins) maintains many mocks that are used by other plugin's unit tests. Would a change in a mock result in unit tests being run for all the tests that import that mock?

As it currently stands, yes. The mocks are inputs into the test actions, and if those change the downstream tests will be re-run.

There are ways to combat this by isolating the mocks with a separate filegroup which would be consumed by the downstream projects.

joshdover · 2021-03-10T10:37:34Z

Can we have nested Bazel packages? If so, are there any caveats to be aware of with this and JavaScript?

On Bazel language there is nested packages all over the place. But probably you are thinking about other cases. Can you give a concrete example to see if we can spot any caveats? At my first thoughts I think there won't be any special ones. \cc @joshdover

I'm curious if we can support a nested structure like this:

x-pack/
  observability/
    BUILD <- where this package exposes a subset of APIs from `o11y_lib` and `apm` to other packages
    apm/
      BUILD
    logs/
      BUILD
    o11y_lib/
      BUILD

tylersmalley · 2021-03-10T16:57:46Z

I'm curious if we can support a nested structure like this...

@joshdover, absolutely. We have discussed breaking out portions of plugins that have very little churn. If there is a large portion of code within a plugin that doesn't frequently change, it might make sense to add a BUILD.bazel file within that portion to create those assets separately. This would decrease the scope and in-turn the time it would take to build the plugin. Lots of options.

joshdover

Super excited about this and looking forward to all the improvements. Thanks for taking the time to write all this out so we can clearly document our decisions!

tylersmalley · 2021-03-16T03:49:29Z

Seeing no pending feedback, I am going to merge the RFC. If any questions come up, don't hesitate to reach out.

tylersmalley added RFC Team:Operations Team label for Operations Team labels Feb 25, 2021

tylersmalley marked this pull request as ready for review February 25, 2021 06:42

tylersmalley added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Feb 25, 2021

[RFC] Bazel

c0fc1d5

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the rfc-bazel branch from 88cf085 to c0fc1d5 Compare February 25, 2021 15:32

mistic mentioned this pull request Feb 25, 2021

chore(NA): manage npm dependencies within bazel #92864

Merged

Updates

dbe1cfa

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

mistic reviewed Feb 26, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

mistic mentioned this pull request Feb 26, 2021

Use Bazel for Kibana builds #69706

Closed

23 tasks

jbudz reviewed Mar 1, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

Update rfcs/text/0015_bazel.md

2dc8401

Co-authored-by: Jonathan Budzenski <jon@budzenski.me>

jbudz reviewed Mar 1, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

Update rfcs/text/0015_bazel.md

a32e978

Co-authored-by: Jonathan Budzenski <jon@budzenski.me>

spalger approved these changes Mar 1, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

Update rfcs/text/0015_bazel.md

29f1c78

Co-authored-by: Spencer <email@spalger.com>

mistic approved these changes Mar 2, 2021

View reviewed changes

Fix typo

1c9ebe6

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

jbudz approved these changes Mar 2, 2021

View reviewed changes

tylersmalley mentioned this pull request Mar 2, 2021

[SECURITY SOLUTIONS] Meta to move cases everywhere #90905

Closed

15 tasks

tylersmalley requested review from stacey-gammon and kobelb March 2, 2021 18:32

legrego reviewed Mar 3, 2021

View reviewed changes

rfcs/text/0015_bazel.md Show resolved Hide resolved

mshustov reviewed Mar 4, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

joshdover reviewed Mar 8, 2021

View reviewed changes

tylersmalley and others added 2 commits March 8, 2021 15:18

Update rfcs/text/0015_bazel.md

ca01faa

Co-authored-by: Josh Dover <1813008+joshdover@users.noreply.github.com>

Update rfcs/text/0015_bazel.md

9745e8c

Co-authored-by: Josh Dover <1813008+joshdover@users.noreply.github.com>

tylersmalley mentioned this pull request Mar 9, 2021

[discuss] Use mapping aliases instead of relative imports between plugins #94034

Closed

tylersmalley force-pushed the rfc-bazel branch from 423b191 to c21f8c2 Compare March 9, 2021 15:35

Expand on CI unit test savings

3878082

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the rfc-bazel branch from c21f8c2 to 3878082 Compare March 9, 2021 15:37

joshdover mentioned this pull request Mar 9, 2021

[meta] Unblock migration of src/core to Bazel #94141

Closed

4 tasks

Add note about optimzeMixin

ccf254e

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

spalger reviewed Mar 9, 2021

View reviewed changes

rfcs/text/0015_bazel.md Outdated Show resolved Hide resolved

Update rfcs/text/0015_bazel.md

85d5013

Co-authored-by: Spencer <email@spalger.com>

kobelb approved these changes Mar 9, 2021

View reviewed changes

mshustov reviewed Mar 10, 2021

View reviewed changes

rudolf reviewed Mar 10, 2021

View reviewed changes

joshdover approved these changes Mar 11, 2021

View reviewed changes

tylersmalley removed the request for review from stacey-gammon March 11, 2021 16:04

pgayvallet approved these changes Mar 11, 2021

View reviewed changes

tylersmalley added the backport:skip This commit does not require backporting label Mar 16, 2021

tylersmalley merged commit d02169e into elastic:master Mar 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Bazel #92758

[RFC] Bazel #92758

tylersmalley commented Feb 25, 2021 •

edited

Loading

elasticmachine commented Feb 25, 2021

spalger left a comment

mistic left a comment

mshustov Mar 4, 2021

mistic Mar 4, 2021

tylersmalley Mar 4, 2021 •

edited

Loading

joshdover left a comment

joshdover Mar 8, 2021

kobelb Mar 8, 2021

tylersmalley Mar 9, 2021 •

edited

Loading

kobelb Mar 9, 2021

tylersmalley commented Mar 9, 2021

mistic commented Mar 9, 2021

tylersmalley commented Mar 9, 2021

mshustov Mar 10, 2021

tylersmalley Mar 10, 2021

rudolf Mar 10, 2021

tylersmalley Mar 10, 2021

tylersmalley Mar 10, 2021

joshdover commented Mar 10, 2021

tylersmalley commented Mar 10, 2021 •

edited

Loading

joshdover left a comment

tylersmalley commented Mar 16, 2021


		In this phase, we will be migrating each of the 135 plugins over to being built and unit tested using Bazel. During this time, the legacy systems will stay in place and run in parallel with Bazel. Once all plugins have been migrated, we can decommission the legacy systems.

		The `BUILD.bazel` files will look similar to that of packages, there will be a target for `web`, `server`, and `jest`. Just like packages, as the Jest unit tests are migrated, they will need to be removed from the root `jest.config.js` file as described in the Unit Testing section.


		The `BUILD.bazel` files will look similar to that of packages, there will be a target for `web`, `server`, and `jest`. Just like packages, as the Jest unit tests are migrated, they will need to be removed from the root `jest.config.js` file as described in the Unit Testing section.

		Plugins are built in a sandbox, so they will no longer be able to use relative imports from one another. For Typescript, relative imports will be replaced with a path reference to the `bazel/bin`.

		Building TypeScript reference definitions and using `@babel/register` will be negated by using the TypeScript compiler directly instead of using Babel. Currently, we use Babel for code generation and `tsc` for type check and type declaration output. Additionally, the TypeScript implementation in [rules_nodejs](https://bazelbuild.github.io/rules_nodejs/TypeScript.html) for Bazel handles incremental builds, resulting in faster re-builds.

		In addition to the benefits of building code, there are also benefits regarding running unit tests. Developers currently need to understand what unit tests to run to validate changes or rely on waiting for CI, which has a long feedback loop. Since Bazel knows the dependency tree, it will only run unit tests for a package or plugin modified or dependent on those modifications. This optimization helps developers and will significantly reduce the amount of work CI needs to do. On CI, unit tests take 35 minutes to complete, where the average single Jest project takes just twenty seconds.


		Developers currently use `yarn test:jest` to efficiently run tests in a given directory without remembering the command or path. This command will continue to work as it does today, but will begin running tests through Bazel for packages or plugins which have been migrated.

		CI will have an additional job to run `bazel test //…:jest`. This will run unit tests for any package or plugin modified or dependent on modifications since the last successful CI run on that branch.

[RFC] Bazel #92758

[RFC] Bazel #92758

Conversation

tylersmalley commented Feb 25, 2021 • edited Loading

elasticmachine commented Feb 25, 2021

spalger left a comment

Choose a reason for hiding this comment

mistic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylersmalley Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

joshdover left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylersmalley Mar 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylersmalley commented Mar 9, 2021

mistic commented Mar 9, 2021

tylersmalley commented Mar 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshdover commented Mar 10, 2021

tylersmalley commented Mar 10, 2021 • edited Loading

joshdover left a comment

Choose a reason for hiding this comment

tylersmalley commented Mar 16, 2021

tylersmalley commented Feb 25, 2021 •

edited

Loading

tylersmalley Mar 4, 2021 •

edited

Loading

tylersmalley Mar 9, 2021 •

edited

Loading

tylersmalley commented Mar 10, 2021 •

edited

Loading