From fa3e0a296b1a1b57d394fd623ee33df84006986d Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Wed, 3 Mar 2021 09:20:32 +0530 Subject: [PATCH 1/6] Overriding env variable for the shadow requests --- runtime/constants.go | 5 ++++ runtime/server_http_request.go | 8 ++++++ runtime/server_http_request_test.go | 43 +++++++++++++++++++++++++++-- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/runtime/constants.go b/runtime/constants.go index 904cc115b..a25be4b5e 100644 --- a/runtime/constants.go +++ b/runtime/constants.go @@ -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" + shadowEnvironment = "shadow" + environmentKey = "env" ) var knownMetrics = []string{ diff --git a/runtime/server_http_request.go b/runtime/server_http_request.go index d3977e5aa..0dddd9497 100644 --- a/runtime/server_http_request.go +++ b/runtime/server_http_request.go @@ -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] @@ -99,6 +100,13 @@ func NewServerHTTPRequest( logFields = append(logFields, endpoint.contextExtractor.ExtractLogFields(ctx)...) } + + val := r.Header.Get(shadowHeader) + if val != "" { + scopeTags[environmentKey] = shadowEnvironment + logFields = append(logFields, zap.String(environmentKey, shadowEnvironment)) + } + ctx = WithScopeTags(ctx, scopeTags) ctx = WithLogFields(ctx, logFields...) diff --git a/runtime/server_http_request_test.go b/runtime/server_http_request_test.go index 468c5f557..21cf9cbd6 100644 --- a/runtime/server_http_request_test.go +++ b/runtime/server_http_request_test.go @@ -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, @@ -2273,13 +2273,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-Uber-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", @@ -2290,6 +2297,11 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) { "pid", "timestamp-finished", } + + if isShadowRequest { + dynamicHeaders = append(dynamicHeaders, "X-Uber-Shadow-Request") + } + for _, dynamicValue := range dynamicHeaders { assert.Contains(t, tags, dynamicValue) delete(tags, dynamicValue) @@ -2297,7 +2309,7 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) { 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", @@ -2319,3 +2331,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) + }) + } +} \ No newline at end of file From a9e07f9d82412dcce4e0a7a1b32d568bf5d4adb3 Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Wed, 3 Mar 2021 09:24:07 +0530 Subject: [PATCH 2/6] lint issues fixes --- runtime/server_http_request_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/runtime/server_http_request_test.go b/runtime/server_http_request_test.go index 21cf9cbd6..fa8ffdf15 100644 --- a/runtime/server_http_request_test.go +++ b/runtime/server_http_request_test.go @@ -2238,7 +2238,7 @@ func TestSpanCreated(t *testing.T) { assert.Equal(t, "200 OK", resp.Status) } -func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, environment string) { +func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, environment string) { gateway, err := benchGateway.CreateGateway( defaultTestConfig, defaultTestOptions, @@ -2276,7 +2276,7 @@ func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, enviro var headers map[string]string if isShadowRequest { headers = map[string]string{ - "X-Uber-Shadow-Request" : "true", + "X-Uber-Shadow-Request": "true", } } _, err = gateway.MakeRequest("GET", "/foo?bar=bar", headers, nil) @@ -2334,8 +2334,8 @@ func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, enviro func TestIncomingHTTPRequestServerLogForDiffRequestTypes(t *testing.T) { tests := []struct { - name string - isShadowRequest bool + name string + isShadowRequest bool expectedEnvironment string }{ { @@ -2355,4 +2355,4 @@ func TestIncomingHTTPRequestServerLogForDiffRequestTypes(t *testing.T) { testIncomingHTTPRequestServerLog(t, tt.isShadowRequest, tt.expectedEnvironment) }) } -} \ No newline at end of file +} From 5ca04c9f410e2680adf1b4eea2ac1a4529c93547 Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Wed, 3 Mar 2021 09:48:14 +0530 Subject: [PATCH 3/6] lint error fixes --- runtime/constants.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/constants.go b/runtime/constants.go index a25be4b5e..15131ab53 100644 --- a/runtime/constants.go +++ b/runtime/constants.go @@ -53,9 +53,9 @@ const ( clientTchannelUnmarshalError = "client.tchannel-unmarshal-error" // shadow headers and environment - shadowHeader = "X-Uber-Shadow-Request" - shadowEnvironment = "shadow" - environmentKey = "env" + shadowHeader = "X-Uber-Shadow-Request" + shadowEnvironment = "shadow" + environmentKey = "env" ) var knownMetrics = []string{ From 899031c6ea30d846f395f34293e6c93916d3d8f6 Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Thu, 4 Mar 2021 09:34:26 +0530 Subject: [PATCH 4/6] Updating shadow header name to a generic header name --- runtime/constants.go | 2 +- runtime/server_http_request_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/constants.go b/runtime/constants.go index 15131ab53..1f61f83d6 100644 --- a/runtime/constants.go +++ b/runtime/constants.go @@ -53,7 +53,7 @@ const ( clientTchannelUnmarshalError = "client.tchannel-unmarshal-error" // shadow headers and environment - shadowHeader = "X-Uber-Shadow-Request" + shadowHeader = "X-Shadow-Request" shadowEnvironment = "shadow" environmentKey = "env" ) diff --git a/runtime/server_http_request_test.go b/runtime/server_http_request_test.go index fa8ffdf15..b4481f48e 100644 --- a/runtime/server_http_request_test.go +++ b/runtime/server_http_request_test.go @@ -2276,7 +2276,7 @@ func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, enviro var headers map[string]string if isShadowRequest { headers = map[string]string{ - "X-Uber-Shadow-Request": "true", + "X-Shadow-Request": "true", } } _, err = gateway.MakeRequest("GET", "/foo?bar=bar", headers, nil) @@ -2299,7 +2299,7 @@ func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, enviro } if isShadowRequest { - dynamicHeaders = append(dynamicHeaders, "X-Uber-Shadow-Request") + dynamicHeaders = append(dynamicHeaders, "X-Shadow-Request") } for _, dynamicValue := range dynamicHeaders { From 0b49f0874a54ffd7a52c5c2b9d6f822e7704e0c1 Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Thu, 4 Mar 2021 12:11:49 +0530 Subject: [PATCH 5/6] Making overriding environment on config based --- examples/example-gateway/config/production.yaml | 2 ++ examples/example-gateway/config/test.yaml | 2 ++ runtime/constants.go | 1 - runtime/router.go | 2 ++ runtime/server_http_request.go | 10 ++++++---- runtime/server_http_request_test.go | 1 + 6 files changed, 13 insertions(+), 5 deletions(-) diff --git a/examples/example-gateway/config/production.yaml b/examples/example-gateway/config/production.yaml index 413caa0fc..5d1a7fb44 100644 --- a/examples/example-gateway/config/production.yaml +++ b/examples/example-gateway/config/production.yaml @@ -90,3 +90,5 @@ grpc.clientServiceNameMapping: echo: echo router.whitelistedPaths: - /path/whitelisted +service.shadow.request.header: x-shadow-request +service.shadow.env.override.enable: true \ No newline at end of file diff --git a/examples/example-gateway/config/test.yaml b/examples/example-gateway/config/test.yaml index 229056103..12d3996bf 100644 --- a/examples/example-gateway/config/test.yaml +++ b/examples/example-gateway/config/test.yaml @@ -89,3 +89,5 @@ clients.baz.alternates: port: 8114 grpc.clientServiceNameMapping: echo: echo +service.shadow.request.header: x-shadow-request +service.shadow.env.override.enable: true \ No newline at end of file diff --git a/runtime/constants.go b/runtime/constants.go index 1f61f83d6..e353ae505 100644 --- a/runtime/constants.go +++ b/runtime/constants.go @@ -53,7 +53,6 @@ const ( clientTchannelUnmarshalError = "client.tchannel-unmarshal-error" // shadow headers and environment - shadowHeader = "X-Shadow-Request" shadowEnvironment = "shadow" environmentKey = "env" ) diff --git a/runtime/router.go b/runtime/router.go index f79bea336..0913910b2 100644 --- a/runtime/router.go +++ b/runtime/router.go @@ -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 @@ -108,6 +109,7 @@ func NewRouterEndpoint( scope: deps.Scope, tracer: deps.Tracer, JSONWrapper: deps.JSONWrapper, + config: deps.Config, } } diff --git a/runtime/server_http_request.go b/runtime/server_http_request.go index 0dddd9497..46ba63a12 100644 --- a/runtime/server_http_request.go +++ b/runtime/server_http_request.go @@ -101,10 +101,12 @@ func NewServerHTTPRequest( logFields = append(logFields, endpoint.contextExtractor.ExtractLogFields(ctx)...) } - val := r.Header.Get(shadowHeader) - if val != "" { - scopeTags[environmentKey] = shadowEnvironment - logFields = append(logFields, zap.String(environmentKey, shadowEnvironment)) + // 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("service.shadow.request.header") && r.Header.Get(endpoint.config.MustGetString("service.shadow.request.header")) != "" { + scopeTags[environmentKey] = shadowEnvironment + logFields = append(logFields, zap.String(environmentKey, shadowEnvironment)) + } } ctx = WithScopeTags(ctx, scopeTags) diff --git a/runtime/server_http_request_test.go b/runtime/server_http_request_test.go index b4481f48e..9e9f69573 100644 --- a/runtime/server_http_request_test.go +++ b/runtime/server_http_request_test.go @@ -2256,6 +2256,7 @@ func testIncomingHTTPRequestServerLog(t *testing.T, isShadowRequest bool, enviro 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( From 20896d2d20081e567b9355cdc873f77143987db9 Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Fri, 5 Mar 2021 11:04:38 +0530 Subject: [PATCH 6/6] code review comments incorporation --- examples/example-gateway/build.yaml | 1 + examples/example-gateway/config/production.yaml | 1 - examples/example-gateway/config/test.yaml | 1 - runtime/server_http_request.go | 5 ++++- test/lib/util/util.go | 4 ++++ 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/examples/example-gateway/build.yaml b/examples/example-gateway/build.yaml index 7368fe601..ff32bcf19 100644 --- a/examples/example-gateway/build.yaml +++ b/examples/example-gateway/build.yaml @@ -35,3 +35,4 @@ moduleSearchPaths: defaultDependencies: endpoint: - middlewares/default/* +shadowRequestHeader: x-shadow-request diff --git a/examples/example-gateway/config/production.yaml b/examples/example-gateway/config/production.yaml index 5d1a7fb44..673836d62 100644 --- a/examples/example-gateway/config/production.yaml +++ b/examples/example-gateway/config/production.yaml @@ -90,5 +90,4 @@ grpc.clientServiceNameMapping: echo: echo router.whitelistedPaths: - /path/whitelisted -service.shadow.request.header: x-shadow-request service.shadow.env.override.enable: true \ No newline at end of file diff --git a/examples/example-gateway/config/test.yaml b/examples/example-gateway/config/test.yaml index 12d3996bf..df818472f 100644 --- a/examples/example-gateway/config/test.yaml +++ b/examples/example-gateway/config/test.yaml @@ -89,5 +89,4 @@ clients.baz.alternates: port: 8114 grpc.clientServiceNameMapping: echo: echo -service.shadow.request.header: x-shadow-request service.shadow.env.override.enable: true \ No newline at end of file diff --git a/runtime/server_http_request.go b/runtime/server_http_request.go index 46ba63a12..b4c2a232e 100644 --- a/runtime/server_http_request.go +++ b/runtime/server_http_request.go @@ -103,7 +103,10 @@ func NewServerHTTPRequest( // 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("service.shadow.request.header") && r.Header.Get(endpoint.config.MustGetString("service.shadow.request.header")) != "" { + 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)) } diff --git a/test/lib/util/util.go b/test/lib/util/util.go index 77b56c323..cc46414da 100644 --- a/test/lib/util/util.go +++ b/test/lib/util/util.go @@ -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", + ), } }