[Infra] Create service in `apm_data_access` to fetch APM colleced hosts #189046

crespocarlos · 2024-07-24T10:03:49Z

Summary

Create a service in APM data access that will return host names collected by APM agents. This change is key to allow the hosts view to list only hosts that are monitored with the system module #188756.

I've made some changes to the Hosts API, simplifying the code. The payload has been slightly changed.

How to test

Start a local kibana and es instances
Run node scripts/synthtrace infra_hosts_with_apm_hosts --live
Navigate to Infrastructure > hosts

obltmachine · 2024-07-24T10:04:02Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

…fix'

crespocarlos · 2024-07-24T16:22:16Z

/ci

crespocarlos · 2024-07-24T19:09:40Z

/ci

crespocarlos · 2024-07-24T20:42:27Z

@elasticmachine merge upstream

crespocarlos · 2024-07-24T20:43:04Z

/ci

elasticmachine · 2024-07-25T09:14:20Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

crespocarlos · 2024-07-25T09:20:30Z

@elasticmachine merge upstream

cauemarcondes · 2024-07-26T14:12:47Z

x-pack/plugins/observability_solution/apm_data_access/server/plugin.ts

+    const getCoreStart = () => core.getStartServices().then(([coreStart]) => coreStart);
+    const getResourcesForServices = async () => {
+      const coreStart = await getCoreStart();
+      const soClient = coreStart.savedObjects.createInternalRepository();


Have you tried changing the APM index patterns on the APM settings page and see if the correct indices are returned with this createInternalRepository?

Just tested it and it worked. APM also uses it here here. It could be due to the fact that when APM indices are changed on the Settings page, Kibana does a full page refresh.

crespocarlos · 2024-07-29T07:03:57Z

@elasticmachine merge upstream

dgieselaar · 2024-07-29T07:33:44Z

x-pack/plugins/observability_solution/apm_data_access/server/services/get_host_names/index.ts

+    const { apmIndices, esClient } = await params.getResourcesForServices();
+
+    const esResponse = await esClient.search({
+      index: [apmIndices.metric, apmIndices.transaction, apmIndices.span, apmIndices.error],


This will be slow, because you're aggregating over all data. Even if it's just a single service, you'll run into performance issues. What would be more efficient here is if you figure out what data source to query first (tx metrics or raw events, 1m vs 10m and 60m), and then execute it.

that would be the same thing that the get_document_sources does. We'd have to move that to apm_data_access. Is that your idea?

yeah, how involved would that be?

it's a relatively big change. A few things would have to be moved (could be more):

document_type, get_document_sources and the types they depend on.

ok, sounds fine to do it in a follow-up but I'd like to make it happen, to set a good first example

yeah, makes sense. I'm going to get a sense of how much effort it would be. it might as well be doable in this PR.

dgieselaar · 2024-07-29T07:35:55Z

x-pack/plugins/observability_solution/apm_data_access/server/services/get_host_names/index.ts

+
+export function createGetHostNames(params: RegisterParams) {
+  return async ({ from: timeFrom, to: timeTo, query, limit }: ServicesHostNamesParams) => {
+    const { apmIndices, esClient } = await params.getResourcesForServices();


I think we should stay away from patterns like this, and expect the consumer to pass in asynchronously resolved dependencies, such as getting the APM indices. If we don't do that, we'll end up with a ton of small requests - I see this happen with e.g. the ML plugin that does a ton of small requests because it encapsulates privilege and other checks in each service call. For instance: #161229

Yeah, I was just having second thoughts about this approach.

though it's kind of weird asking the consumer to pass the APM indices to a service in the apm_data_acess plugin.

it is a little weird, and there might be a better way (e.g. to have a per-request service that does per-request caching, but that also might create some risks).

I can't really see the risks of getting the APM indices when calling a service in the apm_data_access, what could happen? if we just get the APM indices there, do you think it would still end up in the same situation from ML plugin?

I'm more concerned about the fact I'm using the internal user to get both apm indices and typed esClient.

One downside is potentially requesting APM indices twice.

One downside is potentially requesting APM indices twice.

This is my (only) concern. It might end up being more than twice however. Especially in the context of e.g. rule executors this becomes more important. My experience is that if we abstract away async operations, people lose awareness of what is happening as part of the service call, and it leads to performance bottlenecks. I'd be less concerned about this if we have really good insight in production bottlenecks, but we don't.

For the lack of better alternatives, I'll leave it up to the consumer the send the APM indices for now. It's bad DX but for the time being it's better than end up in situation where it might lead to performance problems.

dgieselaar · 2024-07-29T07:36:20Z

x-pack/plugins/observability_solution/apm_data_access/server/services/get_host_names/index.ts

+      aggs: {
+        hostNames: {
+          terms: {
+            field: 'host.name',


there is a constant for this somewhere (HOST_NAME).

not in the apm_data_access. we need to either move the constants from apm plugin here or create a new constants file.

yeah, let's move the constants then

I'll do that in a follow up PR, over 250 files will be changed after this

package might be better at that point, WDYT?

yeah. When doing this #189046 (comment), some types could also be moved to this new package.

kibana-ci · 2024-07-29T22:01:00Z

💔 Build Failed

Buildkite Build
Commit: 1e0c949
Interpreting CI Failures
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-189046-1e0c94993edf

Failed CI Steps

Linting

Metrics [docs]

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
`apmDataAccess`	9	10	+1

Async chunks

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

id	before	after	diff
`infra`	1.5MB	1.5MB	-87.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
`apmDataAccess`	0	1	+1

Unknown metric groups

API count

id	before	after	diff
`apmDataAccess`	9	10	+1

History

💔 Build #224277 failed 5134699
💚 Build #223743 succeeded ce95298
💚 Build #223665 succeeded 35af259
💔 Build #223634 failed 18603df

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

crespocarlos · 2024-08-05T08:59:28Z

I'll open a new PR for this.

Create service in apm_data_service to fetch APM colleced hosts

c1590eb

crespocarlos force-pushed the 188752-fetch-apm-only-hosts branch from 55721b3 to e3a418d Compare July 24, 2024 10:29

crespocarlos added release_note:skip Skip the PR/issue when compiling release notes Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.16.0 labels Jul 24, 2024

Clean up

c9ddb1e

crespocarlos force-pushed the 188752-fetch-apm-only-hosts branch from e3a418d to c9ddb1e Compare July 24, 2024 11:08

kibanamachine and others added 2 commits July 24, 2024 11:20

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

4be42af

…fix'

Fix types

3ebaffd

Update tests

18603df

Merge branch 'main' into 188752-fetch-apm-only-hosts

35af259

crespocarlos marked this pull request as ready for review July 25, 2024 09:14

crespocarlos requested review from a team as code owners July 25, 2024 09:14

Update README.md

4d98192

botelastic bot added the ci:project-deploy-observability Create an Observability project label Jul 25, 2024

Merge branch 'main' into 188752-fetch-apm-only-hosts

ce95298

cauemarcondes reviewed Jul 26, 2024

View reviewed changes

Merge branch 'main' into 188752-fetch-apm-only-hosts

5134699

dgieselaar reviewed Jul 29, 2024

View reviewed changes

Merge branch 'main' into 188752-fetch-apm-only-hosts

1e0c949

crespocarlos requested a review from a team as a code owner July 29, 2024 21:07

crespocarlos closed this Aug 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Infra] Create service in `apm_data_access` to fetch APM colleced hosts #189046

[Infra] Create service in `apm_data_access` to fetch APM colleced hosts #189046

crespocarlos commented Jul 24, 2024 •

edited

Loading

obltmachine commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

elasticmachine commented Jul 25, 2024

crespocarlos commented Jul 25, 2024

cauemarcondes Jul 26, 2024

crespocarlos Jul 26, 2024

crespocarlos commented Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

crespocarlos Jul 29, 2024 •

edited

Loading

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024 •

edited

Loading

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

dgieselaar Jul 29, 2024

crespocarlos Jul 29, 2024

kibana-ci commented Jul 29, 2024 •

edited

Loading

API count

crespocarlos commented Aug 5, 2024

[Infra] Create service in apm_data_access to fetch APM colleced hosts #189046

[Infra] Create service in apm_data_access to fetch APM colleced hosts #189046

Conversation

crespocarlos commented Jul 24, 2024 • edited Loading

Summary

How to test

obltmachine commented Jul 24, 2024

🤖 GitHub comments

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

crespocarlos commented Jul 24, 2024

elasticmachine commented Jul 25, 2024

crespocarlos commented Jul 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos commented Jul 29, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crespocarlos Jul 29, 2024 • edited Loading

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

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

kibana-ci commented Jul 29, 2024 • edited Loading

💔 Build Failed

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

API count

History

crespocarlos commented Aug 5, 2024

[Infra] Create service in `apm_data_access` to fetch APM colleced hosts #189046

[Infra] Create service in `apm_data_access` to fetch APM colleced hosts #189046

crespocarlos commented Jul 24, 2024 •

edited

Loading

crespocarlos Jul 29, 2024 •

edited

Loading

crespocarlos Jul 29, 2024 •

edited

Loading

kibana-ci commented Jul 29, 2024 •

edited

Loading