Skip to content

chore(telemetry): add filter tracking for bulk ops COMPASS-10297 #8015

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.

Sign up for GitHub

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

Open
mabaasit wants to merge 2 commits into
base: main
Choose a base branch
from
+20 −5
Open

chore(telemetry): add filter tracking for bulk ops COMPASS-10297 #8015

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a8f916
add filter tracking for bulk delete ops
mabaasit Apr 23, 2026
1a63824
correct check and fix tests
mabaasit Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/compass-crud/src/stores/crud-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,7 @@ class CrudStoreImpl
'Bulk Update Executed',
{
isUpdatePreviewSupported: this.state.isUpdatePreviewSupported,
has_filter: !!this.queryBar.getLastAppliedQuery('crud').filter,
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_filter is computed using !!...filter, but getLastAppliedQuery() returns a query object where filter is always an object (often {}), which is truthy. This will report has_filter: true even when no filter is set. Compute it like the existing Query Executed telemetry does (e.g., Object.keys(filter ?? {}).length > 0) by first grabbing const { filter } = this.queryBar.getLastAppliedQuery('crud').

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@Anemy Anemy Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot is right here, filter can be {} which would be truthy. 'Query Executed' has the same pattern.

},
this.connectionInfoRef.current
);
Expand Down Expand Up @@ -1956,7 +1957,13 @@ class CrudStoreImpl
}

async runBulkDelete() {
this.track('Bulk Delete Executed', {}, this.connectionInfoRef.current);
this.track(
'Bulk Delete Executed',
Copy link
Copy Markdown
Collaborator Author

@mabaasit mabaasit Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both of these events should be called either after the operation succeeds or just before it (dataservice call). In delete event we show a confirmation modal, to which user can ignore and we still track it. Similarly for update, if the query is invalid, we still track it.

{
has_filter: !!this.queryBar.getLastAppliedQuery('crud').filter,
},
Comment thread
mabaasit marked this conversation as resolved.
this.connectionInfoRef.current
);

const { affected } = this.state.bulkDelete;
this.closeBulkDeleteDialog();
Expand Down
7 changes: 6 additions & 1 deletion packages/compass-telemetry/src/telemetry-events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,8 @@ type BulkUpdateExecutedEvent = ConnectionScopedEvent<{
* Specifies if update preview was supported (the update preview runs inside a transaction.)
*/
isUpdatePreviewSupported: boolean;
/** Specifies if a filter was set in the query */
has_filter: boolean;
};
}>;

Expand All @@ -1018,7 +1020,10 @@ type BulkDeleteOpenedEvent = ConnectionScopedEvent<{
*/
type BulkDeleteExecutedEvent = ConnectionScopedEvent<{
name: 'Bulk Delete Executed';
payload: Record<string, never>;
payload: {
/** Specifies if a filter was set in the query */
has_filter: boolean;
};
Comment on lines 1022 to +1026
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This event payload shape changed from {} to include has_filter. There are existing telemetry assertions in e2e tests that expect an empty payload / only isUpdatePreviewSupported (e.g., packages/compass-e2e-tests/tests/collection-bulk-delete.test.ts and collection-bulk-update.test.ts). These will fail until updated to assert the new property.

Copilot uses AI. Check for mistakes.
}>;

/**
Expand Down
Loading