VSCODE-224: Telemetry document edited from playground #235
Conversation
| isGenuine: boolean; | ||
| dbType: string; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Another way we could do this is add a typescript definitions file to the data-service itself. While this does help our code it doesn't ensure this can't change in the data-service module. Not too much we can do there though.
I think using the data-service's instance function in our telemetry is a bit overkill - it fetches all of the databases and collection info for example, which we don't use/need. Maybe at some point we update it to be a bit more like mongosh where we're only pulling what we need: https://github.com/mongodb-js/mongosh/blob/master/packages/service-provider-server/src/cli-service-provider.ts#L234 . Not related to this pr but I figure it's good to bring up.
There was a problem hiding this comment.
Yeah, would be nice to have types in data service. But since we were talking about getting rid of data service completely for this project, I put a temporary solution here to fix the linting warning.
src/telemetry/telemetryService.ts
Outdated
| } catch (error) { | ||
| log.error('TELEMETRY key error', error); | ||
| } | ||
| this._segmentUserID = storageController.getUserID() || ''; |
There was a problem hiding this comment.
Isn't getUserID always returning a string?
src/telemetry/telemetryService.ts
Outdated
| this.track(TelemetryEventTypes.DOCUMENT_UPDATED, { source, success }); | ||
| } | ||
|
|
||
| trackOpenMongoDBDocumentFromPlayground(source: string): void { |
There was a problem hiding this comment.
Maybe we can make source here use the DOCUMENT_SOURCE_PLAYGROUND and DOCUMENT_SOURCE_TREEVIEW types
| dataService: DataServiceType, | ||
| connectionType: ConnectionTypes | ||
| ): void { | ||
| dataService.instance({}, async (error: any, data: any) => { |
There was a problem hiding this comment.
Since we are doing a lot of things depending on the instance results here - maybe we can promisify this and try catch it. Then if we do end up doing something that breaks it's handled well - maybe we also want to log that error to telemetry.
| } | ||
| }); | ||
| } catch (error) { | ||
| log.error('TELEMETRY track new connection', error); |
There was a problem hiding this comment.
Would it make sense to track the telemetry error here? (We'd probably want to sanitize the error somehow though)
There was a problem hiding this comment.
Max hasn't requested a telemetry event for failed connections, only for ones that succeeded.
There was a problem hiding this comment.
Gotcha - I was thinking about creating a telemetry event around letting us know we've missed tracking a connection because we errored parsing the connection instance information, not an event around a connection failing to connect. It's probably something we'd want a bug tracker to report, but since we don't have one it might make sense to report it to telemetry so we're aware if users are adding connections but we aren't properly tracking them.
There was a problem hiding this comment.
Good idea, pls create a ticket to triage it!
alenakhineika
deleted the
VSCODE-224-telemetry-document-edited-from-playground
branch
2 participants
Description
Track telemetry events when user decides to edit a document from a code lens and when the edited document is saved to MongoDB.
Checklist
Motivation and Context
Types of changes