Override env for shadow request #756
Conversation
runtime/constants.go
Outdated
| @@ -51,6 +51,11 @@ const ( | |||
| clientHTTPUnmarshalError = "client.http-unmarshal-error" | |||
| // clientTchannelReadError is the metric for tracking errors in reading tchannel response | |||
| clientTchannelUnmarshalError = "client.tchannel-unmarshal-error" | |||
|
|
|||
| // shadow headers and environment | |||
| shadowHeader = "X-Uber-Shadow-Request" | |||
There was a problem hiding this comment.
Open source code cannot have uber specific anything. You should probably have a generic shadow header mechanism and set it to a specific value in the uber internal use-case
There was a problem hiding this comment.
Valid point. I have renamed it to a more generic shadow header "X-Shadow-Request"
runtime/server_http_request_test.go
Outdated
| var headers map[string]string | ||
| if isShadowRequest { | ||
| headers = map[string]string{ | ||
| "X-Uber-Shadow-Request": "true", |
There was a problem hiding this comment.
same deal as my other comment
| @@ -90,3 +90,5 @@ grpc.clientServiceNameMapping: | |||
| echo: echo | |||
| router.whitelistedPaths: | |||
| - /path/whitelisted | |||
| service.shadow.request.header: x-shadow-request | |||
There was a problem hiding this comment.
I think these configs should go in the gateway config rather than env based config, see another similar header config example here: https://github.com/uber/zanzibar/blob/master/examples/example-gateway/build.yaml#L20
There was a problem hiding this comment.
Hi @rpatali , Actually I took the reference of http.clients.requestUUIDHeaderKey: x-request-uuid and ""sidecarRouter.default.http.callerHeader": "RPC-Caller"" the way it implemented here (https://github.com/uber/zanzibar/blob/master/examples/example-gateway/config/production.yaml#L53,) but yes your point is valid that this is not environment specific and it should be present while gateway is bootstraping.
I tried to put the shadow header key in the build.yaml of example gateway but I unable to fetch it through static config.
There was a problem hiding this comment.
Request changes have been incorporated.
| @@ -90,3 +90,4 @@ grpc.clientServiceNameMapping: | |||
| echo: echo | |||
| router.whitelistedPaths: | |||
| - /path/whitelisted | |||
| service.shadow.env.override.enable: true | |||
There was a problem hiding this comment.
why is this needed? if you have shadowRequestHeader in build.yaml, you override it otherwise you don't.
There was a problem hiding this comment.
As discussed with Tejas, Zanzibar is used by multiple teams(edge-gateway and presentation), etc.
This shadow header can pass to downstream services as well but we don't want to enforce env override for them as well. So I enable it through config.
Overriding the environment value depending on the request type