-
Notifications
You must be signed in to change notification settings - Fork 34
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
Forward metadata in proxy #167
Conversation
|
||
// Copy incoming header to outgoing if not already present in outgoing. We | ||
// have confirmed in gRPC that incoming is a copy so we can mutate it. | ||
incoming, _ := metadata.FromIncomingContext(ctx) |
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.
Error check?
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.
This isn't an error, this is whether it was present, and we don't care really in our case. incoming
is a type wrapper of map[string][]string
and .Delete
is a wrapper around delete
. So a nil
incoming
is safe.
|
||
// Remove common headers and if there's nothing left, return early | ||
incoming.Delete("user-agent") | ||
incoming.Delete(":authority") |
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.
Is this supposed to have a colon?
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.
Yes, this is an HTTP/2 header
This should not be merged until after a Go SDK release is made with temporalio/sdk-go#1502. We did a type conversion in the Go SDK on options set here so we cannot release an API library version with this updated or SDK users will get compile errors. |
@cretz temporalio/sdk-go#1502 is included in |
Thanks! I will go ahead and merge this after CI passes. |
What changed?
Today headers set by client do not flow through the proxy. This will now forward them by default.