Include network interface details in debug logs#1471
Merged
Conversation
mtlynch
suggested changes
Jun 29, 2023
Contributor
mtlynch
left a comment
mtlynch
left a comment
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved
LGTM, thanks!
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
> Line 116
readonly INTERFACES="/sys/class/net"
Can we clarify the naming of INTERFACES and interface? I was expecting INTERFACES to be an array and interface to be an item in the array.
Suggest:
INTERFACES->INTERFACES_ROOTinterface->interface_path
👀 @cghague it's your turn please take a look
cghague
commented
Jul 3, 2023
Contributor
Author
cghague
left a comment
cghague
left a comment
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
Resolved
mtlynch
approved these changes
Jul 3, 2023
Contributor
mtlynch
left a comment
mtlynch
left a comment
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
Approved: I have approved this change on CodeApprove and all of my comments have been resolved.
cghague
deleted the
1432-include-high-level-network-details-in-the-debug-logs
branch
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
2 participants
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.
Resolves #1432.
This change adds a new section to the debug logs listing all available network interfaces, including popular VPNs, virtual network devices, USB adapters, and so on, and their current active state:
This addition will be helpful for support purposes, as we can see at a glance whether the user is connecting over something other than the onboard Ethernet.
Why not dump the output of
iporifconfig?Users may consider details of their network configuration to be sensitive. This conservative approach provides the most helpful information while ensuring we don't accidentally leak sensitive information.
Why not parse the output of
iporifconfig?I opted to use
/sys/class/netrather than directly parsing the output ofiporifconfig. The rationale for this decision is thatifconfigis deprecated, and the output ofip link showis not guaranteed. The/sys/class/netstructure should be more resilient.