From 679374a8656d39b5a38515c7d5fdaf708a999e4c Mon Sep 17 00:00:00 2001 From: Antoine Toulme Date: Mon, 7 Oct 2024 18:33:39 -0700 Subject: [PATCH] simplify the signature of ProtoFromTraces as it never returns an error (#35560) **Description:** The `ProtoFromTraces` returns an error but never sets it to a value besides nil. Removing it from the signature simplifies code downstream. --- .chloggen/ProtoFromTracesError.yaml | 27 +++++++++++++++++++ .../internal/batch/encode_jaeger.go | 6 +---- exporter/kafkaexporter/jaeger_marshaler.go | 5 +--- .../kafkaexporter/jaeger_marshaler_test.go | 3 +-- exporter/logzioexporter/exporter.go | 9 +++---- exporter/pulsarexporter/jaeger_marshaler.go | 5 +--- .../pulsarexporter/jaeger_marshaler_test.go | 6 ++--- exporter/sapmexporter/exporter.go | 5 +--- exporter/sapmexporter/exporter_test.go | 3 +-- .../jaegerencodingextension/jaeger_test.go | 3 +-- .../jaeger/traces_to_jaegerproto.go | 6 ++--- .../jaeger/traces_to_jaegerproto_test.go | 19 +++---------- receiver/jaegerreceiver/jaeger_agent_test.go | 3 +-- .../jaegerreceiver/trace_receiver_test.go | 3 +-- .../kafkareceiver/jaeger_unmarshaler_test.go | 3 +-- .../pulsarreceiver/jaeger_unmarshaler_test.go | 3 +-- testbed/datasenders/jaeger.go | 8 ++---- 17 files changed, 52 insertions(+), 65 deletions(-) create mode 100644 .chloggen/ProtoFromTracesError.yaml diff --git a/.chloggen/ProtoFromTracesError.yaml b/.chloggen/ProtoFromTracesError.yaml new file mode 100644 index 000000000000..134bd861706d --- /dev/null +++ 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 index 42131ee0ea3d..50f4662a0575 100644 --- 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 index abc73c22f18a..d6d6beb643c3 100644 --- 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 index 81a310c4a353..ca4cd7e7440e 100644 --- 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 index 3254084ba8ce..297c3eafdcb3 100644 --- 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 index 79a927a12f2e..786140d2c67d 100644 --- 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 index e4fb6d06e948..8b0eb9bec1ca 100644 --- 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 index 9893d1c189a8..305bbf956123 100644 --- 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 index d9d863eaeb13..8d0953120ca0 100644 --- 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 index 674b0c0ba82f..a1888a434c36 100644 --- 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 index 5ece80c2e6b1..db79dac374b4 100644 --- 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 index 96cc4bdf1982..672d97c6f320 100644 --- 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 index 93fa35c85888..3a80c3c1804c 100644 --- 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 index c3571fb59f8c..dc10c767c5e2 100644 --- 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 index 0ef0af468ed1..3af93c6ca398 100644 --- 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 index 70849f70ba59..c8446637cf4f 100644 --- 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 index 6e3bedb76543..a8d6a49286ac 100644 --- 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))