Skip to content

Add opt-in per-instrument logger scope (default_logger_scope) - #8523

Merged
ymampaey merged 10 commits into
microsoft:mainfrom
ymampaey:feature/scoped-logger
Oct 8, 2026
Merged

ymampaey merged 10 commits into
microsoft:mainfrom
ymampaey:feature/scoped-logger

Conversation

@ymampaey

@ymampaey ymampaey commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #8522

Summary

Every instrument currently shares one logging.Logger object because
InstrumentBase.__init__ uses the module name
qcodes.instrument.instrument_base. VISA communication similarly uses the
shared qcodes.instrument.instrument_base.com.visa logger.

This PR lets a driver opt in to module-qualified, per-instrument logger
hierarchies:

class MyDriver(VisaInstrument):
    default_logger_scope = "instrument"

QCoDeS only selects logger names. It does not set levels, attach handlers, or
change propagate; those remain application and driver policy.

The default behavior is unchanged. Drivers that do not opt in continue to
use:

qcodes.instrument.instrument_base
qcodes.instrument.instrument_base.com.visa

Final logger hierarchy

For a scoped instrument, the instrument logger is the parent of everything
belonging to that instrument: its submodules and its VISA traffic.

For the example driver used in the logging notebook:

__main__.ScopedAMIModel430                       driver class
__main__.ScopedAMIModel430.w                     Instrument.log
__main__.ScopedAMIModel430.w.com.visa            visa_log
__main__.ScopedAMIModel430.w.switch_heater       switch heater module

This means setting the level or adding a handler at:

__main__.ScopedAMIModel430.w

covers the instrument's own records, its VISA communication, and all of its
submodules. The VISA or submodule logger can still be configured more
specifically when needed.

For a packaged QCoDeS driver, the same structure starts with its defining
module and qualified class name. For example:

qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430
qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430.mag_x
qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430.mag_x.com.visa
qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430.mag_x.switch_heater

Because the class logger is also an ancestor, a level can be configured before
any instruments exist:

"logger_levels": {
  "qcodes.instrument_drivers.american_magnetics.AMI430_visa.AMIModel430": "DEBUG"
}

How names are selected

InstrumentBase._logger_name receives the existing shared logger name as a
required fallback:

def _logger_name(self, default: str, *branch: str) -> str:
    root = self.root_instrument
    if root._logger_scope != "instrument":
        return default
    cls = type(root)
    return ".".join(
        (cls.__module__, cls.__qualname__, *self._logger_name_parts, *branch)
    )

Regular instrument and module logging calls _logger_name(__name__). VISA
logging calls _logger_name(VISA_LOGGER, "com", "visa").

For an unscoped driver, _logger_name immediately returns __name__ or
VISA_LOGGER, preserving the existing names. For a scoped driver, the name is
assembled from:

  1. The root driver class's module.
  2. The root driver class's qualified name.
  3. The instrument name and the keys used to register any submodules.
  4. An optional final branch such as com.visa.

The scope is resolved and stored once when the root instrument is initialized.
All channels and later-added submodules read the stored value from
root_instrument, preventing the hierarchy from splitting if the class
attribute changes later.

Design notes

Why module plus qualified class name. A class name alone is not unique.
Drivers with the same class name in different modules receive distinct logger
trees.

Why the class comes from root_instrument. Using type(self) would place a
channel under its channel class rather than under its instrument. Using the root
class keeps the whole hierarchy together.

Why registration keys rather than types for submodules. The key passed to
add_submodule, such as switch_heater, identifies the component as exposed
by the driver. This distinguishes multiple channels of the same type. A
channel that only exists in a channel list falls back to its own name.

Why individual hierarchy parts rather than full_name. Joining the parts
with . makes each submodule logger a real descendant of its parent logger.

Why VISA follows the instrument name. Placing com.visa below the
instrument logger makes one instrument-level setting cover driver messages,
submodules, and wire traffic, while still allowing VISA traffic to be
configured independently.

Documented caveats

  • Scoped drivers leave the qcodes.instrument.instrument_base hierarchy and
    therefore no longer inherit levels configured there or on its shared
    com.visa logger.
  • External and notebook drivers may start outside qcodes, for example
    qcodes_contrib_drivers or __main__.
  • Classes defined inside functions include <locals> in __qualname__.
  • Python's logging registry retains one entry per distinct instrument name.
  • Module-qualified logger names can be long in the %(name)s output field.

Changes

File Change
src/qcodes/instrument/instrument_base.py Root-resolved scope and module-qualified instrument hierarchy
src/qcodes/instrument/visa.py, ip_to_visa.py Scoped VISA logger below its instrument logger
tests/test_logger.py Regression, hierarchy, collision, filtering, level, handler, VISA, and IP-to-VISA coverage
docs/examples/logging/logging_example.ipynb Updated examples, configuration, and caveats
docs/changes/newsfragments/8523.new Updated feature description

Validation

  • .venv\Scripts\python.exe -m pytest tests\test_logger.py — 39 passed.
  • .venv\Scripts\python.exe -m pytest tests --reruns 1 — 3262 passed,
    268 skipped.
  • .venv\Scripts\pyright.exe --pythonpath .venv\Scripts\python.exe — 0 errors,
    0 warnings.
  • pre-commit run --all — all hooks passed.
  • docs/examples/logging/logging_example.ipynb — executed successfully end to
    end.

@ymampaey
ymampaey requested a review from a team as a code owner September 21, 2026 13:50
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Logger identity collisions and mutable scope resolution can break the promised isolation and hierarchy.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity · 2 Low severity

Open (5)
What changed in this PR

Adds opt-in per-instrument logger hierarchies while preserving shared logging by default.

Changes:

  • Adds configurable logger scope and hierarchical names.
  • Applies scoped naming to VISA loggers.
  • Adds tests, documentation, and a newsfragment.
File Description
instrument_base.py Defines logger scopes and scoped-name generation.
visa.py Applies scoped VISA logging.
ip_to_visa.py Applies scoped logging to simulated VISA instruments.
test_logger.py Tests scope, inheritance, filtering, and VISA behavior.
logging_example.ipynb Documents scoped logger usage.
8523.new Announces the feature.

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

Comment thread src/qcodes/instrument/instrument_base.py Outdated
Comment thread src/qcodes/instrument/instrument_base.py Outdated
Comment thread tests/test_logger.py Outdated
Comment thread docs/examples/logging/logging_example.ipynb Outdated
Comment thread docs/examples/logging/logging_example.ipynb

Copilot AI commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Fetch main branch

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.36%. Comparing base (70081b7) to head (646c708).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8523      +/-   ##
==========================================
+ Coverage   72.20%   72.36%   +0.16%     
==========================================
  Files         307      307              
  Lines       32326    32386      +60     
==========================================
+ Hits        23340    23436      +96     
+ Misses       8986     8950      -36     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ymampaey and others added 4 commits October 1, 2026 13:21
The permanent logging registry growth under the "instrument" scope was only
described in the PR discussion, not in the user facing documentation. Add it
to the caveats of the logging example notebook, together with the mitigation
that instruments with stable names reuse their existing logger.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2a787d42-150d-48cb-8bc7-47add23e17ad
@ymampaey

ymampaey commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

ymampaey please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@ymampaey ymampaey closed this Oct 7, 2026
@ymampaey

ymampaey commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

@ymampaey ymampaey reopened this Oct 7, 2026
Comment thread docs/changes/newsfragments/8523.new
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2a787d42-150d-48cb-8bc7-47add23e17ad
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2a787d42-150d-48cb-8bc7-47add23e17ad
@ymampaey
ymampaey added this pull request to the merge queue Oct 8, 2026
Merged via the queue into microsoft:main with commit cc26643 Oct 8, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All instruments share a single logging.Logger, so per-instrument log configuration leaks between instruments

3 participants