From 02b3cb723260ce2ab2d520c14f383f6a920a18a4 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Mon, 23 Sep 2019 11:52:49 +0300 Subject: [PATCH 1/3] Fix ordering of indexScanKeys after TraceID parsing, closes #1808 Signed-off-by: Michael Burman --- .../badger/spanstore/read_write_test.go | 83 +++++++++++++++++-- plugin/storage/badger/spanstore/reader.go | 5 ++ .../badger/spanstore/rw_internal_test.go | 38 +++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/plugin/storage/badger/spanstore/read_write_test.go b/plugin/storage/badger/spanstore/read_write_test.go index 64dc5732972..b4916229b1b 100644 --- a/plugin/storage/badger/spanstore/read_write_test.go +++ b/plugin/storage/badger/spanstore/read_write_test.go @@ -25,7 +25,7 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" + assert "github.com/stretchr/testify/require" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" @@ -587,14 +587,14 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) { // Opens a badger db and runs a test on it. func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) { + assert := assert.New(tb) f := badger.NewFactory() opts := badger.NewOptions("badger") v, command := config.Viperize(opts.AddFlags) dir := "/mnt/ssd/badger/testRun" err := os.MkdirAll(dir, 0700) - defer os.RemoveAll(dir) - assert.NoError(tb, err) + assert.NoError(err) keyParam := fmt.Sprintf("--badger.directory-key=%s", dir) valueParam := fmt.Sprintf("--badger.directory-value=%s", dir) @@ -608,22 +608,87 @@ func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Wr f.InitFromViper(v) err = f.Initialize(metrics.NullFactory, zap.NewNop()) - assert.NoError(tb, err) + assert.NoError(err) sw, err := f.CreateSpanWriter() - assert.NoError(tb, err) + assert.NoError(err) sr, err := f.CreateSpanReader() - assert.NoError(tb, err) + assert.NoError(err) defer func() { if closer, ok := sw.(io.Closer); ok { err := closer.Close() - assert.NoError(tb, err) + os.RemoveAll(dir) + assert.NoError(err) } else { - tb.FailNow() + assert.FailNow("io.Closer not implemented by SpanWriter") } - }() test(tb, sw, sr) } + +// TestRandomTraceID from issue #1808 +func TestRandomTraceID(t *testing.T) { + runFactoryTest(t, func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader) { + s1 := model.Span{ + TraceID: model.TraceID{ + Low: uint64(14767110704788176287), + High: 0, + }, + SpanID: model.SpanID(14976775253976086374), + OperationName: "/", + Process: &model.Process{ + ServiceName: "nginx", + }, + Tags: model.KeyValues{ + model.KeyValue{ + Key: "http.request_id", + VStr: "first", + VType: model.StringType, + }, + }, + StartTime: time.Now(), + Duration: 1 * time.Second, + } + err := sw.WriteSpan(&s1) + assert.NoError(t, err) + + s2 := model.Span{ + TraceID: model.TraceID{ + Low: uint64(4775132888371984950), + High: 0, + }, + SpanID: model.SpanID(13576481569227028654), + OperationName: "/", + Process: &model.Process{ + ServiceName: "nginx", + }, + Tags: model.KeyValues{ + model.KeyValue{ + Key: "http.request_id", + VStr: "second", + VType: model.StringType, + }, + }, + StartTime: time.Now(), + Duration: 1 * time.Second, + } + err = sw.WriteSpan(&s2) + assert.NoError(t, err) + + params := &spanstore.TraceQueryParameters{ + StartTimeMin: time.Now().Add(-1 * time.Minute), + StartTimeMax: time.Now(), + ServiceName: "nginx", + Tags: map[string]string{ + "http.request_id": "second", + }, + } + traces, err := sr.FindTraces(context.Background(), params) + assert.NoError(t, err) + + // failed with `second` tag query, but success with `first` + assert.Equal(t, 1, len(traces)) + }) +} diff --git a/plugin/storage/badger/spanstore/reader.go b/plugin/storage/badger/spanstore/reader.go index daa69ca78e1..a329e98e6f3 100644 --- a/plugin/storage/badger/spanstore/reader.go +++ b/plugin/storage/badger/spanstore/reader.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "math" + "sort" "time" "github.com/dgraph-io/badger" @@ -291,6 +292,9 @@ func (r *TraceReader) indexSeeksToTraceIDs(query *spanstore.TraceQueryParameters prevTraceID = traceID } } + sort.Slice(ids[i], func(k, h int) bool { + return bytes.Compare(ids[i][k], ids[i][h]) < 0 + }) } return ids, nil } @@ -522,6 +526,7 @@ func (r *TraceReader) scanIndexKeys(indexKeyValue []byte, startTimeMin time.Time } return nil }) + return indexResults, err } diff --git a/plugin/storage/badger/spanstore/rw_internal_test.go b/plugin/storage/badger/spanstore/rw_internal_test.go index 957d1f6f348..fc119033158 100644 --- a/plugin/storage/badger/spanstore/rw_internal_test.go +++ b/plugin/storage/badger/spanstore/rw_internal_test.go @@ -17,6 +17,8 @@ import ( "bytes" "context" "encoding/binary" + "math/rand" + "sort" "testing" "time" @@ -212,3 +214,39 @@ func TestMergeJoin(t *testing.T) { assert.Equal(2, len(merged)) assert.Equal(uint32(2), binary.BigEndian.Uint32(merged[1])) } + +func TestIndexScanReturnOrder(t *testing.T) { + runWithBadger(t, func(store *badger.DB, t *testing.T) { + testSpan := createDummySpan() + cache := NewCacheStore(store, time.Duration(1*time.Hour), true) + sw := NewSpanWriter(store, cache, time.Duration(1*time.Hour), nil) + rw := NewTraceReader(store, cache) + + for i := 0; i < 1000; i++ { + testSpan.TraceID = model.TraceID{ + High: rand.Uint64(), + Low: uint64(rand.Int63()), + } + testSpan.SpanID = model.SpanID(rand.Uint64()) + testSpan.StartTime = testSpan.StartTime.Add(time.Duration(i) * time.Millisecond) + err := sw.WriteSpan(&testSpan) + assert.NoError(t, err) + } + + tqp := &spanstore.TraceQueryParameters{ + ServiceName: "service", + StartTimeMax: testSpan.StartTime.Add(time.Hour), + StartTimeMin: testSpan.StartTime.Add(-1 * time.Hour), + } + + indexSeeks := make([][]byte, 0, 1) + indexSeeks = serviceQueries(tqp, indexSeeks) + + ids := make([][][]byte, 0, len(indexSeeks)+1) + + indexResults, _ := rw.indexSeeksToTraceIDs(tqp, indexSeeks, ids) + assert.True(t, sort.SliceIsSorted(indexResults[0], func(i, j int) bool { + return bytes.Compare(indexResults[0][i], indexResults[0][j]) < 0 + })) + }) +} From aa185578dc2b9ea730adf399d7ba40fdeba7e153 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Mon, 30 Sep 2019 15:21:07 +0300 Subject: [PATCH 2/3] Address comments Signed-off-by: Michael Burman --- plugin/storage/badger/spanstore/read_write_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugin/storage/badger/spanstore/read_write_test.go b/plugin/storage/badger/spanstore/read_write_test.go index b4916229b1b..985668776fa 100644 --- a/plugin/storage/badger/spanstore/read_write_test.go +++ b/plugin/storage/badger/spanstore/read_write_test.go @@ -25,7 +25,8 @@ import ( "testing" "time" - assert "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" @@ -587,7 +588,7 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) { // Opens a badger db and runs a test on it. func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) { - assert := assert.New(tb) + assert := require.New(tb) f := badger.NewFactory() opts := badger.NewOptions("badger") v, command := config.Viperize(opts.AddFlags) @@ -687,8 +688,6 @@ func TestRandomTraceID(t *testing.T) { } traces, err := sr.FindTraces(context.Background(), params) assert.NoError(t, err) - - // failed with `second` tag query, but success with `first` assert.Equal(t, 1, len(traces)) }) } From 86d7b60e335d08b9d0b8029bfaf3639e65fc8f68 Mon Sep 17 00:00:00 2001 From: Michael Burman Date: Mon, 7 Oct 2019 14:54:27 +0300 Subject: [PATCH 3/3] .. Signed-off-by: Michael Burman --- plugin/storage/badger/spanstore/read_write_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/storage/badger/spanstore/read_write_test.go b/plugin/storage/badger/spanstore/read_write_test.go index 985668776fa..d6422edc6ef 100644 --- a/plugin/storage/badger/spanstore/read_write_test.go +++ b/plugin/storage/badger/spanstore/read_write_test.go @@ -26,7 +26,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" @@ -588,7 +587,7 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) { // Opens a badger db and runs a test on it. func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) { - assert := require.New(tb) + assert := assert.New(tb) f := badger.NewFactory() opts := badger.NewOptions("badger") v, command := config.Viperize(opts.AddFlags)