After dropping extra metadata in the rest of this PR, our usage of
joinResults.add* only ever ends creating minimal nodes, from just an
id and topology. Hence joinResults.add* can be invoked with simply an
id and topology instead of a generic node creation function.
Some process nodes may not have a HostNodeID metadata, e.g. when an
endpoint references a pid that we know nothing about. When mapping
processes to containers, we therefore shouldn't rely on
HostNodeID. Instead we can obtain the hostID from the process node ID.
This has been broken for a while, possibly forever.
...when creating Uncontained pseudo nodes. Summarisation of
Uncontained/Umanaged only looks at the ID, which includes the
HostNodeID.
We adjust the promotion of Uncontained to Unmanaged, to operate on the
ID instead of (re)extracting the hostID from the HostNodeID
metadata. With that, nothing looks at the HostNodeID metadata of
Uncontained/Unmanaged nodes.
The ImageID is already the id of the node we are creation, and that's
what summarisation renders in the event we fail to join this node with
a node from the ContainerImage topology that has more metadata.
Nothing is looking at the ImageID metadata field.
This was building a set of all the image ids represented by the same
unversioned image. Well, it was doing that until I broke it with a
silly mistake in #1739 - instead of extracting the imageID from the
original node ID, it's extracting it from the updated ID, which is the
unversioned image. Even if it was working though, it's pointless
since nothing is looking at that info.
The container hostname is already the id of the node, and that's what
summarisation renders. Nothing looks at the docker.ContainerHostname
metadata of nodes in the ContainerHostname group topology.
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.
This eliminates the awkward distinction between ProcessRenderer and
ColorConnectedProcessRenderer.
It also ensures that processes resulting from direct rendering of the
process topology (/api/topology/processes is invoking
ProcessWithContainerNameRenderer and /api/topology/processes-by-name
is invoking ProcessNameRenderer) are colored and hence summarising
them correctly sets the 'linkable' property. This was the behaviour
prior to the revamping of the rendering pipeline. However, it doesn't
actually make a practical difference since process detail panels only
show other processes as connection endpoints, and these are always
marked linkable anyway.
This is preparatory to future refactorings: all existing calls are to
Endpoints which have no children and where we don't want a Counter.
We make addChildAndChildren an obvious extension of addChild even
though it adds a dead code path (we never call addChildAndChildren
with an endpoint).
and add a comment indicating non-memoisation of other, not shared
top-level renderers.
This memoisation is effective when the browser requests multiple
topologies for the same report.
This guarantees that the output won't contain bare nodes containing
just an id and topology, as produced by ipToNode. Previously this was
ensured by calling convention, now it's ensured by construction.
The change is necessitated by the removal of procspied/ebpf endpoint
filtering in the renderers, as a result of which the odd
conntracked-only, unconnected pseudo node can sneak through.
This new way of doing things also makes renderers more composable and
robust, and more directly reflects the objective:
- in the process topologies, filter out all unconnected nodes
- in all other topologies, filter out unconnected pseudo nodes
The filtering of endpoints causes some connections to get missed for
non-eBPF-tracked connections. Furthermore, the filtering of endpoints
is entirely pointless when the probes run eBPF since the filters just
pass through eBPF-tracked endpoints (for good reason too; because
otherwise some connections would be missed). So in that case it is
just costing CPU and removing it actually improves performance.
Note that removing the filtering does not result in over-counting
connections since that is done by source ip:port pairs.
Fixes#2551.
Fixes#2558.
ProcessRenderer was coloring connected nodes because we need that info
for rendering details panels. However, the main process topology view
renderers depending on ProcessRenderer were also doing coloring
themselves. For the 'processes' topology that was literally
duplicating work. For the 'processes-by-name' topology that was
throwing away the process coloring, and then coloring at the name
level.
Solution: remove the coloring from the ProcessRenderer, thus
eliminating the duplicate/thrown-away work, and introduce a
ColorConnectedProcessRenderer which is only used in places that
populate details panels.
