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.

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

Add prelimenary telemetry [easy] #254

Closed
brson opened this Issue Apr 2, 2016 · 13 comments

Comments

Projects
None yet
7 participants
@brson
Copy link
Contributor

brson commented Apr 2, 2016

We have always wanted better insight into what people are doing with Rust. Knowing things like which errors people hit the most will help us know what areas of development to focus on. rustup, with it's compiler proxies is a perfect place to add instrumentation to observe what the compiler is doing.

This is a sensitive subject: data gathering is always controversial so we have to be completely upfront about what we collect, when and why, and we have to legitimately not collect personally-identifying information.

Additionally, every bit of code on the proxy path reduces the compiler's performance slightly. Whatever we do can't degrade performance significantly.

Here's how I'd like to start prototyping:

Define several events that we're interested in, as an enum. To start with:

  • RustcRun - The duration of a rustc invocation, it's exit code, and any error numbers produced.
  • ToolchainUpdate - The normalized name of a toolchain being updated, and whether it succeeded.
  • TargetAdd - The normalized name of a toolchain being added to, the target triple, and whether it succeeded.

Every event is paired with a UTC timestamp.

Before rustup exits, serialize each event to json and emit it as a new line in a log file. Add two new commands: rustup telemetry on, and rustup telemetry off. By default it should be off.

We'll at some point need to discuss with fx developers how they do telemetry, since they've had a lot of experience.

@brson brson changed the title Add prelimenary telemetry Add prelimenary telemetry [easy] Apr 2, 2016

@mitsuhiko

This comment has been minimized.

Copy link

mitsuhiko commented Apr 2, 2016

Please, please, please make this opt-in with any flow going to "no" by default. You can encourage people with various means to opt into it at a later point easily. For instance the compiler could spit out periodically that telemetry is disabled but enabling it might improve your experience going forward, something like this.

@peschkaj

This comment has been minimized.

Copy link
Contributor

peschkaj commented Apr 3, 2016

@brson I'd like to take this on. Is there a set deliverable date or process?

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 4, 2016

Initial implementation of rust-lang#254
- Can currently set the `telemetry` flag.
- Telemetry status is stored in `MULTIRUST_HOME/telemetry`
@jdm

This comment has been minimized.

Copy link

jdm commented Apr 4, 2016

Consider making use of https://github.com/Yoric/telemetry.rs which is written based on Firefox's experiences with telemetry.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Apr 4, 2016

Consider making use of https://github.com/Yoric/telemetry.rs which is written based on Firefox's experiences with telemetry.

CC @Yoric

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 5, 2016

Please, please, please make this opt-in with any flow going to "no" by default. You can encourage people with various means to opt into it at a later point easily. For instance the compiler could spit out periodically that telemetry is disabled but enabling it might improve your experience going forward, something like this.

It'll be opt-in. Right now I'm thinking we'll wait for a week to see if you are a 'serious' Rust user then print a hint.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 5, 2016

@brson I'd like to take this on. Is there a set deliverable date or process?

@peschkaj Please do. I'd like to have this in by the time we announce rustup for wider use so that we firmly establish the precedent that rustup does telemetry from the start. Right now I'm looking at the end of April for that.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 5, 2016

Consider making use of https://github.com/Yoric/telemetry.rs which is written based on Firefox's experiences with telemetry.

Nice! @peschkaj this is pure-Rust. Looks very promising.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 5, 2016

Also, to start let's not worry about reporting the telemetry back to a server - just store and rotate logs.

@peschkaj

This comment has been minimized.

Copy link
Contributor

peschkaj commented Apr 6, 2016

to start let's not worry about reporting the telemetry back to a server - just store and rotate logs.

👍 I was thinking of rotating logs daily by writing into a file named telemetry-YYYY-MM-dd. I figured that pushing logs to a server would come later (especially since you didn't mention it in the initial issue).

I currently have a scaffold in place and was working on getting the JSON serialization going. I'll take a look again at Yoric/telemetry.rs to see if it meets our needs. On my first glance, it looked like it was geared toward creating a histogram from a single run, but that was only a quick glance.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Apr 6, 2016

I am super eager to hear @Yoric's opinions on Rust telemetry.

@Yoric

This comment has been minimized.

Copy link

Yoric commented Apr 6, 2016

The parts of https://github.com/Yoric/telemetry.rs that have landed are indeed targeted towards creating a histogram from a single run. The objective was to provide the core data structures, and then to add additional layers (most likely application-specific) to implement disk storage and server upload.

I haven't had time to do this work on higher layers, unfortunately, but I'd be glad to give a hand if this is useful.

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 6, 2016

WIP on rust-lang#254
- removes logging messages
- matches `rustc` and `rustc.exe` for telemetry and proxying
@peschkaj

This comment has been minimized.

Copy link
Contributor

peschkaj commented Apr 7, 2016

@Yoric Appreciated. I've run into one blocker.

The first is I've found is that our data structures don't seem to match up with what telemetry.rs is expecting. As best as I can tell, telemetry.rs is looking for either a function to measure multiple times (e.g. the fibonacci example), or else it's recording a count/flag of the use of something.

In this use case, we're looking to measure the execution time of rustc as executed through rustup's proxying. It's easy enough to wrap timers around it, but it seems more difficult to wrap the telemetry library around it to give us what we want.

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 7, 2016

Initial implementation of rust-lang#254
- Telemetry can be enabled via `rustup telemetry on`
- Telemetry status is stored in `MULTIRUST_HOME/telemetry-YYYY-MM-DD`

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 10, 2016

Initial implementation of rust-lang#254
- Can currently set the `telemetry` flag.
- Telemetry status is stored in `MULTIRUST_HOME/telemetry`
- Adds routing for telemetry for rustc calls and target add
- Provides basic command proxying for telemetry

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 10, 2016

Initial implementation of rust-lang#254
- Can currently set the `telemetry` flag.
- Telemetry status is stored in `MULTIRUST_HOME/telemetry`
- Adds routing for telemetry for rustc calls and target add
- Provides basic command proxying for telemetry

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 13, 2016

Initial implementation of rust-lang#254
- Can currently set the `telemetry` flag.
- Telemetry status is stored in `MULTIRUST_HOME/telemetry`
- Adds routing for telemetry for rustc calls and target add
- Provides basic command proxying for telemetry

peschkaj added a commit to peschkaj/rustup.rs that referenced this issue Apr 18, 2016

Initial implementation of rust-lang#254
- Can currently set the `telemetry` flag.
- Telemetry status is stored in `MULTIRUST_HOME/telemetry`
- Adds routing for telemetry for rustc calls and target add
- Provides basic command proxying for telemetry

@Diggsey Diggsey added this to Proxying & telemetry in Issue Categorisation May 4, 2017

@Diggsey

This comment has been minimized.

Copy link
Contributor

Diggsey commented May 4, 2017

This exists - lets have separate issues for further development.

@Diggsey Diggsey closed this May 4, 2017

@Diggsey Diggsey removed this from Proxying & telemetry in Issue Categorisation May 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment