diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 974c1ac..867bde4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,13 +16,15 @@ jobs: strategy: matrix: runs-on: [ macos-15, macos-14, macos-13, ubuntu-22.04, ubuntu-20.04, windows-latest, arm-4core-linux ] - go-version: [ "1.23", "1.22" ] + go-version: [ "1.24", "1.23" ] cgo_enabled: [ "0", "1" ] # test it compiles with and without cgo fail-fast: false runs-on: ${{ matrix.runs-on }} steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0 + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Setup Go + uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0 with: go-version: ${{ matrix.go-version }} cache: true @@ -34,13 +36,22 @@ jobs: key: go-pkg-mod-${{ hashFiles('**/go.sum') }} restore-keys: go-pkg-mod- + # Install build-essential on ARM linux runners if CGO is to be enabled + - name: Install build-essential (ARM linux) + if: matrix.runs-on == 'arm-4core-linux' && matrix.cgo_enabled == '1' + run: |- + sudo apt update + sudo apt install -y build-essential + - name: go test shell: bash run: | # Install gotestsum env GOBIN=$PWD go install gotest.tools/gotestsum@latest # Run the tests with gotestsum - env CGO_ENABLED=${{ matrix.cgo_enabled }} ./gotestsum -- -v ./... || true + ./gotestsum -- -v ${{ runner.os == 'Linux' && matrix.cgo_enabled == '1' && '-race' || '' }} ./... + env: + CGO_ENABLED: ${{ matrix.cgo_enabled }} # Same tests but on the official golang container for linux golang-linux-container: @@ -50,14 +61,16 @@ jobs: image: golang:${{ matrix.go-version }}-${{ matrix.distribution }} strategy: matrix: - go-version: [ "1.23", "1.22" ] + go-version: [ "1.24", "1.23" ] distribution: [ bookworm, bullseye, alpine ] cgo_enabled: [ "0", "1" ] # test it compiles with and without cgo fail-fast: false steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 # Install gcc and the libc headers on alpine images - - if: ${{ matrix.distribution == 'alpine' }} + - if: matrix.distribution == 'alpine' + name: Install necessary packages (Alpine) run: apk add gcc musl-dev libc6-compat git - name: Go modules cache @@ -72,7 +85,9 @@ jobs: # Install gotestsum env GOBIN=$PWD go install gotest.tools/gotestsum@latest # Run the tests with gotestsum - env CGO_ENABLED=${{ matrix.cgo_enabled }} ./gotestsum -- -v ./... || true + ./gotestsum -- -v ${{ matrix.cgo_enabled == '1' && '-race' || '' }} ./... + env: + CGO_ENABLED: ${{ matrix.cgo_enabled }} everything: name: All Tests diff --git a/apisec/internal/timed/set.go b/apisec/internal/timed/lru.go similarity index 100% rename from apisec/internal/timed/set.go rename to apisec/internal/timed/lru.go diff --git a/apisec/internal/timed/set_test.go b/apisec/internal/timed/lru_test.go similarity index 87% rename from apisec/internal/timed/set_test.go rename to apisec/internal/timed/lru_test.go index f81e994..da6d88a 100644 --- a/apisec/internal/timed/set_test.go +++ b/apisec/internal/timed/lru_test.go @@ -6,17 +6,18 @@ package timed import ( + "context" "runtime" "sync" - "sync/atomic" "testing" "time" "github.com/DataDog/appsec-internal-go/apisec/internal/config" + "github.com/DataDog/appsec-internal-go/apisec/internal/timed/testutil" "github.com/stretchr/testify/require" ) -func TestSet(t *testing.T) { +func TestLRU(t *testing.T) { t.Run("New", func(t *testing.T) { require.PanicsWithError(t, "NewSet: interval must be at least 1s, got 0s", func() { NewSet(0, UnixTime) }) require.PanicsWithError(t, "NewSet: interval must be at least 1s, got 10ms", func() { NewSet(10*time.Millisecond, UnixTime) }) @@ -54,20 +55,23 @@ func TestSet(t *testing.T) { }) t.Run("rebuild", func(t *testing.T) { - var fakeTime atomic.Int64 - fakeTime.Store(time.Now().Unix()) - fakeClock := func() int64 { return fakeTime.Load() } + goCount := runtime.GOMAXPROCS(0) * 10 + ctx, cancel := context.WithCancel(context.Background()) + clock := testutil.NewFakeClock(ctx, t, goCount) + defer func() { + cancel() + clock.WaitUntilDone() + }() - subject := NewSet(config.Interval, fakeClock) + subject := NewSet(config.Interval, clock.Unix) var ( - goCount = runtime.GOMAXPROCS(0) * 10 startBarrier sync.WaitGroup finishBarrier sync.WaitGroup ) startBarrier.Add(goCount + 1) finishBarrier.Add(goCount) - for g := range goCount { + for range goCount { go func() { defer finishBarrier.Done() startBarrier.Done() @@ -75,9 +79,7 @@ func TestSet(t *testing.T) { for key := range uint64(config.MaxItemCount * 4) { _ = subject.Hit(key) - if g == 0 { - fakeTime.Add(1) - } + clock.WaitForTick() } }() } diff --git a/apisec/internal/timed/table.go b/apisec/internal/timed/table.go index 641bd7c..cfc1c4d 100644 --- a/apisec/internal/timed/table.go +++ b/apisec/internal/timed/table.go @@ -168,11 +168,11 @@ func (d entryData) WithAccessTime(atime uint32) entryData { func (e copiableEntry) Compare(other copiableEntry) int { tst := e.Data.SampleTime() ost := other.Data.SampleTime() - if tst > ost { + if tst < ost { // Receiver was sampled more recently (sorts higher) return 1 } - if tst < ost { + if tst > ost { // Receiver was sampled less recently (sorts lower) return -1 } diff --git a/apisec/internal/timed/testutil/fakeclock.go b/apisec/internal/timed/testutil/fakeclock.go new file mode 100644 index 0000000..7d4fc0b --- /dev/null +++ b/apisec/internal/timed/testutil/fakeclock.go @@ -0,0 +1,80 @@ +package testutil + +import ( + "context" + "sync" + "sync/atomic" + "testing" + "time" +) + +type FakeClock struct { + t testing.TB + closed chan struct{} + goCount int + now int64 + needTick atomic.Bool + wg sync.WaitGroup + cnd sync.Cond +} + +func NewFakeClock(ctx context.Context, t testing.TB, goroutineCount int) *FakeClock { + res := &FakeClock{ + t: t, + closed: make(chan struct{}), + goCount: goroutineCount, + now: time.Now().Unix(), + cnd: *sync.NewCond(&sync.Mutex{}), + } + res.wg.Add(goroutineCount) + go res.tick(ctx) + return res +} + +func (c *FakeClock) Unix() int64 { + return c.now +} + +func (c *FakeClock) WaitForTick() { + c.cnd.L.Lock() + + c.needTick.CompareAndSwap(false, true) + curTime := c.now + + c.wg.Done() + for curTime == c.now && c.now > 0 { + c.cnd.Wait() + } + c.cnd.L.Unlock() +} + +func (c *FakeClock) WaitUntilDone() { + <-c.closed +} + +func (c *FakeClock) tick(ctx context.Context) { + c.t.Log("Start ticker!") + for { + select { + case <-ctx.Done(): + // Context is cancelled, stop the ticker... + c.cnd.L.Lock() + c.now = 0 + c.cnd.L.Unlock() + c.t.Log("Stop ticker!") + close(c.closed) + return + default: + if !c.needTick.Load() { + continue + } + c.wg.Wait() + c.cnd.L.Lock() + c.now++ + c.needTick.Store(false) + c.cnd.Broadcast() + c.wg.Add(c.goCount) + c.cnd.L.Unlock() + } + } +} diff --git a/apisec/sampler_test.go b/apisec/sampler_test.go index 53d2762..f502958 100644 --- a/apisec/sampler_test.go +++ b/apisec/sampler_test.go @@ -17,6 +17,7 @@ import ( "time" "github.com/DataDog/appsec-internal-go/apisec/internal/config" + "github.com/DataDog/appsec-internal-go/apisec/internal/timed/testutil" "github.com/stretchr/testify/assert" ) @@ -77,21 +78,24 @@ func TestSampler(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - ctx, cancel := context.WithCancel(ctx) - defer cancel() - assert.Positive(t, tc.SimulatedTPS) assert.GreaterOrEqual(t, tc.ExpectedKeepRate, 0.0) assert.GreaterOrEqual(t, 1.0, tc.ExpectedKeepRate) + const goroutineCount = 10 // Fixed to ensure consistent keep rate... + ctx, cancel := context.WithCancel(ctx) + clock := testutil.NewFakeClock(ctx, t, goroutineCount) + defer func() { + cancel() + clock.WaitForTick() + }() + var ( - goroutineCount = max(2, runtime.GOMAXPROCS(0)) - clock = newFakeClock(ctx, t, goroutineCount) - subject = newSampler(30*time.Second, clock.Unix) - sb sync.WaitGroup // Start barrier - wg sync.WaitGroup // Completion barrier - kept atomic.Uint64 - dropped atomic.Uint64 + subject = newSampler(30*time.Second, clock.Unix) + sb sync.WaitGroup // Start barrier + wg sync.WaitGroup // Completion barrier + kept atomic.Uint64 + dropped atomic.Uint64 ) sb.Add(1 + goroutineCount) // All child goroutines + this one... wg.Add(goroutineCount) @@ -207,70 +211,6 @@ func BenchmarkSampler(b *testing.B) { } } -type fakeClock struct { - t testing.TB - goCount int - now int64 - needTick atomic.Bool - wg sync.WaitGroup - cnd sync.Cond -} - -func newFakeClock(ctx context.Context, t testing.TB, goroutineCount int) *fakeClock { - res := &fakeClock{ - t: t, - goCount: goroutineCount, - now: time.Now().Unix(), - cnd: *sync.NewCond(&sync.Mutex{}), - } - res.wg.Add(goroutineCount) - go res.tick(ctx) - return res -} - -func (c *fakeClock) Unix() int64 { - return c.now -} - -func (c *fakeClock) WaitForTick() { - c.cnd.L.Lock() - - c.needTick.CompareAndSwap(false, true) - curTime := c.now - - c.wg.Done() - for curTime == c.now && c.now > 0 { - c.cnd.Wait() - } - c.cnd.L.Unlock() -} - -func (c *fakeClock) tick(ctx context.Context) { - c.t.Log("Start ticker!") - for { - select { - case <-ctx.Done(): - // Context is cancelled, stop the ticker... - c.cnd.L.Lock() - c.now = 0 - c.cnd.L.Unlock() - c.t.Log("Stop ticker!") - return - default: - if !c.needTick.Load() { - continue - } - c.wg.Wait() - c.cnd.L.Lock() - c.now++ - c.needTick.Store(false) - c.cnd.Broadcast() - c.wg.Add(c.goCount) - c.cnd.L.Unlock() - } - } -} - func randomOne[T any](list []T) T { return list[rand.Intn(len(list))] }