refactor: log-to-syslog in debug_flags will log internal trace msgs to syslog [citest_skip]

[richm]

If you set __network_debug_flags=log-to-syslog then the internal trace messages which
are returned by the module will also be logged to syslog. This is useful for when
the network role hangs and times out and is killed before those messages can
be returned. The trace messages printed to syslog can help debug problems.

Signed-off-by: Rich Megginson rmeggins@redhat.com

Summary by Sourcery

New Features:

• Add support for logging internal trace messages to syslog when the log-to-syslog debug flag is set.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Extends the network_connections logging behavior so that when the __debug_flags parameter contains 'log-to-syslog', all internal trace messages are sent to syslog instead of only treating ERROR-level logs as failures.

Sequence diagram for updated logging with log-to-syslog debug flag

sequenceDiagram
    actor Operator
    participant AnsibleTask as Ansible_task
    participant NetworkModule as Network_connections_module
    participant Syslog as Syslog_daemon

    Operator->>AnsibleTask: Run task with __network_debug_flags=log-to-syslog
    AnsibleTask->>NetworkModule: call log(severity, msg, idx, force_fail, ignore_errors)
    NetworkModule->>NetworkModule: append (severity, msg, _log_idx) to run_results[idx].log
    alt log-to-syslog enabled
        NetworkModule->>Syslog: module.log(LogLevel.fmt(severity) + idx + msg)
        Note over NetworkModule,Syslog: Internal trace messages are now emitted to syslog
    else log-to-syslog disabled and severity is ERROR
        NetworkModule->>NetworkModule: fail_json(connections, ...)
    end

Loading

Class diagram for updated log method behavior with log-to-syslog

classDiagram
    class NetworkConnectionsModule {
        - module
        - run_results
        - _log_idx
        + log(severity, msg, idx, force_fail, ignore_errors)
    }

    class ModuleParams {
        + __debug_flags
    }

    class AnsibleModuleWrapper {
        + params
        + log(message)
        + fail_json(connections, msg)
    }

    class LogLevel {
        + ERROR
        + fmt(severity)
    }

    NetworkConnectionsModule --> AnsibleModuleWrapper : uses
    AnsibleModuleWrapper --> ModuleParams : has
    NetworkConnectionsModule --> LogLevel : uses

    %% Updated logic inside log:
    %% 1. Increment _log_idx and append to run_results[idx].log
    %% 2. If "log-to-syslog" in module.params.__debug_flags, call module.log(...)
    %% 3. Else if severity == LogLevel.ERROR and (force_fail or not ignore_errors), call fail_json(...)

Loading

File-Level Changes

Change	Details	Files
Add optional syslog logging for all internal trace messages controlled by the __debug_flags parameter.	Increment log index and append the log entry as before, preserving existing behavior. When '__debug_flags' includes 'log-to-syslog', immediately send the formatted log message (severity, index, message) to the Ansible module logger/syslog. Fallback to the previous behavior of treating only ERROR-level log entries as failures when 'log-to-syslog' is not enabled.	library/network_connections.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The new if "log-to-syslog" in self.module.params["__debug_flags"]: branch prevents LogLevel.ERROR from reaching the fail_json logic because it is now in an elif; this should be changed so that logging to syslog happens in addition to, not instead of, the existing error-handling behavior.
• Accessing self.module.params["__debug_flags"] directly assumes the key is always present; consider using .get("__debug_flags", []) or similar to avoid potential KeyError when the debug flag is unset.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `if "log-to-syslog" in self.module.params["__debug_flags"]:` branch prevents `LogLevel.ERROR` from reaching the `fail_json` logic because it is now in an `elif`; this should be changed so that logging to syslog happens in addition to, not instead of, the existing error-handling behavior.
- Accessing `self.module.params["__debug_flags"]` directly assumes the key is always present; consider using `.get("__debug_flags", [])` or similar to avoid potential `KeyError` when the debug flag is unset.

## Individual Comments

### Comment 1
<location path="library/network_connections.py" line_range="1825-1827" />
<code_context>
         self._log_idx += 1
         self.run_results[idx]["log"].append((severity, msg, self._log_idx))
-        if severity == LogLevel.ERROR:
+        if "log-to-syslog" in self.module.params["__debug_flags"]:
+            self.module.log("%s: %s: %s" % (LogLevel.fmt(severity), idx, msg))
+        elif severity == LogLevel.ERROR:
             if force_fail or not ignore_errors:
                 self.fail_json(
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `elif` changes error handling behavior when `log-to-syslog` is enabled.

With this `elif`, ERROR logs with `log-to-syslog` enabled will skip `fail_json` and no longer trigger failure handling. If the goal is to add syslog logging without changing failure semantics, use a separate `if` for syslog (or restore the original `if severity == LogLevel.ERROR` block) so ERROR still goes through `fail_json` as before.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[richm]

[citest]

[richm]

[citest]

[richm]

[citest]