-
Notifications
You must be signed in to change notification settings - Fork 246
chore(compass-collection): Add track events for mock data generator modal CLOUDP-333861 #7482
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
Conversation
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.
Pull Request Overview
This PR adds comprehensive telemetry tracking for the Mock Data Generator modal feature to measure user engagement and feature usage patterns. The changes implement analytics events as specified in the technical design document for tracking user interactions throughout the mock data generation workflow.
Key changes:
- Defined 9 new telemetry event types for tracking button views, modal interactions, and user actions
- Instrumented modal screens and user interactions with tracking calls
- Added test coverage for all new tracking events
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/compass-telemetry/src/telemetry-events.ts |
Defines 9 new event types for Mock Data Generator analytics |
packages/compass-collection/src/components/mock-data-generator-modal/utils.ts |
Exports max collection nesting depth constant for reuse |
packages/compass-collection/src/components/mock-data-generator-modal/types.ts |
Adds enum for data generation steps used in telemetry |
packages/compass-collection/src/components/mock-data-generator-modal/script-screen.tsx |
Tracks script generation and copy events |
packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx |
Tracks screen views, progression, and modal dismissal |
packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx |
Tracks faker method changes |
packages/compass-collection/src/components/mock-data-generator-modal/document-count-screen.tsx |
Tracks document count changes |
packages/compass-collection/src/components/mock-data-generator-modal/constants.ts |
Adds step progression map for telemetry |
packages/compass-collection/src/components/collection-header-actions/collection-header-actions.tsx |
Tracks CTA button views and clicks |
packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx |
Tests for new tracking events in modal |
packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.spec.tsx |
Tests for faker method change tracking |
packages/compass-collection/src/components/collection-header-actions/collection-header-actions.spec.tsx |
Tests for CTA button tracking events |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| output_docs_count: documentCount, | ||
| }); | ||
| } | ||
| }); |
Copilot
AI
Oct 21, 2025
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 useEffect is missing its dependency array, which will cause the tracking event to fire on every render. Add [scriptResult.success, fakerSchema, documentCount, track] as dependencies to ensure it only fires when the script is successfully generated.
| }); | |
| }, [scriptResult.success, fakerSchema, documentCount, 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.
co-pilot is correct here, we want to add deps here.
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.
Added the missing deps
| const onFakerMethodChange = useCallback( | ||
| (newMethod: string) => { | ||
| track('Mock Data Faker Method Changed', { | ||
| field_name: activeJsonType, |
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.
Should be the field name, not the type. We can pass that into this component from mock-data-generator-modal. Also not a bad idea to include the jsonType, let's add json_type: activeJsonType here too
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.
Changed!
| track('Mock Data Generator Dismissed', { | ||
| screen: currentStep, | ||
| gen_ai_features_enabled: false, | ||
| send_sample_values_enabled: false, |
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.
Looks like these hardcoded values snuck in here?
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.
Fixed this, thanks for flagging!
|
Also, missed this but let's add a |
| useEffect(() => { | ||
| if (shouldShowMockDataButton) { | ||
| track('Mock Data Generator CTA Button Viewed', { | ||
| button_enabled: !shouldDisableMockDataButton, | ||
| gen_ai_features_enabled: isAIFeatureEnabled, | ||
| send_sample_values_enabled: isSampleDocumentPassingEnabled, | ||
| }); | ||
| } | ||
| }, [ | ||
| track, | ||
| isAIFeatureEnabled, | ||
| isSampleDocumentPassingEnabled, | ||
| shouldDisableMockDataButton, | ||
| shouldShowMockDataButton, | ||
| ]); |
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.
Here you can use useTrackOnChange hook that '@mongodb-js/compass-telemetry/provider' exports
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.
Oh this is awesome, will use useTrackOnChange!
| }); | ||
| onFakerFunctionSelect(newMethod); | ||
| }, | ||
| [activeJsonType, onFakerFunctionSelect, 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.
Missing fieldName from deps list.
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.
Added this here! it was a late addition
Added the new event! |
| | 'String' | ||
| | 'Number' | ||
| | 'Boolean' | ||
| | 'Date' | ||
| | 'Int32' | ||
| | 'Decimal128' | ||
| | 'Long' | ||
| | 'ObjectId' | ||
| | 'RegExp' | ||
| | 'Symbol' | ||
| | 'MaxKey' | ||
| | 'MinKey' | ||
| | 'Binary' | ||
| | 'Code' | ||
| | 'Timestamp' | ||
| | 'DBRef'; |
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.
We can extract this bit to a type and then use for previous_json_type, new_json_type and within MockDataFakerMethodChangedEvent
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.
Similarly for screens?
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.
Sure, I can add this. Any reason that other events in the file don't use any types and just write out the union of strings?
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 didn't know this until now :) I don't think there's any specific reason for this - its just that this caught my eye especially as the list a big, so made sense to type it out.
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.
fyi - we use these types to generate Compass tracking-plan.
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.
lol I see what you did there
| [track] | ||
| ); | ||
|
|
||
| useEffect(() => { |
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.
You can also use useTrackOnChange here
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.
Done!
jcobis
left a comment
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
Description
CLOUDP-333861
Checklist
Motivation and Context
Types of changes