Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security_Solution][Telemetry] - Update endpoint usage to use agentService #93829

Merged
merged 5 commits into from
Mar 10, 2021

Conversation

michaelolo24
Copy link
Contributor

Summary

This PR addresses the first point made by @nchaulet in this issue: #92311

We were previously accessing the saved_objects for fleet directly to populate endpoint usage telemetry, but should have been using the agentService where possible. This PR adopts that change to continue to reliably provide us with the total_installed and os details information we currently have available for telemetry.

The outstanding issue remains that there is no way to currently reliably obtain the information we are getting from the fleet-agent-events savedObject which helps to populate the active_within_last_24_hours and policies.malware data. With the move to fleet server as detailed in the aforementioned issue, our use of that savedObject will break and we will no longer get this data. Ideally, we'll be able to determine a solution that allows us to retain this information and is better implemented than the current scenario (also detailed here: https://github.com/elastic/endpoint-dev/issues/8382)

Checklist

Delete any items that are not applicable to this PR.

@michaelolo24 michaelolo24 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.13.0 labels Mar 5, 2021
@michaelolo24 michaelolo24 requested a review from a team as a code owner March 5, 2021 19:58
@michaelolo24 michaelolo24 self-assigned this Mar 5, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

};

/*
TODO: AS OF 7.13, this access will no longer work due to the enabling of fleet server. An alternative route will have
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break in 7.13

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also put this information in an issue?

When it breaks in 7.13 will it have an adverse affect on the plugin? Or will it just log and fail silently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't log to prevent accidental leaking of PII, but yea if it stays as is for 7.13, we'll only get the total_installed and os details. Everything else will fail silently. Yea, I'm gonna add it into the issue that @nchaulet created that's referenced in the description here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathan-buttner actually went with a comment here to avoid splitting the discussion

@@ -108,7 +109,7 @@ export const updateEndpointOSTelemetry = (
* the same time span.
*/
export const updateEndpointDailyActiveCount = (
latestEndpointEvent: SavedObject<NewAgentEvent>,
latestEndpointEvent: SavedObject<NewAgentEvent>, // TODO: This information will be lost in 7.13, need to find an alternative route.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break in 7.13

@michaelolo24
Copy link
Contributor Author

@elasticmachine merge upstream

agentService: AgentService | undefined,
esClient: ElasticsearchClient
) => {
const agentData = await agentService?.listAgents(savedObjectsClient, esClient, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just merged the change where we remove the support for saved object, I think this method is probably not accepting a savedObjectClient any more

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the note. I'll update this accordingly!

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good you probably want to merge master, before merging

@michaelolo24
Copy link
Contributor Author

@elasticmachine merge upstream

@michaelolo24
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @michaelolo24

@michaelolo24
Copy link
Contributor Author

michaelolo24 commented Mar 10, 2021

Discussed with @achuguy who wrote the current endpoint_telemetry functional tests. We're deciding to skip them for now until we finalize how telemetry will be handled moving forward. I'll be creating an issue to update these tests

michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Mar 10, 2021
michaelolo24 added a commit that referenced this pull request Mar 10, 2021
…rvice (#93829) (#94366)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit that referenced this pull request Mar 11, 2021
…-action

* 'master' of github.com:elastic/kibana: (43 commits)
  [Console] Update copy when showing warnings in response headers (#94270)
  [TSVB] Type public code. Step 1 (#93231)
  [ML] Functional tests - stabilize slider value selection (#94313)
  skip another suite blocking es promotion (#94367)
  [Security Solution] Eliminates a redundant external link icon (#94194)
  skip another suite blocking es promotion (#94367)
  [App Search] Role mappings migration part 1 (#94346)
  [Security Solution][Detections] Fix flaky indicator enrichment tests (#94241)
  [Workplace Search] Deduplicate icons (#94359)
  [ML] Add latest transform to intro text (#94039)
  skip test failing es promotion (#94367)
  [Maps] convert elasticsearch_utils to TS (#93984)
  [Security_Solution][Telemetry] - Update endpoint usage to use agentService (#93829)
  [Security Solution][Exceptions] Fixes OS adding method for exception enrichment (#94343)
  [ILM] Add support for frozen phase (#93068)
  [App Search] Fixed 2 relevance tuning bugs (#94312)
  remove `try` auth mode (#94287)
  Removing resolver functional tests (#94331)
  migrate warning mixin to core (#94273)
  [App Search] Add routes for Role Mappings (#94221)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/cold_phase/cold_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/phase/phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_issues_context.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants