Integrate amplitude #209
Integrate amplitude #209
Conversation
And merge the sentry and amplitude usage under one “Enable analytics” flag
meilisearch-http/src/main.rs
Outdated
| #[cfg(feature = "analytics")] | ||
| if !opt.no_analytics { | ||
| let analytics_data = data.clone(); | ||
| let analytics_opt = opt.clone(); | ||
| tokio::task::spawn(analytics::analytics_sender(analytics_data, analytics_opt)); | ||
| } |
There was a problem hiding this comment.
Maybe we should prevent data to be sent in debug builds, just like above
There was a problem hiding this comment.
To be sure, are we talking about a MeiliSearch compiled by the user in debug mode and not in release mode? I ask the question to be sure that it does not concern the log level.
If this is the case, I don't see any immediate need to have analytics on the uses of Meilisearch when it is compiled in debug. It also avoids polluting Amplitude with data that doesn't represent the majority of our users.
What is the purpose of the debug build? Using different compiler options for development purposes?
There was a problem hiding this comment.
Yes, you understood perfectly. Right now Meilisearch (and this pr) are sending amplitude analytics even when compiled in Debug mode. Sentry is not sending any analytics while in debug mode though.
Basically, the debug mode is faster to compile, slower to execute and obviously easier to debug.
Every binary we ship is compiled in release mode so someone using a debug build build it himself from the source.
There was a problem hiding this comment.
Thanks for the answer @irevoire. I agreed that we should not send the analytics in debug build! Thanks, guys for the catch 💪
There was a problem hiding this comment.
The spec has been updated to mention the fact that debug build doesn't collect any telemetry!
Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
|
I removed the usage of amplitude when in debug mode 🙂 |
|
Build succeeded: |
And merge the sentry and amplitude usage under one “Enable analytics” flag
closes #180