write results to directory OR tar archive

[jesteria]

Note

If this looks good, it's probably best to merge via rebase, (rather than via a merge commit or squash), as supported by GitHub (below).

Changes

Netrics support

Essentially, we just need to be able to write the result files to a specified path – namely stdout – via a tar file. (A local directory, such as data/, is certainly useful, but inappropriate for measurement devices, the Netrics framework and our data pipeline.)

Otherwise, tar functionality isn't needed, (and writing results to both is redundant).

So, we now write results to tar archive OR directory depending on flags:

• output is specified via single flag -o/--out-path which may be "dir/" or "file.tar[.gz]" or "-" (stdout)
• archive flag removed
• requests confirmation when out path is ambiguous "data" (perhaps directory is intended but will otherwise write archive to that path)
  • only prompts to confirm extension-less output file when stdout is the terminal
• requests confirmation to write archive to terminal unless forced
• data directory is now a temp dir when requested output is an archive
• archive defaults to uncompressed tar, is gzip-compressed iff out-path features extension ".tar.gz" or ".tgz"
• archive members named consistently (without timestamps)

Also:

• non-critical logs may be disabled with -q/--quiet flag

Refactoring

While at it, I undertook some refactoring.

Singular command

Most important, what Netrics requires shouldn't be so unusual, and in this case can be a simple matter of configuration, rather than a separate executable.

(This isn't to say that we can't or shouldn't ever also devise a Netrics-dedicated executable wrapper, when adding this tool to its builtins, for convenience and for "endorsement" of the tool. Rather, the ideal for both users and developers of Traceneck is that we maintain a single, consistent interface, with reasonable options and defaults. It's best that Traceneck not worry about Netrics per se, so much as about the options useful to Traceneck in the Netrics environment. Indeed, the benefit of developing this measurement this way, as opposed to the implementation of the existing measurements, is that we have complete control of the underlying tool and can make this happen. Measurements which are simple variations on ping or ndt don't require this level of development; but, their Python implementations allow them to operate robustly for Netrics. On the other hand, such a Netrics-dedicated wrapper for Traceneck could be very slim – nothing more than an sh script: traceneck -qo- 🤩)

And so, we nix the Netrics build. The Netrics build is by now more than similar enough to the generic build that it doesn't warrant special support. Though this means special configuration for Netrics, this is trivial for Traceneck, (as opposed to less compatible tools); and, current deployment of Netrics entails such configuration, regardless, (to specify either NDT or Ookla speed-testing).

It should now be sufficient to execute Traceneck as follows under Netrics:

traceneck -qo -

a.k.a.:

traceneck --quiet --out-file=-

(i.e. quiet and writing to stdout)

or more fully:

traceneck --quiet --out-file=- --tool=[ndt|ookla]

Otherwise:

• .dockerignore file re-added (missing since repo migration): recommended with Dockerfile COPY commands
• Dockerfile made to target GO version in use to avoid development environment issues
• Dockerfile made to use newer HEREDOC syntax with RUN commands
• docker create used to create inactive container in lieu of run … sleep in CI

Config verification via slice

This is far less important, and perhaps debatable; but, while tweaking config variables such as OutPath/OutDir, it occurred to me that it might be useful to verify/finish config via a slice (array) of closures, returning a generic interface, rather than developing named functions which are each handled specially.

At the same time, it seemed preferable that os.Exit expressions are not evaluated too deep in the stack – particularly now that I've implemented a temporary directory in need of cleaning up….

So, I've defined config "finishers" – in lieu of verifyFlags – and now ensure temp dir clean-up:

• Rather than named config verification/finalization functions invoked specially by verifyFlags, a slice of verification/finalization closures is now defined, which return structs abiding by a defined finalization interface (as well as the error interface). These finalizers are invoked and their results handled by config.finish().
  • config.Parse() now returns any error resulting from the above to main() – such that process termination may (generally) be handled there, and to ensure that clean-up takes place.
• Any temporary working directory may now be reliably torn down (cleaned up) from main() – via config.Teardown()

[jesteria]

Happy to discuss these changes via another medium if that would be helpful.

Otherwise, I believe all that leaves for Traceneck is to slim down the metadata.json.

[staveesh]

Thanks for the PR, @jesteria! I am generally okay with the above changes and agree to post-process the PCAPs to calculate the RTT samples that we need for further analysis. I'll assign @arghyadipchak as a reviewer for the PR and let him provide his comments (if any), as he developed a vast majority of this tool. He will also be able to truncate the metadata (only for the Netrics binary) once we merge this PR.

[jesteria]

@staveesh Sounds good.

Regarding the metadata size: rather than consider the behavior of a separate Netrics binary (which this branch removes), I imagine the metadata verbosity might be controlled by another command line flag. For example, a flag --meta-terse or --terse-meta or --terse-metadata might set a boolean config variable to make the metadata less verbose; or of course we could use an opposite flag like --verbose-metadata (though I'm guessing you all would prefer that verbose metadata is the default).

[staveesh]

I agree. @arghyadipchak let's add the --terse-metadata flag and discard the RTT samples when the flag is set. For our lab experiments, we can continue using the default value (--terse-metadata=False).

[jesteria]

BTW: The --terse-metadata flag works for me.

I see that, with it set, rtt_samples is simply null. This is fine, for now, anyway. (Presumably we hope to write useful aggregate data, eventually.)

[staveesh]

Yes, eventually we will replace these statistics with those critical for locating the bottleneck. We may need a key name change as well. We kept it as null because we wrote a data pipeline for the lab experiments which expects this key to exist.