Tracing works with Broker and Trigger#936
Conversation
In a follow up PR, I plan to include matching of expressions. It turns our that they are somewhat more involved than expected.
knative-prow-robot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Harwayne The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
|
/assign @vaikas-google |
|
The following is the coverage report on pkg/.
|
| "x-request-id", | ||
| ) | ||
| // These MUST be lowercase strings, as they will be compared against lowercase strings. | ||
| forwardPrefixes = []string{ |
There was a problem hiding this comment.
use sets or an array, not both? why both?
There was a problem hiding this comment.
I use a set for the first one so that I can easily do forwardHeaders.Has(n). The second, I iterate over, so slice and set are the same to me.
| for _, prefix := range forwardPrefixes { | ||
| if strings.HasPrefix(lower, prefix) { | ||
| ctx = addHeader(ctx, n, v) | ||
| break |
There was a problem hiding this comment.
Seems like this will only add one header?
There was a problem hiding this comment.
This breaks out of the 'range over forwardPrefixes' for loop, not the 'range over all headers from the context' for loop.
|
/lgtm |
8 participants
Proposed Changes
Release Note