Skip to content
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

chore: improve types and documentation for telemetry #6176

Merged
merged 5 commits into from
Sep 10, 2024
Merged

chore: improve types and documentation for telemetry #6176

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec7a80e
chore: improve types and documentation for telemetry
mcasimir Aug 30, 2024
d1f6d78
fix test and mixed up branches
mcasimir Sep 2, 2024
869592e
add is_favorite and document identify traits
mcasimir Sep 3, 2024
1d0f293
improve modeForTelemetry type
mcasimir Sep 3, 2024
63b9b01
remove unnecessary as const
mcasimir Sep 6, 2024
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
2 changes: 1 addition & 1 deletion packages/compass-aggregations/src/components/stage-editor/stage-editor.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('StageEditor [Component]', function () {
syntaxError={null}
serverError={null}
num_stages={0}
editor_view_type=""
editor_view_type="text"
/>
);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-aggregations/src/components/stage-editor/stage-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type StageEditorProps = {
syntaxError: PipelineParserError | null;
serverError: MongoServerError | null;
num_stages: number;
editor_view_type: string;
editor_view_type: 'text' | 'stage' | 'focus';
className?: string;
onChange: (index: number, value: string) => void;
editorRef?: React.Ref<EditorRef>;
Expand Down Expand Up @@ -143,7 +143,7 @@ export const StageEditor = ({
{
num_stages: num_stages,
stage_index: index + 1,
stage_action: 'stage_content_changed',
stage_action: 'stage_content_changed' as const,
Copy link
Contributor

@lerouxb lerouxb Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not understanding what these as const bits are doing and why they would be necessary. Why not give the whole object a type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as in move as const at the end? They are there so stage_action is not considered a string

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah wait, you are right this should not be needed here though. I'll take another look and remove them where is redundant

stage_name: stageOperator,
editor_view_type: editor_view_type,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-aggregations/src/components/stage-preview/atlas-stage-preview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const AtlasStagePreview = ({
href={ATLAS_LINK}
target="_blank"
onClick={() => {
track('Atlas Link Clicked', { screen: 'agg_builder' });
track('Atlas Link Clicked', { screen: 'agg_builder' as const });
}}
variant="primary"
>
Expand Down
10 changes: 5 additions & 5 deletions packages/compass-aggregations/src/modules/pipeline-builder/stage-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ export const changeStageOperator = (
'Aggregation Edited',
{
num_stages: pipelineFromStore(stages).length,
stage_action: 'stage_renamed',
stage_action: 'stage_renamed' as const,
stage_name: stage.operator,
stage_index: idxInPipeline + 1,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
Expand Down Expand Up @@ -673,7 +673,7 @@ export const addStage = (
'Aggregation Edited',
{
num_stages: pipelineFromStore(stages).length,
stage_action: 'stage_added',
stage_action: 'stage_added' as const,
stage_index: stage.id + 1,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
},
Expand Down Expand Up @@ -713,7 +713,7 @@ export const removeStage = (
'Aggregation Edited',
{
num_stages: pipelineFromStore(stages).length,
stage_action: 'stage_deleted',
stage_action: 'stage_deleted' as const,
stage_name: stage.operator,
stage_index: idxInPipeline + 1,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
Expand Down Expand Up @@ -774,7 +774,7 @@ export const moveStage = (
'Aggregation Edited',
{
num_stages: pipelineFromStore(stages).length,
stage_action: 'stage_reordered',
stage_action: 'stage_reordered' as const,
stage_name: stageAtFromIdx.stageOperator,
stage_index: stageAtFromIdx.idxInPipeline + 1,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
Expand Down Expand Up @@ -944,7 +944,7 @@ export const convertWizardToStage = (
'Aggregation Edited',
{
num_stages: pipelineFromStore(stages).length + 1,
stage_action: 'stage_added',
stage_action: 'stage_added' as const,
stage_name: stage.operator,
stage_index: afterStageIndex + 1,
editor_view_type: 'stage',
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-aggregations/src/modules/saved-pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export const confirmOpenPipeline =
{
id: pipelineData.id,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
screen: 'aggregations',
screen: 'aggregations' as const,
},
connectionInfo
);
Expand Down Expand Up @@ -275,7 +275,7 @@ export const confirmDeletePipeline =
{
id: pipelineId,
editor_view_type: mapPipelineModeToEditorViewType(getState()),
screen: 'aggregations',
screen: 'aggregations' as const,
},
connectionInfo
);
Expand Down
4 changes: 3 additions & 1 deletion packages/compass-connections/src/components/atlas-help/atlas-help.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ export function AtlasHelpSection(): React.ReactElement {
createClusterButtonStyles,
!darkMode && createClusterButtonLightModeStyles
)}
onClick={() => track('Atlas Link Clicked', { screen: 'connect' })}
onClick={() =>
track('Atlas Link Clicked', { screen: 'connect' as const })
}
variant={ButtonVariant.PrimaryOutline}
href="https://www.mongodb.com/cloud/atlas/lp/try4?utm_source=compass&utm_medium=product&utm_content=v1"
target="_blank"
Expand Down
3 changes: 0 additions & 3 deletions packages/compass-connections/src/stores/connections-store-redux.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1510,9 +1510,6 @@ export const connect = (
'Connection Attempt',
{
is_favorite: connectionInfo.savedConnectionType === 'favorite',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

drive by, these attributes aren't meaningful anymore with multiple connections

Copy link
Contributor

@lerouxb lerouxb Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Are they, though? We still have the concept of a favourite that we haven't removed. Can't ever remove it if we don't keep stats on whether it gets used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Relevant thread

I suggested removing them because the significance of is_favorite is not the same as it was before. Earlier is_favorite was the only way to differentiate wether a connection is saved or recent but now all the connections are saved and is_favorite is more UI related, allowing us to group connections together. So for the telemetry point that it used to represent earlier, it does not make sense to keep it anymore. However if we would like to track connections being marked as favorite maybe we can introduce a different attribute?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ahh ok, so is favorite is still a thing, then i will keep it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and is recent doesn't make sense because everything saved now right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think is_recent can be dropped, yes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok re-added is_favorite then! thanks!

is_recent:
!!connectionInfo.lastUsed &&
connectionInfo.savedConnectionType !== 'favorite',
is_new: isNewConnection(getState(), connectionInfo.id),
},
connectionInfo
Expand Down
2 changes: 1 addition & 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 @@ -489,7 +489,7 @@ class CrudStoreImpl
* to update if the labels change at some point.
*/
modeForTelemetry() {
return this.state.view.toLowerCase();
return this.state.view.toLowerCase() as 'list' | 'json' | 'table';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is there a better way?

Copy link
Contributor

Choose a reason for hiding this comment

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

as Lowercase<DocumentView>?

}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/compass-e2e-tests/tests/logging.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ describe('Logging and Telemetry integration', function () {
.events()
.find((entry) => entry.event === 'Connection Attempt');
expect(connectionAttempt.properties.is_favorite).to.equal(false);
expect(connectionAttempt.properties.is_recent).to.equal(false);
expect(connectionAttempt.properties.is_new).to.equal(true);
});

Expand Down
88 changes: 52 additions & 36 deletions packages/compass-export-to-language/src/components/modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,51 +146,67 @@ const ExportToLanguageModal: React.FunctionComponent<
const [wasOpen, setWasOpen] = useState(false);

useEffect(() => {
const trackingEvent =
mode === 'Update Query'
? 'Update Export Opened'
: mode === 'Delete Query'
? 'Delete Export Opened'
: mode === 'Query'
? 'Query Export Opened'
: 'Aggregation Export Opened';

if (modalOpen && !wasOpen) {
const connectionInfo = connectionInfoAccess.getCurrentConnectionInfo();
track(
trackingEvent,
{
...stageCountForTelemetry(inputExpression),
},
connectionInfo
);

if (mode === 'Query') {
track('Query Export Opened', {}, connectionInfo);
} else if (mode === 'Delete Query') {
track('Delete Export Opened', {}, connectionInfo);
} else if (mode === 'Update Query') {
track('Update Export Opened', {}, connectionInfo);
} else if (mode === 'Pipeline') {
track(
'Aggregation Export Opened',
{
...stageCountForTelemetry(inputExpression),
},
connectionInfo
);
}

track('Screen', { name: 'export_to_language_modal' }, connectionInfo);
}

setWasOpen(modalOpen);
}, [modalOpen, wasOpen, mode, inputExpression, track, connectionInfoAccess]);

const trackCopiedOutput = useCallback(() => {
const trackingEvent =
mode === 'Update Query'
? 'Update Exported'
: mode === 'Delete Query'
? 'Delete Exported'
: mode === 'Query'
? 'Query Exported'
: 'Aggregation Exported';

track(
trackingEvent,
{
language: outputLanguage,
with_import_statements: includeImports,
with_drivers_syntax: includeDrivers,
with_builders: useBuilders,
...stageCountForTelemetry(inputExpression),
},
connectionInfoAccess.getCurrentConnectionInfo()
);
const commonProps = {
language: outputLanguage,
with_import_statements: includeImports,
with_drivers_syntax: includeDrivers,
with_builders: useBuilders,
};

if (mode === 'Update Query') {
track(
'Update Exported',
commonProps,
connectionInfoAccess.getCurrentConnectionInfo()
);
} else if (mode === 'Delete Query') {
track(
'Delete Exported',
commonProps,
connectionInfoAccess.getCurrentConnectionInfo()
);
} else if (mode === 'Query') {
track(
'Query Exported',
commonProps,
connectionInfoAccess.getCurrentConnectionInfo()
);
} else if (mode === 'Pipeline') {
track(
'Aggregation Exported',
{
...commonProps,
...stageCountForTelemetry(inputExpression),
},
connectionInfoAccess.getCurrentConnectionInfo()
);
}
}, [
track,
connectionInfoAccess,
Expand Down
6 changes: 4 additions & 2 deletions packages/compass-import-export/src/modules/export.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ export const openExport = (
track(
'Export Opened',
{
type: exportOptions.aggregation ? 'aggregation' : 'query',
type: exportOptions.aggregation
? ('aggregation' as const)
: ('query' as const),
origin: exportOptions.origin,
},
connectionRepository.getConnectionInfoById(exportOptions.connectionId)
Expand Down Expand Up @@ -542,7 +544,7 @@ export const runExport = ({
track(
'Export Completed',
{
type: aggregation ? 'aggregation' : 'query',
type: aggregation ? ('aggregation' as const) : ('query' as const),
all_docs: exportFullCollection,
has_projection:
exportFullCollection || aggregation || !_query
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-import-export/src/modules/import.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ export const startImport = (): ImportThunkAction<Promise<void>> => {
file_type: fileType,
all_fields: exclude.length === 0,
stop_on_error_selected: stopOnErrors,
number_of_docs: err.result.docsWritten,
number_of_docs: err.result?.docsWritten,
success: !err,
aborted: abortSignal.aborted,
ignore_empty_strings: fileType === 'csv' ? ignoreBlanks : undefined,
Expand All @@ -369,7 +369,7 @@ export const startImport = (): ImportThunkAction<Promise<void>> => {
log.error(mongoLogId(1001000081), 'Import', 'Import failed', {
ns,
errorLogFilePath,
docsWritten: err.result.docsWritten,
docsWritten: err.result?.docsWritten,
error: err.message,
});
debug('Error while importing:', err.stack);
Expand Down
4 changes: 2 additions & 2 deletions packages/compass-query-bar/src/components/query-history/favorite-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const FavoriteItem = ({
'Query History Favorite Used',
{
id: query._id,
screen: 'documents',
screen: 'documents' as const,
isUpdateQuery,
},
connectionInfoAccess.getCurrentConnectionInfo()
Expand Down Expand Up @@ -80,7 +80,7 @@ const FavoriteItem = ({
'Query History Favorite Removed',
{
id: query._id,
screen: 'documents',
screen: 'documents' as const,
isUpdateQuery,
},
connectionInfoAccess.getCurrentConnectionInfo()
Expand Down
9 changes: 6 additions & 3 deletions packages/compass-saved-aggregations-queries/src/components/aggregations-queries-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export const AggregationsQueriesList = ({
useTrackOnChange(
(track: TrackFunction) => {
if (filters.database) {
track('My Queries Filter', { type: 'database' });
track('My Queries Filter', { type: 'database' as const });
}
},
[filters.database]
Expand All @@ -153,7 +153,7 @@ export const AggregationsQueriesList = ({
useTrackOnChange(
(track: TrackFunction) => {
if (filters.collection) {
track('My Queries Filter', { type: 'collection' });
track('My Queries Filter', { type: 'collection' as const });
}
},
[filters.collection]
Expand All @@ -169,7 +169,10 @@ export const AggregationsQueriesList = ({
(track: TrackFunction) => {
track('My Queries Sort', {
sort_by: sortState.name,
order: sortState.order === 1 ? 'ascending' : 'descending',
order:
sortState.order === 1
? ('ascending' as const)
: ('descending' as const),
});
},
[sortState]
Expand Down
2 changes: 1 addition & 1 deletion packages/compass-saved-aggregations-queries/src/stores/copy-to-clipboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function copyToClipboard(
: 'Query History Favorite Copied',
{
id: item.id,
screen: 'my_queries',
screen: 'my_queries' as const,
}
);

Expand Down
2 changes: 1 addition & 1 deletion packages/compass-saved-aggregations-queries/src/stores/delete-item.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const confirmDeleteItem = (
: 'Query History Favorite Removed',
{
id: item.id,
screen: 'my_queries',
screen: 'my_queries' as const,
}
);

Expand Down
2 changes: 1 addition & 1 deletion packages/compass-saved-aggregations-queries/src/stores/open-item.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ const openItem =
: 'Query History Favorite Used',
{
id: item.id,
screen: 'my_queries',
screen: 'my_queries' as const,
}
);

Expand Down
2 changes: 1 addition & 1 deletion packages/compass-telemetry/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export { createIpcTrack, createIpcSendTrack } from './ipc-track';
export type { TelemetryServiceOptions } from './generic-track';
export type { TrackFunction } from './types';
export type { TrackFunction, IdentifyTraits } from './types';
Loading
Loading