[EBT] Add Elastic V3 shippers

[afharo]

Summary

Implements the Elastic V3 shippers for both, the UI (resolves #125691) and the server (resolves #125693).

I noticed that the server implementation is loaded in the bundle even when it's not used, so I added a line in our webpack.config.ts file to stop parsing the server implementation.

The shippers needed a shutdown mechanism to flush the queues so I had to add the shutdown methods to the shippers, analytics client and stop to core.analytics service. I preferred shutdown over stop in the package because the verb feels more destructive (you could start after stop). Let me know if I should change this to stop instead.

This PR also registers the V3 shippers in the telemetry plugin (both, public and server sides).

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[spalger]

Why is the server-side shipper implementation being loaded into a front-end bundle? I feel like that's the real problem, not that it's being parsed by webpack.

[spalger]

I think we might need to split this package since it seems to have some server-specific code and some ui-specific code. We should probably have @kbn/analytics-client and @kbn/analytics-server or something. Common code could go into @kbn/analytics-common.

[afharo]

FWIW, the client is the same for both, the UI and the Server. The difference is the implementation of the transport layer that ships the events to our Remote Telemetry Service.

There is a piece of logic deciding which implementation to import and use depending on the environment:

if (isBrowser()) {
  import('./ui_shipper');
} else {
  import('./server_shipper');
}

So, essentially, the server-side will never be used on the UI and should not be included in the bundle. Please, bear in mind that other libraries do similar stuff: https://github.com/segmentio/analytics-next/blob/master/src/core/connection/index.ts <- we are considering Segment as a potential candidate for shipper and they offer the same library for both: UI and Server side.

I wish Webpack were able to identify this and only import the parts of the code that are used. Otherwise, we are bundling entire libraries like lodash or rxjs only to use a small subset of methods. That seems a huge cost in terms of bundle size.

We can split the package in 3 to have common, ui and server. And it'll play nice for our package and monorepo approach in Kibana. However, given that this package will eventually make it to its own repo and NPM module, it feels like we'd be setting ourselves, the Platform Analytics team as the future maintainers of this library, and any other Elastic products using this client to a larger dependency tree. I wonder if we can do anything at the Bazel level to not expose everything at the root level so shippers can be imported on demand by the app without requiring the entire library? This could reduce core bundle size as it doesn't care about the shippers, as well as reducing cloud and telemetry plugins as they'd only import the shipper they are configuring (fullstory and elastic_v3 respectively).
Exposing mocks from the same library is another use case for this that we now need to overcome by creating a separate package.

A potential different approach is to create a package for the client and a different one for each shipper. However, these are intended to be Elastic-approved shippers that should be used when using the client. Having to install a separate library for each one of them feels like making them "optional".

I hope anything in my rambling makes sense. Please, let me know what's the best approach here, bearing in mind that it'll be eventually moved to a separate repo.

[spalger]

I understand the idea, and it's something we've considered a lot in the design of the packaging system. The primary issue with supporting something like this is that it makes the build process for packages require a lot more customization which makes it more fragile and harder to reason about.

Right now every package implements it's own build process with a custom BUILD.bazel file and they are primarily written and maintained by Operations. Once we incorporate all code in the repo into package we can't have hundreds of bespoke build processes in use. Instead we've settled on a base set of build procedures which can be reused across packages. Supporting some level of nesting in these build procedures is technically possible, but complicated, and we've decided that it's not worth implementing because the simpler approach has an additional benefit: it helps discourage dependency bottlenecks in the system.

Dependency bottlenecks are like lodash, rxjs, or the data plugin where a single package is created to contain a bunch of separate things and consumers regularly depend on that package for a small fraction of it's API. These bottlenecks lead to circular dependencies, unnecessary rebuilds and test runs, cache thrashing, and many other issues. Their primary benefit is that there are less packages overall, but they have caused so many problems for us over the years and we're trying to get rid of them going forward. In the package system we're building we want to encourage folks to instead break those packages up into pieces which means more packages, enforcing the need to make it trivial to create packages and to standardize on the build procedures.

Maybe once we get past this initial rollout we can look at making more complicated package types, but for now we see a lot of simplicity in this approach and think the trade-offs are worth the added cost of having more packages. Please try the package generator:

node scripts/generate package --help

Creating new packages shouldn't be hard and will get even easier soon.

I hope this helps explain why we think this is the right way forward, if @kbn/analytics moves out of the Kibana repo then an entire build procedure for that new repo will have to be defined and I think it will be a perfect time to revisit the idea of using separate packages. I'd also be happy to help with ESLint rules at that time which automatically rewrites all the @kbn/analytics-client and @kbn/analytics-server imports.

[afharo]

Makes sense! Thanks for explaining!

node scripts/generate package --help

I actually used it to generate the @elastic/analytics package (only that I manually renamed it to @elastic/ instead of `@kbn/' because of the already mentioned intention to eventually move it outside Kibana. 😇 It worked like a charm 😍

I'll see how I can do to minimize the amount of code duplication while keeping a clear split between UI and shippers. I think the best approach can be:

1. Keep @elastic/analytics to hold the main client and types. This package is imported by core only.
2. Create @elastic/analytics-shipper-fullstory that imports the types only from @elastic/analytics and implements the FullStory shipper. This package is used by the cloud plugin.
3. Create @elastic/analytics-shipper-elastic_v3-common to hold the common logic and helpers for the "elastic V3" shipper.
4. Create @elastic/analytics-shipper-elastic_v3-ui that imports the *-common and includes the ui-side implementation of the shipper. Imported by the telemetry plugin in the public directory.
5. Create @elastic/analytics-shipper-elastic_v3-server that imports the *-common and includes the server-side implementation of the shipper. Imported by the telemetry plugin in the server directory.

With this split, we might not need to import add the package to the shared libs: #130027

[spalger]

Nice! Sounds like you've got a good handle on how you want to lay this out and being able to avoid adding things to shared libs sounds like a win!

[Bamieh]

Code looks good so far. Although I'd love to see a plugin functional test (inside test/plugin_functional/plugins) that actually ships data to different shippers and we test that the pipeline is working.

[Bamieh]

the v3 endpoint accepts a ?debug=true parameter which returns useful information such as bucket path. I wonder if we should accommodate adding this from the start?

[afharo]

It's added as a query parameter instead of the URL builder :)

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 06fadc5
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
cloud	32	41	+9
core	352	361	+9
telemetry	31	54	+23
total			+41

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@elastic/analytics	23	37	+14

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
cloud	26.5KB	32.6KB	+6.1KB
core	132.3KB	132.5KB	+179.0B
total			+6.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
kibana	332	331	-1

Page load bundle

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

id	before	after	diff
core	324.9KB	331.0KB	+6.1KB
telemetry	27.7KB	46.5KB	+18.7KB
total			+24.8KB

Unknown metric groups

API count

id	before	after	diff
@elastic/analytics	106	129	+23
core	2511	2521	+10
total			+33

ESLint disabled line counts

id	before	after	diff
@elastic/analytics	16	34	+18

Total ESLint disabled count

id	before	after	diff
@elastic/analytics	17	35	+18

History

• 💚 Build #38101 succeeded c29e524
• 💚 Build #38096 succeeded b8a1c44
• 💚 Build #38089 succeeded 3fcbf42
• 💔 Build #38077 failed 386bd7c
• 💔 Build #37979 failed 7989aad
• 💔 Build #37976 failed 44033c9

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

[afharo]

I'll move this back to draft until I resolve the split into packages.

[afharo]

PR to split the package: #130574 I'll rebase this one once it's merged.

[afharo]

Closing this PR in favour of #130696