MONGOSH-110: send telemetry events to segment if opted-in #121
Conversation
| let userId; | ||
| let telemetry; | ||
|
|
||
| let analytics = new NoopAnalytics(); |
There was a problem hiding this comment.
use a noop just in case something goes wrong with the definition below. That way the following analytics calls don't fail and pollute user with errors.
| analytics.track({ | ||
| userId, | ||
| event: 'mongosh:error', | ||
| properties: { error } |
There was a problem hiding this comment.
One of the side effects of how the node repl handles errors weirdly, is that we can send errors that we've thrown to telemetry but skip other errors, right? Since we don't want to send every JS error ever triggered on the shell to telemetry? I'm not 100% clear on what errors are getting sent
There was a problem hiding this comment.
good question! so we currently send everything, because all the errors ever make it to the writer get logged. This doesn't include unhandled exceptions, since we don't currently handle those.
Do you think we should be a bit more specific about what errors we send back? For example, only MongoshInternalError? I do agree, I think telemetry will be super polluted if we do everything.
There was a problem hiding this comment.
Yeah I'd say even only Mongosh errors and not generic javascript errors that get written to the logger but not thrown by us.
There was a problem hiding this comment.
ok, now telemetry only gets Mongosh errors. Local logs get everything tho, because I want to keep an eye on all the js throws in case we have missed something.
| @@ -123,7 +123,7 @@ class ShellEvaluator { | |||
| this.saveState(); | |||
| const rewrittenInput = this.asyncWriter.compile(input); | |||
| this.bus.emit( | |||
| 'mongosh:rewrittenAsyncInput', | |||
There was a problem hiding this comment.
We could emit a little more intelligently here if you want - we could only emit when the rewritten input does not match the original input. (We'd have to set a flag to babel to not 'prettify' the generated code, or have the async-rewriter indicate that it has inserted an await, or remove all whitespace before comparison).
As it is, it will emit an event for literally everything that gets run in the shell, so it might not be that useful.
There was a problem hiding this comment.
^ that would be heaps more useful to us and to other users!
| }); | ||
| } | ||
|
|
||
| export default logger; | ||
| // set up a noop, in case we are not able to connect to segment. | ||
| function NoopAnalytics() {} |
There was a problem hiding this comment.
what's the syntax for a noop class? I've only ever written it like this before.
There was a problem hiding this comment.
I meant this:
class NoopAnalytics {
identify() {}
track() {}
}
I was wondering if you had some form of typescript issue with that. Just a formality anyway.
There was a problem hiding this comment.
oh no, nothing to do with typescript! I am just old school and didn't know the new and fancy syntax
There was a problem hiding this comment.
in an unexpected turn of events, typescript does not, in fact like the class syntax.
lrlna
force-pushed
the
add-segment
branch
2 times, most recently
from
2e6b233
to
9037bbc
Compare
|
I am going to merge it in, and see if evergreen will properly inject this file when on master. If not, I'll debug tomorrow. If it doesn't inject the key properly, master will not be broken and everything will continue functioning as is. Segment just won't get any data, but that's not different from what's happening right now. |
Description
Sends information to segment if user opted-in.
API_KEY from segment is to be read from
./config.jsfile that evergreen writes when releasing the binary. This PR adds a task torelease.jsto populate that file. Evergreen already has the appropriate key to be written.This PR also adds a few more events:
mongosh:new-user,mongosh-update-userand
mongosh:toggle-telemetry.Information sent to segment is stripped of possible sensitive information that
could come in queries via
mongodb-redactmodule.Adds documentation on events we listen to in
logger.tsOpen Questions
@durran what's the best possible way for me to get this API key into anEvergreen build?