EMT-146: use ingest agent for status info #63921

nnamdifrankie · 2020-04-18T00:46:30Z

Summary

https://github.com/elastic/endpoint-app-team/issues/146

Expose the AgentStatus in the IngestManager setup contract
Update integration and unit tests
Add new unit tests

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

nnamdifrankie · 2020-04-18T16:10:01Z

@elasticmachine merge upstream

…mdifrankie/kibana into EMT-146_use_agent_service_for_status

nnamdifrankie · 2020-04-19T17:15:52Z

@elasticmachine merge upstream

jfsiii

I'd like to avoid the new AgentService and getAgentStatus this PR currently has. Ingest Manager already has an Agent Service. I'd like to use that and its current function signatures, or update them to make them more useful/consistent/whatever, rather than add export a different one.

I made a comment where I think it might work. Ingest Manager's handlers do the same await agent ... getAgentStatus pattern so I don't think it's unusual for Endpoint to do it.

If getStatusById is useful Ingest Manager could add/export that. I'd prefer either to adding this new function & signature.

jfsiii · 2020-04-20T00:45:38Z

x-pack/plugins/endpoint/server/routes/metadata/index.ts

+    const status = await metadataRequestContext.endpointAppContext.agentService.getAgentStatus(
+      metadataRequestContext.requestHandlerContext,
+      hostMetadata.elastic.agent.id
+    );


Suggested change

const status = await metadataRequestContext.endpointAppContext.agentService.getAgentStatus(

metadataRequestContext.requestHandlerContext,

hostMetadata.elastic.agent.id

);

const { endpointAppContext, requestHandlerContext } = metadataRequestContext;

const { getAgent, getAgentStatus } = endpointAppContext.agentService;

const agent = await getAgent(requestHandlerContext.core.savedObjects.client, hostMetadata.elastic.agent.id);

const status = await getAgentStatus(agent);

I won't be able to try this locally until Monday afternoon, but I think this will work. If so, we can avoid the new getAgentStatus this PR adds in Ingest Manager.

@jfsiii Can we talk about this. I would think a wholesale service will be better that performs the functionality will be better. And this does not isolate from changes to how agent status is derived. With this approach we have too much knowledge of the internal working of Ingest. This will mean will do a wholesale export of the entire services directory?

@nnamdifrankie sure, we can talk anytime. I'm available most days from 1pm-5pm ET.

Before then, I'll try to summary my understanding & suggestions.

As I understand it, this PR adds an Agent Service but Ingest Manager already has one

It also adds a function getAgentStatus which a) is a duplicate of an existing function and b) uses the existing services functions from Ingest.

I'd like to avoid both those issues and think we can do that by moving that function into services/agents/status, changing its name to something like getAgentStatusById, and have it accept a saved object client and string id. That's in line with lots of our other service functions.

That way, all you're really changing at this call site is the function name and the first argument. It's still only calling one function and Ingest is the one in control of that logic.

- const status = await metadataRequestContext.endpointAppContext.agentService.getAgentStatus( - metadataRequestContext.requestHandlerContext, + const status = await metadataRequestContext.endpointAppContext.agentService.getAgentStatusById( + metadataRequestContext.requestHandlerContext.core.savedObjects.client,

Hope that helps. Reach out any time to talk more.

nnamdifrankie · 2020-04-20T01:54:19Z

@elasticmachine merge upstream

…mdifrankie/kibana into EMT-146_use_agent_service_for_status

nchaulet · 2020-04-21T13:31:19Z

x-pack/plugins/ingest_manager/server/plugin.ts

@@ -148,6 +150,9 @@ export class IngestManagerPlugin implements Plugin<IngestManagerSetupContract> {
    }
    return deepFreeze({
      esIndexPatternService: new ESIndexPatternSavedObjectService(),
+      agentService: {


I think we should expose agentService during start, it's probably not going to work if you use it during setup

Thanks for catching that @nchaulet. Yeah, I believe we're encouraged to put as much as possible in start. Especially anything that could Error.

I think this is a setup dependency, because we do the route registration in the setup method which I believe is the place for this action. These are just service definitions are they are not consumed in the setup step but in the handlers. Also if ingest does not setup correctly, then the endpoint is also not supposed to work either at least with how we depend on the ingest plugin today.

@jfsiii @nchaulet should we move esIndexPatternService too? That one is reading from the saved objects in kibana, I suppose that could error as well right?

Yes esIndexPatternService should probably be moved to start too.
If it's too much refacto for this PR I am ok to have a follow up PR for that.

Gotcha, yeah I was talking with @nnamdifrankie about that. Right now endpoint is accessing the ingest contract in the setup function because that's where the routes are being registered. If we move the contract to the start function I think we'd have to use a singleton or something to point to the ingest contract because I don't think we can register the endpoint routes in the start function. It seems like other plugins do this. An example is the encrypted saved objects: https://github.com/elastic/kibana/blob/master/x-pack/plugins/actions/server/plugin.ts#L243

Might be easier to do that in a follow up PR.

@nchaulet @jfsiii @jonathan-buttner
FYI https://github.com/elastic/endpoint-app-team/issues/357

jonathan-buttner · 2020-04-21T17:29:17Z

x-pack/plugins/endpoint/server/routes/metadata/index.ts

+    );
+    hostStatus = HOST_STATUS_MAPPING.get(status) || HostStatus.ERROR;
+  } catch (e) {
+    if (e.isBoom && e.output.statusCode === 404) {


Does the ingest api throw a boom?

Yes, if a saved object is not found is going throw a Boom error

Yes,
server log [10:02:10.301] [warning][endpoint][metadata][plugins] agent with id 023fa40c-411d-4188-a941-4147bfadd095 not found

nnamdifrankie · 2020-04-21T17:41:20Z

@elasticmachine merge upstream

jonathan-buttner

Changes look good from an endpoint side.

nchaulet

Good to me for the ingest manager part

jfsiii

🚀 from me. Especially with the linked follow up PR.

kibanamachine · 2020-04-21T19:31:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f7d0bad

History

💚 Build #42073 succeeded 2bd6601
💚 Build #42026 succeeded e990c47
💚 Build #41977 succeeded 62eba60
💚 Build #41942 succeeded f591e86
💚 Build #41870 succeeded b36c29f

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

* master: (29 commits) [Dashboard] Deangularize navbar, attempt nr. 2 (elastic#61611) refactor action filter creation utils (elastic#62969) Refresh index pattern list before redirecting (elastic#63329) [APM]fixing custom link unit tests (elastic#64045) [Ingest] EPM & Fleet are enabled when Ingest is enabled (elastic#64103) [Alerting] Fixed bug with no possibility to edit the index name after adding (elastic#64033) [Maps] Map settings: min and max zoom (elastic#63714) [kbn-storybook] Use raw loader for text files (elastic#64108) [EPM] /packages/{package} endpoint to support upgrades (elastic#63629) [SIEM] New Platform Saved Objects Registration (elastic#64029) [Endpoint] Hook to handle events needing navigation via Router (elastic#63863) Fixed small issue in clone functionality (elastic#64085) [Endpoint]EMT-146: use ingest agent for status info (elastic#63921) [SIEM] Server NP Followup (elastic#64010) Register uiSettings on New Platform (elastic#64015) [Reporting] Integration polling config with client code (elastic#63754) [Docs]7.7 SIEM doc updates (elastic#63951) [SIEM] [Cases] Tags suggestions (elastic#63878) Include datasource UUID in agent config yaml, adjust overflow height of yaml view (elastic#64027) [DOCS] Add file size setting for Data Visualizer (elastic#64006) ...

[Endpoint]EMT-146: use ingest agent for status info

…ana into task-manager/cancel-logging * 'task-manager/cancel-logging' of github.com:gmmorris/kibana: (28 commits) [Dashboard] Deangularize navbar, attempt nr. 2 (elastic#61611) refactor action filter creation utils (elastic#62969) Refresh index pattern list before redirecting (elastic#63329) [APM]fixing custom link unit tests (elastic#64045) [Ingest] EPM & Fleet are enabled when Ingest is enabled (elastic#64103) [Alerting] Fixed bug with no possibility to edit the index name after adding (elastic#64033) [Maps] Map settings: min and max zoom (elastic#63714) [kbn-storybook] Use raw loader for text files (elastic#64108) [EPM] /packages/{package} endpoint to support upgrades (elastic#63629) [SIEM] New Platform Saved Objects Registration (elastic#64029) [Endpoint] Hook to handle events needing navigation via Router (elastic#63863) Fixed small issue in clone functionality (elastic#64085) [Endpoint]EMT-146: use ingest agent for status info (elastic#63921) [SIEM] Server NP Followup (elastic#64010) Register uiSettings on New Platform (elastic#64015) [Reporting] Integration polling config with client code (elastic#63754) [Docs]7.7 SIEM doc updates (elastic#63951) [SIEM] [Cases] Tags suggestions (elastic#63878) Include datasource UUID in agent config yaml, adjust overflow height of yaml view (elastic#64027) [DOCS] Add file size setting for Data Visualizer (elastic#64006) ...

EMT-146: use ingest agent for status info

ad9dad3

nnamdifrankie added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 labels Apr 18, 2020

nnamdifrankie requested a review from a team as a code owner April 18, 2020 00:46

nnamdifrankie requested a review from a team April 18, 2020 00:46

EMT-146: add integration tests

c653765

elasticmachine and others added 6 commits April 18, 2020 12:10

Merge branch 'master' into EMT-146_use_agent_service_for_status

eb13637

EMT-146: use beforeEach and afterEach

110efc6

Merge branch 'EMT-146_use_agent_service_for_status' of github.com:nna…

e29c087

…mdifrankie/kibana into EMT-146_use_agent_service_for_status

EMT-146: remove failing test

4e53a4f

EMT-146: add back integration test

ba532d0

EMT-146: revert ingest calls

179d09e

EMT-146: clean up

229df97

jfsiii reviewed Apr 20, 2020

View reviewed changes

elasticmachine and others added 7 commits April 19, 2020 21:54

Merge branch 'master' into EMT-146_use_agent_service_for_status

578e0bf

EMT-146: reorder test load

bcd6e4c

Merge branch 'EMT-146_use_agent_service_for_status' of github.com:nna…

b36c29f

…mdifrankie/kibana into EMT-146_use_agent_service_for_status

EMT-146: add ingest calls back

f591e86

EMT-146: remove metadata status test, it cross some boundaries

62eba60

EMT-146: review comments and refactor the code

e990c47

EMT-146: add more documentation and test

2bd6601

nchaulet reviewed Apr 21, 2020

View reviewed changes

nnamdifrankie added 2 commits April 21, 2020 09:35

EMT-146: fall back to host id if elastic id is missing

6d4c1ed

EMT-146: add warning log, and improve logging

9bb3550

jfsiii self-requested a review April 21, 2020 17:11

jonathan-buttner reviewed Apr 21, 2020

View reviewed changes

Merge branch 'master' into EMT-146_use_agent_service_for_status

f7d0bad

jonathan-buttner approved these changes Apr 21, 2020

View reviewed changes

nchaulet approved these changes Apr 21, 2020

View reviewed changes

jfsiii approved these changes Apr 21, 2020

View reviewed changes

nnamdifrankie merged commit 4ca8610 into elastic:master Apr 21, 2020

nnamdifrankie deleted the EMT-146_use_agent_service_for_status branch April 21, 2020 19:51

nnamdifrankie mentioned this pull request Apr 21, 2020

[7.x] [Endpoint]EMT-146: use ingest agent for status info (#63921) #64122

Merged

nnamdifrankie added a commit that referenced this pull request Apr 22, 2020

[Endpoint]EMT-146: use ingest agent for status info (#63921) (#64122)

f570c0d

[Endpoint]EMT-146: use ingest agent for status info

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EMT-146: use ingest agent for status info #63921

EMT-146: use ingest agent for status info #63921

nnamdifrankie commented Apr 18, 2020 •

edited

Loading

nnamdifrankie commented Apr 18, 2020

nnamdifrankie commented Apr 19, 2020

jfsiii left a comment

jfsiii Apr 20, 2020

jfsiii Apr 20, 2020

nnamdifrankie Apr 20, 2020 •

edited

Loading

jfsiii Apr 20, 2020 •

edited

Loading

nnamdifrankie commented Apr 20, 2020

nchaulet Apr 21, 2020

jfsiii Apr 21, 2020

nnamdifrankie Apr 21, 2020 •

edited

Loading

jonathan-buttner Apr 21, 2020

nchaulet Apr 21, 2020

jonathan-buttner Apr 21, 2020

nnamdifrankie Apr 21, 2020

jonathan-buttner Apr 21, 2020

nchaulet Apr 21, 2020

nnamdifrankie Apr 21, 2020

nnamdifrankie commented Apr 21, 2020

jonathan-buttner left a comment

nchaulet left a comment

jfsiii left a comment

kibanamachine commented Apr 21, 2020

-    const status = await metadataRequestContext.endpointAppContext.agentService.getAgentStatus(
-      metadataRequestContext.requestHandlerContext,
-      hostMetadata.elastic.agent.id
-    );
+    const { endpointAppContext, requestHandlerContext } = metadataRequestContext;
+    const { getAgent, getAgentStatus } = endpointAppContext.agentService;
+    const agent = await getAgent(requestHandlerContext.core.savedObjects.client, hostMetadata.elastic.agent.id);
+    const status = await getAgentStatus(agent);

EMT-146: use ingest agent for status info #63921

EMT-146: use ingest agent for status info #63921

Conversation

nnamdifrankie commented Apr 18, 2020 • edited Loading

Summary

Checklist

nnamdifrankie commented Apr 18, 2020

nnamdifrankie commented Apr 19, 2020

jfsiii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nnamdifrankie Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

jfsiii Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

nnamdifrankie commented Apr 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nnamdifrankie Apr 21, 2020 • 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

nnamdifrankie commented Apr 21, 2020

jonathan-buttner left a comment

Choose a reason for hiding this comment

nchaulet left a comment

Choose a reason for hiding this comment

jfsiii left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 21, 2020

💚 Build Succeeded

History

nnamdifrankie commented Apr 18, 2020 •

edited

Loading

nnamdifrankie Apr 20, 2020 •

edited

Loading

jfsiii Apr 20, 2020 •

edited

Loading

nnamdifrankie Apr 21, 2020 •

edited

Loading