[refactor] Consolidate v1/v2 writer factory adapter functionality (jaegertracing#7022)

## Which problem is this PR solving?
- Towards jaegertracing#6979

## Description of the changes
- This PR consolidates the v1/v2 writer adapter functionality by
combining `GetV1Writer` with `NewSpanWriter`

## 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`

Signed-off-by: Mahad Zaryab <[email protected]>

Author: mahadzaryab1

cmd/collector/app/span_processor.go

@@ -227,11 +227,8 @@ func (sp *spanProcessor) saveSpan(span *model.Span, tenant string) {
 }
 
 func (sp *spanProcessor) writeSpan(ctx context.Context, span *model.Span) error {
-	if spanWriter, ok := v1adapter.GetV1Writer(sp.traceWriter); ok {
-		return spanWriter.WriteSpan(ctx, span)
-	}
-	traces := v1adapter.V1BatchesToTraces([]*model.Batch{{Spans: []*model.Span{span}}})
-	return sp.traceWriter.WriteTraces(ctx, traces)
+	spanWriter := v1adapter.GetV1Writer(sp.traceWriter)
+	return spanWriter.WriteSpan(ctx, span)
 }
 
 func (sp *spanProcessor) countSpansInQueue(span *model.Span, _ string /* tenant */) {

cmd/jaeger/internal/extension/jaegerquery/server.go

@@ -21,7 +21,6 @@ import (
 	"github.com/jaegertracing/jaeger/internal/metrics"
 	"github.com/jaegertracing/jaeger/internal/storage/metricstore/disabled"
 	"github.com/jaegertracing/jaeger/internal/storage/v1/api/metricstore"
-	"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
 	"github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore"
 	"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
 	"github.com/jaegertracing/jaeger/internal/storage/v2/v1adapter"
@@ -159,7 +158,8 @@ func (s *server) addArchiveStorage(
 	v2opts.ArchiveTraceReader = traceReader
 	v2opts.ArchiveTraceWriter = traceWriter
 
-	spanReader, spanWriter := getV1Adapters(traceReader, traceWriter)
+	spanReader := v1adapter.GetV1Reader(traceReader)
+	spanWriter := v1adapter.GetV1Writer(traceWriter)
 
 	opts.ArchiveSpanReader = spanReader
 	opts.ArchiveSpanWriter = spanWriter
@@ -181,19 +181,6 @@ func (s *server) initArchiveStorage(f tracestore.Factory) (tracestore.Reader, tr
 	return reader, writer
 }
 
-func getV1Adapters(
-	reader tracestore.Reader,
-	writer tracestore.Writer,
-) (spanstore.Reader, spanstore.Writer) {
-	v1Reader := v1adapter.GetV1Reader(reader)
-	v1Writer, ok := v1adapter.GetV1Writer(writer)
-	if !ok {
-		v1Writer = v1adapter.NewSpanWriter(writer)
-	}
-
-	return v1Reader, v1Writer
-}
-
 func (s *server) createMetricReader(host component.Host) (metricstore.Reader, error) {
 	if s.config.Storage.Metrics == "" {
 		s.telset.Logger.Info("Metric storage not configured")

cmd/jaeger/internal/extension/jaegerquery/server_test.go

@@ -29,13 +29,10 @@ import (
 	"github.com/jaegertracing/jaeger/internal/storage/v1"
 	"github.com/jaegertracing/jaeger/internal/storage/v1/api/metricstore"
 	metricstoremocks "github.com/jaegertracing/jaeger/internal/storage/v1/api/metricstore/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/v2/api/depstore"
 	depstoremocks "github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore/mocks"
 	"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
 	tracestoremocks "github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore/mocks"
-	"github.com/jaegertracing/jaeger/internal/storage/v2/v1adapter"
 	"github.com/jaegertracing/jaeger/internal/telemetry"
 	"github.com/jaegertracing/jaeger/internal/testutils"
 )
@@ -344,39 +341,6 @@ func TestServerAddArchiveStorage(t *testing.T) {
 	}
 }
 
-func TestGetV1Adapters(t *testing.T) {
-	tests := []struct {
-		name           string
-		reader         tracestore.Reader
-		writer         tracestore.Writer
-		expectedReader spanstore.Reader
-		expectedWriter spanstore.Writer
-	}{
-		{
-			name:           "native tracestore.Reader and tracestore.Writer",
-			reader:         &tracestoremocks.Reader{},
-			writer:         &tracestoremocks.Writer{},
-			expectedReader: &v1adapter.SpanReader{},
-			expectedWriter: v1adapter.NewSpanWriter(&tracestoremocks.Writer{}),
-		},
-		{
-			name:           "wrapped spanstore.Reader and spanstore.Writer",
-			reader:         v1adapter.NewTraceReader(&spanstoremocks.Reader{}),
-			writer:         v1adapter.NewTraceWriter(&spanstoremocks.Writer{}),
-			expectedReader: &spanstoremocks.Reader{},
-			expectedWriter: &spanstoremocks.Writer{},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			gotReader, gotWriter := getV1Adapters(test.reader, test.writer)
-			require.IsType(t, test.expectedReader, gotReader)
-			require.Equal(t, test.expectedWriter, gotWriter)
-		})
-	}
-}
-
 func TestServerAddMetricsStorage(t *testing.T) {
 	host := componenttest.NewNopHost()
 

internal/storage/v2/v1adapter/spanwriter.go

@@ -6,8 +6,6 @@ package v1adapter
 import (
 	"context"
 
-	jaegerTranslator "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger"
-
 	"github.com/jaegertracing/jaeger-idl/model/v1"
 	"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
 	"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
@@ -21,13 +19,7 @@ type SpanWriter struct {
 	traceWriter tracestore.Writer
 }
 
-func NewSpanWriter(traceWriter tracestore.Writer) *SpanWriter {
-	return &SpanWriter{
-		traceWriter: traceWriter,
-	}
-}
-
 func (sw *SpanWriter) WriteSpan(ctx context.Context, span *model.Span) error {
-	traces, _ := jaegerTranslator.ProtoToTraces([]*model.Batch{{Spans: []*model.Span{span}}})
+	traces := V1BatchesToTraces([]*model.Batch{{Spans: []*model.Span{span}}})
 	return sw.traceWriter.WriteTraces(ctx, traces)
 }

internal/storage/v2/v1adapter/spanwriter_test.go

@@ -39,7 +39,9 @@ func TestSpanWriter_WriteSpan(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			mockTraceWriter := &tracestoremocks.Writer{}
-			spanWriter := NewSpanWriter(mockTraceWriter)
+			spanWriter := &SpanWriter{
+				traceWriter: mockTraceWriter,
+			}
 
 			now := time.Now().UTC()
 			testSpan := &model.Span{

internal/storage/v2/v1adapter/tracewriter.go

@@ -20,11 +20,13 @@ type TraceWriter struct {
 	spanWriter spanstore.Writer
 }
 
-func GetV1Writer(writer tracestore.Writer) (spanstore.Writer, bool) {
+func GetV1Writer(writer tracestore.Writer) spanstore.Writer {
 	if tr, ok := writer.(*TraceWriter); ok {
-		return tr.spanWriter, ok
+		return tr.spanWriter
+	}
+	return &SpanWriter{
+		traceWriter: writer,
 	}
-	return nil, false
 }
 
 func NewTraceWriter(spanWriter spanstore.Writer) *TraceWriter {

internal/storage/v2/v1adapter/tracewriter_test.go

@@ -16,7 +16,7 @@ import (
 
 	"github.com/jaegertracing/jaeger-idl/model/v1"
 	"github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore"
-	spanstoreMocks "github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore/mocks"
+	spanstoremocks "github.com/jaegertracing/jaeger/internal/storage/v1/api/spanstore/mocks"
 	"github.com/jaegertracing/jaeger/internal/storage/v1/memory"
 	tracestoremocks "github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore/mocks"
 )
@@ -42,7 +42,7 @@ func TestWriteTraces(t *testing.T) {
 }
 
 func TestWriteTracesError(t *testing.T) {
-	mockstore := spanstoreMocks.NewWriter(t)
+	mockstore := spanstoremocks.NewWriter(t)
 	mockstore.On(
 		"WriteSpan",
 		mock.AnythingOfType("context.backgroundCtx"),
@@ -57,20 +57,22 @@ func TestWriteTracesError(t *testing.T) {
 	require.ErrorContains(t, err, "mocked error")
 }
 
-func TestGetV1Writer_NoError(t *testing.T) {
-	memstore := memory.NewStore()
-	traceWriter := &TraceWriter{
-		spanWriter: memstore,
-	}
-	v1Writer, ok := GetV1Writer(traceWriter)
-	require.True(t, ok)
-	require.Equal(t, memstore, v1Writer)
-}
-
-func TestGetV1Writer_Error(t *testing.T) {
-	w := new(tracestoremocks.Writer)
-	_, ok := GetV1Writer(w)
-	require.False(t, ok)
+func TestGetV1Writer(t *testing.T) {
+	t.Run("wrapped v1 writer", func(t *testing.T) {
+		writer := new(spanstoremocks.Writer)
+		traceWriter := &TraceWriter{
+			spanWriter: writer,
+		}
+		v1Writer := GetV1Writer(traceWriter)
+		require.Equal(t, writer, v1Writer)
+	})
+
+	t.Run("native v2 writer", func(t *testing.T) {
+		writer := new(tracestoremocks.Writer)
+		v1Writer := GetV1Writer(writer)
+		require.IsType(t, &SpanWriter{}, v1Writer)
+		require.Equal(t, writer, v1Writer.(*SpanWriter).traceWriter)
+	})
 }
 
 func makeTraces() ptrace.Traces {