router: inject tracing context in the upstream request#20503
router: inject tracing context in the upstream request#20503mattklein123 merged 2 commits intoenvoyproxy:mainfrom
Conversation
|
@wbpcode - please fix missing DCO |
Signed-off-by: wbpcode <wbphub@live.com>
| // Inject the active span's tracing context into the request headers. | ||
| callbacks_->activeSpan().injectContext(headers); |
There was a problem hiding this comment.
What are the implications of this with regard to retries and hedging? I'm concerned this a change in behavior here?
There was a problem hiding this comment.
If start_child_span is set to false, then we may do some repeated injectings when retries occur. This should be safe because in previous implementation, we may re-inject tracing context as well.
There was a problem hiding this comment.
In the #20367, we want to update the injectContext API to add a new parameter to provide target upstream of current upstream request. This PR can also complete some pre-work.
| // No independent child span for current upstream request then inject the parent span's tracing | ||
| // context into the request headers. |
There was a problem hiding this comment.
OK I understand now, thanks for explaining. Do you mind adding a few more comments here on the implications of doing this multiple times for retries, etc.? I understand this is the way it was always done, but it's a little more confusing now. Thank you!
/wait
Signed-off-by: wbpcode <wbphub@live.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
3 participants
Commit Message: router: inject tracing context in the upstream request
Additional Description:
Move tracing context injecting to upstream request. We can reduce some unnecessary injectings by this way. And in the future, we can provide some more upstream infos (for example, upstream host) to
injectContextcall.Risk Level: Low.
Testing: Unit.
Docs Changes: N/A.
Release Notes: N/A.
Platform Specific Features: N/A.