-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Visualizations] Simplify visualization telemetry #77145
[Visualizations] Simplify visualization telemetry #77145
Conversation
@elasticmachine merge upstream |
Pinging @elastic/kibana-app (Team:KibanaApp) |
@elasticmachine merge upstream |
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
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! I have tested it on Safari:
-
Remove all tasks on DevTools:
POST /.kibana_task_manager/_delete_by_query { "query": { "match_all": {} } } -
Made a change to a server file in kibana
-
Went to Management-->Advanced Setting->Telemetry-->See an example of what we collect and check the vis data
Can't see any regressions. 🙂
@flash1293 can you also check it? 🙏
💚 Build SucceededBuild metricsdistributable file count
History
To update your PR or re-run it, just comment with: |
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.
Tested with multiple spaces and works as expected, LGTM
* Move visualizations telemetry into visualizations plugin * Remove x-pack oss_telemetry plugin * Add unit tests * Update docs * Revert not related changes Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/oss_telemetry/server/test_utils/index.ts
}; | ||
const esResponse: ESResponse = await callCluster('search', searchParams); | ||
const size = get(esResponse, 'hits.hits.length'); | ||
if (size < 1) { |
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.
@sulemanof I've noticed this error ever since this PR has been merged (for fresh installed clusters):
server log [10:30:55.212] [warning][collector-set][plugins][usageCollection] TypeError: Cannot read property 'hits' of undefined
at getStats (/src/plugins/visualizations/server/usage_collector/get_usage_collector.ts:64:54)
at process._tickCallback (internal/process/next_tick.js:68:7)
server log [10:30:55.216] [warning][collector-set][plugins][usageCollection] Unable to fetch data from visualization_types collector
The reason: size
is undefined
and undefined < 1
is false
.
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've created #77478 to tackle this
Summary
Resolves #56986
visualizations
plugin;oss_telemetry
plugin;Checklist
Delete any items that are not applicable to this PR.
For maintainers