Skip to content

Commit

Permalink
Add go leak check to Cassandra and Kafka e2e tests (jaegertracing#6336)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
Fixes a part of jaegertracing#5006 
## Description of the changes
1) Cassandra factory is closed in the test manually after the test, not
doing so will fail the leak test probably due to CleanUp function is not
closing factory properly and returning in the test leading to end it
that further fails the leak test.
2) Kafka Producer and Consumer was not closed before leading to failure
of leak test which is fixed in this PR.

## How was this change tested?
- By running integartion tests manually

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Manik2708 <[email protected]>
Signed-off-by: Manik Mehta <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
Manik2708 and yurishkuro authored Dec 11, 2024
1 parent 8f612a8 commit 53a8053
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
13 changes: 12 additions & 1 deletion pkg/testutils/leakcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ func IgnoreOpenCensusWorkerLeak() goleak.Option {
return goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start")
}

// IgnoreGoMetricsMeterLeak prevents the leak created by go-metrics which is
// used by Sarama (Kafka Client) in Jaeger v1. This reason of this leak is
// not Jaeger but the go-metrics used by Samara.
// See these issues for the context
// - https://github.com/IBM/sarama/issues/1321
// - https://github.com/IBM/sarama/issues/1340
// - https://github.com/IBM/sarama/issues/2832
func IgnoreGoMetricsMeterLeak() goleak.Option {
return goleak.IgnoreTopFunction("github.com/rcrowley/go-metrics.(*meterArbiter).tick")
}

// VerifyGoLeaks verifies that unit tests do not leak any goroutines.
// It should be called in TestMain.
func VerifyGoLeaks(m *testing.M) {
Expand All @@ -36,5 +47,5 @@ func VerifyGoLeaks(m *testing.M) {
//
// defer testutils.VerifyGoLeaksOnce(t)
func VerifyGoLeaksOnce(t *testing.T) {
goleak.VerifyNone(t, IgnoreGlogFlushDaemonLeak(), IgnoreOpenCensusWorkerLeak())
goleak.VerifyNone(t, IgnoreGlogFlushDaemonLeak(), IgnoreOpenCensusWorkerLeak(), IgnoreGoMetricsMeterLeak())
}
11 changes: 8 additions & 3 deletions plugin/storage/integration/cassandra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"

"github.com/jaegertracing/jaeger/pkg/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/pkg/testutils"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
"github.com/jaegertracing/jaeger/storage/dependencystore"
)
Expand Down Expand Up @@ -47,6 +49,9 @@ func (*CassandraStorageIntegration) initializeCassandraFactory(t *testing.T, fla
require.NoError(t, command.ParseFlags(flags))
f.InitFromViper(v, logger)
require.NoError(t, f.Initialize(metrics.NullFactory, logger))
t.Cleanup(func() {
assert.NoError(t, f.Close())
})
return f
}

Expand Down Expand Up @@ -78,9 +83,6 @@ func (s *CassandraStorageIntegration) initializeCassandra(t *testing.T) {
s.SamplingStore, err = f.CreateSamplingStore(0)
require.NoError(t, err)
s.initializeDependencyReaderAndWriter(t, f)
t.Cleanup(func() {
require.NoError(t, f.Close())
})
}

func (s *CassandraStorageIntegration) initializeDependencyReaderAndWriter(t *testing.T, f *cassandra.Factory) {
Expand All @@ -99,6 +101,9 @@ func (s *CassandraStorageIntegration) initializeDependencyReaderAndWriter(t *tes

func TestCassandraStorage(t *testing.T) {
SkipUnlessEnv(t, "cassandra")
t.Cleanup(func() {
testutils.VerifyGoLeaksOnce(t)
})
s := newCassandraStorageIntegration()
s.initializeCassandra(t)
s.RunAll(t)
Expand Down
13 changes: 11 additions & 2 deletions plugin/storage/integration/kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/Shopify/sarama"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"
Expand All @@ -20,6 +21,7 @@ import (
"github.com/jaegertracing/jaeger/pkg/config"
"github.com/jaegertracing/jaeger/pkg/kafka/consumer"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/pkg/testutils"
"github.com/jaegertracing/jaeger/plugin/storage/kafka"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand Down Expand Up @@ -53,10 +55,11 @@ func (s *KafkaIntegrationTestSuite) initialize(t *testing.T) {
f.InitFromViper(v, logger)
err = f.Initialize(metrics.NullFactory, logger)
require.NoError(t, err)

t.Cleanup(func() {
assert.NoError(t, f.Close())
})
spanWriter, err := f.CreateSpanWriter()
require.NoError(t, err)

v, command = config.Viperize(app.AddFlags)
err = command.ParseFlags([]string{
"--kafka.consumer.topic",
Expand All @@ -82,6 +85,9 @@ func (s *KafkaIntegrationTestSuite) initialize(t *testing.T) {
traceStore := memory.NewStore()
spanConsumer, err := builder.CreateConsumer(logger, metrics.NullFactory, traceStore, options)
require.NoError(t, err)
t.Cleanup(func() {
assert.NoError(t, spanConsumer.Close())
})
spanConsumer.Start()

s.SpanWriter = spanWriter
Expand Down Expand Up @@ -120,6 +126,9 @@ func (*ingester) FindTraceIDs(context.Context, *spanstore.TraceQueryParameters)

func TestKafkaStorage(t *testing.T) {
SkipUnlessEnv(t, "kafka")
t.Cleanup(func() {
testutils.VerifyGoLeaksOnce(t)
})
s := &KafkaIntegrationTestSuite{}
s.initialize(t)
t.Run("GetTrace", s.testGetTrace)
Expand Down

0 comments on commit 53a8053

Please sign in to comment.