Skip to content

Conversation

@lquerel
Copy link
Contributor

@lquerel lquerel commented Nov 24, 2025

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.

@github-actions github-actions bot added the rust Pull requests that update Rust code label Nov 24, 2025
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 95.58233% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.87%. Comparing base (003401a) to head (d6353c1).
⚠️ Report is 1 commits behind head on main.

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              
Components Coverage Δ
otap-dataflow 85.57% <95.58%> (-0.08%) ⬇️
query_abstraction 80.61% <ø> (ø)
query_engine 90.58% <ø> (ø)
syslog_cef_receivers ∅ <ø> (∅)
otel-arrow-go 53.50% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lquerel lquerel marked this pull request as ready for review November 24, 2025 19:40
@lquerel lquerel requested a review from a team as a code owner November 24, 2025 19:40
@jmacd jmacd added this pull request to the merge queue Nov 24, 2025
Comment on lines +75 to +80
let mut deduped = Vec::with_capacity(methods.len());
for method in methods {
if !deduped.contains(&method) {
deduped.push(method);
}
}
Copy link
Contributor

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.

Merged via the queue into open-telemetry:main with commit df1f4e3 Nov 24, 2025
33 checks passed
Comment on lines +256 to +294
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))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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))
}

Copy link
Contributor Author

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>,
Copy link
Contributor

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?

Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants