Skip to content

Commit

Permalink
Fix PR bpftrace#1416
Browse files Browse the repository at this point in the history
The merged PR which enables `top` and `div` args of `print` for `avg` maps has
two issues that need addressing:

1. It came with subtractions of unsigned integers which can easily cause
negative overflows.
2. The runtime tests it came with are somehow unstable and fails randomly. This
patch brings more straightforward tests.
  • Loading branch information
suyuee committed Jul 23, 2020
1 parent a44618a commit e758f36
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
16 changes: 6 additions & 10 deletions src/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,9 @@ void TextOutput::map_stats(
auto &key = key_count.first;
auto &value = values_by_key.at(key);

if (map.type_.IsAvgTy() && top)
{
if (i++ < (values_by_key.size() - top))
continue;
}
if (map.type_.IsAvgTy() && top && values_by_key.size() > top &&
i++ < (values_by_key.size() - top))
continue;

out_ << map.name_ << map.key_.argument_value_list_str(bpftrace, key) << ": ";

Expand Down Expand Up @@ -610,11 +608,9 @@ void JsonOutput::map_stats(
auto &key = key_count.first;
auto &value = values_by_key.at(key);

if (map.type_.IsAvgTy() && top)
{
if (j++ < (values_by_key.size() - top))
continue;
}
if (map.type_.IsAvgTy() && top && values_by_key.size() > top &&
j++ < (values_by_key.size() - top))
continue;

std::vector<std::string> args = map.key_.argument_value_list(bpftrace, key);
if (i > 0)
Expand Down
14 changes: 7 additions & 7 deletions tests/runtime/call
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,12 @@ RUN bpftrace -v -e 'BEGIN { @[nsecs] = strftime("%m/%d/%y", nsecs); exit();}'
EXPECT @\[[0-9]*\]: [0-9]{2}\/[0-9]{2}\/[0-9]{2}
TIMEOUT 5

NAME print_avg_map_top
RUN bpftrace -e 'i:ms:1 { @[nsecs/100000] = avg(1) } i:ms:10 { print(@, 3, 0); exit(); } END {clear(@)}' | grep -c -v "^ *$"
EXPECT ^4$
NAME print_avg_map_args
RUN bpftrace -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 2, 10); print("END"); clear(@); exit(); }'
EXPECT Attaching 1 probe\.\.\.\n@\[c\]: 3\n@\[d\]: 4\n\nEND
TIMEOUT 1

NAME print_avg_map_div
RUN bpftrace -e 'i:ms:1 { @[nsecs/1000000] = avg(10) } i:ms:10 { print(@, 1, 10) ;exit();} END {clear(@)}'
EXPECT ^@\[[0-9]+\]: 1$
TIMEOUT 1
NAME print_avg_map_with_large_top
RUN bpftrace -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 10, 10); print("END"); clear(@); exit(); }'
EXPECT Attaching 1 probe\.\.\.\n@\[a\]: 1\n@\[b\]: 2\n@\[c\]: 3\n@\[d\]: 4\n\nEND
TIMEOUT 1
9 changes: 7 additions & 2 deletions tests/runtime/json-output
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ EXPECT ^{"type": "value", "data": \[1,2,"string"\]}$
TIMEOUT 1

NAME print_avg_map_args
RUN bpftrace -f json -e 'i:ms:1 { @[nsecs/100000] = avg(10) } i:ms:10 { print(@, 1, 10); exit(); } END {clear(@)}'
EXPECT ^{"type": "stats", "data": {"@": { *"[0-9]*": 1}}}$
RUN bpftrace -f json -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 2, 10); clear(@); exit(); }'
EXPECT {"type": "stats", "data": {"@": { *"c": 3, *"d": 4}}}
TIMEOUT 1

NAME print_avg_map_with_large_top
RUN bpftrace -f json -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 10, 10); clear(@); exit(); }'
EXPECT {"type": "stats", "data": {"@": { *"a": 1, *"b": 2, *"c": 3, *"d": 4}}}
TIMEOUT 1

0 comments on commit e758f36

Please sign in to comment.