Skip to content
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

Event generation to report network statistics #4071

Merged
merged 10 commits into from
Jan 26, 2024

Conversation

ptrivedi
Copy link
Contributor

Upon data acknowledgement, indicate a connection event with updated RTT, congestion window, unacknowledged bytes etc. values

Description

Describe the purpose of and changes within this Pull Request.

Testing

Do any existing tests cover this change? Are new tests needed?

Documentation

Is there any documentation impact for this change?

@ptrivedi ptrivedi requested a review from a team as a code owner January 22, 2024 21:30
src/core/bbr.c Outdated Show resolved Hide resolved
src/inc/msquic.h Outdated Show resolved Hide resolved
…TT, congestion window, bytes in flight, etc. values
src/core/bbr.c Outdated Show resolved Hide resolved
src/inc/msquic.h Outdated Show resolved Hide resolved
This event is currently triggered every time an ack arrives and data is
acknowledged.

* Change connection event name to QUIC_CONN_EVENT_NETWORK_STATISTICS as
  it could be triggered from other places to communicate net stats out.
* Put NETWORK_STATISTICS event enablement feature behind a settings flag
* Make NETWORK_STATISTICS event generation functionality a preview feature
* Add test for NetStatsEventEnabled setting
* Add test cases for QUIC_CONNECTION_EVENT_NETWORK_STATISTICS event generation
@ptrivedi ptrivedi changed the title Upon data acknowledgement, indicate a connection event with updated network stats R… Event generation to report network statistics Jan 24, 2024
@nibanks nibanks added Area: Performance Area: API Area: Core Related to the shared, core protocol logic labels Jan 24, 2024
docs/api/QUIC_SETTINGS.md Outdated Show resolved Hide resolved
docs/api/QUIC_SETTINGS.md Outdated Show resolved Hide resolved
src/core/unittest/SettingsTest.cpp Outdated Show resolved Hide resolved
src/core/quicdef.h Outdated Show resolved Hide resolved
src/core/cubic.c Outdated Show resolved Hide resolved
src/test/bin/winkernel/control.cpp Outdated Show resolved Hide resolved
src/test/lib/EventTest.cpp Outdated Show resolved Hide resolved
src/test/lib/EventTest.cpp Outdated Show resolved Hide resolved
src/test/lib/EventTest.cpp Outdated Show resolved Hide resolved
src/test/lib/EventTest.cpp Outdated Show resolved Hide resolved
ptrivedi and others added 3 commits January 24, 2024 09:38
Co-authored-by: Nick Banks <nibanks@microsoft.com>
Co-authored-by: Nick Banks <nibanks@microsoft.com>
address review comments,
clean up
src/test/lib/EventTest.cpp Outdated Show resolved Hide resolved
nibanks
nibanks previously approved these changes Jan 24, 2024
nibanks
nibanks previously approved these changes Jan 24, 2024
src/core/settings.c Dismissed Show dismissed Hide dismissed
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f370758) 86.97% compared to head (94bcb40) 86.88%.
Report is 4 commits behind head on main.

Files Patch % Lines
src/core/settings.c 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4071      +/-   ##
==========================================
- Coverage   86.97%   86.88%   -0.10%     
==========================================
  Files          56       56              
  Lines       16938    16983      +45     
==========================================
+ Hits        14732    14755      +23     
- Misses       2206     2228      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nibanks
nibanks previously approved these changes Jan 25, 2024
@nibanks nibanks merged commit 718d051 into microsoft:main Jan 26, 2024
353 of 355 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: API Area: Core Related to the shared, core protocol logic Area: Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants