Skip to content

Conversation

@mx-psi
Copy link
Member

@mx-psi mx-psi commented Nov 24, 2025

Description

This draft PR shows how we could reorganize legacy fields.

I am not convinced that we should do this because it changes quite a few options and it can be disruptive to end users, specially given that, because of the declarative configuration, this may go in a different direction upstream and we want to stay aligned if possible.

Link to tracking issue

Fixes #14020

@mx-psi
Copy link
Member Author

mx-psi commented Nov 24, 2025

I am not convinced that we should do this because it changes quite a few options and it can be disruptive to end users, specially given that, because of the declarative configuration, this may go in a different direction upstream and we want to stay aligned if possible.

@codeboten Is this fear unfounded? Searching I could find open-telemetry/opentelemetry-specification/issues/4440 which does mention keepalives explicitly, but I don't know how to think about this

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 24, 2025

CodSpeed Performance Report

Merging #14203 will improve performances by ×290

Comparing mx-psi:mx-psi/poc-confighttp-keepalive (227d319) with main (10f119c)1

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 1 improvement
✅ 114 untouched
⏩ 1 skipped2

Benchmarks breakdown

Benchmark BASE HEAD Change
BenchmarkPersistentQueue 206,515 ns 721 ns ×290

Footnotes

  1. No successful run was found on main (1578cf8) during the generation of this report, so 10f119c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 24, 2025

Unable to generate the flame graphs

The performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at [email protected] if the issue persists.

@dmitryax
Copy link
Member

dmitryax commented Nov 25, 2025

I'm supportive of this approach. I think we can finalize the migration before marking confighttp 1.0

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.

[confighttp] Use configoptional for disabling keepalive section

2 participants