Address Feedback From PR Review

Signed-off-by: Mahad Zaryab <[email protected]>
jaegertracing · Dec 24, 2024 · 56163f6 · 56163f6
1 parent 1c9b19b
commit 56163f6
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 25 deletions.
diff --git a/cmd/jaeger/internal/integration/span_writer.go b/cmd/jaeger/internal/integration/span_writer.go
@@ -68,15 +68,12 @@ func (w *spanWriter) Close() error {
 }
 
 func (w *spanWriter) WriteSpan(ctx context.Context, span *model.Span) error {
-	td, err := jptrace.ProtoToTraces([]*model.Batch{
+	td := jptrace.ProtoToTraces([]*model.Batch{
 		{
 			Spans:   []*model.Span{span},
 			Process: span.Process,
 		},
 	})
-	if err != nil {
-		return err
-	}
 
 	return w.exporter.ConsumeTraces(ctx, td)
 }
diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go
@@ -12,7 +12,6 @@ import (
 
 func modelToOTLP(spans []*model.Span) ptrace.Traces {
 	batch := &model.Batch{Spans: spans}
-	// there is never an error returned from ProtoToTraces
-	tr, _ := jptrace.ProtoToTraces([]*model.Batch{batch})
+	tr := jptrace.ProtoToTraces([]*model.Batch{batch})
 	return tr
 }
diff --git a/internal/jptrace/translator.go b/internal/jptrace/translator.go
@@ -22,14 +22,11 @@ func ProtoFromTraces(traces ptrace.Traces) []*model.Batch {
 
 // ProtoToTraces converts Jaeger model batches ([]*model.Batch)
 // to OpenTelemetry traces (ptrace.Traces).
-func ProtoToTraces(batches []*model.Batch) (ptrace.Traces, error) {
-	traces, err := jaegerTranslator.ProtoToTraces(batches)
-	if err != nil {
-		return ptrace.NewTraces(), err
-	}
+func ProtoToTraces(batches []*model.Batch) ptrace.Traces {
+	traces, _ := jaegerTranslator.ProtoToTraces(batches) // never returns an error
 	spanMap := createSpanMapFromTraces(traces)
 	transferWarningsToOTLPSpans(batches, spanMap)
-	return traces, nil
+	return traces
 }
 
 func createSpanMapFromBatches(batches []*model.Batch) map[model.SpanID]*model.Span {
@@ -67,10 +64,11 @@ func transferWarningsToModelSpans(traces ptrace.Traces, spanMap map[model.SpanID
 			for k := 0; k < spans.Len(); k++ {
 				otelSpan := spans.At(k)
 				warnings := GetWarnings(otelSpan)
-				span := spanMap[model.SpanIDFromOTEL(otelSpan.SpanID())]
-				span.Warnings = append(span.Warnings, warnings...)
-				// filter out the warning tag
-				span.Tags = filterTags(span.Tags, warningsAttribute)
+				if span, ok := spanMap[model.SpanIDFromOTEL(otelSpan.SpanID())]; ok && len(warnings) > 0 {
+					span.Warnings = append(span.Warnings, warnings...)
+					// filter out the warning tag
+					span.Tags = filterTags(span.Tags, warningsAttribute)
+				}
 			}
 		}
 	}
@@ -79,9 +77,8 @@ func transferWarningsToModelSpans(traces ptrace.Traces, spanMap map[model.SpanID
 func transferWarningsToOTLPSpans(batches []*model.Batch, spanMap map[pcommon.SpanID]ptrace.Span) {
 	for _, batch := range batches {
 		for _, span := range batch.Spans {
-			otelSpan := spanMap[span.SpanID.ToOTELSpanID()]
-			for _, warning := range span.Warnings {
-				AddWarning(otelSpan, warning)
+			if otelSpan, ok := spanMap[span.SpanID.ToOTELSpanID()]; ok && len(span.Warnings) > 0 {
+				AddWarning(otelSpan, span.Warnings...)
 			}
 		}
 	}

diff --git a/internal/jptrace/translator_test.go b/internal/jptrace/translator_test.go
@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 
@@ -85,8 +84,7 @@ func TestProtoToTraces_AddsWarnings(t *testing.T) {
 		},
 	}
 	batches := []*model.Batch{batch1, batch2}
-	traces, err := ProtoToTraces(batches)
-	require.NoError(t, err)
+	traces := ProtoToTraces(batches)
 
 	assert.Equal(t, 2, traces.ResourceSpans().Len())
 

diff --git a/storage_v2/v1adapter/reader.go b/storage_v2/v1adapter/reader.go
@@ -60,8 +60,8 @@ func (tr *TraceReader) GetTraces(
 				return
 			}
 			batch := &model.Batch{Spans: t.GetSpans()}
-			tr, err := jptrace.ProtoToTraces([]*model.Batch{batch})
-			if !yield([]ptrace.Traces{tr}, err) || err != nil {
+			tr := jptrace.ProtoToTraces([]*model.Batch{batch})
+			if !yield([]ptrace.Traces{tr}, nil) {
 				return
 			}
 		}
@@ -105,7 +105,7 @@ func (tr *TraceReader) FindTraces(
 		}
 		for _, trace := range traces {
 			batch := &model.Batch{Spans: trace.GetSpans()}
-			otelTrace, _ := jptrace.ProtoToTraces([]*model.Batch{batch})
+			otelTrace := jptrace.ProtoToTraces([]*model.Batch{batch})
 			if !yield([]ptrace.Traces{otelTrace}, nil) {
 				return
 			}