[Nightshift] Add impact entities endpoint

[mykolaharmash]

Part of https://github.com/elastic/nightshift-program/issues/1217

Adds the GET /internal/nightshift/investigations/_impact_entities endpoint to list all entities identified by investigations as impacted by the investigated events in the specified date range.

This endpoint is designed to be used by the Investigations UI as part of the list filtering.

Notable changes

• Returns distinct { name, type? } impact-entity pairs matching investigations.
• Unifies date filtering between the entity list and the main investigations list so they stay in sync.

How to test

This is an endpoint-only change, running unit tests should be enough:

node scripts/jest x-pack/platform/plugins/shared/nightshift_investigations/server/storage/saved_object_investigation_repository.test.ts x-pack/platform/plugins/shared/nightshift_investigations/server/client/investigations_client.test.ts x-pack/platform/plugins/shared/nightshift_investigations/server/routes/get_impact_entities.test.ts

[github-actions]

Reviewed the new _impact_entities endpoint. The route wiring, validation, security scoping, and type refactor look sound. Left one inline comment about the 1000-document cap and missing sort in findImpactEntities, which affects completeness and determinism at scale.

Generated by Claude Reviewer for #290331 · claude · opus · 102 AIC · ⌖ 25.7 AIC · ⊞ 5.5K

[github-actions]

One concern on findImpactEntities: it derives the distinct impact-entity filter list from only the 1,000 most recent matching investigations with no pagination, so entities from older in-range investigations are silently dropped. Flagged inline with a suggested alternative. Otherwise the change looks sound and well tested.

Generated by Claude Reviewer for #290331 · claude · opus · 120.2 AIC · ⌖ 23 AIC · ⊞ 5.5K

[cesco-f]

Question: did you link the wrong issue in the PR description? This work doesn't seem to address the issue you linked.

[cesco-f]

I see the same comment was resolved twice, but I still see the issue?

[mykolaharmash]

I've added sortField: 'created_at', sortOrder: 'desc' and resolved it leaving a fixed perPage: 1000. This seems like a reasonable limit for this endpoint and it avoids the additional logic for pagination, WDYT?

For now, added a comment in the code to make it explicit.

[cesco-f]

Before we settle on the fixed perPage: 1000, two things I'd like to resolve first.

The cap doesn't hold at scale. Past 1,000 matching investigations, entities that only appear in older ones silently drop out, so a user can't filter by an entity that demonstrably exists. How are we planning to handle that?

Can the same name carry different types? So will the filter accept name, or name + type? #1217 says name only, in which case {checkout, service} and {checkout, host} are two options returning identical results. But if it's the pair, I don't think it can be implemented correctly — impact is flattened, which loses field association inside object arrays, so name: checkout AND type: host would match an investigation where checkout is a service and something else is a host.

[mykolaharmash]

Fair points, my thinking here: this endpoint is meant to serve a specific part of the UI:

The date range of the investigation list marches the date filter for impact entities list. Realistically, 1000 limit should be more then enough, we only want to see entities that are part of the current investigation list. But you're probably right, to have it technically complete and be on the safe side, I'll add pagination 👍

Can the same name carry different types? So will the filter accept name, or name + type?

It will be name + type because we need to filter by services specifically. name: checkout AND type: host concern is valid and we need to think how to do it, but it's not part of this change, this one is only about returning the list of entities that the UI can render and use as a filter.

[mykolaharmash]

Added pagination to get all investigations 393f7b4

[cesco-f]

Pagination solves the completeness problem, thanks. One thing it introduces though: perPage: 1000 was also the only bound on cost, and there's nothing replacing it. Since all six date params are optional, a bare GET /_impact_entities builds no filter and the finder now walks every investigation in the space.

Could we require a date range?

[mykolaharmash]

Question: did you link the wrong issue in the PR description? This work doesn't seem to address the issue you linked.

Good point, the issue is correct but this PR addresses only part of it. I've change the description.

[cesco-f]

Before we settle on the fixed perPage: 1000, two things I'd like to resolve first.

The cap doesn't hold at scale. Past 1,000 matching investigations, entities that only appear in older ones silently drop out, so a user can't filter by an entity that demonstrably exists. How are we planning to handle that?

Can the same name carry different types? So will the filter accept name, or name + type? #1217 says name only, in which case {checkout, service} and {checkout, host} are two options returning identical results. But if it's the pair, I don't think it can be implemented correctly — impact is flattened, which loses field association inside object arrays, so name: checkout AND type: host would match an investigation where checkout is a service and something else is a host.

[flash1293]

@mykolaharmash I didn't test this, but do we have to load the whole SO to do the aggregation in node? I thought there was a way to push this down to Elasticsearch by using aggregations (I might be wrong)

If that would be possible, it would also remove the 1000 cap concerns

[mykolaharmash]

@mykolaharmash I didn't test this, but do we have to load the whole SO to do the aggregation in node? I thought there was a way to push this down to Elasticsearch by using aggregations (I might be wrong)

If that would be possible, it would also remove the 1000 cap concerns

I believe in order to do it on the ES side, we'd need to change the mapping of the impact field from flattened to nested, otherwise we don't preserve the field association between name and type. Do you remember why we chose flattened there in the first place?

[kibanamachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 393f7b4
• Build duration: 48 mins

Failed CI Steps

• FTR Configs #7
• Scout Lane #10 - stateful-classic / default

Metrics [docs]

✅ unchanged

Test Failures

• [job] [logs] FTR Configs #7 / Journey[login] Login
• [job] [logs] Scout Lane #10 - stateful-classic / default / local-stateful-classic - pingList query - returns a list of pings for the date range and given size

History

• 💛 Build #501709 was flaky ce95f02
• 💛 Build #500974 was flaky e34a5dd
• 💛 Build #500308 was flaky 20dd668

[cesco-f]

@mykolaharmash I didn't test this, but do we have to load the whole SO to do the aggregation in node? I thought there was a way to push this down to Elasticsearch by using aggregations (I might be wrong)
If that would be possible, it would also remove the 1000 cap concerns

I believe in order to do it on the ES side, we'd need to change the mapping of the impact field from flattened to nested, otherwise we don't preserve the field association between name and type. Do you remember why we chose flattened there in the first place?

Should we add an indexed field (a keyword of type|name pairs) since impact is flattened and loses the name/type association? I'm not sure we can change the type from flattened to nested for impact.

[flash1293]

@cesco-f

I'm not sure we can change the type from flattened to nested for impact.

what's the problem with that? Seems like it's for this kind of stuff. I might miss context here though

[mykolaharmash]

I think as well we're free to change the mapping, at this stage we we don't need to worry about backward compatibility.

[flash1293]

It seems like pushing down the entity discovery to Elasticsearch by nested plus server side agg is the "right way" of doing things here, it also would align well with how I imagine to consolidate this with the shared agentic_investigations plugin in a follow-up: #290802

[cesco-f]

I think as well we're free to change the mapping, at this stage we we don't need to worry about backward compatibility.

I wasn't sure if we can change the mapping of a saved object, but if we can let's do it.

[mykolaharmash]

I wasn't sure if we can change the mapping of a saved object, but if we can let's do it.

no, you're right, we have these saved objects with current mappings on serverless already, we need a backward-compatible solution.

[mykolaharmash]

As discussed, let's postpone the filtering tasks until after the migration to agentic_investigations. It doesn't make sense to work on SO-specific storage issues if we're going to abandon it soon. Let's come back to filtering once we know more details about the new storage.