zipkinv1 implement directly Unmarshaler interface (#3504)

Signed-off-by: Bogdan Drutu <[email protected]>

Author: bogdandrutu

translator/trace/zipkinv1/json.go

@@ -40,28 +40,31 @@ var (
 	msgZipkinV1SpanIDError        = "zipkinV1 span id"
 	msgZipkinV1ParentIDError      = "zipkinV1 span parentId"
 	// Generic hex to ID conversion errors
-	errHexTraceIDWrongLen                     = errors.New("hex traceId span has wrong length (expected 16 or 32)")
-	errHexTraceIDParsing                      = errors.New("failed to parse hex traceId")
-	errHexTraceIDZero                         = errors.New("traceId is zero")
-	errHexIDWrongLen                          = errors.New("hex Id has wrong length (expected 16)")
-	errHexIDParsing                           = errors.New("failed to parse hex Id")
-	errHexIDZero                              = errors.New("ID is zero")
-	_                     pdata.TracesDecoder = (*jsonDecoder)(nil)
+	errHexTraceIDWrongLen = errors.New("hex traceId span has wrong length (expected 16 or 32)")
+	errHexTraceIDParsing  = errors.New("failed to parse hex traceId")
+	errHexTraceIDZero     = errors.New("traceId is zero")
+	errHexIDWrongLen      = errors.New("hex Id has wrong length (expected 16)")
+	errHexIDParsing       = errors.New("failed to parse hex Id")
+	errHexIDZero          = errors.New("ID is zero")
 )
 
-type jsonDecoder struct {
+type jsonUnmarshaler struct {
 	// ParseStringTags should be set to true if tags should be converted to numbers when possible.
 	ParseStringTags bool
 }
 
-// DecodeTraces from JSON bytes.
-func (j jsonDecoder) DecodeTraces(buf []byte) (interface{}, error) {
-	return v1JSONBatchToOCProto(buf, j.ParseStringTags)
+// UnmarshalTraces from JSON bytes.
+func (j jsonUnmarshaler) UnmarshalTraces(buf []byte) (pdata.Traces, error) {
+	tds, err := v1JSONBatchToOCProto(buf, j.ParseStringTags)
+	if err != nil {
+		return pdata.Traces{}, err
+	}
+	return toTraces(tds)
 }
 
 // NewJSONTracesUnmarshaler returns an unmarshaler for Zipkin JSON.
 func NewJSONTracesUnmarshaler(parseStringTags bool) pdata.TracesUnmarshaler {
-	return pdata.NewTracesUnmarshaler(jsonDecoder{ParseStringTags: parseStringTags}, toTranslator{})
+	return jsonUnmarshaler{ParseStringTags: parseStringTags}
 }
 
 // Trace translation from Zipkin V1 is a bit of special case since there is no model

translator/trace/zipkinv1/json_test.go

@@ -38,26 +38,17 @@ func TestSingleJSONV1BatchToTraces(t *testing.T) {
 	blob, err := ioutil.ReadFile("./testdata/zipkin_v1_single_batch.json")
 	require.NoError(t, err, "Failed to load test data")
 
-	got, err := jsonDecoder{ParseStringTags: false}.DecodeTraces(blob)
+	td, err := NewJSONTracesUnmarshaler(false).UnmarshalTraces(blob)
 	require.NoError(t, err, "Failed to translate zipkinv1 to OC proto")
-
-	td := got.([]traceData)
-	spanCount := 0
-
-	for _, data := range td {
-		spanCount += len(data.Spans)
-	}
-
-	assert.Equal(t, 5, spanCount)
+	assert.Equal(t, 5, td.SpanCount())
 }
 
 func TestErrorSpanToTraces(t *testing.T) {
 	blob, err := ioutil.ReadFile("./testdata/zipkin_v1_error_batch.json")
 	require.NoError(t, err, "Failed to load test data")
 
-	got, err := jsonDecoder{ParseStringTags: false}.DecodeTraces(blob)
+	_, err = NewJSONTracesUnmarshaler(false).UnmarshalTraces(blob)
 	assert.Error(t, err, "Should have generated error")
-	assert.Nil(t, got)
 }
 
 func Test_hexIDToOCID(t *testing.T) {

translator/trace/zipkinv1/thrift.go

@@ -32,22 +32,24 @@ import (
 	"go.opentelemetry.io/collector/model/pdata"
 )
 
-var _ pdata.TracesDecoder = (*thriftDecoder)(nil)
+type thriftUnmarshaler struct{}
 
-type thriftDecoder struct{}
-
-// DecodeTraces from Thrift bytes.
-func (t thriftDecoder) DecodeTraces(buf []byte) (interface{}, error) {
+// UnmarshalTraces from Thrift bytes.
+func (t thriftUnmarshaler) UnmarshalTraces(buf []byte) (pdata.Traces, error) {
 	spans, err := jaegerzipkin.DeserializeThrift(buf)
 	if err != nil {
-		return nil, err
+		return pdata.Traces{}, err
+	}
+	tds, err := v1ThriftBatchToOCProto(spans)
+	if err != nil {
+		return pdata.Traces{}, err
 	}
-	return v1ThriftBatchToOCProto(spans)
+	return toTraces(tds)
 }
 
 // NewThriftTracesUnmarshaler returns an unmarshaler for Zipkin Thrift.
 func NewThriftTracesUnmarshaler() pdata.TracesUnmarshaler {
-	return pdata.NewTracesUnmarshaler(thriftDecoder{}, toTranslator{})
+	return thriftUnmarshaler{}
 }
 
 // v1ThriftBatchToOCProto converts Zipkin v1 spans to OC Proto.

translator/trace/zipkinv1/thrift_test.go

@@ -51,17 +51,9 @@ func TestV1ThriftToTraces(t *testing.T) {
 	require.NoError(t, json.Unmarshal(blob, &zSpans), "failed to unmarshal json test file")
 	thriftBytes := zipkin.SerializeThrift(zSpans)
 
-	got, err := thriftDecoder{}.DecodeTraces(thriftBytes)
+	td, err := thriftUnmarshaler{}.UnmarshalTraces(thriftBytes)
 	require.NoError(t, err, "Failed to translate zipkinv1 thrift to OC proto")
-
-	td := got.([]traceData)
-	spanCount := 0
-
-	for _, data := range td {
-		spanCount += len(data.Spans)
-	}
-
-	assert.Equal(t, 5, spanCount)
+	assert.Equal(t, 5, td.SpanCount())
 }
 
 func TestZipkinThriftFallbackToLocalComponent(t *testing.T) {

translator/trace/zipkinv1/to_translator.go

@@ -19,17 +19,7 @@ import (
 	"go.opentelemetry.io/collector/translator/internaldata"
 )
 
-var _ pdata.ToTracesTranslator = (*toTranslator)(nil)
-
-type toTranslator struct{}
-
-// ToTraces converts converts traceData to pdata.Traces.
-func (t toTranslator) ToTraces(src interface{}) (pdata.Traces, error) {
-	ocTraces, ok := src.([]traceData)
-	if !ok {
-		return pdata.Traces{}, pdata.NewErrIncompatibleType([]traceData{}, src)
-	}
-
+func toTraces(ocTraces []traceData) (pdata.Traces, error) {
 	td := pdata.NewTraces()
 
 	for _, trace := range ocTraces {