Merge pull request #26 from zalando/fix_error_attribute

Fix error attribute parsing and remove early returns

Author: silentz

internal/lightstep_common/common.go

@@ -2,6 +2,8 @@ package lightstep_common
 
 import (
 	"errors"
+
+	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
 
@@ -25,3 +27,14 @@ type ProjectTraces struct {
 type OtelTransformer interface {
 	ToOtel() (*ProjectTraces, error)
 }
+
+func IsErrorAttributeValueActuallyError(value pcommon.Value) bool {
+	switch value.Type() {
+	case pcommon.ValueTypeBool:
+		return value.Bool()
+	case pcommon.ValueTypeStr:
+		return value.AsString() == "true"
+	default:
+		return false
+	}
+}

internal/lightstep_pb/transform.go

@@ -3,14 +3,15 @@ package lightstep_pb
 import (
 	"context"
 	"encoding/binary"
+	"strings"
+	"time"
+	"unicode/utf8"
+
 	lightstepConstants "github.com/lightstep/lightstep-tracer-go/constants"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.opentelemetry.io/otel/codes"
 	"go.uber.org/zap"
-	"strings"
-	"time"
-	"unicode/utf8"
 
 	lightstepCommon "github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_common"
 	pb "github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_pb/collectorpb"
@@ -54,26 +55,27 @@ func (r *Request) ToOtel(ctx context.Context) (*lightstepCommon.ProjectTraces, e
 
 	if r.orig.Auth == nil || r.orig.Auth.AccessToken == "" {
 		span.SetStatus(codes.Error, lightstepCommon.ErrNoAccessToken.Error())
-		return result, lightstepCommon.ErrNoAccessToken
+	} else {
+		result.AccessToken = r.orig.Auth.AccessToken
 	}
-	result.AccessToken = r.orig.Auth.AccessToken
 
 	data := ptrace.NewTraces()
 	rs := data.ResourceSpans().AppendEmpty()
 	rAttr := rs.Resource().Attributes()
 
 	nonUtf8Keys, err := r.kvToAttr(r.orig.Reporter.Tags, &rAttr)
-	serviceName, ok := rAttr.Get(lightstepConstants.ComponentNameKey)
-	result.ServiceName = serviceName.Str()
-	if !ok {
-		return result, lightstepCommon.ErrNoServiceName
-	}
 	if err != nil {
 		span.SetStatus(codes.Error, "non-utf8-keys")
 		r.reportNonUtf8(result.ServiceName, nonUtf8Keys)
 	}
 
-	rAttr.PutStr("service.name", serviceName.Str())
+	serviceName, ok := rAttr.Get(lightstepConstants.ComponentNameKey)
+	if ok {
+		result.ServiceName = serviceName.Str()
+		rAttr.PutStr("service.name", serviceName.Str())
+	} else {
+		span.SetStatus(codes.Error, lightstepCommon.ErrNoServiceName.Error())
+	}
 
 	if r.orig.InternalMetrics != nil {
 		for _, m := range r.orig.InternalMetrics.Counts {
@@ -106,9 +108,11 @@ func (r *Request) ToOtel(ctx context.Context) (*lightstepCommon.ProjectTraces, e
 			r.reportNonUtf8(result.ServiceName, nonUtf8Keys)
 		}
 
-		if _, ok := attr.Get("error"); ok {
-			s.Status().SetCode(ptrace.StatusCodeError)
-			attr.Remove("error")
+		if value, ok := attr.Get("error"); ok {
+			if lightstepCommon.IsErrorAttributeValueActuallyError(value) {
+				s.Status().SetCode(ptrace.StatusCodeError)
+				attr.Remove("error")
+			}
 		}
 
 		for _, log := range span.Logs {

internal/lightstep_pb/transform_test.go

@@ -2,6 +2,8 @@ package lightstep_pb
 
 import (
 	"context"
+	"testing"
+
 	"github.com/golang/protobuf/ptypes/timestamp"
 	"github.com/matryer/is"
 	"go.opentelemetry.io/collector/component"
@@ -10,9 +12,7 @@ import (
 	"go.opentelemetry.io/collector/receiver"
 	"go.opentelemetry.io/otel/trace/noop"
 	"go.uber.org/zap"
-	"testing"
 
-	lightstepCommon "github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_common"
 	pb "github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_pb/collectorpb"
 	"github.com/zalando/otelcol-lightstep-receiver/internal/telemetry"
 )
@@ -181,7 +181,7 @@ func TestTransformation_NoServiceName(t *testing.T) {
 		telemetry: initTelemetry(),
 	}
 	_, err := rq.ToOtel(context.Background())
-	is.Equal(err, lightstepCommon.ErrNoServiceName)
+	is.Equal(err, nil)
 }
 
 func TestTransformation_NoAccessToken(t *testing.T) {
@@ -198,7 +198,7 @@ func TestTransformation_NoAccessToken(t *testing.T) {
 		telemetry: initTelemetry(),
 	}
 	_, err := rq.ToOtel(context.Background())
-	is.Equal(err, lightstepCommon.ErrNoAccessToken)
+	is.Equal(err, nil)
 }
 
 func TestTransformation_GetClientDropSpans(t *testing.T) {

internal/lightstep_thrift/transform.go

@@ -3,13 +3,14 @@ package lightstep_thrift
 import (
 	"context"
 	"encoding/hex"
+	"strings"
+	"time"
+
 	lightstepConstants "github.com/lightstep/lightstep-tracer-go/constants"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.opentelemetry.io/otel/codes"
 	"go.uber.org/zap"
-	"strings"
-	"time"
 
 	lightstepCommon "github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_common"
 	"github.com/zalando/otelcol-lightstep-receiver/internal/lightstep_thrift/collectorthrift"
@@ -47,22 +48,23 @@ func (tr *Request) ToOtel(ctx context.Context) (*lightstepCommon.ProjectTraces,
 	result := &lightstepCommon.ProjectTraces{}
 
 	if tr.auth == nil || tr.auth.AccessToken == nil {
-		return result, lightstepCommon.ErrNoAccessToken
+		span.SetStatus(codes.Error, lightstepCommon.ErrNoAccessToken.Error())
+	} else {
+		result.AccessToken = tr.auth.GetAccessToken()
 	}
-	result.AccessToken = tr.auth.GetAccessToken()
 
 	data := ptrace.NewTraces()
 	rs := data.ResourceSpans().AppendEmpty()
 	rAttr := rs.Resource().Attributes()
 
 	tr.kvToAttr(tr.orig.Runtime.Attrs, &rAttr)
 	serviceName, ok := rAttr.Get(lightstepConstants.ComponentNameKey)
-	result.ServiceName = serviceName.Str()
-	if !ok {
+	if ok {
+		result.ServiceName = serviceName.Str()
+		rAttr.PutStr("service.name", serviceName.Str())
+	} else {
 		span.SetStatus(codes.Error, lightstepCommon.ErrNoServiceName.Error())
-		return result, lightstepCommon.ErrNoServiceName
 	}
-	rAttr.PutStr("service.name", serviceName.Str())
 
 	if tr.orig.InternalMetrics != nil {
 		for _, m := range tr.orig.InternalMetrics.Counts {
@@ -92,9 +94,11 @@ func (tr *Request) ToOtel(ctx context.Context) (*lightstepCommon.ProjectTraces,
 			attr.Remove("parent_span_guid")
 		}
 
-		if _, ok := attr.Get("error"); ok {
-			s.Status().SetCode(ptrace.StatusCodeError)
-			attr.Remove("error")
+		if value, ok := attr.Get("error"); ok {
+			if lightstepCommon.IsErrorAttributeValueActuallyError(value) {
+				s.Status().SetCode(ptrace.StatusCodeError)
+				attr.Remove("error")
+			}
 		}
 
 		for _, log := range span.LogRecords {