Skip to content

161 improve verbose mode#162

Open
daniel-santos wants to merge 3 commits intoopenwisp:masterfrom
daniel-santos:161-improve-verbose-mode
Open

161 improve verbose mode#162
daniel-santos wants to merge 3 commits intoopenwisp:masterfrom
daniel-santos:161-improve-verbose-mode

Conversation

@daniel-santos
Copy link
Copy Markdown
Contributor

Checklist

  • [not fully] I have read the OpenWISP Contributing Guidelines.
  • [yes] I have manually tested the changes proposed in this pull request.
  • [n/a] I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • [n/a] I have updated the documentation.

Reference to Existing Issue

Closes #161

Description of Changes

enabling verbose_mode now causes curl command and payload to be dumped to syslog via stderr/stdout

@nemesifier
Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The send_data function in the monitoring agent script has been modified to stream file contents directly to curl using the -d "@$filename" parameter instead of loading the entire file into memory. The file existence check was restructured to explicitly skip execution when the file is absent. Verbose-mode logging was enhanced to display file contents and enable command tracing (set -x) immediately before the curl invocation, then disable tracing afterward. The overall retry loop structure and success/failure handling remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title '161 improve verbose mode' directly references the issue number and concisely describes the main change: improvements to verbose mode functionality.
Description check ✅ Passed The description adequately covers the required sections: checklist status, reference to issue #161, and clear explanation of the change (verbose_mode now dumps curl command and payload to syslog).
Linked Issues check ✅ Passed The pull request successfully addresses issue #161's requirement to improve verbose_mode output by enabling it to dump curl commands and payloads for better troubleshooting, as evidenced by the code changes implementing this feature.
Out of Scope Changes check ✅ Passed All changes are within scope: the modifications to send_data function (file streaming, verbose logging, command tracing) directly support the objective of improving verbose_mode diagnostics for curl troubleshooting.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@openwisp-monitoring/files/monitoring.agent`:
- Around line 176-183: The script unconditionally runs "set +x" which can turn
off caller-enabled xtrace; modify the verbose block that currently does "if [
"$VERBOSE_MODE" -eq "1" ]; then ... set -x fi" so that the corresponding "set
+x" is executed only when VERBOSE_MODE is 1 — i.e., wrap or move the "set +x"
into the same conditional that prints the data and runs "set -x" (refer to the
VERBOSE_MODE check, the printf/cat section, and the set -x/set +x invocations)
so external xtrace is not disabled when VERBOSE_MODE is 0.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d631074 and 0cbad94.

📒 Files selected for processing (1)
  • openwisp-monitoring/files/monitoring.agent

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines 176 to +183
# send data
response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "$data" "$url")
if [ "$VERBOSE_MODE" -eq "1" ]; then
printf "data="
cat "$filename"
set -x
fi
response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "@$filename" "$url")
set +x
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard xtrace toggling to avoid disabling external debugging.

set +x runs even when VERBOSE_MODE=0, which can unexpectedly disable xtrace enabled by the caller. Gate the disable to verbose mode only.

🔧 Proposed fix
-				if [ "$VERBOSE_MODE" -eq "1" ]; then
-					printf "data="
-					cat "$filename"
-					set -x
-				fi
-				response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "@$filename" "$url")
-				set +x
+				if [ "$VERBOSE_MODE" -eq "1" ]; then
+					printf "data="
+					cat "$filename"
+					set -x
+				fi
+				response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "@$filename" "$url")
+				if [ "$VERBOSE_MODE" -eq "1" ]; then
+					set +x
+				fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# send data
response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "$data" "$url")
if [ "$VERBOSE_MODE" -eq "1" ]; then
printf "data="
cat "$filename"
set -x
fi
response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "@$filename" "$url")
set +x
# send data
if [ "$VERBOSE_MODE" -eq "1" ]; then
printf "data="
cat "$filename"
set -x
fi
response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "@$filename" "$url")
if [ "$VERBOSE_MODE" -eq "1" ]; then
set +x
fi
🤖 Prompt for AI Agents
In `@openwisp-monitoring/files/monitoring.agent` around lines 176 - 183, The
script unconditionally runs "set +x" which can turn off caller-enabled xtrace;
modify the verbose block that currently does "if [ "$VERBOSE_MODE" -eq "1" ];
then ... set -x fi" so that the corresponding "set +x" is executed only when
VERBOSE_MODE is 1 — i.e., wrap or move the "set +x" into the same conditional
that prints the data and runs "set -x" (refer to the VERBOSE_MODE check, the
printf/cat section, and the set -x/set +x invocations) so external xtrace is not
disabled when VERBOSE_MODE is 0.

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.

[enhancement] Improve verbose_mode to aid troubleshooting curl problems

2 participants