Skip to content

Commit

Permalink
[fix] Revert changes to tracereader adapter (jaegertracing#6705)
Browse files Browse the repository at this point in the history
## Description of the changes
- This PR reverts the changes to jaegertracing#6701 since v1 implements the `Purger`
and `SamplingStoreFactory` on `storage.Factory` instead of on
`spanstore.Reader`.

## How was this change tested?
- CI

## 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`
  • Loading branch information
mahadzaryab1 authored Feb 11, 2025
1 parent d3c1eaf commit 8bf69c7
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 92 deletions.
30 changes: 2 additions & 28 deletions internal/storage/v2/v1adapter/tracereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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 @@ -32,35 +31,10 @@ func GetV1Reader(reader tracestore.Reader) (spanstore.Reader, bool) {
return nil, false
}

func NewTraceReader(spanReader spanstore.Reader) tracestore.Reader {
traceReader := &TraceReader{
func NewTraceReader(spanReader spanstore.Reader) *TraceReader {
return &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: 0 additions & 64 deletions internal/storage/v2/v1adapter/tracereader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,80 +16,16 @@ 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 8bf69c7

Please sign in to comment.