From c72a8a6389fb6f509f4af8726eb57ab0243bae01 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 15 Sep 2020 00:04:34 -0400 Subject: [PATCH 1/4] Add integration test for ErrTraceNotFound Signed-off-by: Yuri Shkuro --- plugin/storage/integration/integration.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugin/storage/integration/integration.go b/plugin/storage/integration/integration.go index 8887b75b49e..0d984cd4e32 100644 --- a/plugin/storage/integration/integration.go +++ b/plugin/storage/integration/integration.go @@ -188,6 +188,12 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) { if !assert.True(t, found) { CompareTraces(t, expected, actual) } + + t.Run("NotFound error", func(t *testing.T) { + fakeTraceID := model.TraceID{High: 42, Low: 42} + _, err := s.SpanReader.GetTrace(context.Background(), fakeTraceID) + assert.Equal(t, spanstore.ErrTraceNotFound, err) + }) } func (s *StorageIntegration) testFindTraces(t *testing.T) { From f5e1e87601ed1c77f35f2a136636fe3c7af1a6e6 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 15 Sep 2020 02:13:11 -0400 Subject: [PATCH 2/4] Fix badger storage Signed-off-by: Yuri Shkuro --- plugin/storage/badger/spanstore/reader.go | 2 +- plugin/storage/integration/integration.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin/storage/badger/spanstore/reader.go b/plugin/storage/badger/spanstore/reader.go index 1a689540d66..c5d2ccf848f 100644 --- a/plugin/storage/badger/spanstore/reader.go +++ b/plugin/storage/badger/spanstore/reader.go @@ -164,7 +164,7 @@ func (r *TraceReader) GetTrace(ctx context.Context, traceID model.TraceID) (*mod return traces[0], nil } - return nil, nil + return nil, spanstore.ErrTraceNotFound } // scanTimeRange returns all the Traces found between startTs and endTs diff --git a/plugin/storage/integration/integration.go b/plugin/storage/integration/integration.go index 0d984cd4e32..dc29a6ff66c 100644 --- a/plugin/storage/integration/integration.go +++ b/plugin/storage/integration/integration.go @@ -191,8 +191,9 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) { t.Run("NotFound error", func(t *testing.T) { fakeTraceID := model.TraceID{High: 42, Low: 42} - _, err := s.SpanReader.GetTrace(context.Background(), fakeTraceID) + trace, err := s.SpanReader.GetTrace(context.Background(), fakeTraceID) assert.Equal(t, spanstore.ErrTraceNotFound, err) + assert.Nil(t, trace) }) } From b3f0824617f1ef54d21b1e9d729e5be6c7ac6849 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 15 Sep 2020 11:13:07 -0400 Subject: [PATCH 3/4] fix badger test Signed-off-by: Yuri Shkuro --- plugin/storage/badger/spanstore/read_write_test.go | 2 +- plugin/storage/integration/integration.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/storage/badger/spanstore/read_write_test.go b/plugin/storage/badger/spanstore/read_write_test.go index 6cd35a779fa..0c5f921dd76 100644 --- a/plugin/storage/badger/spanstore/read_write_test.go +++ b/plugin/storage/badger/spanstore/read_write_test.go @@ -306,7 +306,7 @@ func TestFindNothing(t *testing.T) { assert.Equal(t, 0, len(trs)) tr, err := sr.GetTrace(context.Background(), model.TraceID{High: 0, Low: 0}) - assert.NoError(t, err) + assert.Equal(t, spanstore.ErrTraceNotFound, err) assert.Nil(t, tr) }) } diff --git a/plugin/storage/integration/integration.go b/plugin/storage/integration/integration.go index dc29a6ff66c..bc56c890ab4 100644 --- a/plugin/storage/integration/integration.go +++ b/plugin/storage/integration/integration.go @@ -190,7 +190,7 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) { } t.Run("NotFound error", func(t *testing.T) { - fakeTraceID := model.TraceID{High: 42, Low: 42} + fakeTraceID := model.TraceID{High: 0, Low: 0} trace, err := s.SpanReader.GetTrace(context.Background(), fakeTraceID) assert.Equal(t, spanstore.ErrTraceNotFound, err) assert.Nil(t, trace) From d0194aaa1f58ced7022f94b84ae42b770594800d Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 15 Sep 2020 11:30:15 -0400 Subject: [PATCH 4/4] add API docs Signed-off-by: Yuri Shkuro --- storage/spanstore/interface.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/storage/spanstore/interface.go b/storage/spanstore/interface.go index e383da1b0d2..deaefa372b1 100644 --- a/storage/spanstore/interface.go +++ b/storage/spanstore/interface.go @@ -35,10 +35,31 @@ type Writer interface { // Reader finds and loads traces and other data from storage. type Reader interface { + // GetTrace retrieves the trace with a given id. + // + // If no spans are stored for this trace, it returns ErrTraceNotFound. GetTrace(ctx context.Context, traceID model.TraceID) (*model.Trace, error) + + // GetServices returns all service names known to the backend from spans + // within its retention period. GetServices(ctx context.Context) ([]string, error) + + // GetOperations returns all operation names for a given service + // known to the backend from spans within its retention period. GetOperations(ctx context.Context, query OperationQueryParameters) ([]Operation, error) + + // FindTraces returns all traces matching query parameters. There's currently + // an implementation-dependent abiguity whether all query filters (such as + // multiple tags) must apply to the same span within a trace, or can be satisfied + // by different spans. + // + // If no matching traces are found, the function returns (nil, nil). FindTraces(ctx context.Context, query *TraceQueryParameters) ([]*model.Trace, error) + + // FindTraceIDs does the same search as FindTraces, but returns only the list + // of matching trace IDs. + // + // If no matching traces are found, the function returns (nil, nil). FindTraceIDs(ctx context.Context, query *TraceQueryParameters) ([]model.TraceID, error) }