Skip to content

Commit

Permalink
Update the broctl top command to not show the "Proc" column
Browse files Browse the repository at this point in the history
Removed the "Proc" column (it showed "parent" or "child" for each bro
process) because after the switch to broker, Bro no longer runs as
two processes.

This simplified some code, and now the broctl "top" command should
run faster because it no longer needs to run the "get-childs" helper
script.

Note that these changes do not change the format of the broctl stats.log
file (the "parent" field should probably be removed in a future release).
  • Loading branch information
Daniel Thayer committed Sep 7, 2018
1 parent 4461807 commit 364c277
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 104 deletions.
82 changes: 30 additions & 52 deletions BroControl/control.py
Expand Up @@ -1005,48 +1005,29 @@ def df(self, nodes):
return results

# Returns a list of tuples of the form (node, error, vals) where 'error' is
# an error message string, or None if there was no error. 'vals' is a list
# of dicts which map tags to their values. Tags are "pid", "proc", "vsize",
# an error message string, or None if there was no error. 'vals' is a
# dict which maps tags to their values. Tags are "pid", "vsize",
# "rss", "cpu", and "cmd".
def get_top_output(self, nodes):

results = []
cmds = []

running = self._isrunning(nodes)

# Get all the PIDs first.

pids = {}
parents = {}

for (node, isrunning) in running:
if isrunning:
pid = node.getPID()
pids[node.name] = [pid]
parents[node.name] = pid

cmds += [(node, "get-childs", [str(pid)])]
pids[node.name] = node.getPID()
else:
results += [(node, "not running", [{}])]
results += [(node, "not running", {})]
continue

if not cmds:
if not pids:
return results

for (node, success, output) in self.executor.run_helper(cmds):
if not success:
results += [(node, "cannot get child pids", [{}])]
continue

try:
pidlist = [int(line) for line in output.splitlines()]
except ValueError as err:
results += [(node, "invalid PID: %s" % err, [{}])]
continue

pids[node.name] += pidlist

cmds = []
hosts = {}

Expand Down Expand Up @@ -1080,41 +1061,41 @@ def get_top_output(self, nodes):
# The error msg gets written to stats.log, so we only want
# the first line.
errmsg = output.splitlines()[0] if output else ""
results += [(node, "top failed: %s" % errmsg, [{}])]
results += [(node, "top failed: %s" % errmsg, {})]
continue

if not output:
results += [(node, "no output from top", [{}])]
results += [(node, "no output from top", {})]
continue

# Get a list of all bro processes, where each bro process is a list
# of fields from the "top" helper.
# Get the bro process info, which is a list of fields from
# the "top" helper.
procinfo = []
try:
procs = [line.split() for line in output.splitlines() if int(line.split()[0]) in pids[node.name]]
for line in output.splitlines():
if int(line.split()[0]) == pids[node.name]:
procinfo = line.split()
break
except (IndexError, ValueError) as err:
results += [(node, "bad output from top: %s" % err, [{}])]
results += [(node, "bad output from top: %s" % err, {})]
continue

if not procs:
if not procinfo:
# It's possible that the process is no longer there.
results += [(node, "not running", [{}])]
results += [(node, "not running", {})]
continue

vals = []
vals = {}

try:
for p in procs:
d = {}
pid = int(p[0])
d["pid"] = pid
d["proc"] = "parent" if pid == parents[node.name] else "child"
d["vsize"] = int(float(p[1])) #May be something like 2.17684e+9
d["rss"] = int(float(p[2]))
d["cpu"] = p[3]
d["cmd"] = " ".join(p[4:])
vals += [d]
pid = int(procinfo[0])
vals["pid"] = pid
vals["vsize"] = int(float(procinfo[1])) #May be something like 2.17684e+9
vals["rss"] = int(float(procinfo[2]))
vals["cpu"] = procinfo[3]
vals["cmd"] = " ".join(procinfo[4:])
except (IndexError, ValueError) as err:
results += [(node, "unexpected top output: %s" % err, [{}])]
results += [(node, "unexpected top output: %s" % err, {})]
continue

results += [(node, None, vals)]
Expand All @@ -1127,21 +1108,18 @@ def top(self, nodes):

for (node, error, vals) in self.get_top_output(nodes):
top_info = {"name": node.name, "type": node.type,
"host": node.host, "pid": None, "proc": None,
"host": node.host, "pid": None,
"vsize": None, "rss": None, "cpu": None,
"cmd": None, "error": None}
if error:
top_info["error"] = error
results.set_node_data(node, False, {"procs": [top_info]})
results.set_node_data(node, False, {"procs": top_info})
continue

proclist = []
for d in vals:
top_info2 = top_info.copy()
top_info2.update(d)
proclist.append(top_info2)
top_info2 = top_info.copy()
top_info2.update(vals)

results.set_node_data(node, True, {"procs": proclist})
results.set_node_data(node, True, {"procs": top_info2})

return results

Expand Down
7 changes: 2 additions & 5 deletions BroControl/cron.py
Expand Up @@ -56,11 +56,8 @@ def log_stats(self, interval):
with open(self.config.statslog, "a") as out:
for (node, error, vals) in top:
if not error:
for proc in vals:
parentchild = proc["proc"]
for (val, key) in sorted(proc.items()):
if val != "proc":
out.write("%s %s %s %s %s\n" % (t, node, parentchild, val, key))
for (val, key) in sorted(vals.items()):
out.write("%s %s parent %s %s\n" % (t, node, val, key))
else:
out.write("%s %s error error %s\n" % (t, node, error))

Expand Down
1 change: 0 additions & 1 deletion CMakeLists.txt
Expand Up @@ -101,7 +101,6 @@ InstallShellScript(share/broctl/scripts bin/update)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/check-pid)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/df)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/first-line)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/get-childs)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/start)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/stop)
InstallShellScript(share/broctl/scripts/helpers bin/helpers/top)
Expand Down
41 changes: 19 additions & 22 deletions bin/broctl.in
Expand Up @@ -263,41 +263,38 @@ class BroCtlCmdLoop(brocmd.ExitValueCmd):
hostwidth = 16
data = results.get_node_data()
if data:
proclist = data[0][2]["procs"]
if proclist[0]["type"] == "standalone":
procinfo = data[0][2]["procs"]
if procinfo["type"] == "standalone":
# In standalone mode, we need a wider "type" column.
typewidth = 10
hostwidth = 13

lines = ["%-12s %-*s %-*s %-7s %-7s %-6s %-4s %-5s %s" % ("Name",
typewidth, "Type", hostwidth, "Host", "Pid", "Proc", "VSize",
lines = ["%-12s %-*s %-*s %-7s %-6s %-4s %-5s %s" % ("Name",
typewidth, "Type", hostwidth, "Host", "Pid", "VSize",
"Rss", "Cpu", "Cmd")]
for data in results.get_node_data():
proclist = data[2]["procs"]
for top_info in proclist:
msg = ["%-12s" % top_info["name"]]
msg.append("%-*s" % (typewidth, top_info["type"]))
msg.append("%-*s" % (hostwidth, top_info["host"]))
if top_info["error"]:
msg.append("<%s>" % top_info["error"])
else:
msg.append("%-7s" % top_info["pid"])
msg.append("%-7s" % top_info["proc"])
msg.append("%-6s" % util.number_unit_str(top_info["vsize"]))
msg.append("%-4s" % util.number_unit_str(top_info["rss"]))
msg.append("%3s%% " % top_info["cpu"])
msg.append("%s" % top_info["cmd"])
procinfo = data[2]["procs"]
msg = ["%-12s" % procinfo["name"]]
msg.append("%-*s" % (typewidth, procinfo["type"]))
msg.append("%-*s" % (hostwidth, procinfo["host"]))
if procinfo["error"]:
msg.append("<%s>" % procinfo["error"])
else:
msg.append("%-7s" % procinfo["pid"])
msg.append("%-6s" % util.number_unit_str(procinfo["vsize"]))
msg.append("%-4s" % util.number_unit_str(procinfo["rss"]))
msg.append("%3s%% " % procinfo["cpu"])
msg.append("%s" % procinfo["cmd"])

lines.append(" ".join(msg))
lines.append(" ".join(msg))

return (results.ok, lines)

def do_top(self, args):
"""- [<nodes>]
For each of the nodes, prints the status of the two Bro
processes (parent process and child process) in a *top*-like
format, including CPU usage and memory consumption. If
For each of the nodes, prints the status of the Bro process in
a *top*-like format, including CPU usage and memory consumption. If
executed interactively, the display is updated frequently
until key ``q`` is pressed. If invoked non-interactively, the
status is printed only once."""
Expand Down
8 changes: 0 additions & 8 deletions bin/helpers/get-childs

This file was deleted.

7 changes: 3 additions & 4 deletions doc/broctl.rst
Expand Up @@ -5,7 +5,7 @@
.. Note: This file includes further autogenerated ones.
..
.. Version number is filled in automatically.
.. |version| replace:: 1.7-107
.. |version| replace:: 1.7-122

==========
BroControl
Expand Down Expand Up @@ -715,9 +715,8 @@ nodes if none are given.
.. _top:

*top* *[<nodes>]*
For each of the nodes, prints the status of the two Bro
processes (parent process and child process) in a *top*-like
format, including CPU usage and memory consumption. If
For each of the nodes, prints the status of the Bro process in
a *top*-like format, including CPU usage and memory consumption. If
executed interactively, the display is updated frequently
until key ``q`` is pressed. If invoked non-interactively, the
status is printed only once.
Expand Down
10 changes: 5 additions & 5 deletions testing/Baseline/command.top/all.out
@@ -1,5 +1,5 @@
Name Type Host Pid Proc VSize Rss Cpu Cmd
manager manager localhost 37156 parent 57M 57M 2% bro
proxy-1 proxy localhost 37273 parent 56M 56M 3% bro
worker-1 worker localhost 37402 parent 56M 56M 2% bro
worker-2 worker localhost 37403 parent 56M 56M 2% bro
Name Type Host Pid VSize Rss Cpu Cmd
manager manager localhost 37156 57M 57M 2% bro
proxy-1 proxy localhost 37273 56M 56M 3% bro
worker-1 worker localhost 37402 56M 56M 2% bro
worker-2 worker localhost 37403 56M 56M 2% bro
4 changes: 2 additions & 2 deletions testing/Baseline/command.top/onenode.out
@@ -1,2 +1,2 @@
Name Type Host Pid Proc VSize Rss Cpu Cmd
worker-1 worker localhost 37402 parent 56M 56M 2% bro
Name Type Host Pid VSize Rss Cpu Cmd
worker-1 worker localhost 37402 56M 56M 2% bro
2 changes: 1 addition & 1 deletion testing/Baseline/command.top/stopped.out
@@ -1,4 +1,4 @@
Name Type Host Pid Proc VSize Rss Cpu Cmd
Name Type Host Pid VSize Rss Cpu Cmd
manager manager localhost <not running>
proxy-1 proxy localhost <not running>
worker-1 worker localhost <not running>
Expand Down
8 changes: 4 additions & 4 deletions testing/Scripts/diff-top-output
Expand Up @@ -10,10 +10,10 @@ awk '{
# format is expected (some fields have unpredictable length, but
# we need a constant-width string of Xs).
if ( $4 ~ /^[0-9]+$/ ) { $4 = "XXXXX" } # Pid
if ( $6 ~ /^[0-9]+[KMG]$/ ) { $6 = "XXX" } # VSize
if ( $7 ~ /^[0-9]+[KMG]$/ ) { $7 = "XXX" } # Rss
if ( $8 ~ /^[0-9]+%$/ ) { $8 = "XX%" } # Cpu
if ( $9 ~ /^(bro|[Pp]ython.*)$/ ) { $9 = "XXX" } # Cmd
if ( $5 ~ /^[0-9]+[KMG]$/ ) { $5 = "XXX" } # VSize
if ( $6 ~ /^[0-9]+[KMG]$/ ) { $6 = "XXX" } # Rss
if ( $7 ~ /^[0-9]+%$/ ) { $7 = "XX%" } # Cpu
if ( $8 ~ /^(bro|[Pp]ython.*)$/ ) { $8 = "XXX" } # Cmd
}
print
Expand Down

0 comments on commit 364c277

Please sign in to comment.