[tor-dev] Proposal: Controller events to better understand connection/circuit usage

[Damian Johnson]

Hi Karsten. Looks good to me, just a few comments on the spec parts...

> 5.1. Adding an ID field to ORCONN events

Please define the format of ConnId. This would belong in section 2.4,
expanding...

; Unique identifiers for streams or circuits.  Currently, Tor only
; uses digits, but this may change
StreamID = 1*16 IDChar
CircuitID = 1*16 IDChar
IDChar = ALPHA / DIGIT

> 5.2. Bandwidth used on an OR or DIR or EXIT connection

Minor nit pick, but the normal way of showing an enumeration is...

ConnType = "OR" / "DIR" / "EXIT"

Please make it clear in the spec if new ConnType are allowed or not.

> 5.3. Per-circuit cell stats

Again, please specify the format of all of these cells. Most are
obviously numeric, but a formal specification helps answer questions
like 'can I assume this can't be negative?'.

Are you sure that you want to use positional arguments for all of
these? If you do then you'll lose a lot of flexibility. Making them
positional means...

* these values will always be mandatory, even if you want to deprecate
them in the future
* you'll always need to keep this ordering, with new values appended to the end

> "650" SP "TB_EMPTY" SP ["GLOBAL" || "RELAY" || "ORCONN" SP ConnID]
> SP ReadBucketEmpty SP WriteBucketEmpty SP LastRefill CRLF

Ewww! No, please don't do this. Pretty please with a cherry on top?

Having either one *or* two positional arguments for the first slot
makes this a special snowflake among tor events (and hence need
special handling). ConnID should be an optional keyword argument
instead, documented to only exist if the
event type is "ORCONN".

Same cautioning as before about positional vs keyword arguments, and
defining the values.

Cheers! -Damian