Registration tracker#268
Conversation
ewust
left a comment
There was a problem hiding this comment.
Overall looks good, some minor comments that aren't blockers but could help improve the tool in general.
| EXE_DIR=./bin | ||
|
|
||
| all: rust libtd conjure app registration-server ${PROTO_RS_PATH} | ||
| all: rust libtd conjure app registration-server registration-tracker ${PROTO_RS_PATH} |
There was a problem hiding this comment.
Is registration-tracker something we want built when we build the station? I would think this is more of a utility, so it wouldn't require building every time we make a production station, more for when we want to debug.
| ) | ||
|
|
||
|
|
||
| type transportStats struct { |
There was a problem hiding this comment.
This is fine, but it may make more sense to have this just be a map of int64s, keyed on the transport.Name(), since that's what ends up happening in IncrementCount(). It would make it easier to add new transports (the name just has to be unique), and then printing it out is simply selecting what set of names/keys you want to output.
| ) | ||
| } | ||
| } | ||
| rs.v4Registrations = make(map[uint]*transportStats) |
There was a problem hiding this comment.
Does this leak the old rs.v4Registrations? I would guess no, but might be good to double check that we don't need to delete the old one
| select { | ||
| case <- ctx.Done(): | ||
| logger.Fatal("closing all ingest threads") | ||
| case msgChan <- data: |
There was a problem hiding this comment.
Why have regChan return an array of msgChans that you have to read from? Is there a reason to avoid it providing the messages directly?
| continue | ||
| } | ||
|
|
||
| go func(ip net.IP, registration *cj.DecoyRegistration) { |
There was a problem hiding this comment.
You may want to turn this into a worker model, rather than launching a new thread per registration. New threads have the downside that you may launch too many (if a storm of new registrations comes in), and there's likely some upper-bound of useful threads doing this.
If this tool never runs on a production server it's probably okay, but if it does we would want to bound the number of concurrent threads launched by this tool to prevent us from consuming too much CPU.
A new independent module that tracks registrations and aggregates them by their originating ASN.