From cf7c4e53282f1b41215dcdfdb9d56360587a3823 Mon Sep 17 00:00:00 2001 From: Oleg Shatnyuk <36113375+hinch5@users.noreply.github.com> Date: Sun, 19 Apr 2020 09:02:29 +0300 Subject: [PATCH] fix zipkin without local endpoint with service name (#644) --- example/zipkin/main.go | 1 + exporters/trace/zipkin/exporter_test.go | 32 +++--- exporters/trace/zipkin/model.go | 22 ++-- exporters/trace/zipkin/model_test.go | 128 ++++++++++++++---------- exporters/trace/zipkin/zipkin.go | 21 ++-- 5 files changed, 118 insertions(+), 86 deletions(-) diff --git a/example/zipkin/main.go b/example/zipkin/main.go index dbc33c5d0..f1b1368e1 100644 --- a/example/zipkin/main.go +++ b/example/zipkin/main.go @@ -35,6 +35,7 @@ func initTracer() { // Create Zipkin Exporter exporter, err := zipkin.NewExporter( "http://localhost:9411/api/v2/spans", + "zipkin-example", zipkin.WithLogger(logger), ) if err != nil { diff --git a/exporters/trace/zipkin/exporter_test.go b/exporters/trace/zipkin/exporter_test.go index ada127ca6..848f38867 100644 --- a/exporters/trace/zipkin/exporter_test.go +++ b/exporters/trace/zipkin/exporter_test.go @@ -182,12 +182,14 @@ func TestExportSpans(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "exporter-test", + }, RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ @@ -208,12 +210,14 @@ func TestExportSpans(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "bar", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 15, 0, time.UTC), - Duration: 30 * time.Second, - Shared: false, - LocalEndpoint: nil, + Name: "bar", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 15, 0, time.UTC), + Duration: 30 * time.Second, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "exporter-test", + }, RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ @@ -227,7 +231,9 @@ func TestExportSpans(t *testing.T) { defer collector.Close() ls := &logStore{T: t} logger := logStoreLogger(ls) - exporter, err := NewExporter(collector.url, WithLogger(logger)) + _, err := NewExporter(collector.url, "", WithLogger(logger)) + require.Error(t, err, "service name must be non-empty string") + exporter, err := NewExporter(collector.url, "exporter-test", WithLogger(logger)) require.NoError(t, err) ctx := context.Background() require.Len(t, ls.Messages, 0) diff --git a/exporters/trace/zipkin/model.go b/exporters/trace/zipkin/model.go index bd0a4c027..30a5e5042 100644 --- a/exporters/trace/zipkin/model.go +++ b/exporters/trace/zipkin/model.go @@ -26,23 +26,25 @@ import ( export "go.opentelemetry.io/otel/sdk/export/trace" ) -func toZipkinSpanModels(batch []*export.SpanData) []zkmodel.SpanModel { +func toZipkinSpanModels(batch []*export.SpanData, serviceName string) []zkmodel.SpanModel { models := make([]zkmodel.SpanModel, 0, len(batch)) for _, data := range batch { - models = append(models, toZipkinSpanModel(data)) + models = append(models, toZipkinSpanModel(data, serviceName)) } return models } -func toZipkinSpanModel(data *export.SpanData) zkmodel.SpanModel { +func toZipkinSpanModel(data *export.SpanData, serviceName string) zkmodel.SpanModel { return zkmodel.SpanModel{ - SpanContext: toZipkinSpanContext(data), - Name: data.Name, - Kind: toZipkinKind(data.SpanKind), - Timestamp: data.StartTime, - Duration: data.EndTime.Sub(data.StartTime), - Shared: false, - LocalEndpoint: nil, // *Endpoint + SpanContext: toZipkinSpanContext(data), + Name: data.Name, + Kind: toZipkinKind(data.SpanKind), + Timestamp: data.StartTime, + Duration: data.EndTime.Sub(data.StartTime), + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: serviceName, + }, RemoteEndpoint: nil, // *Endpoint Annotations: toZipkinAnnotations(data.MessageEvents), Tags: toZipkinTags(data), diff --git a/exporters/trace/zipkin/model_test.go b/exporters/trace/zipkin/model_test.go index 6a3a50540..52a021584 100644 --- a/exporters/trace/zipkin/model_test.go +++ b/exporters/trace/zipkin/model_test.go @@ -321,12 +321,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -358,12 +360,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -395,12 +399,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -432,12 +438,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -469,12 +477,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "CLIENT", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "CLIENT", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -506,12 +516,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "PRODUCER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "PRODUCER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -543,12 +555,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "CONSUMER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "CONSUMER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -580,12 +594,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: nil, Tags: map[string]string{ @@ -608,12 +624,14 @@ func TestModelConversion(t *testing.T) { Sampled: nil, Err: nil, }, - Name: "foo", - Kind: "SERVER", - Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), - Duration: time.Minute, - Shared: false, - LocalEndpoint: nil, + Name: "foo", + Kind: "SERVER", + Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC), + Duration: time.Minute, + Shared: false, + LocalEndpoint: &zkmodel.Endpoint{ + ServiceName: "model-test", + }, RemoteEndpoint: nil, Annotations: []zkmodel.Annotation{ { @@ -631,7 +649,7 @@ func TestModelConversion(t *testing.T) { }, }, } - gottenOutputBatch := toZipkinSpanModels(inputBatch) + gottenOutputBatch := toZipkinSpanModels(inputBatch, "model-test") require.Equal(t, expectedOutputBatch, gottenOutputBatch) } diff --git a/exporters/trace/zipkin/zipkin.go b/exporters/trace/zipkin/zipkin.go index ff251c1cd..525a33bdf 100644 --- a/exporters/trace/zipkin/zipkin.go +++ b/exporters/trace/zipkin/zipkin.go @@ -30,9 +30,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 - client *http.Client - logger *log.Logger + url string + serviceName string + client *http.Client + logger *log.Logger } var ( @@ -63,10 +64,13 @@ func WithClient(client *http.Client) Option { } // NewExporter creates a new zipkin exporter. -func NewExporter(collectorURL string, os ...Option) (*Exporter, error) { +func NewExporter(collectorURL string, serviceName string, os ...Option) (*Exporter, error) { if _, err := url.Parse(collectorURL); err != nil { return nil, fmt.Errorf("invalid collector URL: %v", err) } + if serviceName == "" { + return nil, fmt.Errorf("service name must be non-empty string") + } opts := Options{} for _, o := range os { o(&opts) @@ -75,9 +79,10 @@ func NewExporter(collectorURL string, os ...Option) (*Exporter, error) { opts.client = http.DefaultClient } return &Exporter{ - url: collectorURL, - client: opts.client, - logger: opts.logger, + url: collectorURL, + client: opts.client, + logger: opts.logger, + serviceName: serviceName, }, nil } @@ -88,7 +93,7 @@ func (e *Exporter) ExportSpans(ctx context.Context, batch []*export.SpanData) { e.logf("no spans to export") return } - models := toZipkinSpanModels(batch) + models := toZipkinSpanModels(batch, e.serviceName) body, err := json.Marshal(models) if err != nil { e.logf("failed to serialize zipkin models to JSON: %v", err)