-
Notifications
You must be signed in to change notification settings - Fork 586
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.
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
Changes from all commits
448e9bb
9343b05
8af3862
e9ed508
4fd2d36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -821,6 +821,14 @@ message MeshConfig { | |
| // This controls both downstream request header extraction and upstream request header injection. | ||
| // The default value is USE_B3 to maintain backward compatibility. | ||
| TraceContextOption trace_context_option = 6; | ||
|
|
||
| // Optional. The timeout for the HTTP request to the Zipkin collector. | ||
| // If not specified, the default timeout from Envoy's configuration will be used (which is 5 seconds currently). | ||
| google.protobuf.Duration timeout = 7; | ||
|
|
||
| // 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; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
It may be generally useful but want to get other @istio/technical-oversight-committee's opinion before approving There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| } | ||
|
|
||
| // Defines configuration for a Lightstep tracer. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
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"
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.
The default timeout if 5seconds, it exists in the envoy source code here
Uh oh!
There was an error while loading. Please reload this page.
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.
Can you update the protobuf comment?
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.
updated !