Require non-empty values in SetDestinationService

SetDestinationService must not send an empty Name if a
non-empty Resource is specified (and vice versa), as
this will lead to a validation error in apm-server.

Change SetDestinationService to set neither if either
one is empty.

Author: axw

spancontext.go

@@ -197,7 +197,13 @@ func (c *SpanContext) SetDestinationAddress(addr string, port int) {
 }
 
 // SetDestinationService sets the destination service info in the context.
+//
+// Both service.Name and service.Resource are required. If either is empty,
+// then SetDestinationService is a no-op.
 func (c *SpanContext) SetDestinationService(service DestinationServiceSpanContext) {
+	if service.Name == "" || service.Resource == "" {
+		return
+	}
 	c.destinationService.Name = truncateString(service.Name)
 	c.destinationService.Resource = truncateString(service.Resource)
 	c.destination.Service = &c.destinationService

spancontext_test.go

@@ -19,6 +19,7 @@ package apm_test
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/url"
 	"testing"
@@ -122,3 +123,52 @@ func TestSpanContextSetHTTPRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestSetDestinationService(t *testing.T) {
+	type testcase struct {
+		name        string
+		resource    string
+		expectEmpty bool
+	}
+
+	testcases := []testcase{{
+		name:        "",
+		resource:    "",
+		expectEmpty: true,
+	}, {
+		name:        "",
+		resource:    "nonempty",
+		expectEmpty: true,
+	}, {
+		name:        "nonempty",
+		resource:    "",
+		expectEmpty: true,
+	}, {
+		name:     "nonempty",
+		resource: "nonempty",
+	}}
+
+	for _, tc := range testcases {
+		t.Run(fmt.Sprintf("%s_%s", tc.name, tc.resource), func(t *testing.T) {
+			_, spans, _ := apmtest.WithTransaction(func(ctx context.Context) {
+				span, _ := apm.StartSpan(ctx, "name", "span_type")
+				span.Context.SetDestinationAddress("testing.invalid", 123)
+				span.Context.SetDestinationService(apm.DestinationServiceSpanContext{
+					Name:     tc.name,
+					Resource: tc.resource,
+				})
+				span.End()
+			})
+			require.Len(t, spans, 1)
+			if tc.expectEmpty {
+				assert.Nil(t, spans[0].Context.Destination.Service)
+			} else {
+				assert.Equal(t, &model.DestinationServiceSpanContext{
+					Name:     tc.name,
+					Resource: tc.resource,
+					Type:     "span_type",
+				}, spans[0].Context.Destination.Service)
+			}
+		})
+	}
+}

validation_test.go

@@ -118,13 +118,18 @@ func TestValidateDatabaseSpanContext(t *testing.T) {
 }
 
 func TestValidateDestinationSpanContext(t *testing.T) {
-	validateSpan(t, func(s *apm.Span) {
-		s.Context.SetDestinationAddress(strings.Repeat("x", 1025), 0)
-		s.Context.SetDestinationService(apm.DestinationServiceSpanContext{
-			Name:     strings.Repeat("x", 1025),
-			Resource: strings.Repeat("x", 1025),
-		})
-	})
+	overlong := strings.Repeat("x", 1025)
+	for _, name := range []string{"", overlong} {
+		for _, resource := range []string{"", overlong} {
+			validateSpan(t, func(s *apm.Span) {
+				s.Context.SetDestinationAddress(overlong, 0)
+				s.Context.SetDestinationService(apm.DestinationServiceSpanContext{
+					Name:     name,
+					Resource: resource,
+				})
+			})
+		}
+	}
 }
 
 func TestValidateContextUser(t *testing.T) {