Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add irtt report command #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add irtt report command #15

wants to merge 2 commits into from

Conversation

mlouielu
Copy link

This make irtt can reproduce the result and display on the screen from output json file

$ irtt report output.json

                        Min    Mean  Median     Max  Stddev
                        ---    ----  ------     ---  ------
                RTT   1.5ms  2.02ms  1.99ms  5.59ms   238µs
         send delay  -1.28s  -1.28s  -1.28s  -1.27s   858µs
      receive delay   1.28s   1.28s   1.28s   1.28s   831µs
                                                           
      IPDV (jitter)   201ns   175µs  69.7µs  3.57ms   287µs
          send IPDV    25ns   142µs  55.4µs  1.55ms   219µs
       receive IPDV      0s  47.4µs  23.1µs  3.49ms   163µs
                                                           
     send call time  7.35µs  41.2µs          91.6µs  12.9µs
        timer error     7ns  16.7µs           189µs  22.4µs
  server proc. time  7.56µs  21.9µs            55µs  3.38µs

                duration: 10s (wait 16.76ms)
   packets sent/received: 983/983 (0.00% loss)
 server packets received: 983/983 (0.00%/0.00% loss up/down)
     bytes sent/received: 169076/169076
       send/receive rate: 135.4 Kbps / 135.4 Kbps
           packet length: 172 bytes
             timer stats: 17/1000 (1.70%) missed, 0.17% error

This make irtt reproduce the result and display on screen
from output json file
@heistp
Copy link
Owner

heistp commented Dec 6, 2018

Hi, thanks for your pull request! I can see that I should have some things cleaner in my code so that this would have been easier to implement. I'll do some refactoring as a result, but first, I have some questions/requests for changes to the pull request:

  • Any reason to add the fields FirstSend, LastSent, FirstReceived and LastReceived?
  • In irtt_report.go, please replace the condition and call to panic with a call to exitOnError(err, exitCodeRuntimeError)
  • Instead of duplicating the json struct in cconfig.go, can you find a clean way to share the same struct with MarshalJSON?
  • Please change the usage text to be consistent with the other commands, so instead of "read output data and display the result" it could be "reads output JSON and displays the result"
  • The client -o flag can produce gzip'ed output. Please detect that and gunzip as necessary.
  • Instead of reading the output file into a slice with ioutil.ReadFile then using json.Unmarshal, use json.Decoder and take the data from an io.Reader, to reduce memory consumption for large files.
  • Instead of adding a single UnmarshalJSON method, add UnmarshalJSON methods for each nested type that needs it (ReceivedStats, StampAt, Clock, etc).
  • You can leave UnmarshalJSON for DurationStats as it is if it works, but this is an example of where some refactoring would be useful. We're not restoring DurationStats to its original state here, so this may be brittle after future changes.

In case you don't get a chance to make these changes, I've added the report command to the plans for version 0.9.2, but have some other work to do for 0.9.1 before I'll get to it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants