-
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
[Code] frontend telemetry #46154
[Code] frontend telemetry #46154
Conversation
Pinging @elastic/code |
This is a follow up PR of #45467 |
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! Mostly are good. leave some minor comments below
@@ -129,6 +130,7 @@ export class CodeFileTree extends React.Component<Props, State> { | |||
const path = flattenFrom ? flattenFrom.path! : node.path!; | |||
this.toggleTree(path); | |||
this.onClick(node); | |||
trackCodeUiMetric(METRIC_TYPE.COUNT, 'fileTree'); |
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.
can we add the metrics name/enum to this place: https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/code/model/usage_telemetry_metrics.ts#L7, instead of hardwiring them in code?
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.
And it would be great if you can elaborate on what does each metric mean there as comments.
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
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.
@Bamieh Can you take a look on this as well?
@@ -8,4 +8,12 @@ export enum CodeUsageMetrics { | |||
ENABLED = 'enabled', | |||
REPOSITORIES = 'repositories', | |||
LANGUAGE_SERVERS = 'langserver', | |||
ADMIN_PAGE_LOAD_COUNT = 'adminPageLoadCount', |
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.
one last comment before merging this PR. Let's separate the metrics for backend and frontend. Moves these new ones to sth like CodeUsageUiMetrics
?
💚 Build Succeeded |
@Bamieh friendly ping on the review. |
Summary
click counts on
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)- [ ] Documentation was added for features that require explanation or tutorials- [ ] Unit or functional tests were updated or added to match the most common scenarios- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately