[tor-bugs] #22428 [Metrics/CollecTor]: Add webstats module
Tor Bug Tracker & Wiki
blackhole at torproject.org
Mon Jan 22 15:37:56 UTC 2018
#22428: Add webstats module
-------------------------------+---------------------------------
Reporter: iwakeh | Owner: iwakeh
Type: enhancement | Status: needs_revision
Priority: High | Milestone: CollecTor 1.5.0
Component: Metrics/CollecTor | Version:
Severity: Normal | Resolution:
Keywords: metrics-2018 | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-------------------------------+---------------------------------
Comment (by iwakeh):
Replying to [comment:46 karsten]:
> I just reviewed the last commits (086e904, d7a2835, 45dd764) altogether.
Here's what I found:
Thanks for the quick review! No objections to the suggested changes and
fine to start testing more.
A few comments inline:
>
> - We'll have to clean up `CHANGELOG.md` before this branch gets merged.
It contains duplicate/overlapping entries for the new changes. I can do
that as part of merging.
+1
> - Let's also not forget to release metrics-lib 2.2.0 before merging
this branch and updating to 2.2.0 rather than 2.2.0-dev. I can do that as
part of merging, too.
+1
> - New copyrights should be changed to 2018. I can do this, too.
Thanks, I tried to catch all places, but didn't succeed.
> - Can I ask you to reference all member attributes or methods with
`this.`? I know it's syntactically irrelevant for the compiler, but for me
as a human being it improves readability a lot.
Sure. Do you intend to add the missing ones before testing or should I
check and supply a branch?
> - There's a line in `SanitizeWeblogs.java` , namely
`SortedSet<LocalDate> sorted = new TreeSet();`, which is missing a pair of
`<>`. This is the only actual code change I have, which is why I didn't
include this change in a patch.
An oversight, please change.
> - The default values `WebstatsPeriodMinutes = 31` and
`WebstatsOffsetMinutes = 0` seem a bit unusual. Wouldn't it make more
sense to start with an offset of 1 minute and then run every 30 minutes?
And regardless of the period being unusual, isn't that far too often? How
about 360 as period and, say, 31 as offset?
That was arbitrarily chosen. The larger intervals make more sense in
production.
> - Other than that, I'd say let's try it out when you say it's ready. I
think that's more efficient to find remaining bugs than re-reading and
tweaking the code. Just let me know when I should do some tests.
Fine to go!
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22428#comment:48>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list