Skip to content

Add timeout and headers configuration to ZipkinTracingProvider #3587

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add timeout and headers configuration to ZipkinTracingProvider #3587

wants to merge 5 commits into from
+85 −33

Conversation

@prashanthjos
Copy link
Contributor

Description

This PR enhances the ZipkinTracingProvider in MeshConfig with two new configuration options to improve flexibility in communicating with Zipkin collectors:

Envoy changes: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/zipkin.proto#:~:text=maintain%20backward%20compatibility.-,collector_service,-(config.core

Changes

  1. Timeout (timeout)

    • Configures the timeout for HTTP requests to the Zipkin collector
    • Type: google.protobuf.Duration
    • Falls back to Envoy's default timeout if not specified
    • Field number: 7

  2. Custom Headers (headers)

    • Allows adding custom HTTP headers to requests sent to the Zipkin collector
    • Useful for authentication, authorization, or custom metadata
    • Supports both direct values and environment variable references via HttpHeader message
    • Field number: 8

Testing

Documentation

  • Release notes updated in releasenotes/notes/zipkin-tracing-enhancements.yaml
  • Field documentation added to proto file with clear descriptions

@prashanthjos
Add timeout and headers configuration to ZipkinTracingProvider
448e9bb 
- Add timeout field to configure HTTP request timeout to Zipkin collector
- Add headers field to allow custom HTTP headers in Zipkin requests

This enhancement provides more control over Zipkin collector communication,
enabling custom authentication, authorization, and metadata headers.
@prashanthjos prashanthjos requested a review from a team as a code owner October 18, 2025 01:44
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test labels Oct 18, 2025
@istio-testing
Copy link
Collaborator

Hi @prashanthjos. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

zirain
zirain reviewed Oct 18, 2025
View reviewed changes
@zirain
Copy link
Member

zirain commented Oct 18, 2025

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Oct 18, 2025
@prashanthjos
Add timeout and headers configuration to ZipkinTracingProvider
9343b05 
- Add timeout field to configure HTTP request timeout to Zipkin collector
- Add headers field to allow custom HTTP headers in Zipkin requests

This enhancement provides more control over Zipkin collector communication,
enabling custom authentication, authorization, and metadata headers.
@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 18, 2025
@zirain zirain added the release-notes-none Indicates a PR that does not require release notes. label Oct 18, 2025
@prashanthjos
Add timeout and headers configuration to ZipkinTracingProvider
8af3862 
- Add timeout field to configure HTTP request timeout to Zipkin collector
- Add headers field to allow custom HTTP headers in Zipkin requests

This enhancement provides more control over Zipkin collector communication,
enabling custom authentication, authorization, and metadata headers.# On branch master
@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 18, 2025
ramaraochavali
ramaraochavali reviewed Oct 20, 2025
View reviewed changes
Copy link
Contributor

@ramaraochavali ramaraochavali left a comment

Choose a reason for hiding this comment

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

Generally looks good. Couple of comments

mesh/v1alpha1/config.proto

// Optional. The timeout for the HTTP request to the Zipkin collector.
// If not specified, the default timeout from Envoy's configuration will be used.
google.protobuf.Duration timeout = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please specify what is the current Envoy default?

"the default timeout from Envoy's configuration will be used" -> "the default timeout of 5s(or whatever) will be used"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default timeout if 5seconds, it exists in the envoy source code here

const uint64_t timeout =
        runtime_.snapshot().getInteger("tracing.zipkin.request_timeout", 5000U);

Copy link
Contributor

@ramaraochavali ramaraochavali Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you update the protobuf comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated !

mesh/v1alpha1/config.proto

// Optional. Additional HTTP headers to include in the request to the Zipkin collector.
// These headers will be added to the HTTP request when sending spans to the collector.
repeated HttpHeader headers = 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if there is a strong use case to add this to Istio APIs now for two reasons

  • Increasing API surface area
  • This field is new in Envoy. So want to avoid version checks that we do for backward compatibility if possible

It may be generally useful but want to get other @istio/technical-oversight-committee's opinion before approving

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @ramaraochavali. Specifically, this field serves as a uniform mechanism for specifying headers, including existing fields such as collector_host. This field also allows adding Host, authentication or authorization cookies, and other custom headers, this is not possible today.

The intent is to harmonizing existing configuration and improving forward compatibility, creating a cleaner, more extensible interface for header management across telemetry, AuthN/AuthZ, and custom integrations.

@prashanthjos
Add timeout and headers configuration to ZipkinTracingProvider
e9ed508 
- Add timeout field to configure HTTP request timeout to Zipkin collector
- Add headers field to allow custom HTTP headers in Zipkin requests

This enhancement provides more control over Zipkin collector communication,
enabling custom authentication, authorization, and metadata headers.
@prashanthjos
Add timeout and headers configuration to ZipkinTracingProvider
4fd2d36 
- Add timeout field to configure HTTP request timeout to Zipkin collector
- Add headers field to allow custom HTTP headers in Zipkin requests

This enhancement provides more control over Zipkin collector communication,
enabling custom authentication, authorization, and metadata headers.
@prashanthjos
Copy link
Contributor Author

@istio/technical-oversight-committee can someone please take a look at this.

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

Reviewers

@ramaraochavali ramaraochavali ramaraochavali left review comments

@zirain zirain zirain approved these changes

@hzxuzhonghu hzxuzhonghu hzxuzhonghu approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. release-notes-none Indicates a PR that does not require release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@prashanthjos @istio-testing @zirain @hzxuzhonghu @ramaraochavali