[metrics-bugs] #22112 [Metrics/Metrics website]: Replace torperf.csv with onionperf.csv

[Tor Bug Tracker & Wiki]

#22112: Replace torperf.csv with onionperf.csv
-------------------------------------+------------------------------
 Reporter:  karsten                  |          Owner:  metrics-team
     Type:  enhancement              |         Status:  needs_review
 Priority:  Medium                   |      Milestone:
Component:  Metrics/Metrics website  |        Version:
 Severity:  Normal                   |     Resolution:
 Keywords:                           |  Actual Points:
Parent ID:                           |         Points:
 Reviewer:                           |        Sponsor:
-------------------------------------+------------------------------
Changes (by karsten):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:2 iwakeh]:
 > Committed to master means 'productive', but the current onionperf.csv
 links on metrics.tp.o lead to 404; is that intended?

 Ah, no, that's an oversight.  Fixed!  (Just not yet deployed.)

 > init-onionperf.sql:  The last "group by" on the second to the last line
 contains a `3`, which seems to refer to `source`, but `source` in this
 select is set to `''`.  What is intended here?

 The intention was to aggregate over all sources and to include `''` in
 every resulting row, so that there's the same number and type of columns
 as in the `SELECT` above.  That third column, `'' AS source`, is not an
 aggregate function, so I included it in the `GROUP BY`.  But I just tried
 out to simply drop that column from the `GROUP BY`, which worked just
 fine.  Changed.

 > Why are there columns in the measurements table that are not used later
 on (mostly the unrecognized key lines))?  Shouldn't these be omitted?
 > Here it seems as if only 'endpointremote' is used to determine the
 content of column 'server'.  If just that is needed it ought to be filled
 by the java code initially.  But, I might have overlooked something?

 I tried to make the schema as complete as possible, so that we can write
 different views in the future more easily.  In fact, I only left out path
 information and build times, because I wanted to avoid making the schema
 too complex while not using the data at all.  But I could see how we're
 adding those parts, too.  Regarding your question why we included
 unrecognized keys, they will all be recognized in the next metrics-lib
 version, in which case we'll update this code.

 > In some other module we used constants for column-names in Java.  As the
 db redesign will be future work, is this intentionally left to be improved
 then?

 Well, I'm not a big fan of how we used string constants there, because
 they give a false sense of code robustness by protecting against typos,
 but they cannot be used in prepared statements and also don't provide any
 type safety.  And at least during development I ran much more often into
 the latter two issues than into typos.  But maybe you're right and string
 constants are better than nothing.  Changed.

 Please find [https://gitweb.torproject.org/karsten/metrics-
 web.git/log/?h=task-22112 my task-22112 branch] with the fixes.  What do
 you think?  Ready to remove the beta label and call this done? :)

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