Skip to content

Commit

Permalink
[refactor] Conditionally implement interfaces in v1adapter (#6701)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Towards #6697

## Description of the changes
- This is a prequel to
#6699. This PR refactors the
`TraceReader` to conditionally propagate the implementations of
`storage.Purger` and `storage.SamplingStoreFactory`. Without this
change, the implementation of the aforementioned interfaces get lost
when we wrap a v1 `SpanReader`.

## How was this change tested?
- Added unit tests

## 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`: `npm run lint` and `npm run test`

---------

Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 authored Feb 9, 2025
1 parent 1ac7d6d commit 97188f4
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 2 deletions.
30 changes: 28 additions & 2 deletions internal/storage/v2/v1adapter/tracereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/jaegertracing/jaeger-idl/model/v1"
"github.com/jaegertracing/jaeger/internal/storage/v1"
"github.com/jaegertracing/jaeger/internal/storage/v1/api/dependencystore"
"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore"
Expand All @@ -31,10 +32,35 @@ func GetV1Reader(reader tracestore.Reader) (spanstore.Reader, bool) {
return nil, false
}

func NewTraceReader(spanReader spanstore.Reader) *TraceReader {
return &TraceReader{
func NewTraceReader(spanReader spanstore.Reader) tracestore.Reader {
traceReader := &TraceReader{
spanReader: spanReader,
}
var (
purger, isPurger = spanReader.(storage.Purger)
sampler, isSampler = spanReader.(storage.SamplingStoreFactory)
)

switch {
case isPurger && isSampler:
return struct {
tracestore.Reader
storage.Purger
storage.SamplingStoreFactory
}{traceReader, purger, sampler}
case isPurger:
return struct {
tracestore.Reader
storage.Purger
}{traceReader, purger}
case isSampler:
return struct {
tracestore.Reader
storage.SamplingStoreFactory
}{traceReader, sampler}
default:
return traceReader
}
}

func (tr *TraceReader) GetTraces(
Expand Down
64 changes: 64 additions & 0 deletions internal/storage/v2/v1adapter/tracereader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,80 @@ import (
"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/jaegertracing/jaeger-idl/model/v1"
"github.com/jaegertracing/jaeger/internal/storage/v1"
dependencyStoreMocks "github.com/jaegertracing/jaeger/internal/storage/v1/api/dependencystore/mocks"
"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
spanStoreMocks "github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore/mocks"
"github.com/jaegertracing/jaeger/internal/storage/v1/memory"
v1StorageMocks "github.com/jaegertracing/jaeger/internal/storage/v1/mocks"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
tracestoremocks "github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore/mocks"
"github.com/jaegertracing/jaeger/pkg/iter"
)

func TestNewTraceReader(t *testing.T) {
mockSpanReader := new(spanStoreMocks.Reader)
mockPurger := new(v1StorageMocks.Purger)
mockSamplingStoreFactory := new(v1StorageMocks.SamplingStoreFactory)

tests := []struct {
name string
spanReader spanstore.Reader
expectedInterfaces []any
}{
{
name: "No extra interfaces",
spanReader: mockSpanReader,
expectedInterfaces: []any{(*tracestore.Reader)(nil)},
},
{
name: "Implements Purger",
spanReader: struct {
spanstore.Reader
storage.Purger
}{mockSpanReader, mockPurger},
expectedInterfaces: []any{
(*tracestore.Reader)(nil),
(*storage.Purger)(nil),
},
},
{
name: "Implements SamplingStoreFactory",
spanReader: struct {
spanstore.Reader
storage.SamplingStoreFactory
}{mockSpanReader, mockSamplingStoreFactory},
expectedInterfaces: []any{
(*tracestore.Reader)(nil),
(*storage.SamplingStoreFactory)(nil),
},
},
{
name: "Implements both Purger and SamplingStoreFactory",
spanReader: struct {
spanstore.Reader
storage.Purger
storage.SamplingStoreFactory
}{mockSpanReader, mockPurger, mockSamplingStoreFactory},
expectedInterfaces: []any{
(*tracestore.Reader)(nil),
(*storage.Purger)(nil),
(*storage.SamplingStoreFactory)(nil),
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
traceReader := NewTraceReader(test.spanReader)
for _, i := range test.expectedInterfaces {
require.Implements(t, i, traceReader)
}
})
}
}

func TestGetV1Reader_NoError(t *testing.T) {
memstore := memory.NewStore()
traceReader := &TraceReader{
Expand Down

0 comments on commit 97188f4

Please sign in to comment.