[Cloud Security] Adding telemetry collection condition based on render condition by JordanSh · Pull Request #208758 · elastic/kibana

JordanSh · 2025-01-29T14:43:57Z

Summary

Insight components telemetry is now reporting based on the rendering condition of the component

elasticmachine · 2025-01-29T14:43:59Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

maxcold · 2025-01-30T10:25:52Z

...urity_solution/public/flyout/document_details/shared/components/misconfiguration_insight.tsx

-  const hasMisconfigurationFindings = totalFindings > 0;
+  const shouldRender = totalFindings > 0; // this component only renders if there are findings
+
+  useEffect(() => {


While it's a fix over the previous version where we tracked even when the particular insight wasn't even rendered, I wonder what do we want to track overall? With this change the counter will go up with every re-render of the component, for example when a user expands the left pane, switches between Overview, Table, JSON tabs and who knows when else as it's hard to reason about reliable component rerender in React. If our goal is to understand how many users had the component showing up at least once, I guess the current approach would do. But it's harder to make more complex conclusions from this telemetry. Probably not for this PR, but we have a preview link in the component from the counter. We could count clicks to track actual interactions with the component. Or if we want to see for how many alerts per user the data is present, we need to add an alert uuid to tracking to be able to then group by re-renders for the same alerts

good point max. I believe this was solved by [Cloud Security] Collecting telemetry of graph visualization usage

I’m curious why the condition is checked in MisconfigurationsInsight (and in VulnerabilitiesInsight) before calling them. In other words, why not call these functions only if there are findings?

not sure what you mean, this is exactly whats happening. flow is:

Checking we have findings, which is also the render condition

const shouldRender = totalFindings > 0; // this component only renders if there are findings

if shouldRender is true (meaning we have findings), report metric

if (shouldRender && telemetryKey) { uiMetricService.trackUiMetric(METRIC_TYPE.COUNT, telemetryKey); }

Meaning we only call the function if there are findings

@JordanSh I checked and every switch between Overview/Table/JSON tabs still increases the counter. Again, not sure if it's a problem concerning this particular PR, I'm just wondering what we actually want to count with the current approach and if we are able to clean those meaningful counts on the reporting side

@JordanSh I have a PR open to update the count badges: when user clicks on the count, it will go to the respective cloud insight table. If you do want to track clicks, you can do it in that call back.

We can add that as well, i dont think it replaces this view count that we are trying to measure. Thanks for letting me know!
@CohenIdo do we want to add tracker for those counter clicks?

…-fix'

…ry-improvements

…etry

JordanSh · 2025-02-02T12:06:27Z

x-pack/platform/packages/shared/kbn-cloud-security-posture/common/utils/ui_metrics.ts

+export const MISCONFIGURATION_INSIGHT = 'misconfiguration-insight-v2' as const;
+export const VULNERABILITIES_INSIGHT = 'vulnerabilities-insight-v2' as const;


As requested by @CohenIdo, added version to namings so it will be easier to track fixed version telemetry reports

JordanSh · 2025-02-02T12:07:02Z

/ci

CohenIdo · 2025-02-02T14:23:27Z

@JordanSh, please add a backport to 8.x and 9.0, we should ensure it’s included in the next BC. Otherwise, we might lose track of it for the upcoming release.

christineweng · 2025-02-05T21:31:06Z

...urity_solution/public/flyout/document_details/shared/components/misconfiguration_insight.tsx

+  const shouldRender = totalFindings > 0; // this component only renders if there are findings
+
+  useEffect(() => {
+    if (shouldRender && telemetryKey) {


Have you considered typing telemetryKey it as required? Looks like it is always passed

~~yeah makes sense, can do that~~

ended up not doing that, it isn't required for tests, and in general only required for telemetry and not for the component it self in order to function. for those reasons i've decided to keep it optional for now.

christineweng · 2025-02-05T21:37:38Z

...urity_solution/public/flyout/document_details/shared/components/misconfiguration_insight.tsx

-  const hasMisconfigurationFindings = totalFindings > 0;
+  const shouldRender = totalFindings > 0; // this component only renders if there are findings
+
+  useEffect(() => {


@JordanSh I have a PR open to update the count badges: when user clicks on the count, it will go to the respective cloud insight table. If you do want to track clicks, you can do it in that call back.

…-fix'

JordanSh · 2025-02-11T12:31:04Z

/ci

elasticmachine · 2025-02-11T14:08:53Z

💚 Build Succeeded

Buildkite Build
Commit: 12eb8de

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	21.4MB	21.4MB	+55.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	92.3KB	92.3KB	+6.0B

History

💔 Build #275060 failed 12eb8de
💔 Build #272513 failed b972974
💔 Build #272511 failed 1749092
💔 Build #272444 failed 8927342
💔 Build #272035 failed e46bdd0
💔 Build #271589 failed f27fd78

cc @JordanSh

kibanamachine · 2025-02-11T16:33:23Z

Starting backport for target branches: 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/13267784020

…r condition (elastic#208758) (cherry picked from commit c1aaf6f)

kibanamachine · 2025-02-11T16:40:02Z

💚 All backports created successfully

Status	Branch	Result
✅	8.18
✅	8.x
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

… render condition (#208758) (#210636) # Backport This will backport the following commits from `main` to `9.0`: - [[Cloud Security] Adding telemetry collection condition based on render condition (#208758)](#208758)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jordan <51442161+JordanSh@users.noreply.github.com>

…n render condition (#208758) (#210634) # Backport This will backport the following commits from `main` to `8.18`: - [[Cloud Security] Adding telemetry collection condition based on render condition (#208758)](#208758)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jordan <51442161+JordanSh@users.noreply.github.com>

…on-206439 * main: (402 commits) [Search]: Fix Number type field to have correct property (elastic#210462) Change filter for rule monitoring gaps (elastic#209983) Update Logs Explorer deprecation messages (elastic#201307) [APM] Remove `error.id` in `getErrorGroupMainStatistics` query as it's not used (elastic#210613) [Embeddable] Fix presentation panel styles (elastic#210113) [ci] enable Scout reporter for on-merge-unsupported-ftrs (elastic#210627) [Fix][Synonyms UI]Add navigation link to the Detail breadcrumb. (elastic#209574) chore(dep): bump `store2` from `2.12.0` to `2.14.4` (elastic#210530) [scout] adding test helper `@kbn/scout-oblt` package and uptate onboarding tests (elastic#209761) [Cloud Security] Asset Inventory table flyout controls (elastic#208452) [ML] Fix model deployment check in file uploader (elastic#209585) Updates archive again (elastic#209828) [Security Solution] Added concurrency limits and request throttling to prebuilt rule routes (elastic#209551) [Search] [Onboarding] Update search api to use EventEmitter instead of Provider (elastic#209784) [maps] lazy load map actions (elastic#210252) [Cloud Security] Adding telemetry collection condition based on render condition (elastic#208758) [Solution nav] Use flyout for Stack Management in Search and Observability solutions (elastic#208632) [Search] Fix Add Inference Endpoint API call (elastic#210243) [Agentless Connectors] Integration overview panel (elastic#210222) [Lens] Restore dynamic colouring by value for Last value agg (elastic#209110) ...

… render condition (#208758) (#210635) # Backport This will backport the following commits from `main` to `8.x`: - [[Cloud Security] Adding telemetry collection condition based on render condition (#208758)](#208758)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jordan <51442161+JordanSh@users.noreply.github.com>

added telemetry collection condition based on render condition

f27fd78

JordanSh added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related backport:prev-minor labels Jan 29, 2025

JordanSh self-assigned this Jan 29, 2025

JordanSh requested a review from a team as a code owner January 29, 2025 14:43

maxcold reviewed Jan 30, 2025

View reviewed changes

JordanSh and others added 4 commits January 30, 2025 17:30

merge

79f4ef8

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

e46bdd0

…-fix'

lint

6ad1171

merge

8927342

JordanSh requested review from CohenIdo and animehart February 2, 2025 09:37

JordanSh added 2 commits February 2, 2025 11:46

Merge branch 'main' of https://github.com/elastic/kibana into telemet…

1749092

…ry-improvements

added version to naming so it will be easier to track after fix telem…

b972974

…etry

JordanSh requested a review from a team as a code owner February 2, 2025 12:05

JordanSh commented Feb 2, 2025

View reviewed changes

CohenIdo approved these changes Feb 3, 2025

View reviewed changes

christineweng reviewed Feb 5, 2025

View reviewed changes

JordanSh and others added 2 commits February 11, 2025 12:35

merge

b4f4a9a

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

12eb8de

…-fix'

JordanSh added v9.0.0 v8.18.0 v9.1.0 v8.19.0 backport:version Backport to applied version labels labels Feb 11, 2025

JordanSh requested a review from CohenIdo February 11, 2025 14:27

CohenIdo approved these changes Feb 11, 2025

View reviewed changes

JordanSh requested review from animehart and removed request for animehart February 11, 2025 14:29

animehart approved these changes Feb 11, 2025

View reviewed changes

JordanSh enabled auto-merge (squash) February 11, 2025 16:28

christineweng approved these changes Feb 11, 2025

View reviewed changes

JordanSh merged commit c1aaf6f into elastic:main Feb 11, 2025
11 checks passed

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 11, 2025

[Cloud Security] Adding telemetry collection condition based on rende…

1d7a195

…r condition (elastic#208758) (cherry picked from commit c1aaf6f)

kibanamachine mentioned this pull request Feb 11, 2025

[8.18] [Cloud Security] Adding telemetry collection condition based on render condition (#208758) #210634

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 11, 2025

[Cloud Security] Adding telemetry collection condition based on rende…

4e3034a

…r condition (elastic#208758) (cherry picked from commit c1aaf6f)

kibanamachine mentioned this pull request Feb 11, 2025

[8.x] [Cloud Security] Adding telemetry collection condition based on render condition (#208758) #210635

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 11, 2025

[Cloud Security] Adding telemetry collection condition based on rende…

d3bee9a

…r condition (elastic#208758) (cherry picked from commit c1aaf6f)

kibanamachine mentioned this pull request Feb 11, 2025

[9.0] [Cloud Security] Adding telemetry collection condition based on render condition (#208758) #210636

Merged

		export const MISCONFIGURATION_INSIGHT = 'misconfiguration-insight-v2' as const;
		export const VULNERABILITIES_INSIGHT = 'vulnerabilities-insight-v2' as const;

Conversation

JordanSh commented Jan 29, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented Jan 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanSh Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanSh commented Feb 2, 2025

Uh oh!

CohenIdo commented Feb 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanSh Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanSh commented Feb 11, 2025

Uh oh!

elasticmachine commented Feb 11, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

Uh oh!

kibanamachine commented Feb 11, 2025

Uh oh!

kibanamachine commented Feb 11, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

JordanSh commented Jan 29, 2025 •

edited by kibanamachine

Loading

JordanSh Feb 2, 2025 •

edited

Loading

JordanSh Feb 6, 2025 •

edited

Loading