[Security Solution] AI Assistant telemetry#162653
Merged
stephmilovic merged 46 commits intoelastic:mainfrom Aug 15, 2023
Merged
[Security Solution] AI Assistant telemetry#162653stephmilovic merged 46 commits intoelastic:mainfrom
stephmilovic merged 46 commits intoelastic:mainfrom
Conversation
stephmilovic
added
release_note:skip
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Contributor
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
stephmilovic
commented
Jul 27, 2023
| }, | ||
| "actionTypeId": { | ||
| "type": "keyword" | ||
| }, |
Contributor
Author
There was a problem hiding this comment.
I made a change in x-pack/plugins/actions/server/saved_objects/mappings.ts, so I believe i will need it here too?
Contributor
There was a problem hiding this comment.
We'd really prefer the mappings not be extended. Can you not access this field from the script in action_telemetry.ts? If for some reason it's not in doc, it should be in source. I suspect you could also "add" a field during the telemetry query as a runtime-field added to the search.
stephmilovic
commented
Jul 27, 2023
| setActiveTab(TimelineTabs.securityAssistant); | ||
| }, [setActiveTab]); | ||
| if (activeTab !== TimelineTabs.securityAssistant) { | ||
| reportAssistantInvoked({ |
Contributor
Author
There was a problem hiding this comment.
I tried to do all of the telemetry from the assistant package, but the rendering in timeline proved difficult and I did not want a bunch of consumer related code (ie: isTimeline) in the assistant package. Therefore, this is the sole EBT tracker called from security solution for assistant
stephmilovic
commented
Jul 27, 2023
| state.types.put(actionType, state.types.containsKey(actionType) ? state.types.get(actionType) + 1 : 1); | ||
| if (actionType =~ /.gen-ai/) { | ||
| String genAiActionType = actionType +"__"+ doc['action.config.apiProvider'].value; | ||
| state.types.put(genAiActionType, state.types.containsKey(genAiActionType) ? state.types.get(genAiActionType) + 1 : 1); |
Contributor
Author
There was a problem hiding this comment.
@ymao1 The only enhancement I needed to make to this telemetry was to answer the question, what apiProvider is the connector? In order to stay out of the telemetry repo and change as little as possible, I decided to split the .gen-ai connectors count up by the provider. Please let me know if you agree with that approach or if there is any impact I may not have considered? I'm wondering about the response-ops data view and if I need to do something to get those fields to be in that data view by default, I currently added them as Runtime Fields to get my dashboard working
before:
count_by_type: {
'__gen-ai': 2,
}
after:
count_by_type: {
'__gen-ai__Azure OpenAI': 1,
'__gen-ai__OpenAI': 1
}
In my dashboard, I accounted for the sum of the three possible values to count the total of GenAI connectors for pre 8.10 data:
Contributor
Author
There was a problem hiding this comment.
As discussed, I am now creating a new field countGenAiProviderTypes. I am still doing the key as explained above in painless, but putting the data in the agreed structure with JS below. This is tested in actions_telemetry.test.ts by it('getCounts' ...
pmuellr
requested changes
Jul 27, 2023
Contributor
pmuellr
left a comment
pmuellr
left a comment
There was a problem hiding this comment.
I think we'd prefer to not modify the connector mappings. Changing the telemetry keys for gen-ai to include the provider in they key could also be problematic/confusing.
Makes me wonder if we should really be doing this as a completely separate piece of telemetry, vs part of the connector telemetry.
Not sure. Thoughts @mikecote?
| count_connector_types_by_action_run_outcome_per_day: schema.recordOf( | ||
| schema.string(), | ||
| schema.number() | ||
| schema.recordOf(schema.string(), schema.number()) |
Contributor
There was a problem hiding this comment.
huh, same change is being made in this PR #161096 ...
| }, | ||
| "actionTypeId": { | ||
| "type": "keyword" | ||
| }, |
Contributor
There was a problem hiding this comment.
We'd really prefer the mappings not be extended. Can you not access this field from the script in action_telemetry.ts? If for some reason it's not in doc, it should be in source. I suspect you could also "add" a field during the telemetry query as a runtime-field added to the search.
pmuellr
approved these changes
Aug 2, 2023
Contributor
pmuellr
left a comment
pmuellr
left a comment
There was a problem hiding this comment.
LGTM - noticed there's a typo I often make in regexp's; not escaping ., as typically it matches any single char. Code will work as is, since . matches "any single char", but we probably want to fix that, as it matches too widely.
| }): { fn: keyof AssistantTelemetry; params: AssistantTelemetry[keyof AssistantTelemetry] } => | ||
| fn({ | ||
| ...rest, | ||
| conversationId: getAnonymizedConversationId(conversationId), |
Contributor
There was a problem hiding this comment.
this maps all non-default conversationIds that flow through the useAssistantTelemetry hook to a single value: Custom
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
|
I did an experiment on this PR, added an extra telemetry field ✅ Data can be found on staging Test 1assistant_telemetry.movTest 2assistant_telemetry_2.mov |
andrew-goldstein
approved these changes
Aug 14, 2023
Contributor
andrew-goldstein
left a comment
andrew-goldstein
left a comment
There was a problem hiding this comment.
Thanks @stephmilovic for adding this telemetry 🙏
LGTM
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
Author
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Assistant InvokedAssistant Message SentAssistant Quick PromptUpdate 8/2: @andrew-goldstein pointed out that user input should not be tracked, and we allow custom input for both
conversationIdand quick prompt titles. I added a commit to mask any user input id/quick prompt title with "Custom"apiProviderin a new field in theresponse-opstelemetry index this field is mapped to:connectors.all.totalByGenAiProviderTypetelemetry-v2-staging:I've created a gorgeous Dashboard to show off this new telemetry. The top 3 visualizations are from the connector usage collector telemetry (Data View:
response-ops) and the bottom 3 visualizations are the new EBT (Data View:ebt-kibana-browser)