diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c84c02acd2..a57e7a48bd3 100644 --- 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 index b3a09970919..b9cf0ca6f80 100644 --- 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 index 38813cad97e..26df037ff72 100644 --- 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 index f41826f0608..e4575728560 100644 --- 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 index 63b3a3e9d9f..9da317ee709 100644 --- 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) diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index 30ba5469342..fce98f033e4 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/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))