[tor-bugs] #7359 [Tor]: Design/implement method for collecting/reporting statistics

[Tor Bug Tracker & Wiki]

#7359: Design/implement method for collecting/reporting statistics
------------------------------------------------------------------------+---
 Reporter:  robgjansen                                                  |          Owner:                    
     Type:  enhancement                                                 |         Status:  needs_review      
 Priority:  normal                                                      |      Milestone:  Tor: 0.2.5.x-final
Component:  Tor                                                         |        Version:                    
 Keywords:  performance, simulation, statistics, tor-relay, tor-client  |         Parent:  #7357             
   Points:                                                              |   Actualpoints:                    
------------------------------------------------------------------------+---
Changes (by karsten):

  * status:  needs_revision => needs_review


Comment:

 Replying to [comment:9 nickm]:
 > quick comments:

 I finally made it through the list!  Please see my updated morestats4
 branch.  It compiles, makes check-spaces happy, passes `--enable-gcc-
 warnings`, and runs in Shadow, but still I expect we'll need another
 iteration or two.  Below I'm replying to all comments that require more
 than just: "done."

 > General:
 >   * This needs to go along with a patch to control-spec.txt .

 Sure.  I converted proposal 218 into a control-spec.txt patch here:

 https://gitweb.torproject.org/user/karsten/torspec.git/shortlog/refs/heads/morestats4

 > e54d664f7bb1205162c1df3495f8ebc30c23d867
 >  * The last time we added K=V arguments in the middle of controller
 event, it created trouble for controllers, even though it wasn't supposed
 to do so, since the spec says we can.  Since nobody's actually hacking on
 Vidalia, would it be possible to just put the new K=V arguments at the
 end, to avoid possible trouble?

 rransom, I agree that sane controllers should handle this, but I also
 agree that we should change this, just to be safe.

 > 8d1f78c556abb570bb80ea84261c954d9746cf33
 >   * Instead of being controlled by TestingTorNetwork, should there be a
 separate option for EVENT_CONN_BW events?

 Added a new option TestingEnableConnBwEvent that depends on
 TestingTorNetwork being set, is 0 by default, and is changed to 1 if
 TestingTorNetwork is set.  I also added similar options for the other new
 events that are specified for test networks only.

 > c386d2d6ce4c4f58163acb385c7a5de1da8c5e30
 >  * cell_command_to_string doesn't really belong in control.c

 Moved to command.c, though I'm not entirely sure it belongs there.  If
 not, where would you want it to be?

 >  * append_cell_stats_by_command doesn't seem to document how long the
 arrays are.  I might be a bit more comfortable if it took a structure
 instead.  This

 Changed to a structure.  How does the last sentence end?

 >  * Is this really slow in practice? It seems like it's likely to be
 really slow.

 Do you mean slow or fast in your second sentence?

 If you refer to the comment explaining why we're re-using arrays for all
 circuits, maybe I didn't just mean slow, but also bad for memory
 fragmentation.

 >  * Prefer TOR_SIMPLEQ to manual list management for new queues.
 >  * This data structure ... how does it work out in practice? It looks
 like it's going to be hideously inefficient.

 I can't really think of a more efficient data structure.  What do you have
 in mind?  (I'll hold off changing the code to TOR_SIMPLEQ until I'm sure
 we want to use a queue there.)

 >  * This code needs unit tests and functional abstraction badly.

 I think functional abstraction is improved and code is now easier to test.
 I didn't write tests yet, because I'm not quite sure where to start.
 Would you mind writing one example test that I could use as a start for
 these functions?
  - connection_buckets_note_empty_ts in connection.c
  - bucket_millis_empty in connection.c
  - sum_up_cell_stats_by_command in control.c
  - append_cell_stats_by_command in control.c
  - format_cell_stats in control.c
 Are there any other new functions that need testing badly?

 Thanks!

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7359#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online