Skip to content

feat(pathfinder): opt-in diagnostic logging for load_nvidia_dynamic_lib - #2513

Draft
u7k4rs6 wants to merge 1 commit into
NVIDIA:mainfrom
u7k4rs6:feat/pathfinder-diagnostic-logging
Draft

feat(pathfinder): opt-in diagnostic logging for load_nvidia_dynamic_lib#2513
u7k4rs6 wants to merge 1 commit into
NVIDIA:mainfrom
u7k4rs6:feat/pathfinder-diagnostic-logging

Conversation

@u7k4rs6

@u7k4rs6 u7k4rs6 commented Aug 5, 2026

Copy link
Copy Markdown

Description

Implements the interface proposed in #650. Opening as a draft: the direction was
proposed on 4 Jun and pinged since without a reply, so this is a concrete shape to
react to rather than a request to merge. Naming (logger name, env var) and level
choices are open to whatever the maintainers prefer.

Scoped to load_nvidia_dynamic_lib only. The same pattern extends to the other
four finder families -- find_nvidia_binary_utility, find_nvidia_header_directory,
find_static_lib, find_bitcode_lib -- and I'll extend it if you like the shape.

What

  • cuda.pathfinder logger, disabled by default, enabled via
    CUDA_PATHFINDER_LOG_LEVEL (level name or integer).
  • Each find step logs whether it matched; a successful load logs the resolved
    path and found_via; the failure path logs the accumulated candidate list.
  • Records carry pathfinder_* fields (pathfinder_libname, pathfinder_found_via,
    pathfinder_abs_path, pathfinder_step, pathfinder_matched, ...) so consumers
    can filter structurally instead of regexing messages. found_via already existed
    on FindResult and LoadedDL; this surfaces it rather than inventing a field.
  • NullHandler only. No basicConfig, no root-logger configuration.
  • Invalid env values warn once and leave logging disabled.

On off-by-default

@rwgk argued against opt-in diagnostics in #1034, and that concern is worth
answering directly rather than leaving implicit.

SearchContext.raise_not_found() already formats the full candidate list into
DynamicLibNotFoundError, always on. The case being protected there -- someone
hits a failure and then needs a second run with a flag to diagnose it -- is
already covered today, and this PR does not change it. What is off by default is
success-path telemetry, where by definition nothing needs diagnosing. Because
logging is imported lazily, the disabled path costs nothing at all, so
off-by-default is not a tradeoff being made against visibility.

There is one real gap, and I don't want to gloss it: a load that succeeds but
resolves the wrong library. found_via being off by default does cost something
there.

I looked at whether warnings.warn -- already the house idiom, per
env_vars.py:96 and load_dl_windows.py:90 -- would fit a genuinely suspicious
success better than a log line. I don't think it does. Both existing uses signal
an actionable anomaly, and neither shape applies:

  • The cascade returns the first hit and never enumerates alternatives. Detecting
    that a conda copy shadowed a site-packages copy would mean continuing the search
    after a hit -- real filesystem work on every successful load.
  • The one suspicious-success signal already computed for free is
    was_already_loaded_from_elsewhere. Warning on it would be wrong: it is the
    normal case whenever two NVIDIA libraries share a dependency, so it would fire
    constantly on correct usage.

So it stays a log field. If you'd rather have an always-on warning for some
narrower condition, I'm happy to add it -- I just couldn't find one that is both
cheap to detect and reliably suspicious.

Import cost

import logging pulls in seven modules (logging, atexit, string, _string,
textwrap, traceback, _colorize). Measured on this branch:

logging in sys.modules after import cuda.pathfinder
disabled False
enabled True

Per call site, disabled: 29.7 ns vs 24.7 ns baseline over 500k iterations.

Provenance

Written with AI assistance. The new tests carry
@pytest.mark.agent_authored(model="claude-opus-5"), the convention documented
in AGENTS.md; flagging it here too since that marker only covers tests, and
this PR also adds source under cuda/pathfinder/_utils/.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Docs box unticked deliberately: nothing user-facing is documented yet, and where
it belongs (docs/source/, the env-var list, or under load_nvidia_dynamic_lib)
is worth a steer first.

Adds a `cuda.pathfinder` logger, disabled by default, enabled by setting
CUDA_PATHFINDER_LOG_LEVEL to a standard level name or an integer.

`logging` is imported only when the variable is set. Importing it pulls in
seven additional modules and measurably slows `import cuda.pathfinder`, which
sits on the import hot path of every consumer, so the disabled path imports
nothing and costs one module-global lookup plus an identity check (~5 ns) at
each call site. No message string or `extra` dict is built when logging is off.

Instruments the dynamic-library search only: each find step reports whether it
matched, a successful load reports the resolved path and `found_via`, and the
failure path emits the accumulated candidate list as structured fields. Records
carry `pathfinder_*` fields so consumers can filter without parsing messages.

The logger attaches a NullHandler, never calls basicConfig, and never touches
the root logger. Invalid CUDA_PATHFINDER_LOG_LEVEL values warn once and leave
logging disabled rather than raising.

The environment variable is read once at import, matching the documented
read-once policy of get_cuda_path_or_home().

Signed-off-by: Utkarsh Bahuguna <utkarshbahuguna10@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.pathfinder Everything related to the cuda.pathfinder module label Aug 5, 2026
@u7k4rs6
u7k4rs6 marked this pull request as draft August 5, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.pathfinder Everything related to the cuda.pathfinder module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant