Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Zipkin UI failing #644

Merged
merged 1 commit into from
Apr 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions example/zipkin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 19 additions & 13 deletions exporters/trace/zipkin/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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)
Expand Down
22 changes: 12 additions & 10 deletions exporters/trace/zipkin/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
128 changes: 73 additions & 55 deletions exporters/trace/zipkin/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
Expand All @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's surprising to me that this is what gofmt does, but the tests pass, so...

LocalEndpoint: &zkmodel.Endpoint{
ServiceName: "model-test",
},
RemoteEndpoint: nil,
Annotations: []zkmodel.Annotation{
{
Expand All @@ -631,7 +649,7 @@ func TestModelConversion(t *testing.T) {
},
},
}
gottenOutputBatch := toZipkinSpanModels(inputBatch)
gottenOutputBatch := toZipkinSpanModels(inputBatch, "model-test")
require.Equal(t, expectedOutputBatch, gottenOutputBatch)
}

Expand Down
21 changes: 13 additions & 8 deletions exporters/trace/zipkin/zipkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand All @@ -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)
Expand Down