breaking(zipkin): removes serviceName from zipkin exporter.

CHANGELOG.md

@@ -22,6 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - `"go.opentelemetry.io/sdk/metric/controller.basic".WithPusher` is replaced with `WithExporter` to provide consistent naming across project. (#1656)
 - Added non-empty string check for trace `Attribute` keys. (#1659)
 - Add `description` to SpanStatus only when `StatusCode` is set to error. (#1662)
+- Removes `serviceName` parameter from Zipkin exporter and uses resource instead. (#1549)
 
 ### Removed
 

exporters/trace/zipkin/model.go

@@ -23,6 +23,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/semconv"
 	"go.opentelemetry.io/otel/trace"
 )
 
@@ -31,15 +32,26 @@ const (
 	keyInstrumentationLibraryVersion = "otel.instrumentation_library.version"
 )
 
-func toZipkinSpanModels(batch []*export.SpanSnapshot, serviceName string) []zkmodel.SpanModel {
+func toZipkinSpanModels(batch []*export.SpanSnapshot) []zkmodel.SpanModel {
 	models := make([]zkmodel.SpanModel, 0, len(batch))
 	for _, data := range batch {
-		models = append(models, toZipkinSpanModel(data, serviceName))
+		models = append(models, toZipkinSpanModel(data))
 	}
 	return models
 }
 
-func toZipkinSpanModel(data *export.SpanSnapshot, serviceName string) zkmodel.SpanModel {
+func getServiceName(attrs []attribute.KeyValue) string {
+	for _, kv := range attrs {
+		if kv.Key == semconv.ServiceNameKey {
+			return kv.Value.AsString()
+		}
+	}
+
+	// Resource holds a default value so this might not be reach.
+	return ""
+}
+
+func toZipkinSpanModel(data *export.SpanSnapshot) zkmodel.SpanModel {
 	return zkmodel.SpanModel{
 		SpanContext: toZipkinSpanContext(data),
 		Name:        data.Name,
@@ -48,7 +60,7 @@ func toZipkinSpanModel(data *export.SpanSnapshot, serviceName string) zkmodel.Sp
 		Duration:    data.EndTime.Sub(data.StartTime),
 		Shared:      false,
 		LocalEndpoint: &zkmodel.Endpoint{
-			ServiceName: serviceName,
+			ServiceName: getServiceName(data.Resource.Attributes()),
 		},
 		RemoteEndpoint: nil, // *Endpoint
 		Annotations:    toZipkinAnnotations(data.MessageEvents),

exporters/trace/zipkin/model_test.go

@@ -22,16 +22,23 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	zkmodel "github.com/openzipkin/zipkin-go/model"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
 	"go.opentelemetry.io/otel/sdk/instrumentation"
+	"go.opentelemetry.io/otel/sdk/resource"
+	"go.opentelemetry.io/otel/semconv"
 	"go.opentelemetry.io/otel/trace"
 )
 
 func TestModelConversion(t *testing.T) {
+	resource := resource.NewWithAttributes(
+		semconv.ServiceNameKey.String("model-test"),
+	)
+
 	inputBatch := []*export.SpanSnapshot{
 		// typical span data
 		{
@@ -64,6 +71,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data with no parent (same as typical, but has
 		// invalid parent)
@@ -97,6 +105,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data of unspecified kind
 		{
@@ -129,6 +138,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data of internal kind
 		{
@@ -161,6 +171,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data of client kind
 		{
@@ -193,6 +204,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data of producer kind
 		{
@@ -225,6 +237,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data of consumer kind
 		{
@@ -257,6 +270,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data with no events
 		{
@@ -276,6 +290,7 @@ func TestModelConversion(t *testing.T) {
 			MessageEvents: nil,
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// span data with an "error" attribute set to "false"
 		{
@@ -307,6 +322,7 @@ func TestModelConversion(t *testing.T) {
 			},
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 	}
 
@@ -652,7 +668,7 @@ func TestModelConversion(t *testing.T) {
 			},
 		},
 	}
-	gottenOutputBatch := toZipkinSpanModels(inputBatch, "model-test")
+	gottenOutputBatch := toZipkinSpanModels(inputBatch)
 	require.Equal(t, expectedOutputBatch, gottenOutputBatch)
 }
 
@@ -780,3 +796,14 @@ func Test_toZipkinTags(t *testing.T) {
 		})
 	}
 }
+
+func TestServiceName(t *testing.T) {
+	attrs := []attribute.KeyValue{}
+	assert.Empty(t, getServiceName(attrs))
+
+	attrs = append(attrs, attribute.String("test_key", "test_value"))
+	assert.Empty(t, getServiceName(attrs))
+
+	attrs = append(attrs, semconv.ServiceNameKey.String("my_service"))
+	assert.Equal(t, "my_service", getServiceName(attrs))
+}

exporters/trace/zipkin/zipkin.go

@@ -36,11 +36,10 @@ import (
 // the SpanBatcher interface, so it needs to be used together with the
 // WithBatcher option when setting up the exporter pipeline.
 type Exporter struct {
-	url         string
-	serviceName string
-	client      *http.Client
-	logger      *log.Logger
-	o           options
+	url    string
+	client *http.Client
+	logger *log.Logger
+	o      options
 
 	stoppedMu sync.RWMutex
 	stopped   bool
@@ -82,7 +81,7 @@ func WithSDK(config *sdktrace.Config) Option {
 }
 
 // NewRawExporter creates a new Zipkin exporter.
-func NewRawExporter(collectorURL, serviceName string, opts ...Option) (*Exporter, error) {
+func NewRawExporter(collectorURL string, opts ...Option) (*Exporter, error) {
 	if collectorURL == "" {
 		return nil, errors.New("collector URL cannot be empty")
 	}
@@ -102,18 +101,17 @@ func NewRawExporter(collectorURL, serviceName string, opts ...Option) (*Exporter
 		o.client = http.DefaultClient
 	}
 	return &Exporter{
-		url:         collectorURL,
-		client:      o.client,
-		logger:      o.logger,
-		serviceName: serviceName,
-		o:           o,
+		url:    collectorURL,
+		client: o.client,
+		logger: o.logger,
+		o:      o,
 	}, nil
 }
 
 // NewExportPipeline sets up a complete export pipeline
 // with the recommended setup for trace provider
-func NewExportPipeline(collectorURL, serviceName string, opts ...Option) (*sdktrace.TracerProvider, error) {
-	exporter, err := NewRawExporter(collectorURL, serviceName, opts...)
+func NewExportPipeline(collectorURL string, opts ...Option) (*sdktrace.TracerProvider, error) {
+	exporter, err := NewRawExporter(collectorURL, opts...)
 	if err != nil {
 		return nil, err
 	}
@@ -128,8 +126,8 @@ func NewExportPipeline(collectorURL, serviceName string, opts ...Option) (*sdktr
 
 // InstallNewPipeline instantiates a NewExportPipeline with the
 // recommended configuration and registers it globally.
-func InstallNewPipeline(collectorURL, serviceName string, opts ...Option) error {
-	tp, err := NewExportPipeline(collectorURL, serviceName, opts...)
+func InstallNewPipeline(collectorURL string, opts ...Option) error {
+	tp, err := NewExportPipeline(collectorURL, opts...)
 	if err != nil {
 		return err
 	}
@@ -152,7 +150,7 @@ func (e *Exporter) ExportSpans(ctx context.Context, ss []*export.SpanSnapshot) e
 		e.logf("no spans to export")
 		return nil
 	}
-	models := toZipkinSpanModels(ss, e.serviceName)
+	models := toZipkinSpanModels(ss)
 	body, err := json.Marshal(models)
 	if err != nil {
 		return e.errf("failed to serialize zipkin models to JSON: %v", err)

exporters/trace/zipkin/zipkin_test.go

@@ -33,19 +33,19 @@ import (
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/codes"
 	export "go.opentelemetry.io/otel/sdk/export/trace"
+	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/semconv"
 	"go.opentelemetry.io/otel/trace"
 )
 
 const (
 	collectorURL = "http://localhost:9411/api/v2/spans"
-	serviceName  = "zipkin-test"
 )
 
 func TestInstallNewPipeline(t *testing.T) {
 	err := InstallNewPipeline(
 		collectorURL,
-		serviceName,
 	)
 	assert.NoError(t, err)
 	assert.IsType(t, &sdktrace.TracerProvider{}, otel.GetTracerProvider())
@@ -86,7 +86,6 @@ func TestNewExportPipeline(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			tp, err := NewExportPipeline(
 				collectorURL,
-				serviceName,
 				tc.options...,
 			)
 			assert.NoError(t, err)
@@ -103,13 +102,11 @@ func TestNewExportPipeline(t *testing.T) {
 }
 
 func TestNewRawExporter(t *testing.T) {
-	exp, err := NewRawExporter(
+	_, err := NewRawExporter(
 		collectorURL,
-		serviceName,
 	)
 
 	assert.NoError(t, err)
-	assert.EqualValues(t, serviceName, exp.serviceName)
 }
 
 func TestNewRawExporterShouldFailInvalidCollectorURL(t *testing.T) {
@@ -121,7 +118,6 @@ func TestNewRawExporterShouldFailInvalidCollectorURL(t *testing.T) {
 	// cannot be empty
 	exp, err = NewRawExporter(
 		"",
-		serviceName,
 	)
 
 	assert.Error(t, err)
@@ -131,7 +127,6 @@ func TestNewRawExporterShouldFailInvalidCollectorURL(t *testing.T) {
 	// invalid URL
 	exp, err = NewRawExporter(
 		"localhost",
-		serviceName,
 	)
 
 	assert.Error(t, err)
@@ -239,6 +234,10 @@ func logStoreLogger(s *logStore) *log.Logger {
 }
 
 func TestExportSpans(t *testing.T) {
+	resource := resource.NewWithAttributes(
+		semconv.ServiceNameKey.String("exporter-test"),
+	)
+
 	spans := []*export.SpanSnapshot{
 		// parent
 		{
@@ -255,6 +254,7 @@ func TestExportSpans(t *testing.T) {
 			MessageEvents: nil,
 			StatusCode:    codes.Error,
 			StatusMessage: "404, file not found",
+			Resource:      resource,
 		},
 		// child
 		{
@@ -271,6 +271,7 @@ func TestExportSpans(t *testing.T) {
 			MessageEvents: nil,
 			StatusCode:    codes.Error,
 			StatusMessage: "403, forbidden",
+			Resource:      resource,
 		},
 	}
 	models := []zkmodel.SpanModel{
@@ -336,7 +337,7 @@ func TestExportSpans(t *testing.T) {
 	defer collector.Close()
 	ls := &logStore{T: t}
 	logger := logStoreLogger(ls)
-	exporter, err := NewRawExporter(collector.url, "exporter-test", WithLogger(logger))
+	exporter, err := NewRawExporter(collector.url, WithLogger(logger))
 	require.NoError(t, err)
 	ctx := context.Background()
 	require.Len(t, ls.Messages, 0)
@@ -361,7 +362,7 @@ func TestExporterShutdownHonorsTimeout(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()
 
-	exp, err := NewRawExporter(collectorURL, serviceName)
+	exp, err := NewRawExporter(collectorURL)
 	require.NoError(t, err)
 
 	innerCtx, innerCancel := context.WithTimeout(ctx, time.Nanosecond)
@@ -374,7 +375,7 @@ func TestExporterShutdownHonorsCancel(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()
 
-	exp, err := NewRawExporter(collectorURL, serviceName)
+	exp, err := NewRawExporter(collectorURL)
 	require.NoError(t, err)
 
 	innerCtx, innerCancel := context.WithCancel(ctx)
@@ -383,7 +384,7 @@ func TestExporterShutdownHonorsCancel(t *testing.T) {
 }
 
 func TestErrorOnExportShutdownExporter(t *testing.T) {
-	exp, err := NewRawExporter(collectorURL, serviceName)
+	exp, err := NewRawExporter(collectorURL)
 	require.NoError(t, err)
 	assert.NoError(t, exp.Shutdown(context.Background()))
 	assert.NoError(t, exp.ExportSpans(context.Background(), nil))