Skip to content
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

Override env for shadow request #756

Merged
merged 6 commits into from Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/example-gateway/build.yaml
Expand Up @@ -35,3 +35,4 @@ moduleSearchPaths:
defaultDependencies:
endpoint:
- middlewares/default/*
shadowRequestHeader: x-shadow-request
1 change: 1 addition & 0 deletions examples/example-gateway/config/production.yaml
Expand Up @@ -90,3 +90,4 @@ grpc.clientServiceNameMapping:
echo: echo
router.whitelistedPaths:
- /path/whitelisted
service.shadow.env.override.enable: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed? if you have shadowRequestHeader in build.yaml, you override it otherwise you don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

1 change: 1 addition & 0 deletions examples/example-gateway/config/test.yaml
Expand Up @@ -89,3 +89,4 @@ clients.baz.alternates:
port: 8114
grpc.clientServiceNameMapping:
echo: echo
service.shadow.env.override.enable: true
4 changes: 4 additions & 0 deletions runtime/constants.go
Expand Up @@ -51,6 +51,10 @@ 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
shadowEnvironment = "shadow"
environmentKey = "env"
)

var knownMetrics = []string{
Expand Down
2 changes: 2 additions & 0 deletions runtime/router.go
Expand Up @@ -89,6 +89,7 @@ type RouterEndpoint struct {
contextLogger ContextLogger
scope tally.Scope
tracer opentracing.Tracer
config *StaticConfig
}

// NewRouterEndpoint creates an endpoint that can be registered to HTTPRouter
Expand All @@ -108,6 +109,7 @@ func NewRouterEndpoint(
scope: deps.Scope,
tracer: deps.Tracer,
JSONWrapper: deps.JSONWrapper,
config: deps.Config,
}
}

Expand Down
13 changes: 13 additions & 0 deletions runtime/server_http_request.go
Expand Up @@ -88,6 +88,7 @@ func NewServerHTTPRequest(
}
if endpoint.contextExtractor != nil {
headers := map[string]string{}

for k, v := range r.Header {
// TODO: this 0th element logic is probably not correct
headers[k] = v[0]
Expand All @@ -99,6 +100,18 @@ func NewServerHTTPRequest(

logFields = append(logFields, endpoint.contextExtractor.ExtractLogFields(ctx)...)
}

// Overriding the environment for shadow requests
if endpoint.config != nil {
if endpoint.config.ContainsKey("service.shadow.env.override.enable") &&
endpoint.config.MustGetBoolean("service.shadow.env.override.enable") &&
endpoint.config.ContainsKey("shadowRequestHeader") &&
r.Header.Get(endpoint.config.MustGetString("shadowRequestHeader")) != "" {
scopeTags[environmentKey] = shadowEnvironment
logFields = append(logFields, zap.String(environmentKey, shadowEnvironment))
}
}

ctx = WithScopeTags(ctx, scopeTags)
ctx = WithLogFields(ctx, logFields...)

Expand Down
44 changes: 41 additions & 3 deletions runtime/server_http_request_test.go
Expand Up @@ -2238,7 +2238,7 @@ func TestSpanCreated(t *testing.T) {
assert.Equal(t, "200 OK", resp.Status)
}

func TestIncomingHTTPRequestServerLog(t *testing.T) {
func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, environment string) {
gateway, err := benchGateway.CreateGateway(
defaultTestConfig,
defaultTestOptions,
Expand All @@ -2256,6 +2256,7 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
Logger: bgateway.ActualGateway.Logger,
ContextLogger: bgateway.ActualGateway.ContextLogger,
Tracer: bgateway.ActualGateway.Tracer,
Config: bgateway.ActualGateway.Config,
}
err = bgateway.ActualGateway.HTTPRouter.Handle(
"GET", "/foo", http.HandlerFunc(zanzibar.NewRouterEndpoint(
Expand All @@ -2273,13 +2274,20 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
)
assert.NoError(t, err)

_, err = gateway.MakeRequest("GET", "/foo?bar=bar", nil, nil)
var headers map[string]string
if isShadowRequest {
headers = map[string]string{
"X-Shadow-Request": "true",
}
}
_, err = gateway.MakeRequest("GET", "/foo?bar=bar", headers, nil)
assert.NoError(t, err)

allLogs := bgateway.AllLogs()
assert.Equal(t, 1, len(allLogs["Finished an incoming server HTTP request with 200 status code"]))

tags := allLogs["Finished an incoming server HTTP request with 200 status code"][0]

dynamicHeaders := []string{
"requestUUID",
"remoteAddr",
Expand All @@ -2290,14 +2298,19 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
"pid",
"timestamp-finished",
}

if isShadowRequest {
dynamicHeaders = append(dynamicHeaders, "X-Shadow-Request")
}

for _, dynamicValue := range dynamicHeaders {
assert.Contains(t, tags, dynamicValue)
delete(tags, dynamicValue)
}

expectedValues := map[string]interface{}{
"msg": "Finished an incoming server HTTP request with 200 status code",
"env": "test",
"env": environment,
"level": "debug",
"zone": "unknown",
"service": "example-gateway",
Expand All @@ -2319,3 +2332,28 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
assert.Equal(t, expectedValue, tags[expectedKey], "unexpected header %q", expectedKey)
}
}

func TestIncomingHTTPRequestServerLogForDiffRequestTypes(t *testing.T) {
tests := []struct {
name string
isShadowRequest bool
expectedEnvironment string
}{
{
"Test incoming http request server log for normal request",
false,
"test",
},
{
"Test incoming http request server log for shadow request",
true,
"shadow",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
testIncomingHTTPRequestServerLog(t, tt.isShadowRequest, tt.expectedEnvironment)
})
}
}
4 changes: 4 additions & 0 deletions test/lib/util/util.go
Expand Up @@ -52,6 +52,10 @@ func DefaultConfigFiles(serviceName string) []string {
"..", "..", "..", "examples", "example-gateway", "config",
"example-gateway", "test.yaml",
),
filepath.Join(
getDirName(),
"..", "..", "..", "examples", "example-gateway", "build.yaml",
),
}
}

Expand Down