[Entity Analytics] Update entity maintainers to use new relationship schema (ids object)#262911
Conversation
Tracks the changes needed in communicates_with and accesses maintainers to align with the EntityRelationship schema introduced in elastic#262242. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ema update Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ects for accesses relationships
…s schema Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update ProcessedEntityRecord usage in run_maintainer.test.ts to use
the { ids: string[] } object format matching the updated type definition,
and apply eslint auto-fix formatting to postprocess_records.test.ts.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to { ids: string[] } schema
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Pinging @elastic/siem (Team:SIEM) |
1 similar comment
|
Pinging @elastic/siem (Team:SIEM) |
|
Pinging @elastic/contextual-security-apps (Team:Cloud Security) |
|
Pinging @elastic/security-entity-analytics (Team:Entity Analytics) |
ApprovabilityVerdict: Needs human review Schema change wrapping relationship arrays in You can customize Macroscope's approvability policy. Learn more. |
… write Fixes elastic#262869 When multiple integrations produce ProcessedEntityRecord objects with the same entityId, the accesses maintainer previously mapped them 1:1 to bulk update objects. Last-writer-wins meant only one integration's host IDs survived. Add mergeRecordsByEntityId to accumulate host IDs across all records for the same entity before writing. The precedence rule ensures that if a host appears in accesses_frequently from any integration it is promoted out of accesses_infrequently.
… in accesses update
…ntainers-relationship-schema-262893
tiansivive
left a comment
There was a problem hiding this comment.
Good stuff, just a few suggestions
| } | ||
|
|
||
| return merged; | ||
| } |
There was a problem hiding this comment.
This works, but I found it harder to follow than it needs to be because the logic is spread across nested loops, branching, and multiple mutation phases. As a reader, I have to mentally track state changes while also figuring out the actual business rules.
I think this would be much cleaner if it were structured more linearly, with explicit steps like filtering valid records, grouping by entityId, merging ids, then applying the precedence rule.
It would make the intent much easier to read at a glance.
There was a problem hiding this comment.
This is a bit nicer, and the comments do help, but I think the main readability issue is still the structure rather than the lack of explanation. It still relies on branching, nested iteration, and mutation across multiple phases, so I still have to trace the control flow pretty closely to understand it.
I was hoping for something where the steps are more directly reflected in the shape of the code, instead of needing comments to explain the flow.
Extracting to small named utility functions and then doing one chain of array methods like .map/.filter/.reduce or a similarly styled loop would help a lot
There was a problem hiding this comment.
I took a bit more time on this, I think this should satisfy your concerns.
I tried to find more common ground between the maintainers for a shared function but it was minimal.
This code will likely no longer exist in 9.5
- Restructure mergeRecordsByEntityId into explicit linear steps (filter, group, merge, precedence) to improve readability - Replace `as Entity` coercion with `satisfies Entity` for stronger type safety Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| for (const t of r.communicates_with.ids) existing.targets.add(t); | ||
| } else { | ||
| merged.set(r.entityId, { | ||
| entityType: r.entityType, |
There was a problem hiding this comment.
Nit: Not directly part of this PR, but I noticed an inconsistency between the two update_entities files:
- In communicates_with/update_entities,
entityTypeis typed as string (fromMergedEntity) - In accesses/update_entities,
EntityTypeis the Zod-derived enum from @kbn/entity-store/common
Unless MergedEntity.entityType intentionally needs to accept unknown values from raw data, it would be more consistent for both to use the EntityType enum, and runtime validation too.
There was a problem hiding this comment.
Thanks for noticing that! I didn't have an agent review pass on the last commit.
CAWilson94
left a comment
There was a problem hiding this comment.
Desk Tested - working as described. Added one nit, otherwise LGTM with changes added 🚀
Threads EntityType (the enum) through ProcessedEntityRecord.entityType, postProcessEsqlResults, and MergedEntity instead of widening to string. Removes the as BulkObject['type'] coercion and switches as Entity to satisfies Entity for consistency with the accesses maintainer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e steps Extract a shared generic groupByEntityId utility and restructure both accesses and communicates_with update_entities into explicit named functions (filterValid / seed / merge / applyPrecedence) composed in a clear pipeline, removing the need for inline comments to explain control flow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tiansivive
left a comment
There was a problem hiding this comment.
Thanks for addressing everything
⏳ Build in-progress
History
cc @seanrathier |
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/24534917239 |
…schema (ids object) (elastic#262911) (cherry picked from commit 33ee38a)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…nship schema (ids object) (#262911) (#263917) # Backport This will backport the following commits from `main` to `9.4`: - [[Entity Analytics] Update entity maintainers to use new relationship schema (ids object) (#262911)](#262911) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"seanrathier","email":"sean.rathier@gmail.com"},"sourceCommit":{"committedDate":"2026-04-16T21:27:50Z","message":"[Entity Analytics] Update entity maintainers to use new relationship schema (ids object) (#262911)","sha":"33ee38aa5ceacecdec388067c718661cbcaa6387","branchLabelMapping":{"^v9.5.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Cloud Security","ci:cloud-deploy","Team:Entity Analytics","backport:version","v9.4.0","v9.5.0"],"title":"[Entity Analytics] Update entity maintainers to use new relationship schema (ids object)","number":262911,"url":"https://github.com/elastic/kibana/pull/262911","mergeCommit":{"message":"[Entity Analytics] Update entity maintainers to use new relationship schema (ids object) (#262911)","sha":"33ee38aa5ceacecdec388067c718661cbcaa6387"}},"sourceBranch":"main","suggestedTargetBranches":["9.4"],"targetPullRequestStates":[{"branch":"9.4","label":"v9.4.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.5.0","branchLabelMappingKey":"^v9.5.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/262911","number":262911,"mergeCommit":{"message":"[Entity Analytics] Update entity maintainers to use new relationship schema (ids object) (#262911)","sha":"33ee38aa5ceacecdec388067c718661cbcaa6387"}}]}] BACKPORT--> Co-authored-by: seanrathier <sean.rathier@gmail.com>
Summary
Closes #262893
Updates the
communicates_withandaccessesentity maintainers to write relationship values as{ ids: string[] }objects instead of plain string arrays, aligning with theEntityRelationshipschema introduced in #262242.Both maintainers now carry
{ ids: string[] }end-to-end through their internal pipelines — fromProcessedEntityRecordthroughpostprocessEsqlResultstobuildEntityDoc/updateEntityRelationships.Changes
communicates_withmaintainertypes.ts—ProcessedEntityRecord.communicates_with:string[]→{ ids: string[] }postprocess_records.ts— wraps ESQL output in{ ids: [...] }run_maintainer.ts— logging updated to use.idsupdate_entities.ts—mergeRecordsByEntityIdreads.idsfor iteration/deduplicationaccessesmaintainertypes.ts—ProcessedEntityRecord.accesses_frequently/infrequently:string[]→{ ids: string[] }postprocess_records.ts— wraps ESQL output in{ ids: [...] }(+ new test file)run_maintainer.ts— logging updated to use.idsupdate_entities.ts— guards updated to.ids.length > 0(+ new test file)How to test
node scripts/jest x-pack/solutions/security/plugins/security_solution/server/lib/entity_analytics/maintainers --no-coverage— all 207 tests passnode scripts/type_check --project x-pack/solutions/security/plugins/security_solution/tsconfig.json— no errors{ "ids": [...] }under relationship fields