Retrieve tracing header from request headers during report

[bianpengyuan]

Fixes istio/istio#13391

Even though tracing happens before all filters inside envoy (https://github.com/envoyproxy/envoy/blob/master/source/common/http/conn_manager_impl.cc#L735), the tracing header injection actually happens at envoy.router filter (https://github.com/envoyproxy/envoy/blob/master/source/common/router/router.cc#L1218).

The current behavior of mixer filter is that it extracts request headers once at decodeHeaders callback during check time. However request header is not yet ready by that time. This PR extracts b3 tracing headers at report time and add them into request.header attribute.

[bianpengyuan]

@kyessenov @JimmyCYJ ptal thanks!

[kyessenov]

I think we still need an integration test in istio.

[mandarjog]

/lgtm

integration test it is.

[istio-testing]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bianpengyuan, mandarjog
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rshriram

If they are not already assigned, you can assign the PR to them by writing /assign @rshriram in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[bianpengyuan]

@duderino Could you help to merge this? This is a bug fix for mixer tracing adapter due to a behavior change in envoy. Mixer adapter relies on b3 headers to instrument spans. Mixer client assumes request headers are all ready at check time so it only processes request header once. However envoy injects those tracing headers at envoy.router filter, which causes mixer filter missing tracing headers, or mistakenly using b3 headers for downstream span to instrument upstream span. This PR adds functionality to extract tracing headers at report time, an integration test has already been added. I will add another test in istio/istio with sha update. Thanks!

[bianpengyuan]

@duderino integration test is in a pending PR now: istio/istio#13671

[bianpengyuan]

@duderino gentle ping. Let me know if this should be merged to master.

[kyessenov]

@bianpengyuan I think we need to manually cherry pick this PR to master and link both PRs to the underlying issue.

[douglas-reid]

Can we merge this PR?

[silencehe09]

Can we merge this PR?

When will this PR be released ?

[douglas-reid]

@duderino many thanks!

@silencehe09 hopefully a subsequent 1.1.X series of releases will include this change.

@bianpengyuan can you track the use of this in istio/istio and merge up into master ?

[bianpengyuan]

@douglas-reid istio/istio#13671 pick this change into istio/istio and #2197 picks the change into istio/proxy master.

[zhongfox]

hi @douglas-reid, seems we encounter the same issue described in issue
istio/istio#13391, we are using istio 1.1.7

when and which istio version may merge this pr?
thanks.

[douglas-reid]

@fpesce @duderino can you provide any insights on 1.1.X and 1.2 release plans that would include this PR ?

[bianpengyuan]

@douglas-reid @duderino @fpesce The change is already in 1.2. Question is do we still want to cherry pick this into 1.1 and do a 1.1.8 release or we should just wait for 1.2.