-
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
[Step 3] VisEditors Telemetry enhancements (add new agg-based and lens telemetries) #135615
Conversation
de5557a
to
a442730
Compare
fe2609a
to
c299d4f
Compare
@@ -442,7 +442,7 @@ export class VisualizeEmbeddable | |||
private async updateHandler() { | |||
const parentContext = this.parent?.getInput().executionContext || getExecutionContext().get(); | |||
const child: KibanaExecutionContext = { | |||
type: 'visualization', | |||
type: 'agg_based', |
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.
For people who will review: Please pay attention to this.
@@ -34,7 +34,7 @@ export const getGoalVisTypeDefinition = ( | |||
addTooltip: true, | |||
addLegend: false, | |||
isDisplayWarning: false, | |||
type: 'gauge', | |||
type: 'goal', |
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.
For people who will review: Please pay attention to this.
renderComplete={handlers.done} | ||
renderComplete={() => { | ||
// Renaming according to business requirements | ||
const visTypeTelemetryMap: Record<string, string> = { |
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.
For people who will review: Please pay attention to this.
@elasticmachine merge upstream |
28ee150
to
814923d
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.
kibana-gis changes LGTM after changes map
to regionmap
.
x-pack/plugins/maps/public/lens/choropleth_chart/expression_renderer.tsx
Outdated
Show resolved
Hide resolved
const renderComplete = () => { | ||
const executionContext = handlers.getExecutionContext(); | ||
const containerType = extractContainerType(executionContext); | ||
const visualizationType = extractVisualizationType(executionContext); | ||
|
||
if (containerType && visualizationType) { | ||
plugins.usageCollection?.reportUiCounter(containerType, METRIC_TYPE.COUNT, [ | ||
`render_${visualizationType}_metric`, | ||
]); | ||
} | ||
|
||
handlers.done(); | ||
}; |
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.
Great.
@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.
Response Ops changes LGTM! Code review only.
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, thanks for the tests addition
Co-authored-by: Michael Dokolin <dokmic@gmail.com>
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.
Expressions plugin changes LGTM 👍
|
|
|
|
I can't reproduce the multi-counter anymore, maybe it got fixed by an unrelated change. However, I noticed another problem - all charts rendering the default "no results found" view (e.g. tsvb table or agg based pie, but not TSVB timeseries) are not emitting a counter event at all |
@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.
Tested and didn't find any further issues, LGTM. Can't wait to tap that data!
# Conflicts: # x-pack/plugins/lens/public/drag_drop/drag_drop.tsx # x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx # x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx # x-pack/plugins/lens/public/lens_ui_telemetry/index.ts # x-pack/plugins/lens/public/plugin.ts
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Canvas Sharable Runtime
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: cc @alexwizp |
Part of #132396