simplify the signature of ProtoFromTraces as it never returns an error (

open-telemetry#35560) **Description:** The `ProtoFromTraces` returns an error but never sets it to a value besides nil. Removing it from the signature simplifies code downstream.
EOjeah · Oct 8, 2024 · 679374a · 679374a
1 parent b62cc39
commit 679374a
Show file tree

Hide file tree

Showing 17 changed files with 52 additions and 65 deletions.
diff --git a/.chloggen/ProtoFromTracesError.yaml b/.chloggen/ProtoFromTracesError.yaml
@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: breaking
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: pkg/translator/jaeger
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Remove error from method signature as it always returns nil
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [35560]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]
diff --git a/exporter/awskinesisexporter/internal/batch/encode_jaeger.go b/exporter/awskinesisexporter/internal/batch/encode_jaeger.go
@@ -6,7 +6,6 @@ package batch // import "github.com/open-telemetry/opentelemetry-collector-contr
 import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/jaegertracing/jaeger/model"
-	"go.opentelemetry.io/collector/consumer/consumererror"
 	"go.opentelemetry.io/collector/pdata/plog"
 	"go.opentelemetry.io/collector/pdata/pmetric"
 	"go.opentelemetry.io/collector/pdata/ptrace"
@@ -30,10 +29,7 @@ type jaegerEncoder struct {
 var _ Encoder = (*jaegerEncoder)(nil)
 
 func (je jaegerEncoder) Traces(td ptrace.Traces) (*Batch, error) {
-	traces, err := jaeger.ProtoFromTraces(td)
-	if err != nil {
-		return nil, consumererror.NewTraces(err, td)
-	}
+	traces := jaeger.ProtoFromTraces(td)
 
 	bt := New(je.batchOptions...)
 

diff --git a/exporter/kafkaexporter/jaeger_marshaler.go b/exporter/kafkaexporter/jaeger_marshaler.go
@@ -22,10 +22,7 @@ type jaegerMarshaler struct {
 var _ TracesMarshaler = (*jaegerMarshaler)(nil)
 
 func (j jaegerMarshaler) Marshal(traces ptrace.Traces, topic string) ([]*sarama.ProducerMessage, error) {
-	batches, err := jaeger.ProtoFromTraces(traces)
-	if err != nil {
-		return nil, err
-	}
+	batches := jaeger.ProtoFromTraces(traces)
 	var messages []*sarama.ProducerMessage
 
 	var errs error

diff --git a/exporter/kafkaexporter/jaeger_marshaler_test.go b/exporter/kafkaexporter/jaeger_marshaler_test.go
@@ -25,8 +25,7 @@ func TestJaegerMarshaler(t *testing.T) {
 	span.SetEndTimestamp(pcommon.Timestamp(20))
 	span.SetTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})
 	span.SetSpanID([8]byte{1, 2, 3, 4, 5, 6, 7, 8})
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 
 	batches[0].Spans[0].Process = batches[0].Process
 	jaegerProtoBytes, err := batches[0].Spans[0].Marshal()

diff --git a/exporter/logzioexporter/exporter.go b/exporter/logzioexporter/exporter.go
@@ -182,10 +182,7 @@ func mergeMapEntries(maps ...pcommon.Map) pcommon.Map {
 func (exporter *logzioExporter) pushTraceData(ctx context.Context, traces ptrace.Traces) error {
 	// a buffer to store logzio span and services bytes
 	var dataBuffer bytes.Buffer
-	batches, err := jaeger.ProtoFromTraces(traces)
-	if err != nil {
-		return err
-	}
+	batches := jaeger.ProtoFromTraces(traces)
 	for _, batch := range batches {
 		for _, span := range batch.Spans {
 			span.Process = batch.Process
@@ -195,7 +192,7 @@ func (exporter *logzioExporter) pushTraceData(ctx context.Context, traces ptrace
 			if transformErr != nil {
 				return transformErr
 			}
-			_, err = dataBuffer.Write(append(logzioSpan, '\n'))
+			_, err := dataBuffer.Write(append(logzioSpan, '\n'))
 			if err != nil {
 				return err
 			}
@@ -220,7 +217,7 @@ func (exporter *logzioExporter) pushTraceData(ctx context.Context, traces ptrace
 			}
 		}
 	}
-	err = exporter.export(ctx, exporter.config.ClientConfig.Endpoint, dataBuffer.Bytes())
+	err := exporter.export(ctx, exporter.config.ClientConfig.Endpoint, dataBuffer.Bytes())
 	// reset the data buffer after each export to prevent duplicated data
 	dataBuffer.Reset()
 	return err

diff --git a/exporter/pulsarexporter/jaeger_marshaler.go b/exporter/pulsarexporter/jaeger_marshaler.go
@@ -22,10 +22,7 @@ type jaegerMarshaler struct {
 var _ TracesMarshaler = (*jaegerMarshaler)(nil)
 
 func (j jaegerMarshaler) Marshal(traces ptrace.Traces, _ string) ([]*pulsar.ProducerMessage, error) {
-	batches, err := jaeger.ProtoFromTraces(traces)
-	if err != nil {
-		return nil, err
-	}
+	batches := jaeger.ProtoFromTraces(traces)
 
 	var errs error
 	messages := make([]*pulsar.ProducerMessage, 0, len(batches))

diff --git a/exporter/pulsarexporter/jaeger_marshaler_test.go b/exporter/pulsarexporter/jaeger_marshaler_test.go
@@ -30,8 +30,7 @@ func buildTraces() ptrace.Traces {
 
 func TestJaegerJsonBatchMarshaler(t *testing.T) {
 	ptraces := buildTraces()
-	batches, err := jaeger.ProtoFromTraces(ptraces)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(ptraces)
 
 	jsonMarshaler := &jsonpb.Marshaler{}
 	buffer := new(bytes.Buffer)
@@ -48,8 +47,7 @@ func TestJaegerJsonBatchMarshaler(t *testing.T) {
 
 func TestJaegerProtoBatchMarshaler(t *testing.T) {
 	ptraces := buildTraces()
-	batches, err := jaeger.ProtoFromTraces(ptraces)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(ptraces)
 
 	jaegerProtoBytes, err := batches[0].Marshal()
 	require.NoError(t, err)

diff --git a/exporter/sapmexporter/exporter.go b/exporter/sapmexporter/exporter.go
@@ -96,10 +96,7 @@ func (se *sapmExporter) pushTraceData(ctx context.Context, td ptrace.Traces) err
 
 	accessToken := se.retrieveAccessToken(ctx, rss.At(0))
 
-	batches, err := jaeger.ProtoFromTraces(td)
-	if err != nil {
-		return consumererror.NewPermanent(err)
-	}
+	batches := jaeger.ProtoFromTraces(td)
 
 	// Cannot remove the access token from the pdata, because exporters required to not modify incoming pdata,
 	// so need to remove that after conversion.

diff --git a/exporter/sapmexporter/exporter_test.go b/exporter/sapmexporter/exporter_test.go
@@ -92,8 +92,7 @@ func TestFilterToken(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			traces := buildTestTraces(tt.useToken)
-			batches, err := jaeger.ProtoFromTraces(traces)
-			require.NoError(t, err)
+			batches := jaeger.ProtoFromTraces(traces)
 			assert.Equal(t, tt.useToken, hasToken(batches))
 			filterToken(batches)
 			assert.False(t, hasToken(batches))

diff --git a/extension/encoding/jaegerencodingextension/jaeger_test.go b/extension/encoding/jaegerencodingextension/jaeger_test.go
@@ -24,8 +24,7 @@ func TestUnmarshalJaeger(t *testing.T) {
 	span.SetEndTimestamp(pcommon.Timestamp(20))
 	span.SetTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})
 	span.SetSpanID([8]byte{1, 2, 3, 4, 5, 6, 7, 8})
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 
 	protoBytes, err := batches[0].Spans[0].Marshal()
 	require.NoError(t, err)

diff --git a/pkg/translator/jaeger/traces_to_jaegerproto.go b/pkg/translator/jaeger/traces_to_jaegerproto.go
@@ -15,11 +15,11 @@ import (
 
 // ProtoFromTraces translates internal trace data into the Jaeger Proto for GRPC.
 // Returns slice of translated Jaeger batches and error if translation failed.
-func ProtoFromTraces(td ptrace.Traces) ([]*model.Batch, error) {
+func ProtoFromTraces(td ptrace.Traces) []*model.Batch {
 	resourceSpans := td.ResourceSpans()
 
 	if resourceSpans.Len() == 0 {
-		return nil, nil
+		return nil
 	}
 
 	batches := make([]*model.Batch, 0, resourceSpans.Len())
@@ -31,7 +31,7 @@ func ProtoFromTraces(td ptrace.Traces) ([]*model.Batch, error) {
 		}
 	}
 
-	return batches, nil
+	return batches
 }
 
 func resourceSpansToJaegerProto(rs ptrace.ResourceSpans) *model.Batch {

diff --git a/pkg/translator/jaeger/traces_to_jaegerproto_test.go b/pkg/translator/jaeger/traces_to_jaegerproto_test.go
@@ -220,12 +220,10 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 		name string
 		td   ptrace.Traces
 		jb   *model.Batch
-		err  error
 	}{
 		{
 			name: "empty",
 			td:   ptrace.NewTraces(),
-			err:  nil,
 		},
 
 		{
@@ -234,13 +232,11 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 			jb: &model.Batch{
 				Process: generateProtoProcess(),
 			},
-			err: nil,
 		},
 
 		{
 			name: "no-resource-attrs",
 			td:   generateTracesResourceOnlyWithNoAttrs(),
-			err:  nil,
 		},
 
 		{
@@ -254,7 +250,6 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 					generateProtoSpanWithTraceState(),
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "library-info",
@@ -267,7 +262,6 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 					generateProtoSpanWithLibraryInfo("io.opentelemetry.test"),
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "two-spans-child-parent",
@@ -281,7 +275,6 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 					generateProtoChildSpan(),
 				},
 			},
-			err: nil,
 		},
 
 		{
@@ -296,7 +289,6 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 					generateProtoFollowerSpan(),
 				},
 			},
-			err: nil,
 		},
 
 		{
@@ -310,7 +302,6 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 					generateJProtoSpanWithEventAttribute(),
 				},
 			},
-			err: nil,
 		},
 		{
 			name: "a-spans-with-two-parent",
@@ -330,8 +321,7 @@ func TestInternalTracesToJaegerProto(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			jbs, err := ProtoFromTraces(test.td)
-			assert.EqualValues(t, test.err, err)
+			jbs := ProtoFromTraces(test.td)
 			if test.jb == nil {
 				assert.Empty(t, jbs)
 			} else {
@@ -348,8 +338,7 @@ func TestInternalTracesToJaegerProtoBatchesAndBack(t *testing.T) {
 		"../../../internal/coreinternal/goldendataset/testdata/generated_pict_pairs_spans.txt")
 	assert.NoError(t, err)
 	for _, td := range tds {
-		protoBatches, err := ProtoFromTraces(td)
-		assert.NoError(t, err)
+		protoBatches := ProtoFromTraces(td)
 		tdFromPB, err := ProtoToTraces(protoBatches)
 		assert.NoError(t, err)
 		assert.Equal(t, td.SpanCount(), tdFromPB.SpanCount())
@@ -388,7 +377,7 @@ func BenchmarkInternalTracesToJaegerProto(b *testing.B) {
 
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
-		_, err := ProtoFromTraces(td)
-		assert.NoError(b, err)
+		batches := ProtoFromTraces(td)
+		assert.NotEmpty(b, batches)
 	}
 }
diff --git a/receiver/jaegerreceiver/jaeger_agent_test.go b/receiver/jaegerreceiver/jaeger_agent_test.go
@@ -190,8 +190,7 @@ func testJaegerAgent(t *testing.T, agentEndpoint string, receiverConfig *configu
 
 	// 3. Now finally send some spans
 	td := generateTraceData()
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 	for _, batch := range batches {
 		require.NoError(t, jexp.EmitBatch(context.Background(), modelToThrift(batch)))
 	}

diff --git a/receiver/jaegerreceiver/trace_receiver_test.go b/receiver/jaegerreceiver/trace_receiver_test.go
@@ -95,8 +95,7 @@ func TestReception(t *testing.T) {
 	_, port, _ := net.SplitHostPort(addr)
 	collectorAddr := fmt.Sprintf("http://localhost:%s/api/traces", port)
 	td := generateTraceData()
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 	for _, batch := range batches {
 		require.NoError(t, sendToCollector(collectorAddr, modelToThrift(batch)))
 	}

diff --git a/receiver/kafkareceiver/jaeger_unmarshaler_test.go b/receiver/kafkareceiver/jaeger_unmarshaler_test.go
@@ -24,8 +24,7 @@ func TestUnmarshalJaeger(t *testing.T) {
 	span.SetEndTimestamp(pcommon.Timestamp(20))
 	span.SetTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})
 	span.SetSpanID([8]byte{1, 2, 3, 4, 5, 6, 7, 8})
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 
 	protoBytes, err := batches[0].Spans[0].Marshal()
 	require.NoError(t, err)

diff --git a/receiver/pulsarreceiver/jaeger_unmarshaler_test.go b/receiver/pulsarreceiver/jaeger_unmarshaler_test.go
@@ -25,8 +25,7 @@ func TestUnmarshalJaeger(t *testing.T) {
 	span.SetEndTimestamp(pcommon.Timestamp(20))
 	span.SetTraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})
 	span.SetSpanID([8]byte{1, 2, 3, 4, 5, 6, 7, 8})
-	batches, err := jaeger.ProtoFromTraces(td)
-	require.NoError(t, err)
+	batches := jaeger.ProtoFromTraces(td)
 
 	protoBytes, err := batches[0].Spans[0].Marshal()
 	require.NoError(t, err)

diff --git a/testbed/datasenders/jaeger.go b/testbed/datasenders/jaeger.go
@@ -17,7 +17,6 @@ import (
 	"go.opentelemetry.io/collector/config/configretry"
 	"go.opentelemetry.io/collector/config/configtls"
 	"go.opentelemetry.io/collector/consumer"
-	"go.opentelemetry.io/collector/consumer/consumererror"
 	"go.opentelemetry.io/collector/exporter"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 	"go.opentelemetry.io/collector/exporter/exportertest"
@@ -147,17 +146,14 @@ func (s *protoGRPCSender) pushTraces(
 	td ptrace.Traces,
 ) error {
 
-	batches, err := jaeger.ProtoFromTraces(td)
-	if err != nil {
-		return consumererror.NewPermanent(fmt.Errorf("failed to push trace data via Jaeger exporter: %w", err))
-	}
+	batches := jaeger.ProtoFromTraces(td)
 
 	if s.metadata.Len() > 0 {
 		ctx = metadata.NewOutgoingContext(ctx, s.metadata)
 	}
 
 	for _, batch := range batches {
-		_, err = s.client.PostSpans(
+		_, err := s.client.PostSpans(
 			ctx,
 			&jaegerproto.PostSpansRequest{Batch: *batch}, grpc.WaitForReady(s.waitForReady))