Hi Karsten, just finished throwing together a script that does seven of the eighteen DocTor checks. The rest shouldn't be hard, just take a little elbow grease...

https://gitweb.torproject.org/atagar/tor-utils.git/commitdiff/1e49c33

As for the website, why is that part of the same codebase as the monitors? The site doesn't look to make use of the derived warnings. Is this simply a kludge since they both make use of the same descriptor data?

The website might be a good use case for Hyde (http://ringce.com/hyde). That said, this feels like it should belong in the metrics-web repository...

Cheers! -Damian

I quickly looked at the commit, and this is probably due to the early state of the script, but I thought I'd mention it anyway: I wondered how the single try block around get_consensuses, get_votes, and run_checks would respond to single directory authorities being unavailable, closing connections in the middle of the download, taking longer than 60 seconds, etc. I think the 60 seconds thing might be handled fine, but would the other I/O errors make the script not run any checks?

Correct, when an authority is unavailable the script only reports a single error - that outage. It didn't cross my mind that we would want to run the checks on a subset of the authorities, but that's an easy tweak to make.

The kludge is that checks and website don't share code, not that there's a website. The idea of the typical use case is that people receive a warning via email or IRC and then go to the website to learn more details.

I disagree. This repository contains two very distinct applications:

* monitors for issues with the votes * a website that renders the present content of the votes

The use cases for each are associated, but bundling them together makes about as much sense as lumping vidalia and tor within the same repository.

Personally I'm a big fan of these monitors, but less so the website. I don't think it's especially useful (precious few people have cause to find a side-by-side comparison of vote attributes to be interesting, and fewer still would opt for this over reading the documents). But that said, it's not overly much code. I might toy with Hyde to generate the site after finishing the monitors but no promises. That part is not something I would want to own for the long term, though.

Note that the large table at the end of the current consensus-health page is probably different, because it contains much more information than what's required to further investigate a warning message. We should not include that table in your Python rewrite. I just took it out from the metrics website to see if anybody cares.

Ahhh, much better. With the table page loads were painfully slow (25s) but now it's 0.8s. Much more usable.

I'm only interested in providing directory authority operators with the information they need to fix problems with the voting process.

Hi Karsten. I'm going to focus on the monitors for now and come back to discussion of a website after Sebastian and Peter have a chance to respond. That said, we totally agree on this point - the goal of this project is solely to help detect and resolve issues with authorities and consensus generation. If the website is vital to that then great (I still think it should be its own repository, but it's then definitely worth keeping). However, if we can do just as well by including additional information in the warnings then that would be even more maintainable in the long term.

My thoughts on this are probably best explained by a tangent. For years I've run a sybil checker called consensusTracker.py [1]. This checker did a bit more than just watch for sybil attacks. It also generated a pretty html report of the wax and wane in relay counts over the week among other things.

While these bells and whistles were pretty, no one cared about them and rightfully so. The sole purpose of a sybil checker is to provide us a notification saying "Oi! Potential problem here, recent relay additions are...".

The html report, while pretty, was both useless and made the code far, far more complicated than it needed to be. Finally I replaced that 601 line script with a far simpler 116 line counterpart that does just what we want and nothing more [2]. The monitor is now more reliable, maintainable, and can easily be updated in the future when necessary (something I couldn't do with the mess that was the previous script).

The lesson that I learned from this was "Start with the ending goal of a project and code toward that. Anything else will just result in feature creep." This isn't to necessarily say the DocTor website is unnecessary (only the site's intended audience like Peter can tell us that), but I definitely think we should figure out what he needs before resolving to keep it.

Cheers! -Damian

[1] https://gitweb.torproject.org/atagar/tor-utils.git/blob/e537044:/consensusTr... [2] https://gitweb.torproject.org/atagar/tor-utils.git/commitdiff/9b7de30

Hi Damian,

On 8/19/13 5:26 AM, Damian Johnson wrote:

Hi Karsten, I've finished a replacement for the DocTor consensus monitors...

https://gitweb.torproject.org/atagar/tor-utils.git/blob/HEAD:/consensus_heal... https://gitweb.torproject.org/atagar/tor-utils.git/blob/HEAD:/data/consensus...

This is awesome! Thanks! Replying to this mail first, then adding a first code review.

Like the other checkers this is presently running hourly and sending results my way. My vote would be to have them start sending results to tor-consensus-health@ [1] instead. This will double the amount of noise on the list but it should help us flush out any issues with the scripts. Once we have confidence in it we can shut down DocTor's checks.

Yes, I agree that we should have your script start sending results to the mailing list. Want to set that up? Not sure if your mails will bounce until somebody approves your sender address, but I guess we'll find out.

A code review would also be much appreciated. If there's any portions that you find confusing then let me know.

See below.

As for the DocTor website, I'm a little surprised Peter and Sebastian didn't reply. Not sure how we'd like to proceed there...

I just asked Peter on IRC. He didn't reply before, because of tl;dr.

So, Peter is fine with all consensus-health info being in a mail and not on the website. When he looks at the website, he's mostly interested in two things which aren't yet contained in status emails:

1. Do the authorities voting on BadExit agree about relays they assign this flag to? The warning might contain the diff of relay fingerprints they voted or didn't vote BadExit on. We'll probably want to rate-limit this warning to every 6 or 12 hours.

2. Do the bandwidth authorities report roughly the same number of Measured lines, or do these numbers diverge beyond a given threshold (say, 20%)? This warning should probably be rate-limited to every 24 hours, because new bandwidth authorities take some time to measure the network.

What do you think about adding these two warnings to your script? It seems we'd make the website obsolete, at least for Peter, by doing so.

How about we start a new thread on this list discussing only the part where we're planning to kill the website and move everything to the status emails? This thread is probably as long by now that nobody besides us reads it and we could as well discuss private stuff without anybody noticing like that I still disagree with you about Pepper Jack being tasty cheese and that I can highly recommend Muenster cheese.

Thoughts? -Damian

And here's the code review. As usual, feel free to ignore comments you don't agree with:

- Can you add a license to your script?

- Your rate-limiting logic seems to work only with full hours (and days). This may lead to unexpected results if two script executions don't start at the exact minute and second of an hour. In one case a message might be suppressed, in another case it might not. That's why I defined all rate-limiting intervals as X:30 hours. Maybe simply add or subtract 30 minutes from all intervals just to be sure, or add a minutes parameter with a default of 30.

- DocTor produces two output files containing warnings, one with new warnings and one with all warnings (which is empty if there are zero new warnings). New warnings are sent to the IRC bot and all warnings are sent to the mailing list. I agree that this approach is somewhat complicated, so maybe it's sufficient to just send the new warnings to both the IRC bot and the mailing list. Unless you didn't realize there was such a distinction and think it's worth adding to your script. Up to you.

- Are old warnings ever removed from your last_notified.cfg? Not sure if it matters though.

- Is the %is in `log.info("Suppressing %s, time remaining is %is"` supposed to be a %s?

- Your rule about not downloading a vote from an authority that didn't provide a consensus before seems quite strict. In theory, we could ask another authority for that first authority's vote. Maybe the first authority just didn't want to talk to us but was happy to talk to all other authorities. We should probably learn about problems with that authority's vote (or the absence of problems) even if it doesn't talk to us. I'm aware that this will make the download logic somewhat more complex.

- Typo in `unknown_consensus_parameteres`.

- Typo in `incompatable_authorities`.

- In `certificate_expiration`, is your check that a vote has exactly one authority really necessary? Wouldn't stem complain if this were not the case?

- Speaking of stem complaining about invalid votes or consensuses: what does your script do in such a case?

- I wonder if your check in `has_expected_fingerprints` would complain if somebody set up a fake "moria1". It probably shouldn't. That's why I added IP address and port to checkAuthorityRelayIdentityKeys.

- Maybe I missed them while reading your code, but did you add checks for checkContainedVotes and checkConsensusSignatures? Both checks are quite important, because even if an authority claims to add a vote, it may not have become part of the consensus, or the authority may not have signed the consensus that it contributed a vote to before.

That's all. Thanks for working on a DocTor replacement!

All the best, Karsten

On 8/20/13 4:41 AM, Damian Johnson wrote:

Thanks, Karsten, for the code review! I decided to take a long weekend so was able to address all of it...

Hi Damian,

neat, thanks for your quick response!

Replying inline, but leaving out the parts where I'd only write "okay".

...

Yes, I agree that we should have your script start sending results to the mailing list. Want to set that up? Not sure if your mails will bounce until somebody approves your sender address, but I guess we'll find out.

Nope, not getting through. Asking for my addresses to be whitelisted....

https://trac.torproject.org/projects/tor/ticket/9537

Looks like this is already solved.

...

1. Do the authorities voting on BadExit agree about relays they assign

this flag to? The warning might contain the diff of relay fingerprints they voted or didn't vote BadExit on. We'll probably want to rate-limit this warning to every 6 or 12 hours.

Done...

https://gitweb.torproject.org/atagar/tor-utils.git/commitdiff/e6d378a

Btw, it looks like they presently *are* out of sync...

NOTICE: Authorities disagree about the BadExit flag for 7E6A3AA70A156167E7AE543E50EF54321EC80AF0 (with flag: Faravahar, without flag: tor26, moria1) NOTICE: Authorities disagree about the BadExit flag for ADF62D3A1305F0B5404D41EEDADA68ECD294FC60 (with flag: Faravahar, without flag: tor26, moria1)

Are you sure? AFAIK, Faravahar never voted on BadExit. Which valid-after time was this?

...

How about we start a new thread on this list discussing only the part where we're planning to kill the website and move everything to the status emails?

Personally I don't see value in forking this thread, but I don't care strongly either.

It seems I learned some input from Peter on IRC, and Sebastian promised some feedback via email to you. Guess that's a fine start. We'll probably have a new discussion once we run both DocTors in parallel or when we finally switch over.

...

• DocTor produces two output files containing warnings, one with new

warnings and one with all warnings (which is empty if there are zero new warnings). New warnings are sent to the IRC bot and all warnings are sent to the mailing list. I agree that this approach is somewhat complicated, so maybe it's sufficient to just send the new warnings to both the IRC bot and the mailing list. Unless you didn't realize there was such a distinction and think it's worth adding to your script. Up to you.

I've ignored the IRC bot notifications since I'm both unfamiliar with it and presently lack a mechanism to provide it with notifications.

I take it as if the bot periodically reads files from disk then dumps the contents into an IRC channel? Who maintains the bot and might we switch it to another notification mechanism? Maybe it should read tor-consensus-health@ instead?

Alternatively I can change my send() function to do whatever the bot maintainer wants, though dumping files to disk seems a bit odd to me.

Peter runs the IRC bot. The interface on which it accepts input is simply an email address. You could send it the same content that you send to the mailing list, or a different output.

PS. A couple other things that need to be addressed at some point are:

• Running this on a TPO host rather than my desktop, and maybe also use a

TPO address to send the emails too.

Sure, we can do that. The easiest way would be that I run your script on yatei. Should I do that, similar to how I run DocTor on it?

We can later give you access to yatei (which you'll also need for co-co-maintaining metrics services), or we could run the script on a new, tiny VM.

• Moving these scripts to a real repository rather than my 'tor-utils' user

repo. Not sure though if we should keep the name 'DocTor' or opt for something more descriptive like 'DescriptorMonitors'.

I prefer repository names that aren't too descriptive, because if we ever want to extend or narrow scope of a tool, the name might confuse new people. That's what happened to metrics-db which doesn't even use a database anymore since I split off the metrics-web part.

How about we add your script to the DocTor repository? Again, to be quick, you could simply send me a patch produced with `git format-patch` or tell me a repository to pull from.

And if we want to do this right, we should give you a personal doctor.git and push rights to the official doctor.git.

Of course, if your preference is to start a new repository, I'm fine with that, too.

Thanks for spending your long weekend on this!

All the best, Karsten

On 8/20/13 6:21 PM, Damian Johnson wrote:

...

...

Done...

https://gitweb.torproject.org/atagar/tor-utils.git/commitdiff/e6d378a

Btw, it looks like they presently *are* out of sync...

NOTICE: Authorities disagree about the BadExit flag for 7E6A3AA70A156167E7AE543E50EF54321EC80AF0 (with flag: Faravahar, without flag: tor26, moria1) NOTICE: Authorities disagree about the BadExit flag for ADF62D3A1305F0B5404D41EEDADA68ECD294FC60 (with flag: Faravahar, without flag: tor26, moria1)

Are you sure? AFAIK, Faravahar never voted on BadExit. Which valid-after time was this?

Ack! Had a bug...

https://gitweb.torproject.org/atagar/tor-utils.git/commitdiff/9b8a883

tor26 and moria1 actually did disagree about those fingerprints, but Faravahar isn't involved. Odd, I thought we had three authorities voting on the BadExit flag.

Interesting that tor26 and moria1 disagreed. Just confirmed this for the votes from 01:00:00 UTC today. Looks like this warning is going to be quite useful. Great!

I think turtles was voting on BadExit but stopped doing so a short while ago.

...

Peter runs the IRC bot. The interface on which it accepts input is simply an email address. You could send it the same content that you send to the mailing list, or a different output.

Sounds good. If he tells me the address I'll send notifications to it.

Okay. Please talk to him about details here.

...

Sure, we can do that. The easiest way would be that I run your script on yatei. Should I do that, similar to how I run DocTor on it?

Could, the only gotcha is that it's presently sending through my auxiliary gmail account so we'll need to swap it to metrics@yatei.torproject.org (or something else) first. How are you sending emails?

`cat out/status/all-warnings | mail -E -s 'Consensus issues' tor-consensus-health@lists.torproject.org`

Don't ask me how the mail setup on yatei looks like. "It just works."

...

We can later give you access to yatei (which you'll also need for co-co-maintaining metrics services), or we could run the script on a new, tiny VM.

My vote would be for another VM since this is relatively unrelated to the present metrics infrastructure.

Here's a draft of a Trac ticket I was just about to file. Figured it's better to agree on requirements first before asking Peter to create a VM that is too small/large. Please file it or tell me to do so:

Summary: Can we have a tiny VM for DocTor?

Description: """ Damian rewrote the consensus-health checker which currently runs on yatei. We'd like to deploy his new tool on a new VM, unrelated to yatei. Can we have a tiny VM for this, say, with 256 MiB RAM and 1 GiB free disk space? It should have a doctor user that atagar and karsten can `sudo -u` to. Thanks! """

Component: Tor Sysadmin Team

...

I prefer repository names that aren't too descriptive, because if we ever want to extend or narrow scope of a tool, the name might confuse new people. That's what happened to metrics-db which doesn't even use a database anymore since I split off the metrics-web part.

Good point.

...

How about we add your script to the DocTor repository? Again, to be quick, you could simply send me a patch produced with `git format-patch` or tell me a repository to pull from.

And if we want to do this right, we should give you a personal doctor.git and push rights to the official doctor.git.

Sounds good! Would you mind filing the ticket to grant me push permission for doctor.git and create a user/atagar/doctor.git repo? I'll then push my tor-utils.git history into a new branch in doctor.git.

Added as #9545.

All the best, Karsten