-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
feat(investigation): add usage collector #197659
feat(investigation): add usage collector #197659
Conversation
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
3b77d02
to
215979c
Compare
f89344c
to
494da04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for telemetry and added dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM!
@kdelemme I haven't worked with snapshot telemetry, how can I test it locally?
const finder = context.soClient.createPointInTimeFinder<StoredInvestigation>({ | ||
type: SO_INVESTIGATION_TYPE, | ||
perPage: 1000, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be potentially very memory heavy task, if we let's say store lot of data as part of investigation, i imagine storing images, screenshots, lens embeddables attributes JSON, LLM output, all of that data is stored in items right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's possible indeed - I can reduce the batch size to something smaller, like 10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the status part could be fetched as an aggregation, the length of notes and items arrays is tricky to get since that isn't mapped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah let's reduce the perPage for now. i can't think of anything else. since we can't use fields
as well. If we really want to optimise, we can save length of items and notes as separate fields and get those. that way items and notes we won't have to fetch. but i guess it might be overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed with 89bc326
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💚 Build Succeeded
Metrics [docs]
History
cc @kdelemme |
Starting backport for target branches: 8.x |
(cherry picked from commit 5b9908e)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [feat(investigation): add usage collector (#197659)](#197659) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kevin Delemme","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-12T18:56:01Z","message":"feat(investigation): add usage collector (#197659)","sha":"5b9908ee622506c35873d4d8ae74a21e9b46d6e4","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","v8.17.0"],"title":"feat(investigation): add usage collector","number":197659,"url":"https://github.com/elastic/kibana/pull/197659","mergeCommit":{"message":"feat(investigation): add usage collector (#197659)","sha":"5b9908ee622506c35873d4d8ae74a21e9b46d6e4"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197659","number":197659,"mergeCommit":{"message":"feat(investigation): add usage collector (#197659)","sha":"5b9908ee622506c35873d4d8ae74a21e9b46d6e4"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Kevin Delemme <[email protected]>
Resolves #191644
🌸 Summary
This PR collects usage of the investigation app when enabled.
The data we collect is: