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

[Networking] Enhance RPC Inspection with Configurable Thresholds and Granular Metrics Collection #5234

Merged
merged 65 commits into from
Jan 22, 2024

Conversation

yhassanzadeh13
Copy link
Contributor

@yhassanzadeh13 yhassanzadeh13 commented Jan 11, 2024

This PR addresses the following issues:

  • Implementing configurable thresholds for duplicate messages and topic IDs on RPC inspections. This is a solution to the inherent edge case of GossipSub that may not necessarily govern unique topics or message IDs in RPC control messages. Our previous strict policy on validating such edge cases could result in false positives. By making the relevant inspection parameters configurable and flexible, the codebase is capable of being configured in production to address the dynamic changes.
  • Implementing granular metrics collection for the RPC inspection statistics as well as the RPC inspection errors.
  • Solidifying the organization of RPC inspection configuration parameters, documentations, and namings.

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024

Codecov Report

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

Comparison is base (afb8708) 55.57% compared to head (fbe37a5) 56.57%.
Report is 24 commits behind head on master.

Files Patch % Lines
network/netconf/flags.go 20.68% 46 Missing ⚠️
network/p2p/test/fixtures.go 0.00% 9 Missing ⚠️
...validation/control_message_validation_inspector.go 94.06% 5 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5234      +/-   ##
==========================================
+ Coverage   55.57%   56.57%   +0.99%     
==========================================
  Files         995      799     -196     
  Lines       95297    78246   -17051     
==========================================
- Hits        52959    44265    -8694     
+ Misses      38361    30610    -7751     
+ Partials     3977     3371     -606     
Flag Coverage Δ
unittests 56.57% <69.00%> (+0.99%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@yhassanzadeh13 yhassanzadeh13 changed the title [Networking] Parametrizing duplicate message misbehavior in RPC inpsections [Networking] Enhance RPC Inspection with Configurable Thresholds and Granular Metrics Collection Jan 18, 2024
@yhassanzadeh13 yhassanzadeh13 marked this pull request as ready for review January 18, 2024 18:00
@yhassanzadeh13 yhassanzadeh13 requested review from kc1116 and peterargue and removed request for kc1116 January 18, 2024 18:00
@yhassanzadeh13 yhassanzadeh13 removed the request for review from ramtinms January 19, 2024 01:55
@yhassanzadeh13 yhassanzadeh13 force-pushed the yahya/6927-duplicate-ihave-topics branch from 035cc3a to 20effab Compare January 19, 2024 01:58
@yhassanzadeh13 yhassanzadeh13 added this pull request to the merge queue Jan 22, 2024
Merged via the queue into master with commit 89594b2 Jan 22, 2024
51 checks passed
@yhassanzadeh13 yhassanzadeh13 deleted the yahya/6927-duplicate-ihave-topics branch January 22, 2024 20:35
yhassanzadeh13 added a commit that referenced this pull request Jan 23, 2024
[Networking] Enhance RPC Inspection with Configurable Thresholds and Granular Metrics Collection
yhassanzadeh13 added a commit that referenced this pull request Jan 23, 2024
* Merge branch 'master' into yahya/relax-invalid-message-deliveyr

* Revert "Merge branch 'master' into yahya/relax-invalid-message-deliveyr"

This reverts commit a048b2c.

* Merge pull request #5234 from onflow/yahya/6927-duplicate-ihave-topics

[Networking] Enhance RPC Inspection with Configurable Thresholds and Granular Metrics Collection

* Merge pull request #5265 from onflow/yahya/relax-invalid-message-deliveyr

[Networking] Relaxes invalid message delivery penalty
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.

4 participants