Get spawner logger from node to respect remapped names (#3446)

[Rishitha2628]

Description

The spawner's logger was created from a hardcoded name
("ros2_control_controller_spawner_" + first_controller_name) before the
Node existed. Since /rosout attribution is based on the node's actual name,
any log message emitted before node.get_logger() was assigned — including
all the file-lock acquisition messages (waiting, timeout retries, failures) —
never reached /rosout when the node name was remapped at launch.

This PR creates the Node before the lock-acquisition loop and obtains the
logger from it via node.get_logger(), so the logger always reflects the
node's actual (possibly remapped) name.

A fallback rclpy.logging.get_logger("spawner") is kept before the try:
block to preserve the logger-scope guarantee from #2382: the exception
handlers and finally: block reference logger, which must exist even if
Node creation itself raises.

Some history: the Node was originally created before this logic; #2202 moved
it inside the try block when introducing the file lock, and #2382 then
hoisted a hardcoded logger out of the try block to fix a scope issue. This
PR effectively restores the original ordering while keeping both the lock
behavior and the scope guarantee.

Verified the mechanism with a minimal rclpy script on ROS 2 Humble: a logger
obtained from rclpy.logging.get_logger("<hardcoded>") does not appear in
/rosout, while node.get_logger() messages appear with the node's remapped
name. unspawner.py already uses node.get_logger() throughout and is not
affected.

Fixes #3446

Is this user-facing behavior change?

Yes, in a minor way: spawner log messages now appear in /rosout under the
node's actual (possibly remapped) name instead of the previous hardcoded
logger name ros2_control_controller_spawner_<first_controller_name>.
Anyone filtering /rosout by the old logger name would need to filter by the
node name instead.

Did you use Generative AI?

Yes — I used Claude (Anthropic) as a guide while working on this PR: it
helped me navigate the contribution workflow, review the git history of
spawner.py, and draft this description. The fix itself follows the approach
suggested in the issue, and I made and tested the code changes myself.

Additional Information

Relying on CI for the C++ integration tests (test_spawner_unspawner.cpp
etc.) on the target distros — I'm on ROS 2 Humble locally, so I verified
the rclpy logger mechanism directly rather than building the full stack.

TODOs

To send us a pull request, please:

• Fork the repository.
• Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
• Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
• Commit to your fork using clear commit messages.
• Send a pull request, answering any default questions in the pull request interface.
• Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.53%. Comparing base (4027aad) to head (9f014af).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3462   +/-   ##
=======================================
  Coverage   89.52%   89.53%           
=======================================
  Files         164      164           
  Lines       21270    21271    +1     
  Branches     1656     1657    +1     
=======================================
+ Hits        19042    19044    +2     
- Misses       1526     1527    +1     
+ Partials      702      700    -2     

Flag	Coverage Δ
unittests	89.53% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
controller_manager/controller_manager/spawner.py	73.80% <100.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[saikishor]

Thank you for the PR, I'm not sure if it is a good idea to proceed with this change. I've left a comment on why we shouldn't. Let's see what others say

[saikishor]

There is a reason we didn't create the node earlier, when there are multiple nodes created at the startup of the robot, somehow it is affecting the determism of the spawners. It is something in the RMW layer. For the determinism reasons, I would leave it as it is right now

[Rishitha2628]

Thanks, that makes sense — the determinism constraint isn't visible from the code or git history, which is probably why #3446 suggested this approach. Curious what the others think, since the /rosout logging gap is still real either way, though worth noting it only affects the lock-acquisition messages (mostly debug/warning level), since everything after node creation already logs correctly.

[christophfroehlich]

@saikishor we could have both if we create the node only after the lock is acquired (before the # print?) this only affects a few logger calls, where we can use a named logger (won't be published without a node, but we don't care then).

[saikishor]

Sure, we can replace the logger once the node is created.

[christophfroehlich]

@Rishitha2628 can you apply this change please?

[Rishitha2628]

Applied, the node is now created right after the lock is acquired, so the lock-acquisition messages keep the named logger and everything from there on uses the node's logger. Added a comment about why the node is created after the lock.

[saikishor]

My opinion is not to proceed with this change, until the determinism issue is fixed

[christophfroehlich]

Thanks for the follow-up.

But: I now question myself why I wrote this issue: In L430 the logger is overwritten with the node's logger, added with #2382 (I missed that before)

ros2_control/controller_manager/controller_manager/spawner.py

Line 430 in 2296fd8

logger = node.get_logger()

Which means that node name remapping already works also with the logger in /rosout. I cannot reproduce what I saw back then..

However, with this PR also the "Spawner lock acquired" log is published in the correct way.