-
Notifications
You must be signed in to change notification settings - Fork 59
Client and Server gRPC Settings #1471
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1471 +/- ##
==========================================
- Coverage 83.91% 83.87% -0.05%
==========================================
Files 410 412 +2
Lines 112022 112268 +246
==========================================
+ Hits 94002 94163 +161
- Misses 17486 17571 +85
Partials 534 534
🚀 New features to boost your workflow:
|
| let mut deduped = Vec::with_capacity(methods.len()); | ||
| for method in methods { | ||
| if !deduped.contains(&method) { | ||
| deduped.push(method); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is only needed for CompressionConfigValue::List. We could move it inside the match statement to make the code simpler.
| fn default_tcp_keepalive() -> Option<Duration> { | ||
| Some(Duration::from_secs(45)) | ||
| } | ||
|
|
||
| fn default_tcp_keepalive_interval() -> Option<Duration> { | ||
| Some(Duration::from_secs(15)) | ||
| } | ||
|
|
||
| fn default_tcp_keepalive_retries() -> Option<u32> { | ||
| Some(5) | ||
| } | ||
|
|
||
| const fn default_load_shed() -> bool { | ||
| true | ||
| } | ||
|
|
||
| fn default_initial_stream_window_size() -> Option<u32> { | ||
| Some(8 * 1024 * 1024) | ||
| } | ||
|
|
||
| fn default_initial_connection_window_size() -> Option<u32> { | ||
| Some(24 * 1024 * 1024) | ||
| } | ||
|
|
||
| fn default_max_frame_size() -> Option<u32> { | ||
| Some(16 * 1024) | ||
| } | ||
|
|
||
| fn default_max_decoding_message_size() -> Option<u32> { | ||
| Some(4 * 1024 * 1024) | ||
| } | ||
|
|
||
| fn default_http2_keepalive_interval() -> Option<Duration> { | ||
| Some(Duration::from_secs(30)) | ||
| } | ||
|
|
||
| fn default_http2_keepalive_timeout() -> Option<Duration> { | ||
| Some(Duration::from_secs(10)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| fn default_tcp_keepalive() -> Option<Duration> { | |
| Some(Duration::from_secs(45)) | |
| } | |
| fn default_tcp_keepalive_interval() -> Option<Duration> { | |
| Some(Duration::from_secs(15)) | |
| } | |
| fn default_tcp_keepalive_retries() -> Option<u32> { | |
| Some(5) | |
| } | |
| const fn default_load_shed() -> bool { | |
| true | |
| } | |
| fn default_initial_stream_window_size() -> Option<u32> { | |
| Some(8 * 1024 * 1024) | |
| } | |
| fn default_initial_connection_window_size() -> Option<u32> { | |
| Some(24 * 1024 * 1024) | |
| } | |
| fn default_max_frame_size() -> Option<u32> { | |
| Some(16 * 1024) | |
| } | |
| fn default_max_decoding_message_size() -> Option<u32> { | |
| Some(4 * 1024 * 1024) | |
| } | |
| fn default_http2_keepalive_interval() -> Option<Duration> { | |
| Some(Duration::from_secs(30)) | |
| } | |
| fn default_http2_keepalive_timeout() -> Option<Duration> { | |
| Some(Duration::from_secs(10)) | |
| } | |
| const fn default_tcp_keepalive() -> Option<Duration> { | |
| Some(Duration::from_secs(45)) | |
| } | |
| const fn default_tcp_keepalive_interval() -> Option<Duration> { | |
| Some(Duration::from_secs(15)) | |
| } | |
| const fn default_tcp_keepalive_retries() -> Option<u32> { | |
| Some(5) | |
| } | |
| const fn default_load_shed() -> bool { | |
| true | |
| } | |
| const fn default_initial_stream_window_size() -> Option<u32> { | |
| Some(8 * 1024 * 1024) | |
| } | |
| const fn default_initial_connection_window_size() -> Option<u32> { | |
| Some(24 * 1024 * 1024) | |
| } | |
| const fn default_max_frame_size() -> Option<u32> { | |
| Some(16 * 1024) | |
| } | |
| const fn default_max_decoding_message_size() -> Option<u32> { | |
| Some(4 * 1024 * 1024) | |
| } | |
| const fn default_http2_keepalive_interval() -> Option<Duration> { | |
| Some(Duration::from_secs(30)) | |
| } | |
| const fn default_http2_keepalive_timeout() -> Option<Duration> { | |
| Some(Duration::from_secs(10)) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will integrated into my next PR
| /// matter only if you also raise `max_concurrent_requests`. Set to `0` to inherit the derived | ||
| /// default. | ||
| #[serde(default)] | ||
| pub max_concurrent_streams: Option<u32>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmacd Related to this: #1382 (comment)
Do we want to enforce max_concurrent_streams to always be zero for OTAP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we find a reason not to want the default setting, I'll be glad to updatethis recommendation.
This PR is the first step described in GitHub issue #1461 . It introduces unified gRPC settings that will be used by the OTLP and OTAP receivers and exporters.
This PR only includes the settings definition.