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] Add additional properties to attack discovery telemetry #182249

Merged
merged 7 commits into from
May 2, 2024

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented May 1, 2024

Summary

Adds 3 new properties to the reportAttackDiscoveriesGenerated telemetry event: durationMs, alertsCount, and configuredAlertsCount. These fields represent how long the request took, how many alerts were used for the generation, and how many alerts were configured by the user for the generation.

I also added the provider and model arguments which were already defined, but not passed. Here is an example of what this looks like for Bedrock and Azure:

Screenshot 2024-05-01 at 12 56 07 PM Screenshot 2024-05-01 at 12 57 06 PM

CODEOWNERS change

Added the attack_discovery dir to be under @elastic/security-generative-ai ownership 👍

@stephmilovic stephmilovic added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Security Generative AI Security Generative AI v8.14.0 v8.15.0 Feature:Attack Discovery Attack discovery uses generative AI to identify active attacks labels May 1, 2024
@stephmilovic stephmilovic requested a review from a team as a code owner May 1, 2024 14:34
@elasticmachine
Copy link
Contributor

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

reportAttackDiscoveriesGenerated({
actionTypeId,
durationMs,
alertCount: knowledgeBase.latestAlerts,
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider something like:

      uniq(
        selectedConnectorAttackDiscoveries.flatMap((attackDiscovery) => attackDiscovery.alertIds)
      ).length,

to calculate alertCount, and adding an additional stat like configuredAlertsCount to represent the value of knowledgeBase.latestAlerts.

@@ -11,6 +11,7 @@ import {
useAssistantContext,
useLoadConnectors,
} from '@kbn/elastic-assistant';
import { uniq } from 'lodash';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consider lodash/fp

alertsCount: {
type: 'integer',
_meta: {
description: 'Number of alerts evaluated',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Consider something like Number of unique alerts referenced in the attack discoveries, because this may be less than the number of alerts evaluated.

Comment on lines 30 to 41
if (!connector?.isPreconfigured) {
const config = (connector as ActionConnectorProps<GenAiConfig, unknown>)?.config;
if (config?.apiProvider === OpenAiProviderType.AzureAi) {
return {
...config,
defaultModel: getAzureApiVersionParameter(config.apiUrl ?? ''),
};
}

return (connector as ActionConnectorProps<GenAiConfig, unknown>)?.config;
}
return undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider something like the following to eliminate the second cast and most references to config:

  if (!connector?.isPreconfigured) {
    const config = (connector as ActionConnectorProps<GenAiConfig, unknown>)?.config;
    const { apiProvider, apiUrl, defaultModel } = config ?? {};

    return {
      apiProvider,
      apiUrl,
      defaultModel:
        apiProvider === OpenAiProviderType.AzureAi
          ? getAzureApiVersionParameter(apiUrl ?? '')
          : defaultModel,
    };
  }

  return undefined; // the connector is neither available nor editable

@stephmilovic stephmilovic requested a review from a team as a code owner May 2, 2024 03:30
Copy link
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

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

Thanks for the additional telemetry @stephmilovic!
✅ Desk tested locally
LGTM 🚀

@stephmilovic stephmilovic enabled auto-merge (squash) May 2, 2024 03:55
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

The CI Stats report is too large to be displayed here, check out the CI build annotation for this information.

History

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

@stephmilovic stephmilovic merged commit e306074 into elastic:main May 2, 2024
35 checks passed
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.14 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.14:
- Don't render exceptions flyout if data is loading (#181588)
- [Obs AI Assistant] register alert details context in observability plugin (#181501)
- [Security Solution][Endpoint] Add validation to ensure a SentinelOne agent ID exists when enabling the Respond Take Action option (#182158)
- [Obs AI Assistant] Remove unneeded plugin references (#182116)

Manual backport

To create the backport manually run:

node scripts/backport --pr 182249

Questions ?

Please refer to the Backport tool documentation

@stephmilovic
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.14

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

stephmilovic added a commit to stephmilovic/kibana that referenced this pull request May 8, 2024
…emetry (elastic#182249)

(cherry picked from commit e306074)

# Conflicts:
#	.github/CODEOWNERS
stephmilovic added a commit that referenced this pull request May 8, 2024
…ery telemetry (#182249) (#182997)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Security solution] Add additional properties to attack discovery
telemetry (#182249)](#182249)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"stephanie.milovic@elastic.co"},"sourceCommit":{"committedDate":"2024-05-02T04:48:36Z","message":"[Security
solution] Add additional properties to attack discovery telemetry
(#182249)","sha":"e306074aa9e8943ab342270a6f795e17ee37c1da","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:
SecuritySolution","Team:Security Generative
AI","v8.14.0","v8.15.0","Feature:Attack
Discovery"],"number":182249,"url":"#182249
solution] Add additional properties to attack discovery telemetry
(#182249)","sha":"e306074aa9e8943ab342270a6f795e17ee37c1da"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182249
solution] Add additional properties to attack discovery telemetry
(#182249)","sha":"e306074aa9e8943ab342270a6f795e17ee37c1da"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Attack Discovery Attack discovery uses generative AI to identify active attacks release_note:skip Skip the PR/issue when compiling release notes Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants