acceptance: de-flake test_missing_log_output #19402
Conversation
|
This change is |
|
@knz I'm having trouble running this test locally for unrelated reasons (slow bandwidth and no builder image present), but I'll loop it a few times in CI to see that it really does fix the problem. |
| # Note that grpc sends WARNING messages which we accept in this | ||
| # test for now. | ||
| # See, for example, https://github.com/cockroachdb/cockroach/issues/19397. | ||
| send "echo marker; $argv quit 2>&1 | grep '^\\(\[IEF\]\[0-9\]\\)' \r" |
There was a problem hiding this comment.
This is probably a bit too permissive now - we're just interested in permitting the one grpc warning message (rather than all warning messages). How about ... | grep -vE '^W[0-9]+ grpc:' | grep '^[IWEF][0-9]' (note that I've also removed the capture groups, which appear to be useless here.
tamird
force-pushed
the
tcl-missing-log
branch
from
09ce934 to
86abdce
Compare
|
Per our discussion, updated this for you. |
tamird
force-pushed
the
tcl-missing-log
branch
from
86abdce to
db95339
Compare
tamird
force-pushed
the
tcl-missing-log
branch
from
db95339 to
d0e58d2
Compare
| # interested in the absence of INFO messages, remove the warnings from | ||
| # the output. | ||
| send "echo marker; $argv quit --logtostderr 2>&1 | grep -v '^W'\r" | ||
| send "echo marker; $argv quit --logtostderr 2>&1 | grep -vE '^\[IWEF\]\[0-9\]+ .\* grpc: '\r" |
There was a problem hiding this comment.
The test is supposed to check no info messages are printed because the default logging level should be WARNING. The grep should not remove them.
There was a problem hiding this comment.
LGTM, though if you're eating CI anyway, you may want to add the type of message we're special casing in the commit msg:
W171020 12:24:14.884065 21 vendor/google.golang.org/grpc/clientconn.go:1034 grpc: addrConn.resetTransport failed to create client transport: ...
| # interested in the absence of INFO messages, remove the warnings from | ||
| # the output. | ||
| send "echo marker; $argv quit --logtostderr 2>&1 | grep -v '^W'\r" | ||
| send "echo marker; $argv quit --logtostderr 2>&1 | grep -vE '^\[IWEF\]\[0-9\]+ .\* grpc: '\r" |
tamird
force-pushed
the
tcl-missing-log
branch
from
d0e58d2 to
db23363
Compare
|
LGTM. Since it's now basically your PR, I'd say it's between you and @knz to merge. Thanks! |
tamird
force-pushed
the
tcl-missing-log
branch
from
db23363 to
7130cb9
Compare
|
Reviewed 1 of 1 files at r3. Comments from Reviewable |
tamird
force-pushed
the
tcl-missing-log
branch
2 times, most recently
from
6da8a73 to
3566f7b
Compare
...by filtering out all GRPC log messages. The specific offender is https://github.com/grpc/grpc-go/blob/v1.7.0/clientconn.go#L937; this is the same error message that we throttle in the grpcutil log bridge. It is a warning that reads: > grpc: addrConn.resetTransport failed to create client transport: ... Turns out there are other offenders, so we just filter out any log messages that originates from GRPC. Another example is logged from https://github.com/grpc/grpc-go/blob/v1.7.0/clientconn.go#L696 and reads: > Failed to dial ...: ...; please retry. Fixes cockroachdb#19397. Fixes cockroachdb#19388.
tamird
force-pushed
the
tcl-missing-log
branch
from
3566f7b to
7dae7e6
Compare
Fixes #19397.
Fixes #19388.