Opened 4 years ago

Closed 4 years ago

#17617 closed enhancement (implemented)

Parse flag thresholds and provide them as double values in BridgeNetworkStatus and RelayNetworkStatusVote

Reported by: karsten Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Sample data:

https://collector.torproject.org/recent/bridge-descriptors/statuses/20151116-150531-4A0CCD2DDC7995083D73F5D667100C8A5831F16D

flag-thresholds stable-uptime=3768284 stable-mtbf=2721929 fast-speed=55000 guard-wfu=98.000% guard-tk=691200 guard-bw-inc-exits=364000 guard-bw-exc-exits=366000 enough-mtbf=1 ignoring-advertised-bws=0

https://collector.torproject.org/recent/relay-descriptors/votes/2015-11-16-15-00-00-vote-EFCBE720AB3A82B99F9E953CD5BF50F7EEFC7B97-0B4498C736621BDD0867DE4D56D2D08B9C7B7215

flag-thresholds stable-uptime=1291379 stable-mtbf=460242 fast-speed=102000 guard-wfu=98.000% guard-tk=691200 guard-bw-inc-exits=4670000 guard-bw-exc-exits=3820000 enough-mtbf=1 ignoring-advertised-bws=1

Child Tickets

Change History (4)

comment:1 Changed 4 years ago by karsten

Oh, turns we already parse the "flag-thresholds" line in votes and provide its various elements via separate getters.

We could use the same code for bridge network statuses, ideally by moving RelayNetworkStatusVoteImpl.parseFlagThresholdsLine() to NetworkStatusImpl.

comment:2 Changed 4 years ago by karsten

Status: newneeds_review

comment:3 Changed 4 years ago by iwakeh

Why not make the newly added method comments javadoc?
And, maybe verify the other comments in the same interface and make them javacoc, too?
Would be a start for the javadoc issue.

In addition, metrics-lib should actually upgrade to java 7.

Oracle.com

WARNING: These older versions of the JRE and JDK are provided to help developers debug issues in 
older systems. They are not updated with the latest security patches and are not recommended for use 
in production.
...

It's really outdated by now (onionoo also uses 7) and the timing with the first release would be right.
It could also avoid the endless if-else-if statements using string switches.

comment:4 in reply to:  3 Changed 4 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Replying to iwakeh:

Why not make the newly added method comments javadoc?
And, maybe verify the other comments in the same interface and make them javacoc, too?
Would be a start for the javadoc issue.

I'd rather want to have a discussion about what exactly to include in JavaDoc comments first, because it's just too easy to make them very verbose and very useless. Once we agree on something, it's probably easier if I sit down for a few hours and edit all comments to turn them into JavaDoc comments. Let's discuss this on #16873. This shouldn't stop us from merging this patch.

In addition, metrics-lib should actually upgrade to java 7.

Oracle.com

WARNING: These older versions of the JRE and JDK are provided to help developers debug issues in 
older systems. They are not updated with the latest security patches and are not recommended for use 
in production.
...

It's really outdated by now (onionoo also uses 7) and the timing with the first release would be right.
It could also avoid the endless if-else-if statements using string switches.

Yep, we already switched to Java 7 just in time for the 1.0.0 release.

If you have suggestions for making the code cleaner now that we're using Java 7, please create another ticket. Happy to make the code cleaner if we can!

There, I just merged this patch into master, with the only change being an entry in the change log. Resolving. Please re-open if I missed something. Thanks!

Note: See TracTickets for help on using tickets.