Refactor process mappers into taggers.

[tomwilkie]

• Removed the cgroup mapper
• Adds topology tagger

[peterbourgon]

LGTM

[peterbourgon]

Address is an e.g. IP address-to-address topology. It's not new, it's a rename of the currently (badly) named Network.

I agree with your sentiment about the error handling... but remember that tagging is and will always be best effort — we can't assume that all Endpoint nodes will always have processes attached to them, for example.

We've uncovered something here, though. Take the docker tagger as an example. If we stick a docker_container_id key/val into every endpoint node, and then merge multiple nodes together in report.Render, as it stands now we're going to lose information, the last writer to the merged metadata will win. (In the current version this works by having an appendable list of Origins, no information is lost there.)

And I think this is what you were hinting at — so what about this —

• Eliminate tagger concept altogether
• Docker tagger becomes Container Topology generator (as you said)
• Every unique docker container ID, name, etc. is represented in just one node in that topology
• All cross-topology links are encoded as foreign keys, e.g. endpoint node has process_node_id FK, process node has container_node_id FK
• report.Render then resolves those FKs to produce Origins list in a RenderableNode, kind of like we have now

Make sense?

edit: none of that is done in this PR

[tomwilkie]

Yes this makes good sense. I though this was the plan all along. Yay code review!

I'm still not comfortable with this tagging interface, but I can't come up with anything nicer. We'll probably have to come back to it at some later point.

[peterbourgon]

Well if you agree with above we can rm the interface...

[tomwilkie]

Lets not introduce the containers topology in the this PR; it will make the rest of #123 unmergable.

But I think what you're saying is that, given above, tagger will not need to do foreign key lookups? And therefore, we can massively simplify this interface?

[peterbourgon]

Yes I think that's correct.

[tomwilkie]

Do you want to have a crack at that (if you're not busy on marketing)? I'm trying to break out some other changes right now.

[peterbourgon]

If you can merge this and we do that other work in a separate PR — LGTM

I guess we're doing that above-mentioned work in this PR.

[peterbourgon]

Motivation

Right now, the component that reads from Docker attaches metadata to existing nodes in the topologies. For example, if one container is responsible for 10 connections, the same set of Docker metadata is duplicated across those 10 nodes in the process topology — inefficient. And, if we try to merge those nodes, it's last-writer-wins on which information ends up in the merged node — buggy.

What we want instead is for the Docker component to populate its own topology, with each node representing a Docker container, and the metadata existing in that single place. Then, using the same example, if one container is responsible for 10 connections, those 10 nodes in the process topology get a single container_node_id key/val, pointing to the relevant node in the container topology. When we merge the nodes, we'll perform special handling on foreign-key-style key/vals, merging them via set-union.

This will also require changes to report rendering, so that it understands how to follow those links.

Gameplan

This change will occur in 4 stages:

1. Introduce package spy, describing things that can spy on the host machine and produce topologies. Port current func spy to spy.Proc, producing process and address topologies
2. Add spy.Docker, producing the (new) container topology, and populating the foreign keys to the other topologies as appropriate
3. Update Render to perform foreign-key resolution
4. Remove tagging interface and components

[peterbourgon]

Step 3 presents a speed bump. If Render will be able to perform foreign-key resolution, it will need to have access to all topologies. That means it needs to be defined on the full Report, rather than a single Topology. Starting that work, it became apparent that it would involve porting a significant portion of the work already done in #123, specifically implementing something roughly equivalent to its Render function but with lots of small changes to keep the diff only to the immediately relevant components. Some of those are quite tricky, for example reverting the definition of the pseudo func, and therefore the behavior of pseudo node generation, to the previous broken/nondeterminstic mode (previously described here). And which would eventually be undone, anyway, as we continue this path.

This is enough of a problem that I'd like to ask for your opinion, @tomwilkie, on how to proceed. I'm afraid that it will be a waste of time. Perhaps there is a better strategy.

[tomwilkie]

Some (friendly) comments:

• Stop renaming things! At least, stop renaming them in the same PR as you introduce new features. Its just bloats the PRs and makes them harder to review.
• Stop doing more than one thing per PR! We don't need to introduce the container topology in this PR. Lets get the taggers -> mappers refactoring in, then work on the next bit.

This change has already been open for too long; lets do the minimal thing to get it ready and then merge it. I think that minimal thing is to simplify / remove the tagger interface.

[peterbourgon]

The mapper creates new topologies. I don't understand how we can make this PR without introducing a container topology?

[tomwilkie]

Don't make the mapper create new topologies? Preserve the existing behaviour?

[peterbourgon]

The end result of this PR is that Tagger operates on just a Report. @tomwilkie PTAL.

[tomwilkie]

Please squash this down to one change and rebase on top of master.

[tomwilkie]

\o/ LGTM \o/