From f13cd8337580dc8b3d77ed718ada4ff1358d41b3 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 28 Jul 2021 16:48:03 +0200 Subject: [PATCH 1/3] Set lookback in ES rollover to distant past Signed-off-by: Pavol Loffay --- plugin/storage/es/spanstore/reader.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 3e405dfca84..75c1f10fa8c 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -129,10 +129,17 @@ type SpanReaderParams struct { // NewSpanReader returns a new SpanReader with a metrics. func NewSpanReader(p SpanReaderParams) *SpanReader { + maxSpanAge := p.MaxSpanAge + // If rollover is enabled set lookback far into the past + // In rollover only read alias is used to query the data so looking far into the past should + // not affect performance. + if p.UseReadWriteAliases { + maxSpanAge = time.Hour * 24 * 365 * 100 + } return &SpanReader{ client: p.Client, logger: p.Logger, - maxSpanAge: p.MaxSpanAge, + maxSpanAge: maxSpanAge, serviceOperationStorage: NewServiceOperationStorage(p.Client, p.Logger, 0), // the decorator takes care of metrics spanIndexPrefix: indexNames(p.IndexPrefix, spanIndex), serviceIndexPrefix: indexNames(p.IndexPrefix, serviceIndex), From f604e7425ec1f9d059a4486fef64c33dc898a65a Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 5 Aug 2021 13:10:40 +0200 Subject: [PATCH 2/3] Add test Signed-off-by: Pavol Loffay --- plugin/storage/es/spanstore/reader.go | 9 +++--- plugin/storage/es/spanstore/reader_test.go | 36 ++++++++++++++++------ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/plugin/storage/es/spanstore/reader.go b/plugin/storage/es/spanstore/reader.go index 75c1f10fa8c..09c4cadf141 100644 --- a/plugin/storage/es/spanstore/reader.go +++ b/plugin/storage/es/spanstore/reader.go @@ -60,6 +60,8 @@ const ( tagValueField = "value" defaultNumTraces = 100 + + rolloverMaxSpanAge = time.Hour * 24 * 365 * 100 ) var ( @@ -130,11 +132,10 @@ type SpanReaderParams struct { // NewSpanReader returns a new SpanReader with a metrics. func NewSpanReader(p SpanReaderParams) *SpanReader { maxSpanAge := p.MaxSpanAge - // If rollover is enabled set lookback far into the past - // In rollover only read alias is used to query the data so looking far into the past should - // not affect performance. + // Setting the maxSpanAge to a large duration will ensure all spans in the "read" alias are accessible by queries (query window = [now - maxSpanAge, now]). + // When read/write aliases are enabled, which are required for index rollovers, only the "read" alias is queried and therefore should not affect performance. if p.UseReadWriteAliases { - maxSpanAge = time.Hour * 24 * 365 * 100 + maxSpanAge = rolloverMaxSpanAge } return &SpanReader{ client: p.Client, diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index 64410d35394..af272875216 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -29,7 +29,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/uber/jaeger-lib/metrics" "github.com/uber/jaeger-lib/metrics/metricstest" "go.uber.org/zap" @@ -132,14 +131,33 @@ func withArchiveSpanReader(readAlias bool, fn func(r *spanReaderTest)) { var _ spanstore.Reader = &SpanReader{} // check API conformance func TestNewSpanReader(t *testing.T) { - client := &mocks.Client{} - reader := NewSpanReader(SpanReaderParams{ - Client: client, - Logger: zap.NewNop(), - MaxSpanAge: 0, - MetricsFactory: metrics.NullFactory, - IndexPrefix: ""}) - assert.NotNil(t, reader) + tests := []struct { + name string + params SpanReaderParams + maxSpanAge time.Duration + }{ + { + name: "no rollover", + params: SpanReaderParams{ + MaxSpanAge: time.Hour * 72, + }, + maxSpanAge: time.Hour * 72, + }, + { + name: "rollover enabled", + params: SpanReaderParams{ + MaxSpanAge: time.Hour * 72, + UseReadWriteAliases: true, + }, + maxSpanAge: time.Hour * 24 * 365 * 100, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + reader := NewSpanReader(test.params) + require.NotNil(t, reader) + }) + } } func TestSpanReaderIndices(t *testing.T) { From d35426a03379939b6fab74263c6410b9769fbb23 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 5 Aug 2021 13:35:19 +0200 Subject: [PATCH 3/3] Fix test Signed-off-by: Pavol Loffay --- plugin/storage/es/spanstore/reader_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/plugin/storage/es/spanstore/reader_test.go b/plugin/storage/es/spanstore/reader_test.go index af272875216..b7a0ce276c6 100644 --- a/plugin/storage/es/spanstore/reader_test.go +++ b/plugin/storage/es/spanstore/reader_test.go @@ -156,6 +156,7 @@ func TestNewSpanReader(t *testing.T) { t.Run(test.name, func(t *testing.T) { reader := NewSpanReader(test.params) require.NotNil(t, reader) + assert.Equal(t, test.maxSpanAge, reader.maxSpanAge) }) } }