Great work! (can't think of anything else other than known pending issues e.g., PORT_EPOCH and version which is blocked by need to generate a tag; getting this far we can totally leverage this for runtime testing to get to a tag-point where we are happy with the runtime performance).

In D10006#249766, @julian wrote:

In D10006#206784, @gnn wrote:

New, BSD license, code does not need to be in cddl. Most of the new D scripts and programs go into either share/dtrace or the DTrace Toolkit port. Since this is a script I'd put it into share/dtrace if you want it in the src tree.

I'd agree with this... My gut feeling is that I'd look for it in /usr/share/dtrace or maybe /usr/share/examples/dtrace, probably the former. where it is on porridge (/usr/share/dtrace/watch_kill)

In D10006#249765, @julian wrote:

so, like is this getting committed some time?

Fixed typo as part of last update. Marking inline comments as done.

In D10006#206993, @markj wrote:

I've only just started playing with it, but I'll point out that the usage seems misleading: dwatch doesn't really take a (dtrace) probe as input, it takes a function or syscall name and constructs a probe name.

I am running an open public beta of this software. To test the software that is currently submitted here for review, head over to pkg.fraubsd.org in your browser, pick an architecture, and either follow the instructions for setting up the FrauBSD pkg repository and say "pkg install fraubsd/dwatch" or download the dwatch tarball and install with "pkg add PKGFILENAME.txz"

Complete overhaul in an effort to address earlier feedback.

In D8734#225425, @emaste wrote:

@dteske will you commit it?

LGTM

LGTM

In D10585#219534, @emaste wrote:

"MFC after" and "X-MFC-after" are two different things.

That's precisely my point. X-MFC-after is being used intentionally to avoid being parsed by the MFC script, because the commit author believes the MFC reminder script will not be able to parse the value. That's distinct from X-MFC-to or X-MFC-with, where MFC to or MFC with would not cause trouble for the reminder script.

In D10585#219533, @emaste wrote:

Can we add "MFC before:"?

I don't want to make this too long and would prefer to avoid special cases that have only occasional use. Right now it is unlikely "MFC before" could be automatically parsed. By its nature it will be somewhat unrestricted text, and even if we did parse it I don't think we want to build tooling to make the reminder script aware of upcoming branch / release dates.

Can we add "MFC before:"? E.g., "If the commit should be merged before some particular branch creation, specify the branch name(s)." Example: "MFC before: RC-3". Used by bz and gjb. See revisions 285733, 285082, 285078, 285076, 259427, 259426, 185435, and 148871.

Update to apply cleanly to HEAD
Add "-x" option to enable probe tracing
Don't set caller_execname unless hooked on syscall::execve
Fix indentation in generated dtrace(1) code
Beautify generated dtrace(1) code
Fix nested DTrace predicates; "-t test" now works with profiles
Postpone jail predicate generation until after profile loading
Fix a missing ".Ar" in dwatch(8) manual
Update examples/profile_template to include EVENT_TEST info
Change "kill" profile to print unmodified pid_t argument
Fix signedness in printf for signal argument in "kill" profile
Add code to "vop_*" profiles to NULL-ify transient variables
Optimize vnode-walking process in VFS profiles
Fix ellipsis generation for paths exceeding DEPTH_MAX in VFS profiles
Update VFS profiles to use EVENT_TEST instead of EVENT for predicates
Fix incorrect "probe ID" comments generated by vop_rename profile

Update patch to apply cleanly to HEAD
Fix generation of ellipsis for trailing arguments
Remove double-quotes on $COUNT which is guaranteed to be a number

Add "-t test" option for customizing dtrace(1) predicate
Add "-j jail" option for limiting events to a jail name/jid
Change "-p" option (disable profiles) to "-P"
Add "-p pid" option for watching a particular process id
When given "-D-" perform all error checks before reading stdin
Issue an error and exit if group in "-g name" does not exist
Fix spurious double-error when given bad user in "-u name"
Given "-u user" and "-g group" but not "-q", display user before group
Use walltimestamp instead of timestamp for event date/time
Very minor refactoring for code readability
Make ending semi-colon for "-D code" optional
Whitespace and comments
Minor fixes to grammar and punctuation in manual
Sort examples in manual by flag being demonstrated
Add the following examples to manual:
dwatch -f '(mk|rm)dir' execve
dwatch -g wheel execve
dwatch -j 0 execve
dwatch -j myjail execve
dwatch -l 'read$'
dwatch -p 1234 execve
dwatch -q -t 'arg2<10' -D 'printf("%d",arg2)' write
dwatch -v -p 1234 execve

Update diff to apply cleanly to HEAD
Make -q flag squelch errors from dtrace(1)

I wanted to take a moment to explain why the probe syntax for dwatch(8) is "[provider:[module:]]function" and not "[provider:[module:[function:]]]name" as is the probe syntax for dtrace(1).

Incorporate much feedback from previous comments on this review
Update patch to apply cleanly to head
Make lchmod a hard link to chmod
Make vop_{lookup,mkdir,mknod,remove,rmdir} a hard link to vop_create
Change terminology to name dwatch(8) modules to instead profiles
NB: Prevents term modules from being confused with dtrace(1)
Improve readability and simplify code
Move -e code option to -D code for injecting DTrace event detail code
Move -p action option to -e name for modifying the event action
Move -M option to -p for disabling profiles (formerly termed modules)
Update usage statement to be more clear about what a "probe" is
NB: Instead of "probe" show that we expect "[provider:[module:]]function"
Simplify profile loading
Set $FILE $PROFILE and $PROBE before loading profile(s)
Consolidate file descriptor operations
Fix a bug where -u root or -g root did not work as expected
NB: Also fixed -u 0 and -g 0 which suffered from the same issue
Add further clarification to the manual page with respect to probe format
Add examples to the manual page
Move examples/module_template to examples/profile_template
Improve comments in examples/profile_template
Reduce size of profiles, removing comments inherited from profile_template
Add numerical suffixes in nanosleep profile (s for seconds; ns for nanosec)
Add support for -l -e name [pattern] syntax
NB: The -l [pattern] syntax did not support -e name before

I have thought of a way to massively reduce the size of the diff. In the spirit of asomers observation that several of the modules are quite similar, I've decided to improve the functionality provided to modules which will allow the modules to be linked to each other whilst preserving explicit probe value.

In D10006#206993, @markj wrote:

The first thing I tried (without having fully read the manpage) was "dwatch sched:::on-cpu", which of course gives an error.

Optimize filtration

When a dwatch(8) module does not define DETAILS, default to pproc_dump()

Fix mdoc warning: .It macros in lists of type `tag-list' require arguments

Allow modules to contain hyphen in their name

Add `-p action' flag, example usage: dwatch -p on-cpu sched::

Update patch to apply cleanly against latest HEAD

Move back to cddl/usr.sbin to live amongst similarly [BSD] licensed zfsd
Update ObsoleteFiles.inc to remove deprecated/removed watch_* scripts
Fix a copy/paste issue in the manual (s/lchmod/nanosleep/)
Add usr/libexec/dwatch to etc/mtree/BSD.usr.dist

In D10006#207420, @asomers wrote:

@dteske Don't forget to update etc/mtree/BSD.usr.dist

In D10006#206993, @markj wrote:

I'll point out that the usage seems misleading: dwatch doesn't really take a (dtrace) probe as input,
it takes a function or syscall name and constructs a probe name.

A few observations that may help answer the Q of "usr.sbin vs cddl/usr.sbin"...

Back out of cddl and whack deprecated share/dtrace scripts

In D10006#206784, @gnn wrote:

New, BSD license, code does not need to be in cddl.

Address comments

Move utility from usr.sbin/dwatch to cddl/usr.sbin/dwatch

In D10006#206732, @asomers wrote:

Since dwatch depends on dtrace, which is CDDL-licensed, shouldn't dwatch live in cddl/usr.sbin/dwatch instead of usr.sbin/dwatch? That way it can easily be excluded from a CDDL-free build.

For reference, here's where this all stemmed from https://fraubsd.org/doc/nginx_plus_with_ports_openssl_and_aesni.png

Tried closing this revision but because asomers blocked it, phabricator tells me it can't be closed.

Bumping the .so revision puts a big onus on some vendors to recompile their software for almost zero reason.

I think we need a "PORTREVISION= 1"

In D9419#194594, @ngie wrote:

2. Could you please run igor on the manpage?

In D9419#194594, @ngie wrote:

1. Wasn't AUTHORS supposed to be above HISTORY?

Bump .Dd