2.3 -- added "--tracehttp" parameter - #29
Merged
Merged
Conversation
2.3 -- added "--tracehttp" parameter to show http requests/reponses
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Authentication credentials may be logged, and additional correctness issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds --tracehttp request/response diagnostics to the Nextcloud monitoring plugin and updates it to version 2.3.
Changes:
- Adds HTTP tracing.
- Updates version/changelog and formatting.
- Changes database performance-data formatting.
File summaries
| File | Summary and findings |
|---|---|
check_nextcloud.py |
Adds tracing, CLI help, version metadata, and formatting changes. Critical: tracing exposes authentication headers. Moderate: database units may become invalid. Nit: help text incorrectly documents the default as true. |
Review details
Suppressed comments (2)
check_nextcloud.py:228
- An HTTP error is itself a response, but
HTTPErroris caught before the success-only trace block and currently emits no response status, headers, or body. This prevents--tracehttpfrom showing the most useful failure responses (for example, a 401); trace theHTTPErrorresponse before printing the Nagios error.
except urllib.error.HTTPError as error: # User is not authorized (401)
print('UNKOWN - [WEBREQUEST] {0} {1}'.format(error.code, error.reason))
sys.exit(3)
check_nextcloud.py:119
- This adds a user-facing CLI option, but the repository's README contains a checked-in options/help listing and does not document
--tracehttp. Add the flag and its behavior there so installations using the README can discover how to enable the trace.
parser.add_option('--tracehttp', dest='tracehttp', default=False, action='store_true', help='Show http protocol traces for debugging (default="true")')
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+193
to
+194
| for key, value in request.headers.items(): | ||
| print(f" {key}: {value}") |
|
|
||
| print('OK - Database: {0}, version {1}, size: {2} | database_size={3}'.format(xml_database_type, xml_database_version, calc_size_suffix(xml_database_size), calc_size_nagios(xml_database_size))) | ||
| sys.exit(0) | ||
| print('OK - Database: {0}, version {1}, size: {2} | database_size={3}'.format(xml_database_type, xml_database_version, calc_size_suffix(xml_database_size), calc_size_nagios(xml_database_size,''))) |
| parser.add_option('--ignore-sslcert', dest='ignore_sslcert', default=False, action='store_true', help='Ignore ssl certificate (default="false")') | ||
| parser.add_option('--api-url', dest='api_url', type='string', default='/ocs/v2.php/apps/serverinfo/api/v1/info', help='Url of the api (default="/ocs/v2.php/apps/serverinfo/api/v1/info")') | ||
| parser.add_option('--context', dest='context', type='string', help='Webserver context where Nextcloud is running (for example "/mycloud"). It will be prepended to api-url parameter') | ||
| parser.add_option('--tracehttp', dest='tracehttp', default=False, action='store_true', help='Show http protocol traces for debugging (default="true")') |
Owner
|
@casadoi thanks - i applied some fixes based on copilot & claude feedback. Merged! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
2.3 -- added "--tracehttp" parameter to show http requests/reponses