[metrics-bugs] #34191 [Metrics/Onionperf]: Combine multiple analysis files into single data set

[Tor Bug Tracker & Wiki]

#34191: Combine multiple analysis files into single data set
-------------------------------+--------------------------------
 Reporter:  karsten            |          Owner:  acute
     Type:  enhancement        |         Status:  needs_revision
 Priority:  Medium             |      Milestone:
Component:  Metrics/Onionperf  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:                     |  Actual Points:  0.5
Parent ID:  #33321             |         Points:  0.5
 Reviewer:  karsten            |        Sponsor:  Sponsor59-must
-------------------------------+--------------------------------
Changes (by karsten):

 * status:  needs_review => needs_revision


Comment:

 Thanks for working on this and providing a branch! I read your comment and
 the code but did not test anything yet. Two concerns:

  - I'm not sure if we can make it work with a variable number of arguments
 to the `-d` parameter. The usage string is really misleading by saying `-d
 PATH [LABEL ...]`, indicating that it accepts a single `PATH` and zero or
 more `LABEL`s. We would want it to say `-d PATH [PATH ...] LABEL` there.
 Do you know how to override that string? I didn't find something and
 stopped looking at options that felt like hacking argparse.

  - Regarding the limitation that the `LABEL` cannot be a path anymore, I
 think that's problematic. For one, it's turning this change into a
 backward incompatible one. And it's also not intuitive for new users who
 don't know how the parameter works today. Imagine a case where somebody
 puts all files for OnionPerf instance op-hk into one directory `op-hk/`
 and all files for op-ab into another one `op-ab/`. If they want to
 visualize these files using OnionPerf instance names as labels, they'd
 either have to pick different labels or rename the directories. Doable,
 but for sure surprising.

 Maybe it's better to keep this simple by allowing just two arguments as we
 do right now. Users will understand that they have to put all files for
 one data set into a directory. No surprises, they'll get what they want.
 What do you think?

 Apart from these concerns about the user interface, the patch looks good
 to me. The decisions about using `*json*` as pattern (you might want to
 use `*onionperf.analysis.json*` here, now that I think about it) or using
 functionality from the reprocessing module look good to me. I'd say if we
 can figure out the potential user interface issues, this will be a quick
 review. Thanks!

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