EntityAnalytics AD: add support for ECS entity fields#18337
EntityAnalytics AD: add support for ECS entity fields#18337chemamartinez wants to merge 12 commits into
Conversation
Vale Linting ResultsSummary: 2 warnings found
|
| File | Line | Rule | Message |
|---|---|---|---|
| packages/entityanalytics_ad/docs/README.md | 350 | Elastic.Latinisms | Latin terms and abbreviations are a common source of confusion. Use 'for example' instead of 'e.g'. |
| packages/entityanalytics_ad/docs/README.md | 351 | Elastic.Latinisms | Latin terms and abbreviations are a common source of confusion. Use 'for example' instead of 'e.g'. |
The Vale linter checks documentation changes against the Elastic Docs style guide.
To use Vale locally or report issues, refer to Elastic style guide for Vale.
🚀 Benchmarks reportTo see the full report comment with |
| } | ||
| return result; | ||
| } | ||
| def buildRel(def dns) { |
There was a problem hiding this comment.
🟢 Low ingest_pipeline/device.yml:286
In the buildRel function, when a DN lacks a CN= or DC= component, the corresponding name or domain entry is skipped entirely while id is still added. This produces arrays of mismatched lengths — for example, 3 IDs but only 2 names — making it impossible for consumers to correlate which name belongs to which ID by index. Consider restructuring to emit parallel arrays where each position corresponds across fields, such as an array of objects where each object contains the matched id, name, and domain for a single DN.
🤖 Copy this AI Prompt to have your agent fix this:
In file packages/entityanalytics_ad/data_stream/entity/elasticsearch/ingest_pipeline/device.yml around line 286:
In the `buildRel` function, when a DN lacks a `CN=` or `DC=` component, the corresponding `name` or `domain` entry is skipped entirely while `id` is still added. This produces arrays of mismatched lengths — for example, 3 IDs but only 2 names — making it impossible for consumers to correlate which name belongs to which ID by index. Consider restructuring to emit parallel arrays where each position corresponds across fields, such as an array of objects where each object contains the matched `id`, `name`, and `domain` for a single DN.
There was a problem hiding this comment.
We're aware of this limitation, but there are a few reasons why we've opted to keep flat parallel arrays rather than restructuring to nested objects:
-
Nested mappings are not an option as ES|QL doesn't support nested field types (see Elasticsearch docs).
-
Index correlation is already broken at the storage layer — since the sub-fields (id, name, domain) are mapped as flat keyword arrays under a group, Elasticsearch provides no positional guarantee between them regardless of how the script emits the data.
-
Consumers of these fields only need the full set of values for each attribute independently (e.g. all related IDs, all related names). Correlating a specific name to a specific ID within the same relationship group is not a requirement we need to support right now.
Given these constraints, emitting a flat list of each value without cross-field correlation is the right trade-off for us.
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
Moved to draft again because of an ongoing discussion about subfields in relationships fields. |
| type: keyword | ||
| - name: domain | ||
| type: keyword | ||
| - name: administered_by |
There was a problem hiding this comment.
Entity store already uses administers and not administered_by, so we'll stick with that (that was one of Andrew's suggestions).
💚 Build Succeeded
History
|
efd6
left a comment
There was a problem hiding this comment.
Can you update the proposed commit message to more accurately reflect the change?
| fields: | ||
| - name: last_activity | ||
| type: date | ||
| - name: relationships |
There was a problem hiding this comment.
This is not yet defined in ECS. Is there an issue or PR for it?
There was a problem hiding this comment.
There is an RFC at the moment: elastic/ecs#2598.
Once the RFC is merged, they are going to add to ECS the missing fields.
There was a problem hiding this comment.
Suggest bumping the ECS version to v9.3.0, then we have entity.attributes and entity.lifecycle defined.
There was a problem hiding this comment.
Even though we bump ECS version, entity.attributes and entity.lifecycle are defined right now as an object without any subfield. This is not the desired final form of these fields, where fields inside should be defined as well.
There was a problem hiding this comment.
This in conjunction with #18337 (comment) makes me reluctant to approve yet.
Proposed commit message
Checklist
changelog.ymlfile.Related issues