-
Notifications
You must be signed in to change notification settings - Fork 385
RI-7218 Add telemetry collection for Create Vector Search Index wizard #4807
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
RI-7218 Add telemetry collection for Create Vector Search Index wizard #4807
Conversation
@@ -11,8 +11,8 @@ export interface IBulkActionOverview { | |||
databaseId: string; | |||
duration: number; | |||
type: BulkActionType; | |||
status: BulkActionStatus; | |||
filter: IBulkActionFilterOverview; | |||
status: BulkActionStatus; // Note: This can be null, according to the API response |
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.
Maybe, we should update the interface itself, but I was not brave enough to do it at this point, afraid of breaking something unexpected.
redisinsight/ui/src/mocks/factories/workbench/commandExectution.factory.ts
Show resolved
Hide resolved
@@ -12,6 +12,7 @@ module.exports = { | |||
'\\.(css|less|sass|scss)$': 'identity-obj-proxy', | |||
'\\.scss\\?inline$': '<rootDir>/redisinsight/__mocks__/scssRaw.js', | |||
'uiSrc/(.*)': '<rootDir>/redisinsight/ui/src/$1', | |||
'apiSrc/(.*)': '<rootDir>/redisinsight/api/src/$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.
This alias exist for the regular bundler, but was not available for Jest, causing the tests that rely on such imports to fail.
0661ce5
to
3f64658
Compare
Code Coverage - Integration Tests
|
Code Coverage - Backend unit tests
Test suite run success2948 tests passing in 286 suites. Report generated by 🧪jest coverage report action from 30560d7 |
Code Coverage - Frontend unit tests
Test suite run success4952 tests passing in 654 suites. Report generated by 🧪jest coverage report action from 68252b0 |
bf65847
to
f487ba9
Compare
…ex wizard re #RI-7218
…ex wizard re #RI-7218
…ex wizard re #RI-7218
f487ba9
to
39baa30
Compare
…ector search index wizard re #RI-7218
39baa30
to
4e8a96b
Compare
redisinsight/ui/src/mocks/factories/workbench/commandExectution.factory.ts
Show resolved
Hide resolved
redisinsight/ui/src/mocks/factories/workbench/commandExectution.factory.ts
Show resolved
Hide resolved
useEffect(() => { | ||
collectTelemetry(step) | ||
}, [step]) | ||
|
||
const collectTelemetry = (step: number): void => { | ||
switch (step) { | ||
case 1: | ||
collectStartStepTelemetry() | ||
break | ||
case 2: | ||
collectIndexInfoStepTelemetry() | ||
break | ||
case 3: | ||
collectCreateIndexStepTelemetry() | ||
break |
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'm not a big fan of this. It's not even code that's worth extracting to a custom hook (all the telemetry callbacks).
I think it's better to have a single method, defined outside this component/file. On step change - call it with all params, let it decide what to track
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.
Yes, I wanted to do the same, but in order to extract them as a custom hook, we should prop drill all the state params (necessary for the telemetry to work), and at the same time, we don't have a shared context to do it in a clean way. This is the "best" I could do without additional refactoring of the main state.
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.
not a hook but a method is what I suggested. Having these functions inside the component increase it's complexity unneededly
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.
Ok, I made some refactoring in e554f1a to offload the current component, as you suggested. I moved it out to a hook (instead of just a function) so we can pass down less props (like instanceId
).
Let me know if that was what you suggested 🙂
Description
Added telemetry collection events to track users' interaction with the Create Vector Search Index wizard
How to test it
VECTOR_SEARCH_ONBOARDING_TRIGGERED
)VECTOR_SEARCH_ONBOARDING_PROCEED_TO_INDEX_INFO
)VECTOR_SEARCH_ONBOARDING_VIEW_COMMAND_PREVIEW
)VECTOR_SEARCH_ONBOARDING_PROCEED_TO_QUERIES
)Screen.Recording.2025-08-05.at.14.48.58.mov
Verify Telemetry events
We have the new events for the Create Vector Search index wizard collected successfully.
Note: Telemetry is collected only if the user decides to opt-in to that.
You control it from Settings page -> Privacy section -> Usage Data toggle.