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] Cleanup endpoint telemetry #71950

Merged
merged 3 commits into from
Jul 20, 2020

Conversation

michaelolo24
Copy link
Contributor

@michaelolo24 michaelolo24 commented Jul 15, 2020

Summary

This PR is to improve the logic for endpoint usage telemetry.

  1. It removes an unnecessary reduce and confines all updating activities to a single loop
  2. It cleans up our savedObjects requests to only require the necessary details for our telemetry
  3. Improves policy usage logic and provides error handling in the case of missing data and request failures
  4. Added tests

Checklist

For maintainers

@michaelolo24 michaelolo24 force-pushed the optimize-endpoint-telemetry branch from b502aa4 to bba51b4 Compare July 15, 2020 20:47
@michaelolo24 michaelolo24 force-pushed the optimize-endpoint-telemetry branch from bba51b4 to 9175e9e Compare July 16, 2020 17:35
@@ -69,13 +71,14 @@ export const getDefaultEndpointTelemetry = (): EndpointUsage => ({
});

/**
* @description this fun
* @description this function updates the os telemetry. We use the fullName field as the key as it contains the name and version details.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oatkiller fixed 😆

type: AGENT_EVENT_SAVED_OBJECT_TYPE,
filter: `${AGENT_EVENT_SAVED_OBJECT_TYPE}.attributes.agent_id: ${agentId} and ${AGENT_EVENT_SAVED_OBJECT_TYPE}.attributes.message: "${FLEET_ENDPOINT_PACKAGE_CONSTANT}"`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed that initial agent.id check as it's redundant giving the search and searchFields parameters below

@@ -29,9 +37,11 @@ export const getLatestFleetEndpointEvent = async (
agentId: string
) =>
savedObjectsClient.find<AgentEventSOAttributes>({
// Get the most recent endpoint event.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oatkiller, after speaking with @rudolf yesterday, there doesn't seem to be a way we can really batch this. I.e. we cannot say "given a list of agent id's, give me the most recent event for each id".

Thinking about it more, we may be able to say, "give me every event for each of these agent id's using an or in the filter, but that would be a massive response to then filter". This will definitely need perf testing

endpointTelemetry: EndpointUsage
): Promise<EndpointUsage> => {
const updatedEndpointTelemetry = { ...endpointTelemetry };
export const updateEndpointDailyActiveCount = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oatkiller, moved the daily count logic here

if (agentId) {
let agentEvents;
try {
const response = await getLatestFleetEndpointEvent(soClient, agentId);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will gladly take any suggestions people may have on optimizing this, as currently there is the potential for us to make 10000 of these requests if there are that many agents.

} else if (isAnActiveMalwareState && failedToEnable) {
updatedPoliciesTracker.malware.failure += 1;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently tracking the situation where an endpoint failed to turn the policy off as it's unlikely / not a high priority. Also, since we're in FF, I'm not sure if I can add an additional field to telemetry, @afharo?

Copy link
Member

Choose a reason for hiding this comment

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

since we're in FF, I'm not sure if I can add an additional field to telemetry, @afharo?

If it's a bug, I guess you can add it. If it isn't, you'll have to wait :)

@michaelolo24 michaelolo24 marked this pull request as ready for review July 16, 2020 17:50
@michaelolo24 michaelolo24 requested review from a team as code owners July 16, 2020 17:50
@michaelolo24 michaelolo24 changed the title cleanup endpoint telemetry [Security Solution] Cleanup endpoint telemetry Jul 16, 2020
@michaelolo24 michaelolo24 added release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.9.0 v8.0.0 labels Jul 16, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

Copy link
Contributor

@oatkiller oatkiller left a comment

Choose a reason for hiding this comment

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

thanks!

@michaelolo24
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@michaelolo24 michaelolo24 merged commit 2094f33 into elastic:master Jul 20, 2020
@michaelolo24 michaelolo24 deleted the optimize-endpoint-telemetry branch July 20, 2020 16:28
michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jul 20, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jul 20, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jul 20, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 20, 2020
* master: (60 commits)
  [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335)
  [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337)
  [Resolver] no longer pass related event stats to process node component (elastic#72435)
  Revert "skip flaky suite (elastic#72146)"
  [Security Solution] Cleanup endpoint telemetry (elastic#71950)
  Unskip dashboard embeddable rendering tests (elastic#71824)
  [ENDPOINT] Added unerolling status for host. (elastic#72303)
  [Alerting][Connectors] Increase the size of the logos (elastic#72419)
  [SECURITY] [Timeline] Raw events not displayed (elastic#72387)
  [ML] Fixes display of regression stop stats if one is NaN (elastic#72412)
  [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239)
  Fix match phrase and not match phrase comparators (elastic#71850)
  [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040)
  [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152)
  [Resolver] Selector performance (elastic#72380)
  [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026)
  [Ingest Manager] Do not bumb config revision during config creation (elastic#72270)
  [ML] Adding missing index pattern name to new job wizards (elastic#72400)
  [ML] improve annotation flyout performance (elastic#72299)
  [APM] Testing error rate API and restructuring folders (elastic#72257)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 20, 2020
* master: (26 commits)
  [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335)
  [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337)
  [Resolver] no longer pass related event stats to process node component (elastic#72435)
  Revert "skip flaky suite (elastic#72146)"
  [Security Solution] Cleanup endpoint telemetry (elastic#71950)
  Unskip dashboard embeddable rendering tests (elastic#71824)
  [ENDPOINT] Added unerolling status for host. (elastic#72303)
  [Alerting][Connectors] Increase the size of the logos (elastic#72419)
  [SECURITY] [Timeline] Raw events not displayed (elastic#72387)
  [ML] Fixes display of regression stop stats if one is NaN (elastic#72412)
  [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239)
  Fix match phrase and not match phrase comparators (elastic#71850)
  [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040)
  [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152)
  [Resolver] Selector performance (elastic#72380)
  [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026)
  [Ingest Manager] Do not bumb config revision during config creation (elastic#72270)
  [ML] Adding missing index pattern name to new job wizards (elastic#72400)
  [ML] improve annotation flyout performance (elastic#72299)
  [APM] Testing error rate API and restructuring folders (elastic#72257)
  ...
michaelolo24 added a commit that referenced this pull request Jul 20, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
michaelolo24 added a commit that referenced this pull request Jul 20, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 21, 2020
…feature-privileges

* alerting/consumer-based-rbac: (45 commits)
  fixed alerts test
  [SIEM][Detection Engine][Lists] Adds list permissions (elastic#72335)
  [SIEM][Detection Engine][Lists] Adds conflict versioning and io-ts improvements to lists (elastic#72337)
  [Resolver] no longer pass related event stats to process node component (elastic#72435)
  Revert "skip flaky suite (elastic#72146)"
  [Security Solution] Cleanup endpoint telemetry (elastic#71950)
  Unskip dashboard embeddable rendering tests (elastic#71824)
  [ENDPOINT] Added unerolling status for host. (elastic#72303)
  [Alerting][Connectors] Increase the size of the logos (elastic#72419)
  [SECURITY] [Timeline] Raw events not displayed (elastic#72387)
  [ML] Fixes display of regression stop stats if one is NaN (elastic#72412)
  [Ingest Pipelines] Processor Editor Move Tooltip (elastic#72239)
  Fix match phrase and not match phrase comparators (elastic#71850)
  [Plugin Generator] Generate tsconfig and useDefaultBehaviors (elastic#72040)
  [Security Solution][Timeline] Fix timeline styling and createFrom beh… (elastic#72152)
  allow user to disable alert even if they dont have privileges to the underlying action
  [Resolver] Selector performance (elastic#72380)
  [Ingest Manager] Set `_meta` in the index.mappings (elastic#72026)
  [Ingest Manager] Do not bumb config revision during config creation (elastic#72270)
  [ML] Adding missing index pattern name to new job wizards (elastic#72400)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants