Introduce a perf testing framework for Mixer. #1931

ozevren · 2017-11-30T01:13:37Z

What this PR does / why we need it:
This PR introduces a new perf testing framework for Mixer. A substantial documentation can be found in the package documentation in setup.go.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

I am sending this out a bit early: it is going to conflict with some of the PRs that I know to be in-flight. I'll incorporate those changes when they land, before checking this PR in.

Also, this PR does not include an actual perf test (just a proof of concept one). I'll try to come up with a basic test separately, and submit it in a separate PR.

Release note:

NONE

istio-merge-robot · 2017-11-30T01:14:21Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: apicl

Assign the PR to them by writing /assign @apicl in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

mixer/OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

codecov · 2017-11-30T16:51:21Z

Codecov Report

Merging #1931 into master will decrease coverage by 2.04%.
The diff coverage is 66.9%.

@@            Coverage Diff             @@
##           master    #1931      +/-   ##
==========================================
- Coverage   83.87%   81.83%   -2.05%     
==========================================
  Files         145      218      +73     
  Lines       10095    17388    +7293     
==========================================
+ Hits         8467    14229    +5762     
- Misses       1387     2650    +1263     
- Partials      241      509     +268

Flag	Coverage Δ
#broker	`47.27% <ø> (+2.82%)`	⬆️
#mixer	`83.15% <66.9%> (-1.68%)`	⬇️
#pilot	`80.54% <ø> (?)`
#security	`88.88% <ø> (+1.64%)`	⬆️

Impacted Files	Coverage Δ
mixer/pkg/perf/setup.go	`100% <100%> (ø)`
mixer/pkg/perf/run.go	`21.42% <21.42%> (ø)`
mixer/pkg/mock/server.go	`89.18% <33.33%> (ø)`
mixer/pkg/perf/env.go	`33.33% <33.33%> (ø)`
mixer/pkg/perf/server.go	`47.54% <47.54%> (ø)`
mixer/pkg/perf/common.go	`47.61% <47.61%> (ø)`
mixer/pkg/mock/client.go	`60% <60%> (ø)`
mixer/pkg/perf/client.go	`74.19% <74.19%> (ø)`
mixer/pkg/perf/controller.go	`75.38% <75.38%> (ø)`
mixer/pkg/perf/clientserver.go	`87.14% <87.14%> (ø)`
... and 112 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96cba10...45523ab. Read the comment docs.

geeknoid · 2017-12-01T17:16:28Z

mixer/pkg/perf/client.go

+
+// shutdown indicates that the client should perform graceful shutdown and cleanup of resources and get ready
+// to exit.
+func (c *client) shutdown() {


Consider calling this Close and passing back the return value of c.conn.Close()

geeknoid · 2017-12-01T17:17:54Z

mixer/pkg/perf/client.go

+		for _, r := range requests {
+			switch r.(type) {
+			case *mixerpb.ReportRequest:
+				c.mixer.Report(context.Background(), r.(*mixerpb.ReportRequest))


I think we need to track error codes here. Otherwise, some misconfiguration or other problem could lead to totally incorrect perf measurements.

geeknoid · 2017-12-01T17:19:36Z

mixer/pkg/perf/clientserver.go

+	"net/rpc"
+)
+
+// ClientServer is an RPC rpcServer that the Controller connects to remotely control a Mixer perf test client.


An "RPC rpcServer"? Feels like a search-and-replace error...

geeknoid · 2017-12-01T17:20:41Z

mixer/pkg/perf/clientserver.go

+	}
+
+	err := server.initializeRPCServer()
+	if err != nil {


Combine with previous line.

geeknoid · 2017-12-01T17:20:56Z

mixer/pkg/perf/clientserver.go

+	}
+
+	err = server.registerWithController(controllerLoc)
+	if err != nil {


Combine with previous line.

geeknoid · 2017-12-01T18:13:23Z

mixer/pkg/perf/server.go

+	}
+
+	err = write(path.Join(dir, "srvc.yaml"), []byte(setup.Config.Service))
+	if err != nil {


Combine with previous line.

geeknoid · 2017-12-01T18:13:55Z

mixer/pkg/perf/server.go

+	dir := path.Join(os.TempDir(), discriminator)
+
+	err := os.MkdirAll(dir, os.ModePerm)
+	if err != nil {


Combine with previous line.

Failure paths below should clean up this directory.

I started out that way, but later decided against it to help debuggability.

geeknoid · 2017-12-01T18:14:33Z

mixer/pkg/perf/server.go

+	}
+
+	_, err = f.Write(bytes)
+	if err != nil {


Combine with previous line.

geeknoid · 2017-12-01T18:15:56Z

mixer/pkg/perf/setup.go

+// seamlessly with the Go perf testing infrastructure.
+//
+// The entry point to the tests is the "Run*" methods. The benchmarks are expected to call one of the methods, passing in
+// testing.B, the test declaration (i.e. Setup struct), an environment variable that contains the ambient template,


Might be worthwhile to express "environment variable" differently, since it's not OS env vars, right?

geeknoid · 2017-12-01T18:16:18Z

mixer/pkg/perf/setup.go

+// adapter info (i.e. Env struct), along with any other Run-method specific inputs.
+//
+// The top-level struct for test declaration is the Setup struct. This contains two major fields, namely Config and Load.
+// Config field contains the full configuration needed for a Mixer Server, including global and service config Yaml files,


The Config field

ldemailly · 2017-12-04T19:30:18Z

subscribe

guptasu

Looks good. Minor comments.

guptasu · 2017-12-04T18:46:49Z

mixer/pkg/mock/client.go

+)
+
+// NewClient returns a Mixer Grpc client.
+func NewClient(address string) (mixerpb.MixerClient, *grpc.ClientConn, error) {


nit: address -> mixsAddr

guptasu · 2017-12-04T18:53:27Z

mixer/pkg/perf/client.go

+
+// initialize is the first method to be called. The client is expected to perform initialization by setting up
+// any local state using setup, and connecting to the Mixer rpc server at the given address.
+func (c *client) initialize(address string, setup *Setup) error {


The content of this func suggest that we can simply rename this func to func NewClient(mixsAddr string, setup *Setup) (*client, err) {...}

Not sure what you mean by that? Initialize matches to the incoming initialize call from the controller. I think keeping them aligned makes sense.

The current model is to first construct using client{}, then initialize and then run, which can be compressed into just two steps of NewClient and Run

guptasu · 2017-12-04T18:56:01Z

mixer/pkg/perf/client.go

+}
+
+// run indicates that the client should execute the load against the Mixer rpc server multiple times,
+// as indicated by the iterations parameter.


nit: the comment is not adding much value, IMO

guptasu · 2017-12-04T19:12:03Z

mixer/pkg/perf/client.go

+// run indicates that the client should execute the load against the Mixer rpc server multiple times,
+// as indicated by the iterations parameter.
+func (c *client) run(iterations int) error {
+	requests := c.setup.Load.createRequestProtos(c.setup.Config)


Consider passing only the thing that the client needs. the Config has a lot of stuff that is meant for Server. Consider making a another ClientConfig struct and converting the Setup into into what is relevant for the client.

I think that is too costly. Having a single, format keeps things consistent and avoid excessive refactoring cost when we need to change the main data structure (i.e. Setup).

guptasu · 2017-12-04T19:22:07Z

mixer/pkg/perf/clientserver.go

+	return ServiceLocation{Address: s.listener.Addr().String(), Path: s.rpcPath}
+}
+
+func (s *ClientServer) close() {


Do we need to call shutdown on the underlying Mixer client as well ?

Good catch, thanks.

guptasu · 2017-12-04T20:03:04Z

mixer/pkg/perf/controller_test.go

+		t.Fatalf("Error creating controller: %v", err)
+	}
+
+	s, err := NewClientServer(c.location())


It will be awesome if the controller takes a config of all the Clients and just inits it for me and waits etc...
From users point of view, this model has two ways to trigger runClient, one by invoking runClients on the controller and other by directly executing the Run method on the NewClientServer object.

Controller is meant to be internal only. The only reason it is "public" is because the RPC framework requires it. The only supported way to execute things is to call the "Run*" methods.

guptasu · 2017-12-04T20:04:51Z

mixer/pkg/perf/controller_test.go

+		t.Fatalf("Error creating controller: %v", err)
+	}
+
+	s, err := NewClientServer(c.location())


For now, should we just add a function on the controller, "AddClient(name string) and let the controller manager the wait etc for me and I don't need to know there exists a NewClientServer

Currently, such logic is located in the "Run" methods. In the future, we can refactor and make the Controller to be a bit more logic-heavy, with figuring out how to launch the clients themselves as well. However, I want to avoid checking a bare minimum infrastructure first, and make improvements on it later.

guptasu · 2017-12-04T20:06:10Z

mixer/pkg/perf/load.go

+}
+
+// MarshalJSON marshal the load as JSON.
+func (l *Load) MarshalJSON() ([]byte, error) {


Consider renaming to Marshal and Unmarshal. Json is just implementation detail

These are interface methods, used by the JSON marshaller. I can't change them.

guptasu · 2017-12-04T20:16:10Z

mixer/test/perf/perfclient/main.go

+
+func main() {
+	if len(os.Args) < 3 {
+		fmt.Printf("Args: <config_path>, <server_address>\n")


controller_address

guptasu · 2017-12-04T20:16:23Z

mixer/test/perf/perfclient/main.go

+
+func main() {
+	if len(os.Args) < 3 {
+		fmt.Printf("Args: <config_path>, <server_address>\n")


controller_rpc_path ?

ozevren

PTAL

istio-merge-robot · 2017-12-09T08:00:58Z

@ozevren PR needs rebase

geeknoid · 2017-12-11T19:10:53Z

It's very hard to track, given GitHub's poor code review system, but I think there are comments from my original review which haven't yet been addressed.

This is a big PR...

ozevren · 2017-12-11T19:12:51Z

Still working on the cleanup for rebase. But most of your comments should be addressed. There are a couple of places where I didn't do the "if err = ..." as it would have yielded more complicated code. If you feel strongly about it I can add them as well. :)

…

On Mon, Dec 11, 2017 at 11:11 AM, Martin Taillefer ***@***.*** > wrote: It's very hard to track, given GitHub's poor code review system, but I think there are comments from my original review which haven't yet been addressed. This is a big PR... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1931 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQR8I2MsayN30g87GvhLL6V9fdUwqaOIks5s_X5JgaJpZM4Qv4O5> .

geeknoid · 2017-12-11T19:17:02Z

I know you commented that: x.y, err = func() if err != nil { Would be more wordy as: if x.y, err = func(); err != nil { Isn't that shorter? Note, this is = and not := On Mon, Dec 11, 2017 at 11:13 AM, Ozben Evren <notifications@github.com> wrote:

…

Still working on the cleanup for rebase. But most of your comments should be addressed. There are a couple of places where I didn't do the "if err = ..." as it would have yielded more complicated code. If you feel strongly about it I can add them as well. :) On Mon, Dec 11, 2017 at 11:11 AM, Martin Taillefer < ***@***.*** > wrote: > It's very hard to track, given GitHub's poor code review system, but I > think there are comments from my original review which haven't yet been > addressed. > > This is a big PR... > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1931 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ AQR8I2MsayN30g87GvhLL6V9fdUwqaOIks5s_X5JgaJpZM4Qv4O5> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1931 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVucHaciDZfGYCLipY_zs-L5tXwsRPdYks5s_X7AgaJpZM4Qv4O5> .

ozevren · 2017-12-11T19:24:18Z

It is about having to hoist var declarations out. if the variable is being declared within that line, it will only be available within the if-block, which is useless. Either I have to flip the condition and rewrite the code, or do explicit variable declaration. The trade-off seems wrong. I am not a fan of the single-line if, but I don't feel strongly about it. I'll rescan and make it uniform. On Mon, Dec 11, 2017 at 11:17 AM, Martin Taillefer <notifications@github.com

…

wrote: I know you commented that: x.y, err = func() if err != nil { Would be more wordy as: if x.y, err = func(); err != nil { Isn't that shorter? Note, this is = and not := On Mon, Dec 11, 2017 at 11:13 AM, Ozben Evren ***@***.***> wrote: > Still working on the cleanup for rebase. But most of your comments should > be addressed. There are a couple of places where I didn't do the "if err = > ..." as it would have yielded more complicated code. If you feel strongly > about it I can add them as well. :) > > On Mon, Dec 11, 2017 at 11:11 AM, Martin Taillefer < > ***@***.*** > > wrote: > > > It's very hard to track, given GitHub's poor code review system, but I > > think there are comments from my original review which haven't yet been > > addressed. > > > > This is a big PR... > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > <#1931 (comment)>, or > mute > > the thread > > <https://github.com/notifications/unsubscribe-auth/ > AQR8I2MsayN30g87GvhLL6V9fdUwqaOIks5s_X5JgaJpZM4Qv4O5> > > . > > > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1931 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe- auth/AVucHaciDZfGYCLipY_zs-L5tXwsRPdYks5s_X7AgaJpZM4Qv4O5> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1931 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQR8I7jHzaaBVZm7PDSKNF4FviRu2-ZFks5s_X_GgaJpZM4Qv4O5> .

geeknoid · 2017-12-11T20:36:30Z

I specifically pointed out my comments are about lines with "=" in it, not with ":=". That means no symbols are being introduced by such a line and so a single-line "if" should be universally usable without changing any variable scopes. On Mon, Dec 11, 2017 at 11:24 AM, Ozben Evren <notifications@github.com> wrote:

…

It is about having to hoist var declarations out. if the variable is being declared within that line, it will only be available within the if-block, which is useless. Either I have to flip the condition and rewrite the code, or do explicit variable declaration. The trade-off seems wrong. I am not a fan of the single-line if, but I don't feel strongly about it. I'll rescan and make it uniform. On Mon, Dec 11, 2017 at 11:17 AM, Martin Taillefer < ***@***.*** > wrote: > I know you commented that: > > x.y, err = func() > if err != nil { > > Would be more wordy as: > > if x.y, err = func(); err != nil { > > Isn't that shorter? > > Note, this is = and not := > > On Mon, Dec 11, 2017 at 11:13 AM, Ozben Evren ***@***.***> > wrote: > > > Still working on the cleanup for rebase. But most of your comments should > > be addressed. There are a couple of places where I didn't do the "if err > = > > ..." as it would have yielded more complicated code. If you feel strongly > > about it I can add them as well. :) > > > > On Mon, Dec 11, 2017 at 11:11 AM, Martin Taillefer < > > ***@***.*** > > > wrote: > > > > > It's very hard to track, given GitHub's poor code review system, but I > > > think there are comments from my original review which haven't yet been > > > addressed. > > > > > > This is a big PR... > > > > > > — > > > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > > > <#1931 (comment)>, or > > mute > > > the thread > > > <https://github.com/notifications/unsubscribe-auth/ > > AQR8I2MsayN30g87GvhLL6V9fdUwqaOIks5s_X5JgaJpZM4Qv4O5> > > > . > > > > > > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > <#1931 (comment)>, or > mute > > the thread > > <https://github.com/notifications/unsubscribe- > auth/AVucHaciDZfGYCLipY_zs-L5tXwsRPdYks5s_X7AgaJpZM4Qv4O5> > > > . > > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1931 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ AQR8I7jHzaaBVZm7PDSKNF4FviRu2-ZFks5s_X_GgaJpZM4Qv4O5> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1931 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVucHUP4QrpoxzgLugabQSdhUVDatH3Dks5s_YFqgaJpZM4Qv4O5> .

ozevren · 2017-12-12T18:25:50Z

Any more comments on this? If not, I'd like to check this in.

guptasu · 2017-12-12T18:35:10Z

I had LGTM'd before, if there has not been any major changes since then, I am good. Thanks, -Sunny

…

On Tue, Dec 12, 2017 at 10:26 AM, Ozben Evren ***@***.***> wrote: Any more comments on this? If not, I'd like to check this in. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1931 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AK6xE6I569KDB_QRcGTY4nBQ1l3TKSw2ks5s_sU6gaJpZM4Qv4O5> .

-- *Thanks,* *-SG*

ozevren · 2017-12-12T22:47:59Z

/retest

This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: c9d2230 Update Envoy SHA to latest with MetricImpl optimizations. (istio#1938) a32a587 Add a check cache test for string map sub keys (istio#1931) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (istio#4190) 36809d80a fuzz: coverage profile generation instructions. (istio#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (istio#4075) 05c0d52d3 build: use clang-6.0. (istio#4168) 01f403ec4 thrift_proxy: introduce header transport (istio#4082) 564d256fb tcp: allow connection pool callers to store protocol state (istio#4131) 3e1d643b9 configs: match latest API changes (istio#4185) bc6a10c2f Fix wrong mock function name. (istio#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (istio#4182) 3d1325e89 Converting envoy configs to V2 (istio#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (istio#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (istio#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (istio#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (istio#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (istio#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (istio#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (istio#4157) 71152b710 ratelimit: Add ratelimit custom response headers (istio#4015) 306287418 ssl: make Ssl::Connection const everywhere (istio#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (istio#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (istio#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (istio#4130) 8c189a552 Make over provisioning factor configurable (istio#4003) 6c08fb43c Making test less flaky (istio#4149) 592775b7b fuzz: bare bones HCM fuzzer. (istio#4118) For istio#7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/istio/proxy: c9d2230 Update Envoy SHA to latest with MetricImpl optimizations. (#1938) a32a587 Add a check cache test for string map sub keys (#1931) Pulling the following changes from github.com/envoyproxy/envoy: c1cc68dda stats: refactoring MetricImpl without strings (#4190) 36809d80a fuzz: coverage profile generation instructions. (#4193) ba40cc933 upstream: rebuild cluster when health check config is changed (#4075) 05c0d52d3 build: use clang-6.0. (#4168) 01f403ec4 thrift_proxy: introduce header transport (#4082) 564d256fb tcp: allow connection pool callers to store protocol state (#4131) 3e1d643b9 configs: match latest API changes (#4185) bc6a10c2f Fix wrong mock function name. (#4187) e994c1c0b Bump yaml-cpp so it builds with Visual Studio 15.8 (#4182) 3d1325e89 Converting envoy configs to V2 (#2957) 8d0680feb Add timestamp to HealthCheckEvent definition (#4119) 497efb95b server: handle non-EnvoyExceptions safely if thrown in constructor. (#4173) 6d6fafdb3 config: strengthen validation for gRPC config sources. (#4171) 132302caf fuzz: reduce log level when running under fuzz engine. (#4161) 7c04ac255 test: fix V6EmptyOptions in coverage with IPv6 enable (#4155) 1b2219bd7 ci: remove deprecated bazel --batch option (#4166) 2db6a4ce1 ci: update clang to version 6.0 in the Ubuntu build image. (#4157) 71152b710 ratelimit: Add ratelimit custom response headers (#4015) 306287418 ssl: make Ssl::Connection const everywhere (#4179) 706e26238 Handle validation of non expiring tokens in jwt_authn filter (#4007) f06e9588a fuzz: tag trivial fuzzers with no_fuzz. (#4156) 27fb1d353 thrift_proxy: add service name matching to router implementation (#4130) 8c189a552 Make over provisioning factor configurable (#4003) 6c08fb43c Making test less flaky (#4149) 592775b7b fuzz: bare bones HCM fuzzer. (#4118) For #7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

istio-testing added the release-note-none label Nov 30, 2017

googlebot added the cla: yes label Nov 30, 2017

ozevren added the do-not-merge Block automatic merging of a PR. label Nov 30, 2017

ozevren requested review from geeknoid and guptasu November 30, 2017 01:15

istio-merge-robot added do-not-merge/post-submit labels Nov 30, 2017

istio-merge-robot added do-not-merge/post-submit and removed do-not-merge/post-submit labels Nov 30, 2017

geeknoid reviewed Dec 1, 2017

View reviewed changes

istio-merge-robot added do-not-merge/post-submit and removed do-not-merge/post-submit labels Dec 1, 2017

guptasu reviewed Dec 4, 2017

View reviewed changes

istio-merge-robot added do-not-merge/post-submit PostSubmit Failed/Contact Oncall and removed do-not-merge/post-submit PostSubmit Failed/Contact Oncall labels Dec 4, 2017

istio-merge-robot added PostSubmit Failed/Contact Oncall and removed PostSubmit Failed/Contact Oncall labels Dec 7, 2017

ozevren commented Dec 7, 2017

View reviewed changes

istio-merge-robot added needs-rebase Indicates a PR needs to be rebased before being merged and removed PostSubmit Failed/Contact Oncall labels Dec 7, 2017

ozben added 6 commits December 11, 2017 12:34

Complete the framework

8da01e8

Fix CR comments.

4f9bf26

Fixing CR comments.

ed3cc0c

Accomodate Cr feedback #2.

eb6f1bb

Fix rebase issues.

c9f8cb7

CR feedback.

45523ab

istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Dec 11, 2017

ozevren removed the do-not-merge Block automatic merging of a PR. label Dec 11, 2017

ozevren merged commit 20960d8 into istio:master Dec 12, 2017

ozevren deleted the perf branch December 12, 2017 23:41

PiotrSikora mentioned this pull request Aug 23, 2018

Update Proxy SHA to latest with MetricImpl optimizations. #8160

Merged

Introduce a perf testing framework for Mixer. #1931

Introduce a perf testing framework for Mixer. #1931

Conversation

ozevren commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-merge-robot commented Nov 30, 2017

Uh oh!

codecov bot commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldemailly commented Dec 4, 2017

Uh oh!

guptasu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ozevren left a comment

Choose a reason for hiding this comment

Uh oh!

istio-merge-robot commented Dec 9, 2017

Uh oh!

ozevren commented Nov 30, 2017 •

edited

Loading

codecov bot commented Nov 30, 2017 •

edited

Loading