The 'create' function passed to addChild is only ever invoked when we
cannot find a matching process in the process topology. In these cases
the host and pid will be _all_ the info we are ever going to have,
both of which are incorporated in the id. Node summarisation renders
that as best it can; adding the PID as metadata does not make a
difference but for a line with that info in the detail panel, which
adds nothing since the PID is already included in the label.
The id and topology are enough to get some basic info for rendering.
This isn't just a fall-through for exceptional cases.
ContainerwithImageNameRenderer produces parent references using the
image name without version as the ID, and containers-by-image topology
uses that for grouping. By contrast, the container-image topology in
the report is keyed on docker image IDs.
We eliminate the custom parent renderer, which was a very partial
implementation MakeBasicNodeSummary.
We also ensure that parents are always rendered in the same, sensible
order. Previously the order was an alphabetic sort of the parent
topology IDs. Now lower level topologies come before higher level
topologies.
The node ID of group nodes is in fact the same value as we get from
looking up the metadata key contained in the group topology id. So
just use that since a) we always have it, and b) we save a LatestMap
lookup.
We fall back to image id when we cannot find a name. We extract the
image id from the node id rather than the docker.ImageID latest map
entry since that way we are guaranteed to get it.
The main change here is to to label the node with its pseudoID as the
last resort.
We also set the rank to the pseudoID instead of the full node id,
since including the "pseudo:" prefix is not conducive to good ranking.
The rest is just moving from 'if' to 'switch'.
We fall back to the truncated container id when we cannot find a
name. NB: this also happens when rendering a container as a parent.
We cope with the absence of an image name and/or host name.
We cope with the absence of the process name and/or container name,
and extract the hostID and pid from the node id rather than metadata
since that way we are guaranteed to get values for them.
This doesn't make any difference to the outcome - we were simply
setting the shape in the NodeSummary to "", which is what it starts
out as - but looks less weird in the code.
It's a dynamic type check of sorts.
In the process we stop abusing ParseNodeID for extracting the host,
which in turn allows us to clarify its purpose.
Pre-allocating slices really pays dividends when we create loads of
them and they don't grow very large.
We take special care to return nil rather than 0-length slices. This
a) saves further on allocation, and b) is required for some crude
tests to pass that match on nil rather than length.
Also, a bunch of code in templates/tables used slices as Maybe's,
i.e. returning nil or a single-element slice, which is horrendously
inefficient. Eliminating these saves a lot of allocations.
This doesn't make any difference to the outcome - we were simply
setting the shape in the NodeSummary to "", which is what it starts
out as - but looks less weird in the code.
Report.Copy() shallow-copies the nodes in Report.Nodes. Hence
Node.Metrics is shared between the original and the copy. Hence bad
things happen when modifying it.
This bug has laid dormant because by luck other tests in detailed_test
involving metrics are executed first.
Any existing list would be mutated by result(), which is bad.
Note that all the existing newJoinResults() call sites pass in nodes
with no adjacencies, so this is purely a safety measure.
Since we seed the joinResult with the nodes from the topology we are
mapping to, we know the 'create' function is only called when there is
no node with the specified id.
This neatly makes the 'create' function only do what it says,
i.e. return _new_ nodes.
