Page MenuHomeFreeBSD

Update dwatch-json to 1.2, maintainership
ClosedPublic

Authored by dteske on Aug 5 2022, 5:45 PM.

Details

Summary

Commit message is as-follows:

sysutils/dwatch-json: update to 1.2, restore maintainership

dwatch itself is a powerful modular abstraction layer atop dtrace in
FreeBSD base. This package installs modules that produce JSON instead
of human readable text. This version update adds abstraction services
atop those modules.

Test Plan

All currently-supported FreeBSD releases have dwatch and bsdconfig in base (the only required dependencies).

Install to FreeBSD release-of-choice ...

Test grafnet:

Copy /usr/local/etc/grafnet/grafnet.conf.sample to non-sample.
Execute "grafnet" to see usage.
Execute "grafnet trace" to see realtime network traffic.
Edit /usr/local/etc/grafnet/grafnet.conf and add a variable.
For example, dns='this->rport == 53'
Execute "grafnet show variable" (where "variable" is the variable you configured above)
For example, grafnet show dns
Execute "grafnet trace variable" to see realtime network traffic matching variable condition.
For example, grafnet trace dns
Execute "grafnet list"
Execute "grafnet start"
Execute "grafnet status" to confirm Dtrace service is running.
Execute "grafnet status -a" to see telegraf service is not running.
Copy /usr/local/etc/grafnet/stats.conf.sample to non-sample.
Execute "grafnet start -a" to start the telegraf service (Dtrace service reports already-running)
Execute "grafnet status -a" to see both Dtrace and telegraf service running.
Execute "tail /var/log/grafnet/grafnet.log" to see time-series data (or run "grafnet tail")
Execute "service grafnet extracommands"
Execute "service grafnet list" to make sure it looks the same as "grafnet list"
Execute "service grafnet trace dns" to make sure it works the same as "grafnet trace dns"
Execute "service grafnet status" and "service grafnet_stats status"
Load dashboard to make sure stats are making it into database from telegraf service.

Test grafio:

Copy /usr/local/etc/grafio/grafio.conf.sample to non-sample.
Execute "grafio" to see usage.
Execute "grafio trace" to see realtime disk activity.
Execute "grafio ls" to see system disks.
Execute "grafio show -l" to see system disk configuration and their probed labels.
Execute "grafio start"
Execute "grafio status" to confirm Dtrace service is running.
Execute "grafio status -a" to see telegraf service is not running.
Copy /usr/local/etc/grafio/stats.conf.sample to non-sample.
Execute "grafio start -a" to start the telegraf service (Dtrace service reports already-running)
Execute "grafio status -a" to see both Dtrace and telegraf service running.
Execute "tail /var/log/grafio/grafio.log" to see time-series data (or run "grafio tail")
Execute "service grafio extracommands"
Execute "service grafio list" to make sure it looks the same as "grafnet list"
Execute "service grafio trace" to make sure it works the same as "grafnet trace dns"
Execute "service grafio status" and "service grafio_stats status"

These series of commands test but only the high-level functionality. The program suite contains far more functionality that is able to be covered in this review.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dteske requested review of this revision.Aug 5 2022, 5:45 PM
dteske created this revision.

I discussed this submission with dteske on IRC and possibly have a few suggestions:

PORTNAME should correspond to the naming convention of installed files and usecase whenever possible, so dwatch-json --> dwatch-json-net and preferably libexec/dwatch/json-net* --> libexec/dwatch/dwatch-json-net*

Grafnet should be broken out, put in a separate port and be set as a dependency (if required) as it doesn't seem to be a hard requirement and/or limited to dwatch-json-net looking at pkg-descr

I discussed this submission with dteske on IRC and possibly have a few suggestions:

PORTNAME should correspond to the naming convention of installed files and usecase whenever possible, so dwatch-json --> dwatch-json-net and preferably libexec/dwatch/json-net* --> libexec/dwatch/dwatch-json-net*

Grafnet should be broken out, put in a separate port and be set as a dependency (if required) as it doesn't seem to be a hard requirement and/or limited to dwatch-json-net looking at pkg-descr

It is true that this was discussed. However, I tried to enlighten that:

  1. Files in $PREFIX/libexec/dwatch are modules and the filename is both the module name and what the user types to invoke the module. It would not only be redundant to force people to type "dwatch -X dwatch-json-net" when there are dozens of profiles in the base Operating System in /usr/libexec/dwatch and other ports (authored by me, such as dwatch-gource) which follow the standard of NOT including "dwatch-" in the dwatch module name. For additional information on how dwatch modules work and are expected to be formatted, see the BSDCAN 2018 slides for my presentation on dwatch at https://fraubsd.org/doc/bsdcan/2018/
  1. The PORTNAME argument ignores my other work and unnecessarily asks me to change the name of the port and then later change it back when I add non-network-related JSON modules.
  1. The argument that dwatch-json is not a hard requirement [of grafnet] is patently false. Requiring authors to internally document in pkg-descr that files that are part of a package rely on other files inside the same package seems overly burdensome.

    https://github.com/FrauBSD/dwatch-json/blob/bc978e4d6d6b075f5426c01d85217132c9d7d63a/grafnet/grafnet#L545 https://github.com/FrauBSD/dwatch-json/blob/bc978e4d6d6b075f5426c01d85217132c9d7d63a/grafnet/grafnet#L400

From my 2018 BSDCAN presentation slides, here are the modules in base ... we're not going to add "dwatch-" prefix to all those simply because a PORT convention says that a PORT should install files named the same as the PORT when the PORT should follow BASE convention already set forth 6 years ago

dteske retitled this revision from Update dwatch-json to 0.5, maintainership to Update dwatch-json to 1.1, maintainership.Aug 22 2022, 11:00 PM
dteske edited the test plan for this revision. (Show Details)
dteske added a reviewer: 0mp.

Abandon the update to 0.5 and update to 1.1

Add missing config to plist for 1.1

Add symbolic link entries from 1.1

Tested and ready for re-review

woodsb02 added a subscriber: woodsb02.

All the proposed changes look fine to me. You’re welcome to commit this to ports yourself with my ports commit bit approval.

Macro lgtm:

This revision is now accepted and ready to land.Aug 23 2022, 12:21 AM
dteske edited the summary of this revision. (Show Details)

You’re probably already aware, but ports has a commit hook so that commit messages should start the first line with “category/portname: description”.

So commit message should have first line:
sysutils/dwatch-json: update to 1.1, restore maintainership

You’re probably already aware, but ports has a commit hook so that commit messages should start the first line with “category/portname: description”.

So commit message should have first line:
sysutils/dwatch-json: update to 1.1, restore maintainership

Thanks! Was not aware of this (new) hook.

This revision now requires review to proceed.Aug 23 2022, 3:33 PM

Fixup package COMMENT to match what I have set for 1.1

Add missing dashboard examples from 1.1

This revision is now accepted and ready to land.Aug 23 2022, 9:33 PM

LGTM -- feel free to ping me to commit, or use Approved by: ler

I was working on committing this and went to go apply the patch to the tree and the patch no longer applies cleanly.

I check the Makefile, and find that the current version is 0.6.3? But I didn't update it. And 0.6.3 isn't the latest version.

Currently investigating how the current version bumped from 0.4.1 (the last version I put up for ports) to 0.6.3 when I was working towards merging this approved commit to bump it to 1.1

Update diff before applying to head

This revision now requires review to proceed.Aug 27 2022, 12:20 AM
ler requested changes to this revision.Aug 27 2022, 12:25 AM

the distinfo change doesn't look right?

This revision now requires changes to proceed.Aug 27 2022, 12:25 AM
This revision is now accepted and ready to land.Aug 27 2022, 12:26 AM
This revision now requires review to proceed.Aug 27 2022, 1:33 AM

I'm a dork -- *NOW* it looks right :)

This revision is now accepted and ready to land.Aug 27 2022, 1:34 AM
dteske retitled this revision from Update dwatch-json to 1.1, maintainership to Update dwatch-json to 1.2, maintainership.
dteske edited the summary of this revision. (Show Details)

Bump port to 1.2 so we catch latest development (the inclusion of a dashboard for the Block I/O stats)

I have tested these changes with:

  1. portlint
  2. make package
  3. pkg upgrade work/pkg/dwatch-json-1.2.pkg
This revision now requires review to proceed.Aug 27 2022, 2:15 AM
This revision is now accepted and ready to land.Aug 27 2022, 3:32 AM