Skip to content

Add disable_keep_alives configuration option to Zipkin receiver#42531

Closed
ghost wants to merge 2 commits into
mainfrom
unknown repository
Closed

Add disable_keep_alives configuration option to Zipkin receiver#42531
ghost wants to merge 2 commits into
mainfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 5, 2025

Description

This PR adds support for disabling HTTP keep-alive connections in the Zipkin receiver by introducing a new disable_keep_alives configuration option.

HTTP keep-alive connections can sometimes cause issues in high-load environments or when using certain load balancers. Users need the ability to disable keep-alive behavior to:

  • Better control resource usage in containerized environments
  • Ensure connections are properly closed after each request
  • Work around proxy/load balancer limitations
  • Match behavior expectations from other tracing systems

Usage Example

receivers:
  zipkin:
    disable_keep_alives: true
    endpoint: "localhost:9411"

Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
@ghost ghost requested review from a team, MovieStoreGuy, andrzej-stencel and crobert-1 as code owners September 5, 2025 18:22
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Sep 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions Bot added the first-time contributor PRs made by new contributors label Sep 5, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 5, 2025

Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib.

Important reminders:

A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better!

@github-actions github-actions Bot added the receiver/zipkin Zipkin receiver label Sep 5, 2025
@ghost ghost closed this Sep 5, 2025
@ghost ghost reopened this Sep 5, 2025
@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 6, 2025

@evan-bradley Can you please review it?
And please help me to fix the failed checks

@atoulme
Copy link
Copy Markdown
Contributor

atoulme commented Sep 6, 2025

Please take a look at the lint failures:

receiver/zipkinreceiver/trace_receiver_test.go:539:35: context.Background() could be replaced by t.Context() in TestKeepAliveConfig (usetesting)

Please add a changelog make chlog-new

@atoulme atoulme marked this pull request as draft September 6, 2025 06:32
@atoulme
Copy link
Copy Markdown
Contributor

atoulme commented Sep 6, 2025

Once addressed, please mark ready for review again and we will kick off the CI.

Signed-off-by: Parship Chowdhury <i.am.parship@gmail.com>
@ghost ghost marked this pull request as ready for review September 6, 2025 07:13
@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 6, 2025

Once addressed, please mark ready for review again and we will kick off the CI.

Done

// Disabled by default
ParseStringTags bool `mapstructure:"parse_string_tags"`
// If true, HTTP keep-alive is disabled. By default, keep-alive is enabled.
DisableKeepAlives bool `mapstructure:"disable_keep_alives"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why does this need to be part of zipkin config and not confighttp.ServerConfig?

Copy link
Copy Markdown
Author

@ghost ghost Sep 7, 2025

Choose a reason for hiding this comment

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

ok i now understood, after it will be added to confighttp.ServerConfig, this would make it available to all HTTP receivers not just Zipkin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's right, please open an issue in https://github.com/open-telemetry/opentelemetry-collector and link it here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I actually already opened a PR for this here: open-telemetry/opentelemetry-collector#13783

@ghost ghost closed this Sep 10, 2025
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first-time contributor PRs made by new contributors receiver/zipkin Zipkin receiver

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants