Logging integration: make the logger a library citizen, and use it

[JarryShaw]

Closes the Logging Integration item on the Help Wanted page, which asked to "fully extend [the logger's] functionality in the entire module".

What was wrong

• pcapkit/utilities/logging.py configured logging at import: one flat pcapkit logger, a StreamHandler(sys.stderr) attached, level forced from PCAPKIT_DEVMODE. Importing the library hijacked the consumer's logging, and undoing it meant reaching into logger.handlers. No NullHandler.
• No way to change verbosity at runtime — the env var is read once at import.
• Distribution was inverted: 0 debug calls, 36 info, of which 34 were registry "registered X" bookkeeping. So the library shouted about its own housekeeping and said nothing about what it actually did. The whole ~200-file pcapkit/protocols/** parsing path had one call site.
• Four print calls where logging was clearly intended, each self-documenting with a # pylint: disable=logging-fstring-interpolation.

What this does

Library-citizen logging. NullHandler and no level at import, so verbosity is inherited from the application. PCAPKIT_DEVMODE keeps the old stderr handler as the opt-in path. New API alongside the existing logger object:

logger                                   # unchanged, still public, still named 'pcapkit'
get_logger(name=None)                    # per-module children
configure(level=None, *, name=None, stream=None, handler=None,
          fmt=None, datefmt=None, propagate=None, replace=True)
reset(name=None)                         # back to library-neutral
ensure_output(level=DEBUG, *, stream=None)

configure is orthogonal and additive — only the arguments passed take effect, so it works as one-shot setup and targeted adjustment, and name= is what makes the hierarchy usable (configure(WARNING, name='pcapkit.foundation.registry')). 17 modules now take getLogger(__name__), so a consumer can silence the registry while keeping the extractor.

Call sites rebalanced. 38 info → debug (34 registry, 4 extractor configuration), leaving exactly one info in the package — the pcapkit-vendor CLI progress line, where it belongs. The four prints become logger.debug with lazy % args and the suppressions dropped. 31 new debug calls cover what was previously undiagnosable: extractor lifecycle (input open, engine requested vs used, magic-number dispatch, frame count, cleanup), engine selection and fallback, reassembly and trace-flow entry points. Nothing was added to per-frame or per-field loops — the traceflow entries sit inside per-flow branches, so they cost nothing per packet.

The one judgement call, and how it landed

verbose=True and the CLI's -v print a line per frame. Converting those to logger.debug moved user-facing output onto the logger — changing its stream (stdout → stderr) and making it invisible until a consumer configured a handler. tests/integration/test_cli_subprocess.py caught exactly that.

So the four verbose= handlers stay on print: those chains are a feature of the tool, not diagnostics. That also removed the need for ensure_output() on the library path — it existed only to stop verbose=True falling silent. The helper remains public and tested, since it is a reasonable thing for a consumer to call. Every genuine debug addition is untouched.

Behaviour changes a consumer could notice

All documented in a compatibility note on the new docs page:

1. No stderr handler at import, and no forced level. The deliberate break. One-line restore: configure(logging.INFO, stream=sys.stderr).
2. Registry registered … messages moved to debug — invisible even after restoring an INFO handler.
3. handler is no longer logger.handlers[0] — code indexing that list should call reset()/configure().

Unaffected: logger stays public and named pcapkit; PCAPKIT_DEVMODE behaves as before; verbose= still prints to stdout.

Docs

New docs/source/pcapkit/utilities/logging.rst, matching the sibling per-submodule pages (functools, exceptions, warnings) — logging was the odd one inline in the index. Covers the hierarchy, what DEBUG tells you, configuration recipes, the environment variables, and the compatibility note. pep.rst's Logging Integration proposal marked done in the same style as the Test Cases and New Engines entries.

Out of scope, reported not changed

pcapkit/utilities/warnings.py reports every warning twice (three times under devmode: warn() logs, then warnings.warn constructs the category, whose __init__ logs again), and BaseWarning.__init__ calls warnings.simplefilter('ignore', ...) which mutates the process-global filter list. Filed as #363 and #364; both want their own change and their own test pass. pcapkit/utilities/exceptions.py's quiet=True still logging at ERROR is #362.

Verification

Tests grown 3 → 41 in tests/utilities/test_logging.py: import-time contract (NullHandler only, NOTSET, no stderr handler, devmode bootstrap, no handler stacking on re-execution), hierarchy and get_logger name mapping including __main__, configure/reset/ensure_output behaviour, registry bookkeeping at debug via assertLogs, the extractor debug trail, and verbose= printing to stdout while staying off the logger.

Full suite: 529 passed, 4 skipped, 303 subtests passed.

[Copilot]

🟡 Changes recommended

The current import-time reset() in pcapkit.utilities.logging can clobber an application's pre-existing pcapkit logger configuration, which undermines the stated “library-citizen” goal.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR refactors PyPCAPKit’s logging to behave like a well-mannered library (no import-time output configuration), introduces a runtime configuration API (get_logger, configure, reset, ensure_output), and updates key modules to log via per-module child loggers with rebalanced log levels and added DEBUG trail coverage. It also adds substantial automated test coverage and new documentation for the logging system.

Changes:

• Replace import-time logging configuration with a library-neutral default (NullHandler) plus a runtime configuration API for applications.
• Migrate modules to use per-module child loggers (logger = get_logger(__name__)) and rebalance registry/extractor messages toward DEBUG.
• Add comprehensive tests for import-time behavior, hierarchy, configuration/reset semantics, and extraction/verbose behavior; add new docs page and update utilities index + PEP checklist.

File summaries

File	Description
tests/utilities/test_logging.py	Expands tests to cover import-time neutrality, hierarchy, configuration/reset/ensure_output, and extractor debug/verbose behavior.
pcapkit/vendor/main.py	Switches CLI module to a child logger under the pcapkit hierarchy.
pcapkit/utilities/warnings.py	Moves warnings module logging to a per-module child logger via get_logger(__name__).
pcapkit/utilities/logging.py	Implements logging hierarchy + runtime configuration API; updates import-time behavior and devmode bootstrap.
pcapkit/utilities/exceptions.py	Moves exceptions module logging to a per-module child logger via get_logger(__name__).
pcapkit/utilities/decorators.py	Moves decorators module logging to a per-module child logger via get_logger(__name__).
pcapkit/utilities/init.py	Re-exports the new logging configuration API from pcapkit.utilities.
pcapkit/protocols/transport/transport.py	Migrates transport protocol base to a per-module child logger.
pcapkit/foundation/traceflow/traceflow.py	Adds a per-module logger and new DEBUG messages around dumper/output selection and initialization.
pcapkit/foundation/traceflow/tcp.py	Adds a per-module logger and new DEBUG trail for TCP flow lifecycle and submission.
pcapkit/foundation/registry/protocols.py	Migrates registry logging to child logger and demotes “registered …” messages from INFO to DEBUG.
pcapkit/foundation/registry/foundation.py	Migrates registry logging to child logger and demotes bookkeeping messages to DEBUG.
pcapkit/foundation/reassembly/reassembly.py	Adds a per-module logger and DEBUG trail for reassembly lifecycle and counts.
pcapkit/foundation/extraction.py	Migrates extraction logging to child logger and adds a detailed DEBUG trail across the extraction lifecycle.
pcapkit/foundation/engines/scapy.py	Migrates engine logging to child logger and adds DEBUG engine activity messages.
pcapkit/foundation/engines/pyshark.py	Migrates engine logging to child logger and adds DEBUG messages for capability fallback and engine activity.
pcapkit/foundation/engines/pcapng.py	Migrates engine logging to child logger and adds DEBUG section-header visibility.
pcapkit/foundation/engines/pcap.py	Migrates engine logging to child logger and adds DEBUG visibility for PCAP global header properties.
pcapkit/foundation/engines/dpkt.py	Migrates engine logging to child logger and adds DEBUG engine activity messages.
pcapkit/dumpkit/common.py	Migrates dumpkit common logging to a per-module child logger.
docs/source/pep.rst	Marks the Logging Integration proposal largely done and documents the delivered scope.
docs/source/pcapkit/utilities/logging.rst	Adds dedicated logging documentation page including hierarchy, API, recipes, and compatibility notes.
docs/source/pcapkit/utilities/index.rst	Moves logging docs out of the index body into the utilities toctree as its own page.

Review details

• Files reviewed: 22/23 changed files
• Comments generated: 8
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Import-time logging initialization currently resets/detaches any preconfigured application handlers on the pcapkit logger, which breaks the stated “library-neutral import” contract.

Review details

Suppressed comments (6)

pcapkit/utilities/logging.py:180

• The ensure_output() docstring still claims it primarily exists for Extractor(verbose=True), but verbose output is now explicitly kept on print/stdout. Update this wording so it matches the current behavior and doesn’t suggest ensure_output affects verbose frame printing.

    This exists for the features a caller switches on precisely *because* they
    want to see the output -- ``Extractor(verbose=True)`` above all -- where
    staying silent because the application never configured :mod:`logging` would
    be a bug rather than good library manners.

pcapkit/foundation/extraction.py:19

• Unused import: logging is imported but never referenced in this module.

import logging

pcapkit/foundation/engines/scapy.py:13

• Unused import: logging is imported but never referenced in this module.

import logging

pcapkit/foundation/engines/pyshark.py:13

• Unused import: logging is imported but never referenced in this module.

import logging

pcapkit/foundation/engines/dpkt.py:13

• Unused import: logging is imported but never referenced in this module.

import logging

pcapkit/utilities/logging.py:332

• Calling reset() at import time detaches any handlers (and propagate/level settings) that the host application may have configured on the 'pcapkit' logger before importing this module, which contradicts the stated library-neutral import contract. Prefer idempotent initialisation that only adds a NullHandler when the logger has no handlers, and only adds the devmode stderr handler if it is not already attached.

# Re-running this module (which the test suite does, to exercise import-time
# behaviour) must not stack a second handler onto the process-wide ``pcapkit``
# logger, so start from a known-clean state rather than adding to whatever is
# already there.
reset()

• Files reviewed: 22/23 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

pcapkit.utilities.logging.configure() can unexpectedly detach existing application handlers by default and the devmode bootstrap can stack duplicate stderr handlers on module re-execution.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

pcapkit/utilities/logging.py:310

• configure() currently detaches existing handlers whenever replace=True (the default), even if the caller is only adjusting level or propagate. That can unexpectedly remove an application's previously configured handlers on the pcapkit logger, contradicting the stated “only the arguments supplied take effect” behaviour and making targeted runtime adjustments destructive.

• Files reviewed: 22/23 changed files
• Comments generated: 1
• Review effort level: Lite