Structured logging to journald - #2969

Draft

darkexplosiveqwx wants to merge 26 commits into

pi-hole:developmentfrom

darkexplosiveqwx:log-journal

darkexplosiveqwx commented Jul 24, 2026

Contributor

Thank you for your contribution to the Pi-hole Community!

Please read the comments below to help us consider your Pull Request.

We are all volunteers and completing the process outlined will help us review your commits quicker.

Please make sure you

Base your code and PRs against the repositories developmental branch.
Sign Off all commits as we enforce the DCO for all contributions
Sign all your commits as they must have verified signatures
File a pull request for any change that requires changes to our documentation at our documentation repo

What does this PR aim to accomplish?:

Discussed in #2897
Stacked on top of #2958, #2960 and #2968
Requires pi-hole/docker-base-images#175 to build.

How does this PR accomplish the above?:

Link documentation PRs if any are needed to support this PR:

By submitting this pull request, I confirm the following:

I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
I have commented my proposed changes within the code and I have tested my changes.
I am willing to help maintain this change if there are issues with it later.
It is compatible with the EUPL 1.2 license
I have squashed any insignificant commits. (git rebase)
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
I give this submission freely, and claim no ownership to its content.

I have read the above and my PR is ready for review. Check this box to confirm

github-actions Bot added the Merge conflicts label

github-actions Bot commented Jul 26, 2026

This pull request has conflicts, please resolve those before we can evaluate the pull request.

darkexplosiveqwx added 2 commits

July 26, 2026 10:04


          Open logfiles in append instead of append+read mode

d110ed8

We never read from FTL.log and webserver.log after opening them, so +read is unnecessary.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          change webserver.log format to match FTL.log

6d93114

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>

darkexplosiveqwx force-pushed the log-journal branch from 98971ad to 13547b5 Compare

July 26, 2026 08:49

github-actions Bot removed the Merge conflicts label

github-actions Bot commented Jul 26, 2026

Conflicts have been resolved.

darkexplosiveqwx added 8 commits

July 26, 2026 11:06


          enrich webserver and dnsmaq log with priority/severity

099a7d0

carries to: FIFO, webserver.log
changes webserver.log format (again)

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          Avoid NULL dereference when no web log file is configured

afda5b5

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          make non-debug log_web() more convenient

90baf05

adresses review:
- `log_web()` now takes `(priority, flag, ...)` and most call sites pass
  `DEBUG_NONE`. Since `log_web_debug()` is already a macro, a matching
  convenience macro for the plain info/error case would drop
  `DEBUG_NONE` from those call sites and read cleaner. It is only a
  handful of sites, so this is readability rather than diff size.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          Remove unused process variable and name parameter from init_FTL_log()

b2da5eb

The static variable process was set by init_FTL_log() but never read
anywhere in the codebase - it was dead code since its introduction in
8f40798 (2021). Removing it also eliminates the name parameter which
is no longer needed.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          fix: make FTLCONF_files_log_ftl work early

edf32d7

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          Reorder some logsites from FTL to webserver log

da8f879

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          Don't move to webserver.log

b2c306c

this would be duplicated with log_web(LOG_INFO, "Initializing HTTP
server on ports \"%s\"", config.webserver.port.v.s);

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          adjust tests for the move FTL -> webserver.log

8e1298a

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>

darkexplosiveqwx force-pushed the log-journal branch from 13547b5 to 3b06c28 Compare

July 26, 2026 09:15

darkexplosiveqwx added 3 commits

July 26, 2026 13:34


          Keep process-health error sites in FTL.log

2f695e9

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          validate the the filepath in getLogFilePathENV()

ad8d014

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          accept empty files_log_ftl in ENV

Code Review:  And FTLCONF_files_log_ftl="" is skipped by
getLogFilePathENV() but accepted by the regular load, which is a small
inconsistency.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>

github-advanced-security AI found potential problems

View reviewed changes

src/log.c

+              // held, so writes to different files never contend.  The reopen flag is
+              // per-file so SIGUSR2 only touches the fd that actually needs it.
+              static bool write_log_line(struct log_fd *log, const char *line, size_t len)
+              {

src/log.c

-              void init_FTL_log(const char *name)
+              // Open cached log fds from config paths.
+              // open_log_fds(true):  open FTL.log only (called early, before full config)

darkexplosiveqwx added 10 commits

July 28, 2026 15:30


          weblog: route errors to FTL.log if webserver.log is unavailible

c023bf3

Code Review:

On the fallback, the direction is right but the snippet has two problems. First, the comparison is inverted: syslog priorities ascend as severity drops (LOG_CRIT 2 < LOG_ERR 3 < LOG_WARNING 4 < LOG_INFO 6), so priority >= LOG_WARNING catches WARNING/NOTICE/INFO/DEBUG and skips exactly the ERROR and CRIT lines we are trying to save. It needs to be priority <= LOG_WARNING. Second, config.files.log.webserver.v.s == NULL will practically never be true: the default is /var/log/pihole/webserver.log, initConfig() always seeds v from d, and we have no empty-means-disabled handling for that key (unlike files.pcap, which uses validate_filepath_empty). Setting files.log.webserver = "" passes validate_webserver_logfile() and then simply fails in fopen(). So the case that actually loses messages is the open failing - empty path, unwritable directory, read-only filesystem - and there we currently drop the line in daemon mode without even the syslog fallback that _FTL_log() has. Please gate on weblog == NULL instead, and rather than duplicating the formatting, just hand the already-formatted buffer over

_log_web() does not honour print_log/print_stdout outside CLI mode, so log_ctrl(false, false) (e.g., src/api/action.c:49) no longer silences everything, and it never prints to stdout in non-daemon mode - so pihole-FTL -f no longer shows the relocated errors on the console the way log_err() did. Routing through _FTL_log() as above fixes the second half for free.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          move even more severe messages back to FTL.log

aa70f13

Code Review:

Two things are still open from my second point, all in src/webserver/webserver.c: the OOM sites (lines 64, 73, 82, 91, 100, 360, 506, 665, 715, 814, 836, 948) and the startup failures - 619 Not starting web server as webserver.port is empty, 652 Initializing HTTP library failed!, 658 Failed to build web paths ... and the whole mg_start2() failure block at 871-874. Those are process-health signals: "web interface will not be available" is what someone greps FTL.log for. Line 874 also became circular - we now write "Hint: Check the webserver log at ..." into the webserver log. Worth noting that a bad files.log.webserver is itself a plausible reason for that block to run, and then all four lines disappear in daemon mode, i.e., no web interface and nothing in the logs.

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          utilize a cached file descriptor for logging & own dnsmasq.log

e86a010

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          We don't implement dnsmasq's logging to stderr

1175d6c

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          log.c: apply O_CLOEXEC to avoid leaking file descriptors

4fe54dc

the embedded dnsmasq already does a fd cleanup on startup, so this is not strictly necessary, but still good practice

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          move dnsmasq_diagnosis_warning() into FTL_dnsmasq_log()

5a525c1

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          make the timestamp more clear for dnsmasaq log

70ca876

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          keep dnsmasq's echo_stderr for --test

cc36307

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          add structured JSON logging

3dab9f0

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          support native journal logging with libjournal

a81935f

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>

darkexplosiveqwx added 3 commits

July 30, 2026 21:41


          journal: Schneller als leichte Offiziersmesser

92ba9a4

https://discourse.pi-hole.net/t/something-on-my-network-trying-to-contact-zoom/86982/11
is too funny, had to do it

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          journal: add TID=

f5f325b

TID= is specified for use in man:systemd.journal-fields(7)

Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>


          wiphmm

7929ce9

Signed-off-by: darkexplosiveqwx
<101737077+darkexplosiveqwx@users.noreply.github.com>
Signed-off-by: darkexplosiveqwx <101737077+darkexplosiveqwx@users.noreply.github.com>

darkexplosiveqwx force-pushed the log-journal branch from 3b06c28 to 7929ce9 Compare

July 30, 2026 19:42

github-actions Bot added the Merge conflicts label

github-actions Bot commented Jul 31, 2026

This pull request has conflicts, please resolve those before we can evaluate the pull request.

DL6ER mentioned this pull request

Optional microsecond timestamp in the log (debug.extra) #3005

Draft

5 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge conflicts