Don't print out maps on exit if END probe is defined

[ajor]

If the END probe is defined, don't print out maps automatically when bpftrace exits.

This will change the behaviour of some existing scripts, so could do with a discussion.

I often see the END probe being used solely to clear() all maps so that they're not printed by bpftrace, e.g.:

bpftrace/tools/biosnoop.bt

Lines 48 to 54 in c8fd334

	END
	{
	clear(@start);
	clear(@iopid);
	clear(@iocomm);
	clear(@disk);
	}

It's tedious to write this in most scripts and keep the list of maps to be cleared up-to-date as the script evolves.

Analysing the current state of the bpftrace tools, 18 scripts use END to clear maps and 18 rely on the implicit printing without specifying an END probe. 3 scripts define and END probe and also rely on the implicit printing. They all take the form:

bpftrace/tools/bitesize.bt

Lines 26 to 29 in c8fd334

	END
	{
	printf("\nI/O size (bytes) histograms by process name:");
	}

But I think this would be better if they explicitly printed out the relevant map anyway, instead of relying on the implicit print-on-exit behaviour.

[danobi]

Agreed that current behavior is kinda annoying.

But could be risky. There might be automated users (eg https://github.com/iovisor/kubectl-trace and others) that rely on the exit printing. If we change the behavior they could lose stats.

[danobi]

Actually, current behavior is beneficial for one liners. Which is probably why it’s like this in the first place

[viktormalik]

I agree with @danobi here. The "feature" has been around for a long time and many automated users of bpftrace may rely on it.

What about we introduced a config option (and possibly a CLI option) that would auto-clear all the maps prior to exiting?

[ajor]

One liners wouldn't typically define an END probe though - in that case I'm saying we should keep the current behaviour and print out all maps. As for why the current behaviour is like this: printing maps on exit was the only way to see the data in maps for a long time. END probes came later, and then print() later still.

For these automated users, do we know if they are they defining their own end probes or not?

I've just gone through and analysed Meta's collection of bpftrace scripts and the proportions are similar to the bpftrace tools in this repo:

• END probe defined to clear all maps: 23
• END probe defined, but rely on implicit map printing: 4
• END probe not defined, rely on implicit map printing: 29

So ~7% of scripts will break with this change. Of those, I think a lot could be improved by explicitly printing the maps they care about instead of the inverse (deleting those they don't want to see). Instead of this:

kprobe:xxx
{
  @a = 1;
  @b = 2;
  @c = 3;
}
END
{
  clear(@b);
  clear(@c);
  // rely on implicit printing of @a
}

It'd be both clearer and shorter as:

kprobe:xxx
{
  @a = 1;
  @b = 2;
  @c = 3;
}
END
{
  print(@a);
}

This behaviour could be toggled with a config option, but I think it should be made the default at some point as based on
the scripts I've seen, it'd help with a lot more than it'd harm. Automated users (like kubectl-trace) could still default to the legacy behaviour if they want.

[jordalgo]

I agree that auto printing of maps at the end is blah AND that maybe too many people might rely on this default behavior to change it (at least without a lot of notice).

What about instead of implicit map clearing with END, which feels a little magical, introducing a single new function clear_all_maps.

[lenticularis39]

I think we could even do both clear_all_maps and a config option/command line switch. The config option way seems more natural to me, since it is not an actual bpftrace statement which prints the maps, but bpftrace itself, thus, it feels like it should be controlled by bpftrace. Clearing all maps so they are not displayed feels more like a hack.

However, having a clear_all_maps function can also be useful for other things, perhaps together with a print_all_maps function. Iteration over all maps is also technically possible, but the current conception of types in bpftrace does not allow it and changing would add a lot of complexity into the language, so having dedicated helpers for common operations seems as the way to go.