breaking(zipkin): removes servicName from zipkin exporter. (#1697)

* breaking(zipkin): removes servicName from zipkin exporter. Resource detector provides a serviceName in all cases, hence we can relay on span resource to obtain the serviceName. Also this is required by the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/zipkin.md\#service-name (#1549). * docs(zipkin): adds changelog. * chore(examples/zipkin): updates example accordingly. Co-authored-by: Anthony Mirabella <[email protected]>
ljmsc · Mar 16, 2021 · 345f264 · 345f264
1 parent 62cbf0f
commit 345f264
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
  These are now returned as a SpanProcessor interface from their respective constructors. (#1638)
 - Removed setting status to `Error` while recording an error as a span event in `RecordError`. (#1663)
 - Removed `WithConfig` from tracer provider to avoid overriding configuration. (#1633)
+- Removed `serviceName` parameter from Zipkin exporter and uses resource instead. (#1549)
 - Removed `jaeger.WithProcess`. (#1673)
 
 ### Fixed

diff --git a/example/zipkin/main.go b/example/zipkin/main.go
@@ -25,7 +25,9 @@ import (
 
  "go.opentelemetry.io/otel"
  "go.opentelemetry.io/otel/exporters/trace/zipkin"
+ "go.opentelemetry.io/otel/sdk/resource"
  sdktrace "go.opentelemetry.io/otel/sdk/trace"
+ "go.opentelemetry.io/otel/semconv"
  "go.opentelemetry.io/otel/trace"
 )
 
@@ -40,7 +42,6 @@ func initTracer(url string) func() {
  // ratio.
  exporter, err := zipkin.NewRawExporter(
  url,
- "zipkin-test",
  zipkin.WithLogger(logger),
  zipkin.WithSDK(&sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
  )
@@ -50,7 +51,12 @@ func initTracer(url string) func() {
 
  batcher := sdktrace.NewBatchSpanProcessor(exporter)
 
- tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(batcher))
+ tp := sdktrace.NewTracerProvider(
+ sdktrace.WithSpanProcessor(batcher),
+ sdktrace.WithResource(resource.NewWithAttributes(
+ semconv.ServiceNameKey.String("zipkin-test"),
+ )),
+ )
  otel.SetTracerProvider(tp)
 
  return func() {

diff --git a/exporters/trace/zipkin/model.go b/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),

diff --git a/exporters/trace/zipkin/model_test.go b/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))
+}
diff --git a/exporters/trace/zipkin/zipkin.go b/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)