[Inventory] Inventory k8s entities fixes #201260

crespocarlos · 2024-11-21T17:56:29Z

closes #201226

Summary

This PR makes the final adjustments on k8s entities after the #196916 was merged.

I had to fix most of the ECS entities because they had their entityId defined with uid field. Most of the ECS K8s entities don't have this field and should use the name field instead (see metricsets)

~~I also had to fix the transforms to include an aggregation for the displayNameTemplate field, when it doesn't match with the entity.Id~~

Real data Otel

The screenshots below are from a tests running the opentemeletry-demo sending otel data

Real data ECS

The screenshots below are from a tests running the opentemeletry-demo with elastic agent installed with Kubernetes integration

Additional test with synthtrace

Fix

ECS k8s service entity was missing the link to its corresponding dashboard

How to test

Start local kibana and es instances
run node scripts/synthtrace k8s_entities.ts --clean --live
Navigate to Inventory and enable EEM

crespocarlos · 2024-11-21T19:47:18Z

/ci

elasticmachine · 2024-11-22T10:31:17Z

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

elasticmachine · 2024-11-22T10:31:19Z

Pinging @elastic/obs-entities (Team:obs-entities)

miltonhultgren · 2024-11-22T10:49:34Z

This PR also closes https://github.com/elastic/elastic-entity-model/issues/170 and you might fix with this new capability https://github.com/elastic/elastic-entity-model/issues/168

miltonhultgren

This looks fine to me, but do note that using UID is only unique within the cluster but I think that's something you guys are aware of already?
If you want a global ID then you need to combine the cluster name and resource name to improve the chances of not having conflicts.

github-actions · 2024-11-22T10:50:38Z

🤖 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!)

crespocarlos · 2024-11-22T11:10:03Z

This looks fine to me, but do note that using UID is only unique within the cluster but I think that's something you guys are aware of already? If you want a global ID then you need to combine the cluster name and resource name to improve the chances of not having conflicts.

@miltonhultgren I think we still need to look into this. cc: @roshan-elastic

klacabane · 2024-11-22T12:01:45Z

x-pack/plugins/entity_manager/server/lib/entities/ingest_pipeline/generate_latest_processors.ts

-  if (!definition.metadata) {
-    return '';
-  }
+  const displayNameField = definition.displayNameTemplate.replace(/[{}]/g, '');


Would this break if displayNameTemplate includes more than a single field ? something like {{field1}} - {{field2}}

I'd rather explicitly setting the metadata aggregation on the definition instead of this logic since it looks hard to get right

I agree with this, I'd rather the user has to pick a single field to use and if that field is not set we fallback to the ID.

I didn't consider displayNameTemplate with multiple fields. I'll change this.

@klacabane fixed here: 2bb2e1e

roshan-elastic · 2024-11-22T12:22:48Z

This looks fine to me, but do note that using UID is only unique within the cluster but I think that's something you guys are aware of already? If you want a global ID then you need to combine the cluster name and resource name to improve the chances of not having conflicts.

@miltonhultgren I think we still need to look into this. cc: @roshan-elastic

Good catch both.

I'm thinking that we could:

Concat the two fields as a means to not accidentally merging data across two pods with the same UID but in different clusters.
Store the pod UID as a metadata field
Keep the display name as the pod name

That way, we don't accidentally merge different pods with the same UID but users can easily see (and filter by) the pod UID).

WDYT?

cauemarcondes

LGTM

roshan-elastic · 2024-11-22T13:51:10Z

This looks fine to me, but do note that using UID is only unique within the cluster but I think that's something you guys are aware of already? If you want a global ID then you need to combine the cluster name and resource name to improve the chances of not having conflicts.

@miltonhultgren I think we still need to look into this. cc: @roshan-elastic

Good catch both.

I'm thinking that we could:

Concat the two fields as a means to not accidentally merging data across two pods with the same UID but in different clusters.

Store the pod UID as a metadata field

Keep the display name as the pod name

That way, we don't accidentally merge different pods with the same UID but users can easily see (and filter by) the pod UID).

WDYT?

Quick Q on this @crespocarlos - I assume the above hasn't gone into the PR (and might be a valid follow up issue)?

crespocarlos · 2024-11-22T14:07:28Z

Quick Q on this @crespocarlos - I assume the above hasn't gone into the PR (and might be a valid follow up issue)?

Exactly. We need a follow up ticket to investigate this.

roshan-elastic · 2024-11-22T14:23:42Z

Quick Q on this @crespocarlos - I assume the above hasn't gone into the PR (and might be a valid follow up issue)?

Exactly. We need a follow up ticket to investigate this.

Cool @crespocarlos - one thing we need to be mindful of is how alerts will be associated with a k8s entity (cc @kpatticha) as I think they 'group by' the entity identifying field.

crespocarlos · 2024-11-25T08:18:10Z

@elasticmachine merge upstream

elasticmachine · 2024-11-25T09:28:29Z

💚 Build Succeeded

Buildkite Build
Commit: 0a111e7
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-201260-0a111e7ec539

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
`@kbn/apm-synthtrace-client`	268	270	+2

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.8MB	1.8MB	+27.0B
`inventory`	236.3KB	236.5KB	+174.0B
total			+201.0B

Page load bundle

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

id	before	after	diff
`observabilityShared`	93.7KB	93.7KB	+15.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/apm-synthtrace-client`	268	270	+2

History

💛 Build #253624 was flaky 2bb2e1e
💚 Build #253494 succeeded 7900589

kibanamachine · 2024-11-26T08:29:41Z

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12026661303

closes [elastic#201226](elastic#201226) ## Summary This PR makes the final adjustments on k8s entities after the elastic#196916 was merged. I had to fix most of the ECS entities because they had their `entityId` defined with `uid` field. Most of the ECS K8s entities don't have this field and should use the `name` field instead (see [metricsets](https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-module-kubernetes.html#_metricsets_41)) ~I also had to fix the transforms to include an aggregation for the `displayNameTemplate` field, when it doesn't match with the `entity.Id`~ ### Real data Otel The screenshots below are from a tests running the `opentemeletry-demo` sending otel data <img width="710" alt="image" src="https://github.com/user-attachments/assets/d223858d-3b99-4bb0-b69e-3b70112a2c17"> <img width="710" alt="image" src="https://github.com/user-attachments/assets/1ef27dcd-682c-4681-b92b-be9b4f2b32b8"> ### Real data ECS The screenshots below are from a tests running the `opentemeletry-demo` with elastic agent installed with Kubernetes integration <img width="710" alt="image" src="https://github.com/user-attachments/assets/e2fb6cbd-60a0-4995-bc94-8ccbc8911db1"> <img width="710" alt="image" src="https://github.com/user-attachments/assets/a3d9dba3-fee7-42af-972b-34a151b52f2b"> ### Additional test with synthtrace <img width="709" alt="image" src="https://github.com/user-attachments/assets/bf56da1e-0bbb-40a0-9f44-06b9f86427a8"> ### Fix ECS k8s service entity was missing the link to its corresponding dashboard <img width="709" alt="image" src="https://github.com/user-attachments/assets/488891a6-1c61-4001-b604-208478e1c798"> <img width="709" alt="image" src="https://github.com/user-attachments/assets/0b30c56b-db52-4d82-a7e7-100dae91a35e"> ### How to test - Start local kibana and es instances - run ` node scripts/synthtrace k8s_entities.ts --clean --live` - Navigate to Inventory and enable EEM --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit aead7b9)

kibanamachine · 2024-11-26T08:34:38Z

💚 All backports created successfully

Status	Branch	Result
✅	8.17
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.17`: - [[Inventory] Inventory k8s entities fixes (#201260)](#201260)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Carlos Crespo <[email protected]>

# Backport This will backport the following commits from `main` to `8.x`: - [[Inventory] Inventory k8s entities fixes (#201260)](#201260)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Carlos Crespo <[email protected]>

closes [elastic#201226](elastic#201226) ## Summary This PR makes the final adjustments on k8s entities after the elastic#196916 was merged. I had to fix most of the ECS entities because they had their `entityId` defined with `uid` field. Most of the ECS K8s entities don't have this field and should use the `name` field instead (see [metricsets](https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-module-kubernetes.html#_metricsets_41)) ~I also had to fix the transforms to include an aggregation for the `displayNameTemplate` field, when it doesn't match with the `entity.Id`~ ### Real data Otel The screenshots below are from a tests running the `opentemeletry-demo` sending otel data <img width="710" alt="image" src="https://github.com/user-attachments/assets/d223858d-3b99-4bb0-b69e-3b70112a2c17"> <img width="710" alt="image" src="https://github.com/user-attachments/assets/1ef27dcd-682c-4681-b92b-be9b4f2b32b8"> ### Real data ECS The screenshots below are from a tests running the `opentemeletry-demo` with elastic agent installed with Kubernetes integration <img width="710" alt="image" src="https://github.com/user-attachments/assets/e2fb6cbd-60a0-4995-bc94-8ccbc8911db1"> <img width="710" alt="image" src="https://github.com/user-attachments/assets/a3d9dba3-fee7-42af-972b-34a151b52f2b"> ### Additional test with synthtrace <img width="709" alt="image" src="https://github.com/user-attachments/assets/bf56da1e-0bbb-40a0-9f44-06b9f86427a8"> ### Fix ECS k8s service entity was missing the link to its corresponding dashboard <img width="709" alt="image" src="https://github.com/user-attachments/assets/488891a6-1c61-4001-b604-208478e1c798"> <img width="709" alt="image" src="https://github.com/user-attachments/assets/0b30c56b-db52-4d82-a7e7-100dae91a35e"> ### How to test - Start local kibana and es instances - run ` node scripts/synthtrace k8s_entities.ts --clean --live` - Navigate to Inventory and enable EEM --------- Co-authored-by: Elastic Machine <[email protected]>

crespocarlos added 2 commits November 21, 2024 18:52

Inventory k8s entities fixes

ac4eebc

Update synthtrace k8s entities

4c628dd

ecs k8s service fix

7900589

crespocarlos force-pushed the 201226-adjustments-k8s-entities branch from 3284ad5 to 7900589 Compare November 22, 2024 10:23

crespocarlos marked this pull request as ready for review November 22, 2024 10:31

crespocarlos requested review from a team as code owners November 22, 2024 10:31

miltonhultgren approved these changes Nov 22, 2024

View reviewed changes

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

klacabane reviewed Nov 22, 2024

View reviewed changes

cauemarcondes approved these changes Nov 22, 2024

View reviewed changes

Revert logic to generate aggregation for displayName; other adjustments

2bb2e1e

crespocarlos requested a review from klacabane November 22, 2024 14:52

Merge branch 'main' into 201226-adjustments-k8s-entities

0a111e7

crespocarlos added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Nov 26, 2024

crespocarlos merged commit aead7b9 into elastic:main Nov 26, 2024
30 checks passed

crespocarlos deleted the 201226-adjustments-k8s-entities branch November 26, 2024 08:29

kibanamachine added the v9.0.0 label Nov 26, 2024

kibanamachine mentioned this pull request Nov 26, 2024

[8.17] [Inventory] Inventory k8s entities fixes (#201260) #201725

Merged

kibanamachine mentioned this pull request Nov 26, 2024

[8.x] [Inventory] Inventory k8s entities fixes (#201260) #201726

Merged

kibanamachine added the v8.18.0 label Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Inventory] Inventory k8s entities fixes #201260

[Inventory] Inventory k8s entities fixes #201260

crespocarlos commented Nov 21, 2024 •

edited by kibanamachine

Loading

crespocarlos commented Nov 21, 2024

elasticmachine commented Nov 22, 2024

elasticmachine commented Nov 22, 2024

miltonhultgren commented Nov 22, 2024

miltonhultgren left a comment

github-actions bot commented Nov 22, 2024

crespocarlos commented Nov 22, 2024

klacabane Nov 22, 2024

klacabane Nov 22, 2024

miltonhultgren Nov 22, 2024

crespocarlos Nov 22, 2024

crespocarlos Nov 22, 2024

roshan-elastic commented Nov 22, 2024

cauemarcondes left a comment

roshan-elastic commented Nov 22, 2024

crespocarlos commented Nov 22, 2024 •

edited

Loading

roshan-elastic commented Nov 22, 2024

crespocarlos commented Nov 25, 2024

elasticmachine commented Nov 25, 2024 •

edited

Loading

API count

kibanamachine commented Nov 26, 2024

kibanamachine commented Nov 26, 2024

[Inventory] Inventory k8s entities fixes #201260

[Inventory] Inventory k8s entities fixes #201260

Conversation

crespocarlos commented Nov 21, 2024 • edited by kibanamachine Loading

Summary

Real data Otel

Real data ECS

Additional test with synthtrace

Fix

How to test

crespocarlos commented Nov 21, 2024

elasticmachine commented Nov 22, 2024

elasticmachine commented Nov 22, 2024

miltonhultgren commented Nov 22, 2024

miltonhultgren left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 22, 2024

🤖 GitHub comments

crespocarlos commented Nov 22, 2024

klacabane Nov 22, 2024

Choose a reason for hiding this comment

klacabane Nov 22, 2024

Choose a reason for hiding this comment

miltonhultgren Nov 22, 2024

Choose a reason for hiding this comment

crespocarlos Nov 22, 2024

Choose a reason for hiding this comment

crespocarlos Nov 22, 2024

Choose a reason for hiding this comment

roshan-elastic commented Nov 22, 2024

cauemarcondes left a comment

Choose a reason for hiding this comment

roshan-elastic commented Nov 22, 2024

crespocarlos commented Nov 22, 2024 • edited Loading

roshan-elastic commented Nov 22, 2024

crespocarlos commented Nov 25, 2024

elasticmachine commented Nov 25, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kibanamachine commented Nov 26, 2024

kibanamachine commented Nov 26, 2024

💚 All backports created successfully

Questions ?

crespocarlos commented Nov 21, 2024 •

edited by kibanamachine

Loading

crespocarlos commented Nov 22, 2024 •

edited

Loading

elasticmachine commented Nov 25, 2024 •

edited

Loading