Apply LOG_LEVEL as a threshold across the resolver and both daemons - #11
Merged
Conversation
added 4 commits
September 4, 2026 00:37
LOG_LEVEL was never compared against a threshold. The only test of its value
anywhere was == "DEBUG", so eight informational lines were gated on LOGGING
alone and printed at every level, and eight more had no guard at all and
printed even with LOGGING: False. The shipped pfui_unbound.yml sets
LOGGING: True with LOG_LEVEL: ERROR, which is exactly the combination that
emits per-query output, so every install did this:
pythonmod: operate, id: 0, module_event_pass
PFUIDNS: Sending '{...}' to /var/run/pfui/pfui_firewall.sock
PFUIDNS: www.example.com. Found IPv4 address 93.184.216.34
Unbound injects log_info and log_err into the interpreter and both write
unconditionally, so there is no framework to filter them and the module has to
do it itself. log_at() is that filter, and every site now names a severity.
log_err stays ungated throughout: the level chooses how much detail accompanies
a fault, not whether faults are reported.
Two of the unguarded lines were failures rather than chatter, so gating them at
DEBUG would have traded per-query noise for a dead firewall invisible at the
default level. Those are errors now: an unacknowledged send and a UDP receive
that exhausted its retries.
LOG_LEVEL is also normalised and checked when the config loads, because the
symptom of a typo is silently quieter logs.
Two further gaps closed while auditing, either of which would have made the
change look wrong:
- log_at("INFO") had no call sites, so INFO would have behaved identically to
ERROR while the yml advertises it for testing.
- The circuit breaker logged opening at error level and closing silently, so
a firewall was seen leaving and never coming back. Recovery is the INFO
tier's content, reported on the transition only, not per successful query.
The cache-report path gains a docstring and a test recording why it does not
honour BLOCKING: a positive cached answer implies access was already allowed by
the rr report that released it, so the report only resets the TTL. Behaviour
there is unchanged.
"Close msg: ACKUPDATE" was the one per-message line in either daemon not gated on LOGGING, and the Rust daemon inherited it from the Python reference. The config the installers ship sets LOGGING: False with LOG_LEVEL: DEBUG, so both daemons wrote a syslog line per DNS answer while verbose logging was nominally off. It is duplicative as well as ungated. A refusal is already reported at error level with its reason, and a success by the "PF Table updated" line, which is gated. Nothing is lost by holding this one behind the same flag as its neighbours, and a test asserts the refusal path still leaves a trace with logging off, so the gate cannot hide why a message was rejected. The Python daemon also normalises LOG_LEVEL before anything reads it, self.stats included, for the same reason the resolver now does: matching the raw text made 'debug' select ERROR.
The match ran against the raw yml text while the CTL match directly below it trims and folds case, so 'debug' or ' DEBUG ' fell through to the catch-all and silently selected ERROR. An operator who lower-cased the value got a daemon that logged nothing but faults and no indication why.
Two documentation gaps, both of which cost an operator time. The blocklist example now uses HaGeZi's lists as its source, with a tier table covering light, pro, pro.plus and tif. HaGeZi publishes no /etc/hosts format, so unbound-adblock's -l and -u options do not apply and the domain-only lists go in through -d or -t instead. Two further paths do not work and are called out: the adblock/ directory carries ABP filter syntax, which unbound-adblock does not parse, and the plain wildcard/*.txt files carry *. wildcards. Only the -onlydomains variants are usable. tools/update_dns_blocklist.sh still fetches StevenBlack in hosts format, which the section notes rather than papers over. BLOCKING had no prose at all, only two example lines that read like something to opt into. It is the default. The new section says so, separates the paths it governs, and records that a cache hit does not wait whatever it is set to, because a positive cached answer implies access was already allowed and the report only resets the TTL. The latency figure is scoped to the round trip it actually measures, since it was being read as a general number.
andylemin
force-pushed
the
logging-levels
branch
from
September 3, 2026 14:44
7ce2568 to
e0acb82
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LOG_LEVEL: ERRORstill emitted per-query informational logs, on the resolverand on both daemons. Three separate causes, all of which the shipped configs
trigger.
The resolver never applied LOG_LEVEL at all
The only test of its value anywhere in
pfui_unbound.pywas== "DEBUG". Eightinformational lines were therefore gated on
LOGGINGalone and printed at everylevel, and eight more had no guard at all and printed even with
LOGGING: False:The shipped
pfui_unbound.ymlsetsLOGGING: TruewithLOG_LEVEL: ERROR,which is precisely the combination that emits them, so every install did this.
Unbound injects
log_infoandlog_errinto the embedded interpreter and bothwrite unconditionally, so there is no framework doing the filtering and the
module has to do it itself.
log_at(level)is that filter and all 22 sites nowname a severity.
log_errstays ungated: the level chooses how much detailaccompanies a fault, not whether faults are reported.
server-pythonescaped this because it uses stdlibloggingwithsetLevel(),and the Rust daemon because
Logger::infosuppresses atERROR.Two of those lines were failures, not chatter
Gating them at
DEBUGwould have traded per-query noise for a dead firewallinvisible at the default level, so they are errors now.
no acknowledgement from {target}, last was ...log_info, ungatedlog_errTimeout udp receive - all retrieslog_info, ungatedlog_errThe daemons' reply line ignored LOGGING
Close msg: ACKUPDATEwas the one per-message line in either daemon not gated onLOGGING; the Rust daemon inherited it from the Python reference. The installersship
LOGGING: FalsewithLOG_LEVEL: DEBUG, so both wrote a syslog line perDNS answer while verbose logging was nominally off. It is duplicative too: a
refusal is already reported at error level with its reason, and a success by the
gated
PF Table updatedline. A test asserts the refusal path still leaves atrace with logging off, so the gate cannot hide why a message was rejected.
LOG_LEVEL was case-sensitive everywhere
The Rust daemon matched raw yml text while the
CTLmatch directly below ittrims and folds case, so
debugfell through to the catch-all and silentlyselected
ERROR. The Python daemon compared raw text the same way,self.statsincluded. Both normalise now, and the resolver says so out loud when a value is
unrecognised, because the symptom of a typo is silently quieter logs.
Two gaps that would have made the fix look wrong
log_at("INFO")had no call sites, soINFOwould have behaved identically toERRORwhile the yml advertises it for testing.operator watched a firewall leave and never saw it return. Recovery is now the
INFOtier's content, reported on the transition only, never per successfulquery.
What each level gives you
ERROR(default)INFODEBUGQuery Unblockedlatency lineDocumentation
BLOCKINGhad no prose, only two example lines that read like something to optinto. It is the default. The new section separates what it governs — a freshly
resolved answer waits for
ACKUPDATE, a cache hit does not wait whatever thesetting is, because a positive cached answer implies access was already allowed
by the
rrreport that released it and the cache report only resets the TTL. Atest pins that, and the behaviour is unchanged. The latency figure is scoped to
the round trip it measures, since it was being read as a general number.
The blocklist example now prefers HaGeZi's lists, with a tier table. HaGeZi
publishes no
/etc/hostsformat, sounbound-adblock's-l/-udo not applyand the domain-only lists go in through
-d/-t. Two further paths do not workand are called out: the
adblock/directory carries ABP filter syntax, whichunbound-adblockdoes not parse, and the plainwildcard/*.txtfiles carry*.wildcards. Only the
-onlydomainsvariants are usable.tools/update_dns_blocklist.shstill fetches StevenBlack in hosts format, whichthe section notes rather than papers over.
Testing
test fails if
log_atreturns unconditionally, the error-visibility test ifno acknowledgementreturns tolog_info, the daemon test ifClose msgloses its gate
load_configstripped whitespace from
LOG_LEVELbutlog_atdid not, so" DEBUG "washonoured at load and ignored at use
Follow-ups, not in this change
pfui_wire.pyimportslz4.frameunconditionally, soCOMPRESS: Falsedoesnot make the package optional the way it is meant to
COMPRESSis one global key per end, so it cannot be off for a local socketand on for a remote resolver
closes only after Redis and the persist write, so a blocking query appears to
wait for work the PF → ACKUPDATE → Redis → persist ordering exists to exclude