[Profiling] Profiling data access plugin

[cauemarcondes]

This is part 1 of a series of PRs to expose the flamegraph to be used by other plugins.

The problem
Currently for plugin-A to show data from plugin-B, it needs to add dependency on plugin-B. If plugin-B wants to show data from plugin-A, it also needs to add plugin-A as a dependency, and here is where the problem happens. In such scenario, we have a cyclic dependency problem.

The solution
To solve this problem a new plugin is created, profiling-data-access. This plugin exposes services that consumer plugins can call in other to have the data they need to show on their end. The profiling plugin is also using this new plugin. For now, only the flamegraph service is available, The idea is to slowly migrate the data fetching from profiling to this new plugin in other to facilitate the integration across plugins.

Why some many files?
As I said, only the flamegraph logic was moved to the new plugin, but it has many files that it needs to properly build the response of the service call. I moved all these files to the common folder inside the new plugin and adjusted the imports in the profiling plugin.

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[thomasdullien]

One (rather general) question: @danielmitterdorfer and me had discussed whether the Flamegraph generation doesn't need to migrate entirely into the ES plugin -- that shouldn't stop us from this PR, but it may be something to keep in mind, design-wise.

[cauemarcondes]

@thomasdullien I talked about that with @danielmitterdorfer last week. I'll check what kind of data processing we're still doing on kibana server side. Then we can check if that makes sense or not to move it to the ES plugin.

[jbcrail]

A few questions for clarification:

• who do we expect to use the new profiling-data-access plugin?
• do we expect the new plugin to be used in Kibana server, Kibana UI, or both? (relevant since there may be NodeJS-specific APIs used in the migrated code)

When reviewing, I realized that there were several deprecated methods still in use so I created #165282 to remove them. This could help reduce the size of the PR.

I also should note that flamegraph creation is spread out across Kibana server and UI for performance reasons, so the new plugin will not yet present a clean abstraction for other users besides profiling. Subsequent work will likely move the entire flamegraph creation pipeline into Elasticsearch so this is probably only a short-term concern.

[cauemarcondes]

who do we expect to use the new profiling-data-access plugin?

Every plugin that wants to have profiling data including the Profiling plugin itself.

do we expect the new plugin to be used in Kibana server, Kibana UI, or both? (relevant since there may be NodeJS-specific APIs used in the migrated code)

Only Kibana server. This plugin exists exclusively to externalize Profiling data.

I also should note that flamegraph creation is spread out across Kibana server and UI for performance reasons, so the new plugin will not yet present a clean abstraction for other users besides profiling. Subsequent work will likely move the entire flamegraph creation pipeline into Elasticsearch so this is probably only a short-term concern.

There are two points here:

1. We don't want any other plugin to have its own version of the flamegraph for instance. So for that, I'm doing a PoC to see if it's worth moving our flamegraph UI to a Kibana package. This package will be stateless, which means it won't know how to fetch any data rather it will receive the necessary data to render the component. Or will create embeddables of our components that can be reused by other plugins.
2. Like @thomasdullien mentioned in a few comments above, we do indeed need to see what kind of data manipulation we are still doing on kibana server and kibana UI, and see if that makes sense to be moved to ES plugin, but that is another issue not related to this one.

[danielmitterdorfer]

I also had a look at the code and I'm under the impression that the majority of the changes here makes sense regardless whether we move flamegraph processing into ES or not. Even then it's useful to keep both options (processing in Kibana and ES) for a short period of time both as risk mitigation as well as for easier comparison of how the move to Elasticsearch affects performance.

[cauemarcondes]

This is the new flow from now on:

• There'll be no direct dependency from Profiling-plugin to APM-plugin or any other plugins. Solve the circular dependency problem.
• Profiling-plugin will have a new dependency on the Profiling-data-access-plugin
• Apm-plugin will have a new dependency on the Profiling-data-access-plugin
• Profiling-data-access-plugin will NEVER depend on another plugin, such as Profiling-plugin or APM-plugin.
• What is still open? The UI part. I'm not sure if I'm going to create a new kibana package and move the UI components there, or if I'll create embeddables in the Profiling-plugin UI to be used by others.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 479b839
• Storybooks Preview

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
profiling	200	199	-1

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
profiling	19	17	-2
profilingDataAccess	-	3	+3
total			+1

Async chunks

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

id	before	after	diff
profiling	335.9KB	335.9KB	+5.0B

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
profilingDataAccess	-	1	+1

Page load bundle

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

id	before	after	diff
profiling	21.9KB	21.3KB	-618.0B

Unknown metric groups

API count

id	before	after	diff
profiling	19	17	-2
profilingDataAccess	-	3	+3
total			+1

History

• 💔 Build #155102 failed 46f7e3d
• 💔 Build #155082 failed 969b0bb
• 💚 Build #154855 succeeded 63d7641
• 💔 Build #154816 failed af1dad2
• 💔 Build #154793 failed 5a614b4
• 💔 Build #154782 failed f973c3f

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

[jbcrail]

Looks good and is working as expected when I tried it out. 👍

I have one architectural thought about clearer boundaries. As we move more things into the access plugin, we should think about the interface we expose via the plugin. At this time a user can directly or indirectly get data from Elasticsearch via ProfilingESClient, registered services, or exported functions. If we can standardize or simplify this, it would give us consistency and a reduced interface footprint (good for letting us change things as needed).

Of course I realize we're still in flux and have yet to work out how non-profiling clients will want to use the new plugin, which is why this is just general feedback and not requirements. :)