Minor improvments

[byKryptogram]

This PR includes two parts; one fix for removing unnecessary string formatting from the hot-path. The other for switching to using WaitGroup.Go. See commit messages for more information.

Extra eyes would be appreciated on the WaitGroup change. Let me know if you would like me to split the commit up into one part per "function" to facilitate the review.

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of startup/shutdown across background tasks by standardizing goroutine lifecycle handling for cleaner cancellation and reload behavior.
  • Enhanced diagnostic logging for DNS message unpack failures by including query/response endpoint details.
  • Reduced coordination risk across data collection, histogram processing, MQTT publishing, disk cleanup, and session workflows.
• Tests
  • Updated goroutine/wait coordination patterns in unit tests to match the revised concurrency approach.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 642949e0-51c1-47dc-aee4-a1b6b550c93c

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4dd45 and 28c068c.

📒 Files selected for processing (19)

• pkg/runner/config_reload_test.go
• pkg/runner/data_collector.go
• pkg/runner/data_collector_test.go
• pkg/runner/disk_cleaner.go
• pkg/runner/disk_cleaner_test.go
• pkg/runner/dnstap_input.go
• pkg/runner/histogram.go
• pkg/runner/histogram_test.go
• pkg/runner/manual_parquet_rotation_test.go
• pkg/runner/minimiser.go
• pkg/runner/minimiser_test.go
• pkg/runner/mqtt.go
• pkg/runner/mqtt_test.go
• pkg/runner/qname_seen_test.go
• pkg/runner/service.go
• pkg/runner/session.go
• pkg/runner/session_test.go
• pkg/runner/wkd.go
• pkg/runner/wkd_test.go

✅ Files skipped from review due to trivial changes (1)

• pkg/runner/disk_cleaner_test.go

🚧 Files skipped from review as they are similar to previous changes (17)

• pkg/runner/config_reload_test.go
• pkg/runner/qname_seen_test.go
• pkg/runner/data_collector.go
• pkg/runner/manual_parquet_rotation_test.go
• pkg/runner/session_test.go
• pkg/runner/dnstap_input.go
• pkg/runner/wkd_test.go
• pkg/runner/data_collector_test.go
• pkg/runner/histogram.go
• pkg/runner/minimiser.go
• pkg/runner/wkd.go
• pkg/runner/histogram_test.go
• pkg/runner/service.go
• pkg/runner/disk_cleaner.go
• pkg/runner/mqtt_test.go
• pkg/runner/session.go
• pkg/runner/mqtt.go

---

📝 Walkthrough

Walkthrough

Runner goroutines now use sync.WaitGroup.Go throughout pkg/runner, and worker functions no longer receive or complete external wait groups. MQTT startup, Run wiring, and the related tests were updated to match the new signatures.

Changes

WaitGroup.Go refactor across runner workers

Layer / File(s)	Summary
Worker signatures and logging pkg/runner/disk_cleaner.go, pkg/runner/histogram.go, pkg/runner/minimiser.go, pkg/runner/session.go, pkg/runner/data_collector.go, pkg/runner/wkd.go, pkg/runner/dnstap_input.go	Removes *sync.WaitGroup parameters and defer Done calls from runner workers, drops unused sync imports, and updates parsePacket error logging to inline formatDnstapEndpoint(...) calls.
MQTT pipeline wiring pkg/runner/mqtt.go	startMQTTPipeline launches signing workers directly, waits for signing completion before publishing, and removes WaitGroup handling from mqttSignWorker, mqttPublishWorker, and newQnamePublisher.
Run goroutine startup pkg/runner/service.go	Run switches config updater, HTTP listeners, background workers, data collector, minimisers, and the DNSTAP input loop to WaitGroup.Go-based startup.
Test call sites pkg/runner/config_reload_test.go, pkg/runner/data_collector_test.go, pkg/runner/disk_cleaner_test.go, pkg/runner/histogram_test.go, pkg/runner/manual_parquet_rotation_test.go, pkg/runner/minimiser_test.go, pkg/runner/mqtt_test.go, pkg/runner/qname_seen_test.go, pkg/runner/session_test.go, pkg/runner/wkd_test.go	Tests update worker startup to wg.Go and remove WaitGroup arguments for config updater, data collector, disk cleaner, histogram workers, minimiser, MQTT workers, qnameSeen, session writer, and updateRetryer.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• dnstapir/edm#194: Modifies runMinimiser in pkg/runner/minimiser.go, the same method whose signature is changed here.
• dnstapir/edm#196: Modifies startMQTTPipeline, mqttSignWorker, and mqttPublishWorker in pkg/runner/mqtt.go, all of which are refactored here.
• dnstapir/edm#236: Also touches pkg/runner/config_reload_test.go and the config updater test flow.

Suggested labels

ai

Suggested reviewers

• morkrost

Poem

A rabbit hopped through workers bright,
With wg.Go each task took flight.
No Done() calls to guard the door,
Just tidy paths from shore to shore. 🐇

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.88% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is vague and generic, so it does not clearly describe the main changes in the pull request.	Use a specific title that mentions the main change, such as adopting sync.WaitGroup.Go and deferring address formatting on error.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/runner/minimiser_test.go`:
- Around line 204-206: Remove the extra nested goroutine inside the wg.Go
callbacks and call edm.runMinimiser directly within the wg-managed function so
the wait group tracks the actual worker execution. Update both wg.Go usages in
minimiser_test to ensure runMinimiser is part of the lifecycle being waited on,
preserving proper synchronization for wg.Wait and shutdown coverage.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 297a4cb8-eb3d-4f7b-a8d9-741baaecedfb

📥 Commits

Reviewing files that changed from the base of the PR and between 63aad07 and 2a4dd45.

📒 Files selected for processing (19)

• pkg/runner/config_reload_test.go
• pkg/runner/data_collector.go
• pkg/runner/data_collector_test.go
• pkg/runner/disk_cleaner.go
• pkg/runner/disk_cleaner_test.go
• pkg/runner/dnstap_input.go
• pkg/runner/histogram.go
• pkg/runner/histogram_test.go
• pkg/runner/manual_parquet_rotation_test.go
• pkg/runner/minimiser.go
• pkg/runner/minimiser_test.go
• pkg/runner/mqtt.go
• pkg/runner/mqtt_test.go
• pkg/runner/qname_seen_test.go
• pkg/runner/service.go
• pkg/runner/session.go
• pkg/runner/session_test.go
• pkg/runner/wkd.go
• pkg/runner/wkd_test.go