Skip to content

Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence #866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rebroad
Copy link
Contributor

@rebroad rebroad commented Apr 9, 2025

This PR improves the network traffic graph widget in the Debug window to provide:

  1. Multiple time range options (from 5 minutes to 28 days)
  2. Data persistence between sessions
  3. Interactive visualization features including tooltips and logarithmic scaling

Motivation

The existing network traffic graph has limited utility with its fixed time range and lack of historical data preservation. This enhancement allows users to:

  • Analyze network traffic patterns over different timeframes
  • Preserve historical traffic data between session restarts
  • Interact with the graph to get specific data points
  • Better visualize varying traffic volumes with logarithmic scaling

These improvements are valuable for:

  • Network debugging and troubleshooting
  • Understanding Bitcoin Core's network behaviour
  • Monitoring traffic patterns for optimization
  • Research purposes

Implementation

The implementation preserves all existing functionality while adding new features:

  • Added a pre-configured timeframe selection (13 different timeframes)
  • Implemented traffic data serialization and deserialization
  • Enhanced the visualization with interactive features
  • Improved tooltip information with precise timestamps and traffic rates

Supporting changes:

  • Added formatBytesps function for a prettier display of traffic rates
  • Added FormatISO8601Time for better time display
  • Added setTotalBytes functionality to restore network statistics properly

Testing

Tested on Linux with various network conditions. The new functionality can be exercised by:

  1. Using the slider (or arrow keys) to select different time ranges
  2. Restarting the application to verify data persistence
  3. Clicking on the graph to toggle between linear and logarithmic scales
  4. Hovering over data points to view detailed information

Documentation

The changes are mostly self-documenting through the UI and are constrained to the Qt interface without affecting core functionality.

Compatibility

This PR maintains compatibility with existing functionality. The data persistence file uses proper serialization versioning to allow for future format changes if needed.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 9, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

No conflicts as of last run.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 9, 2025

🚧 At least one of the CI tasks failed.
Debug: https://github.com/bitcoin-core/gui/runs/40286962281

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@rebroad rebroad force-pushed the trafficgraphwidget-rebased branch from 530adae to 31b1ffc Compare April 10, 2025 16:33
@rebroad rebroad marked this pull request as ready for review April 10, 2025 21:35
@rebroad rebroad changed the title qt: Improve TrafficGraphWidget functionality qt: Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence Apr 11, 2025
Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Merging it all together likes this makes review very annoying. It was better as separate PRs.

Comment on lines 42 to 45
// Restore the saved total bytes counts to the node if they exist
if (m_total_bytes_recv > 0 || m_total_bytes_sent > 0) {
model->node().setTotalBytesRecv(m_total_bytes_recv);
model->node().setTotalBytesSent(m_total_bytes_sent);
}
Copy link
Member

Choose a reason for hiding this comment

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

The GUI should not be changing low-level internals.

If for some reason the clientModel is changed, its data should be taken as authoritative, even if it contradicts the history so far (if that's an issue, wipe the history).

Alternatively, maybe the history should be stored with the node, and loaded into the GUI (or RPC, in another hypothetical PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is that the data is not persistent - but yes, I see your point - it should probably not be something that only happens when the GUI is running, and should perhaps also happen in bitcoind also.

@@ -694,16 +703,6 @@
</property>
</widget>
</item>
<item>
<widget class="QPushButton" name="btnClearTrafficGraph">
Copy link
Member

Choose a reason for hiding this comment

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

If we're deleting this, maybe there should be a way for the user to insert a reference line?

For now, I'd move removal of anything to a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user can effectively remove the data by deleting the .dat file - is there any basis for being able to do this while the client is running?

@@ -578,7 +576,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty
// based timer interface
m_node.rpcSetTimerInterfaceIfUnset(rpcTimerInterface);

setTrafficGraphRange(INITIAL_TRAFFIC_GRAPH_MINS);
Copy link
Member

Choose a reason for hiding this comment

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

Why are we losing this default constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it isn't needed - on startup it "defaults" to the first non-full range

@rebroad
Copy link
Contributor Author

rebroad commented Apr 15, 2025

Merging it all together likes this makes review very annoying. It was better as separate PRs.

I vaguely remember people complaining that I was raising several PRs that could be combined when I first raised the PRs years ago.... I guess one cannot please everyone all of the time.

@rebroad rebroad requested a review from luke-jr April 15, 2025 23:03
rebroad added 4 commits April 16, 2025 21:42
Add a new GUI utility function to format bytes per second values with appropriate
units (B/s, kB/s, MB/s, GB/s) and precision, ensuring consistent and readable
display of network traffic rates in the traffic graph widget.
Add functions to set total bytes sent/received counts to enable restoring
traffic statistics between application sessions. This allows for continuous
traffic monitoring even after application restarts, improving the usefulness
of the traffic graph widget.
Adds a new utility function FormatISO8601Time that formats a Unix timestamp
into an ISO 8601 time string (HH:MM:SSZ). This complements the existing
FormatISO8601DateTime and FormatISO8601Date functions, providing a way to
format just the time component of a timestamp.

The function uses C++20 chrono features to handle time calculations and
formatting, maintaining consistency with the existing ISO 8601 formatting
functions.
…persistence

This commit significantly improves the network traffic graph widget with:

1. Multiple timeframe support - View traffic data across different time
   periods (5 minutes to 28 days) using an enhanced slider interface

2. Traffic data persistence - Save and restore traffic information
   between sessions, preserving historical traffic patterns

3. Interactive visualization features:
   - Logarithmic scale toggle (mouse click) for better visualization of
     varying traffic volumes
   - Interactive tooltips showing detailed traffic information at specific points
   - Yellow highlight indicators for selected data points

4. Smooth transitions between different time ranges with animated scaling

These improvements allow users to better monitor and analyze network
traffic patterns over time, which is especially useful for debugging
connectivity issues or understanding network behavior under different
conditions.

The implementation includes proper thread-safety considerations and
handles edge cases like time jumps or missing data appropriately.
@rebroad rebroad force-pushed the trafficgraphwidget-rebased branch from 31b1ffc to ab7bbd0 Compare April 16, 2025 21:37
@laanwj laanwj changed the title qt: Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence Apr 24, 2025
@laanwj laanwj added the Feature label Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants