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.

Sign up for GitHub

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

Jump to bottom

zapcore: Unflake TestSamplerConcurrent #1012

Merged
merged 2 commits into from
Sep 10, 2021
Merged

zapcore: Unflake TestSamplerConcurrent #1012

merged 2 commits into from
Sep 10, 2021

Commits on Sep 10, 2021

  1. Move controlledClock to ztest.MockClock 

    We need to be able to use the controlled clock for some other tests so
move it from clock_test to the ztest package and rename it to MockClock.

To keep the interface for the MockClock clear,
don't embed the benbjohnson/clock and instead,
use it as an attribute.
    @abhinav
    abhinav committed Sep 10, 2021
    Configuration menu
    Copy the full SHA
    fe9f608 View commit details
    Browse the repository at this point in the history

  2. zapcore: Address TestSamplerConcurrent flakiness 

    The TestSamplerConcurrent test frequently fails with the following error
in CI:

    --- FAIL: TestSamplerConcurrent (0.25s)
        sampler_test.go:198:
        	    Error Trace:	sampler_test.go:198
        	    Error:      	Max difference between 1250 and 1004 allowed is 125, but difference was 246
        	    Test:       	TestSamplerConcurrent
        	    Messages:   	Unexpected number of logs
    FAIL

The test is intended to verify that
despite an onsalught of messages from multiple goroutines,
we only allow at most `logsPerTick` messages per `tick`.

This was accompilshed by spin-looping 10 goroutines for `numTicks`,
each logging one of `numMessages` different messages,
and then verifying the final log count.

The source of flakiness here was the non-determinism in
how far a goroutine would get in logging enough messages such that
the sampler would be engaged.

In #948, we added a `zapcore.Clock` interface with a ticker and
a mock implementation.
Move that to `ztest` for use here.

To unflake the test, use the mock clock to control time and
for each goroutine, log `logsPerTick*2` messages `numTicks` times.
This gives us,

    for numGoroutines (10)
        for numTicks (25)
            log logsPerTick * 2 (50) messages

We end up attempting to log a total of,

    (numGoroutines * numTicks * logsPerTick * 2) messages
    = (10 * 25 * 50) messages
    = 12500 messages

Of these, the following should be sampled:

    numMessages * numTicks * logsPerTick
    = 5 * 10 * 25
    = 1250

Everything else should be dropped.

For extra confidence, use a SamplerHook (added in #813) to verify that
the number of sampled and dropped messages meet expectations.
    @abhinav
    abhinav committed Sep 10, 2021
    Configuration menu
    Copy the full SHA
    f0f2f30 View commit details
    Browse the repository at this point in the history