[webapp] Add Latency Metrics

[mwfarb]

This PR adds background echo latency data and allows the user to collect echo/traceroute data for all paths or one path, both continuously.

Changes:

• Add background echo requests for paths on Apps.
• STMP echo latency results in purple next to PATHs list tree.
• STMP traceroute latency results in purple next to PATHs list tree.
• PATHs list tree shows Last RTT e2e latency results.
• PATHs interfaces list tree shows Last RTT hop latency results.
• Path graph arrows shows Min/Best RTT hop latency results.
• Provides ability to test survey all paths using Run Continuous.
• Closes [webapp] Add bwtester auto-path switching and path colors in tests #26, providing auto-path switching.
• Fixes traceroute DB query bug which ignored some results.
• Added minor formatting and test files.

New Operation:

• Run Continuous with all PATHs collapsed in the list tree will run bwtest/echo/traceroute rotating between each path.
• Run Continuous with one PATH expanded in the list tree will run bwtest/echo/traceroute for that path only.
• Echo/traceroute will update the Last RTT in purple in the path list.
• Traceroute will update the Minimum RTT in purple on the Paths graph. As a result, Run Continuous with traceroute will eventually produce an accurate intra-AS minimum latency performance metric.
• Only background echo is automated, and does not yet handle rapidly changing path availability.

Future:

• Browser/server polling is at/past its limit on usefulness. We should probably move to using WebSockets to update graph results.

---

This change is 

[FR4NK-W]

Browser/server polling is at/past its limit on usefulness

Can you tell us more about this?

Reviewed 14 of 14 files at r1.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @mwfarb)

---

webapp/webapp.go, line 505 at r1 (raw file):

Quoted 8 lines of code…

		thePath := ""
		pathStr := r.PostFormValue("pathStr")
		paths := strings.Split(pathStr, ",")
		if pathIdx >= 0 && pathIdx < len(paths) {
			thePath = paths[pathIdx]
		}
		start := time.Now()
		executeCommand(nil, r, thePath)

I think this would be less confusing:

		path := ""
		paths := strings.Split( r.PostFormValue("pathStr"), ",")
		if pathIdx >= 0 && pathIdx < len(paths) {
			path = paths[pathIdx]
		}
		start := time.Now()
		executeCommand(nil, r, path)

---

webapp/lib/sciond.go, line 55 at r1 (raw file):

log.Error

Returned errors are both logged here and in CheckError.
The only instance where errors are not logged twice is on l. 111. Is that intentional?
If you want to keep CheckError, we could check the new error created on l. 111. Otherwise I don't see the point of having CheckError, if not all errors pass through it and then we still do log them separately.

---

webapp/lib/traceroutecont.go, line 158 at r1 (raw file):

(w, strings.Replace(string(jsonBuf), "%", "%%", -1))

I don't see anything being passed to the format string, so why not use fmt.Fprint?
Also the json marshalling and mangling is duplicated in file echocont.go. Could we factor that part out? Only the data retrieved from the model is different between the two functions.

---

webapp/models/traceroute.go, line 355 at r1 (raw file):

	for i := 0; i < len(result); i++ {
		hop := result[i]

Use for i, hop := range result {
then you can skip the hop line.

---

webapp/web/static/js/asviz.js, line 119 at r1 (raw file):

paths += ",";

Can we use join(",")?

function formatPathStringAll(res, type) {
    var paths = [];
    for (var i = 0; i < res.if_lists.length; i++) {
        paths.push(formatPathString(res, i, type));
    }
    return paths.join();
}

I think it would be more readable (and not produce a dangling ",")

---

webapp/web/static/js/tab-paths.js, line 145 at r1 (raw file):

setTracerouteLatency

Can you add a comment?

---

webapp/web/static/js/tab-paths.js, line 174 at r1 (raw file):

Quoted 9 lines of code…

            if (if_.RespTime1 > 0) {
                path.latency = updateStats(if_.RespTime1, path.latency);
            }
            if (if_.RespTime2 > 0) {
                path.latency = updateStats(if_.RespTime2, path.latency);
            }
            if (if_.RespTime3 > 0) {
                path.latency = updateStats(if_.RespTime3, path.latency);
            }

Can we move this out of the loop, to reduce the nesting level (and change the loop condition to interfaces.length-1)?

---

webapp/web/static/js/tab-paths.js, line 845 at r1 (raw file):

pathLen - 1

Can we use idx here? I think this can be simplified. Maybe also add a comment or two.

---

webapp/web/static/js/tab-topocola.js, line 333 at r1 (raw file):

style

Could we have one class per type and assign that one instead?

---

webapp/web/static/js/webapp.js, line 529 at r1 (raw file):

.match("\\[.*]")

I think this would be more readable we had something like matchHopString (there are two other instances where the HopString is extracted).

---

webapp/web/template/index.html, line 418 at r1 (raw file):

style='background-color: black; font-weight: bold;'

Can we avoid the inline style?
Bootstrap has .font-weight-bold and .bg-dark, or maybe define a new class.