initial draft of last mile latency test

[kyle-macmillan]

Ported the last mile latency test. Very similar to netrics-traceroute. Using scamper to run traceroute while also allowing arbitrary destinations. If the destination is given as a hostname it is resolved because scamper only takes IP addresses as input.

[jesteria]

Nothing big to say, largely just some food for thought.

(And I'm sure these will evolve over time, regardless.)

[jesteria]

That's cool, but can't we expect the json module to parse proper JSON Booleans (rather than strings), and expect the configuration to be proper?

I.e. is this sufficient?

Suggested change

	if str(params['verbose']).lower() in ['true', '1']:
	params['verbose'] = True
	elif str(params['verbose']).lower() in ['false', '0']:
	params['verbose'] = False
	else:
	exit_code = CONFIG_ERROR
	if not isinstance(params['verbose'], bool):
	exit_code = CONFIG_ERROR

[jesteria]

This is just food for thought, but I have wondered if it wouldn't be useful to make use of proper Python exceptions so long as we're in Python, and leave the exit code stuff to the boundary (in the main function). E.g.:

PARAM_KEY_TYPES = (
    (int, ('attempts', 'timeout')),
    (bool, ('verbose',)),
)

def get_params(stdin=sys.stdin, defaults=PARAM_DEFAULTS, key_types=PARAM_KEY_TYPES):
    params = dict(defaults, **json.load(stdin))

    for (key_type, keys) in key_types:
        for key in keys:
            if not isinstance(params[key], key_type):
                raise TypeError(f'{key}: must be {key_type}: {keys}')

    return params

def main():
    try:
        params = get_params()
    except (ValueError, TypeError):
        json.dump({'stdin': {'retcode': CONFIG_ERROR, 'message': '…'}}, sys.stderr)
        sys.exit(CONFIG_ERROR)

[jesteria]

Just to be clear, we expect only one line in the output to both be JSON and have "type": "trace"? (Otherwise, it appears that subsequent passes will overwrite the result key "src".)

If that's the case, I think this could be made clearer: there's no need to define res outside of the loop if it's entirely a product of a particular line. Or, for that matter, this logic of finding the matching line can be separated from the logic of mapping it to a result. Say:

for line in out:
    try:
        record = json.loads(line)
    except ValueError:
        continue

    if record.get('type') == 'trace':
        break
else:
    # no line matched!
    return None

res = {
    'src': record['src'],
    'dst': record['dst'],
    'attempts': record['attempts'],
}

…

return res

[Ref]

[jesteria]

Too fancy perhaps but you could say tersely:

Suggested change

	if "dig" not in stderr_res:
	stderr_res['dig'] = {}
	stderr_res['dig'][params['target']] = stderr_dst
	stderr_res.setdefault('dig', {})[params['target']] = stderr_dst

[jesteria]

Cool, thanks for the ongoing changes. Let me know if you intend to work more on this – you should feel free to do so, but if not, I'm happy to get it incorporated into the rest.

[jesteria]

This work has been merged! 🎆

…Via #32.