-
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
[Infrastructure UI] Host limit telemetry #155726
[Infrastructure UI] Host limit telemetry #155726
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
cf662b4
to
2224bf6
Compare
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
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
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 🚀
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
@@ -24,6 +24,7 @@ const configSchema = schema.object({ | |||
'Host Entry Clicked', // Worst-case scenario once per second - AT RISK, | |||
'Host Flyout Filter Removed', // Worst-case scenario once per second - AT RISK, | |||
'Host Flyout Filter Added', // Worst-case scenario once per second - AT RISK, | |||
'Host View Total Host Count Retrieved', // Worst-case scenario 1 every 2 seconds |
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.
cc @shahinakmal FYI, this PR adds another custom event to FS
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.
After checking with @shahinakmal, we shouldn't allow more custom events to be sent to FS.
Update:
After that call, we'll make a call on whether the event needs to be whitelisted in FS. |
Hey @crespocarlos @shahinakmal - quick update:
As it stands, without the FullStory custom event - we're not getting the telemetry we need. I'll send an email internally for next steps on this... |
Hey @crespocarlos - we can remove the FS whitelisting here and go ahead with this (we'll try to live without the FS custom events given the throttling issues). OK to proceed on this one? @shahinakmal FYI - we'll remove it and go ahead as suggested (i.e. we'll use the telemetry cluster instead of FS for these custom events). |
@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.
Thank you for iterating! LGTM
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
closes: elastic#155567 ## Summary This PR adds a new custom event to track the total number of hosts, as well as adjusts a few `data-test-subj` attribute values to meet the naming convention defined in the observability-dev [docs](https://github.com/elastic/observability-dev/blob/main/docs/how-we-work/telemetry/telemetry-convention.md#naming-convention) ### For Reviewers An option for not allowing yet a new custom event in FS could be triggering the new custom events only for self-managed customers, and in FS watch the element that holds the total number of hosts. But for now, I decided to allow the new custom event in FS for consistency --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 937912b)
💚 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.8`: - [[Infrastructure UI] Host limit telemetry (#155726)](#155726) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Carlos Crespo","email":"crespocarlos@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-05-11T15:44:38Z","message":"[Infrastructure UI] Host limit telemetry (#155726)\n\ncloses: https://github.com/elastic/kibana/issues/155567\r\n\r\n## Summary\r\n\r\nThis PR adds a new custom event to track the total number of hosts, as\r\nwell as adjusts a few `data-test-subj` attribute values to meet the\r\nnaming convention defined in the observability-dev\r\n[docs](https://github.com/elastic/observability-dev/blob/main/docs/how-we-work/telemetry/telemetry-convention.md#naming-convention)\r\n\r\n\r\n### For Reviewers\r\n\r\nAn option for not allowing yet a new custom event in FS could be\r\ntriggering the new custom events only for self-managed customers, and in\r\nFS watch the element that holds the total number of hosts. But for now,\r\nI decided to allow the new custom event in FS for consistency\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"937912b056f5876247f27a7baad5314401e65939","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Metrics UI","Team:Infra Monitoring UI","release_note:skip","backport:prev-minor","Feature:ObsHosts","v8.9.0"],"number":155726,"url":"https://github.com/elastic/kibana/pull/155726","mergeCommit":{"message":"[Infrastructure UI] Host limit telemetry (#155726)\n\ncloses: https://github.com/elastic/kibana/issues/155567\r\n\r\n## Summary\r\n\r\nThis PR adds a new custom event to track the total number of hosts, as\r\nwell as adjusts a few `data-test-subj` attribute values to meet the\r\nnaming convention defined in the observability-dev\r\n[docs](https://github.com/elastic/observability-dev/blob/main/docs/how-we-work/telemetry/telemetry-convention.md#naming-convention)\r\n\r\n\r\n### For Reviewers\r\n\r\nAn option for not allowing yet a new custom event in FS could be\r\ntriggering the new custom events only for self-managed customers, and in\r\nFS watch the element that holds the total number of hosts. But for now,\r\nI decided to allow the new custom event in FS for consistency\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"937912b056f5876247f27a7baad5314401e65939"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/155726","number":155726,"mergeCommit":{"message":"[Infrastructure UI] Host limit telemetry (#155726)\n\ncloses: https://github.com/elastic/kibana/issues/155567\r\n\r\n## Summary\r\n\r\nThis PR adds a new custom event to track the total number of hosts, as\r\nwell as adjusts a few `data-test-subj` attribute values to meet the\r\nnaming convention defined in the observability-dev\r\n[docs](https://github.com/elastic/observability-dev/blob/main/docs/how-we-work/telemetry/telemetry-convention.md#naming-convention)\r\n\r\n\r\n### For Reviewers\r\n\r\nAn option for not allowing yet a new custom event in FS could be\r\ntriggering the new custom events only for self-managed customers, and in\r\nFS watch the element that holds the total number of hosts. But for now,\r\nI decided to allow the new custom event in FS for consistency\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>","sha":"937912b056f5876247f27a7baad5314401e65939"}}]}] BACKPORT--> Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>
closes: #155567
Summary
This PR adds a new custom event to track the total number of hosts, as well as adjusts a few
data-test-subj
attribute values to meet the naming convention defined in the observability-dev docsFor Reviewers
An option for not allowing yet a new custom event in FS could be triggering the new custom events only for self-managed customers, and in FS watch the element that holds the total number of hosts. But for now, I decided to allow the new custom event in FS for consistency