From 0ba19c7891d94a942ccc03f5be239ca544868297 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Wed, 17 Jun 2020 19:05:14 +0000 Subject: [PATCH 1/5] allow users to use their own display name format for spans --- exporter/trace/cloudtrace.go | 17 +++++++++++++++++ exporter/trace/trace.go | 4 ++-- exporter/trace/trace_proto.go | 14 +++++++++----- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/exporter/trace/cloudtrace.go b/exporter/trace/cloudtrace.go index 400dc5f15..1cb8a97a7 100644 --- a/exporter/trace/cloudtrace.go +++ b/exporter/trace/cloudtrace.go @@ -32,6 +32,10 @@ import ( // Option is function type that is passed to the exporter initialization function. type Option func(*options) +// DisplayNameFormatter is is a function that produces the display name of a span +// given its SpanData +type DisplayNameFormatter func(*export.SpanData) string + // options contains options for configuring the exporter. type options struct { // ProjectID is the identifier of the Stackdriver @@ -107,6 +111,11 @@ type options struct { // to Stackdriver Monitoring. This is only used for Proto metrics export // for now. The minimum number of workers is 1. NumberOfWorkers int + + // DisplayNameFormatter is a function that produces the display name of a span + // given its SpanData. + // Optional. Default format for SpanData s is "Span.{s.SpanKind}-{s.Name}" + DisplayNameFormatter } // WithProjectID sets Google Cloud Platform project as projectID. @@ -151,6 +160,14 @@ func WithTimeout(t time.Duration) func(o *options) { } } +// WithDisplayNameFormatter sets the way span's display names will be +// generated from SpanData +func WithDisplayNameFormatter(f DisplayNameFormatter) func(o *options) { + return func(o *options) { + o.DisplayNameFormatter = f + } +} + func (o *options) handleError(err error) { if o.OnError != nil { o.OnError(err) diff --git a/exporter/trace/trace.go b/exporter/trace/trace.go index 196e8993c..7aa7df67e 100644 --- a/exporter/trace/trace.go +++ b/exporter/trace/trace.go @@ -50,7 +50,7 @@ func newTraceExporter(o *options) (*traceExporter, error) { // ExportSpan exports a SpanData to Stackdriver Trace. func (e *traceExporter) ExportSpan(ctx context.Context, sd *export.SpanData) { - protoSpan := protoFromSpanData(sd, e.projectID) + protoSpan := protoFromSpanData(sd, e.projectID, e.o.DisplayNameFormatter) e.uploadFn(ctx, []*tracepb.Span{protoSpan}) } @@ -58,7 +58,7 @@ func (e *traceExporter) ExportSpan(ctx context.Context, sd *export.SpanData) { func (e *traceExporter) ExportSpans(ctx context.Context, sds []*export.SpanData) { pbSpans := make([]*tracepb.Span, len(sds)) for i, sd := range sds { - pbSpans[i] = protoFromSpanData(sd, e.projectID) + pbSpans[i] = protoFromSpanData(sd, e.projectID, e.o.DisplayNameFormatter) } e.uploadFn(ctx, pbSpans) } diff --git a/exporter/trace/trace_proto.go b/exporter/trace/trace_proto.go index d0bc9f228..e0a9843e7 100644 --- a/exporter/trace/trace_proto.go +++ b/exporter/trace/trace_proto.go @@ -60,7 +60,7 @@ const ( var userAgent = fmt.Sprintf("opentelemetry-go %s; cloudtrace-exporter %s", opentelemetry.Version(), version) -func protoFromSpanData(s *export.SpanData, projectID string) *tracepb.Span { +func protoFromSpanData(s *export.SpanData, projectID string, format DisplayNameFormatter) *tracepb.Span { if s == nil { return nil } @@ -69,10 +69,14 @@ func protoFromSpanData(s *export.SpanData, projectID string) *tracepb.Span { spanIDString := s.SpanContext.SpanID.String() name := s.Name - switch s.SpanKind { - // TODO(ymotongpoo): add cases for "Send" and "Recv". - default: - name = fmt.Sprintf("Span.%s-%s", s.SpanKind, name) + if format == nil { + switch s.SpanKind { + // TODO(ymotongpoo): add cases for "Send" and "Recv". + default: + name = fmt.Sprintf("Span.%s-%s", s.SpanKind, name) + } + } else { + name = format(s) } sp := &tracepb.Span{ From 92f163ab3242bb0759f835a00379566e4fe4f742 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Thu, 18 Jun 2020 19:57:58 +0000 Subject: [PATCH 2/5] add test for custom span display name format --- exporter/trace/cloudtrace_test.go | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/exporter/trace/cloudtrace_test.go b/exporter/trace/cloudtrace_test.go index 42b943669..789cd4ad1 100644 --- a/exporter/trace/cloudtrace_test.go +++ b/exporter/trace/cloudtrace_test.go @@ -33,6 +33,7 @@ import ( "go.opentelemetry.io/otel/api/global" sdktrace "go.opentelemetry.io/otel/sdk/trace" + export "go.opentelemetry.io/otel/sdk/export/trace" ) type mockTraceServer struct { @@ -122,6 +123,45 @@ func TestExporter_ExportSpans(t *testing.T) { assert.EqualValues(t, 1, mockTrace.len()) } +func TestExporter_DisplayNameFormatter(t *testing.T) { + // Initial test precondition + mockTrace.spansUploaded = nil + mockTrace.delay = 0 + + spanName := "span1234" + format := func(s *export.SpanData) string { + return "TEST_FORMAT" + s.Name + } + + // Create Google Cloud Trace Exporter + exp, err := texporter.NewExporter( + texporter.WithProjectID("PROJECT_ID_NOT_REAL"), + texporter.WithTraceClientOptions(clientOpt), + // uncomment when exporter is using bundler + // texporter.WithBundleCountThreshold(1), + texporter.WithDisplayNameFormatter(format), + ) + assert.NoError(t, err) + + tp, err := sdktrace.NewProvider( + sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}), + sdktrace.WithBatcher(exp, // add following two options to ensure flush + sdktrace.WithScheduleDelayMillis(1), + sdktrace.WithMaxExportBatchSize(1), + )) + assert.NoError(t, err) + + global.SetTraceProvider(tp) + _, span := global.TraceProvider().Tracer("test-tracer").Start(context.Background(), spanName) + span.End() + assert.True(t, span.SpanContext().IsValid()) + + // wait exporter to flush + time.Sleep(20 * time.Millisecond) + assert.EqualValues(t, 1, mockTrace.len()) + assert.EqualValues(t, "TEST_FORMAT" + spanName, mockTrace.spansUploaded[0].DisplayName.Value) +} + func TestExporter_Timeout(t *testing.T) { // Initial test precondition mockTrace.spansUploaded = nil From 680ab221336807c1074356e20d5a8330be4d96bb Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Thu, 25 Jun 2020 15:54:41 +0000 Subject: [PATCH 3/5] make separate function to generate displayName --- exporter/trace/trace_proto.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/exporter/trace/trace_proto.go b/exporter/trace/trace_proto.go index e0a9843e7..e5f5cb67c 100644 --- a/exporter/trace/trace_proto.go +++ b/exporter/trace/trace_proto.go @@ -60,6 +60,19 @@ const ( var userAgent = fmt.Sprintf("opentelemetry-go %s; cloudtrace-exporter %s", opentelemetry.Version(), version) +func generateDisplayName(s *export.SpanData, format DisplayNameFormatter) (displayName string) { + if format == nil { + switch s.SpanKind { + // TODO(ymotongpoo): add cases for "Send" and "Recv". + default: + displayName = fmt.Sprintf("Span.%s-%s", s.SpanKind, s.Name) + } + } else { + displayName = format(s) + } + return +} + func protoFromSpanData(s *export.SpanData, projectID string, format DisplayNameFormatter) *tracepb.Span { if s == nil { return nil @@ -68,21 +81,12 @@ func protoFromSpanData(s *export.SpanData, projectID string, format DisplayNameF traceIDString := s.SpanContext.TraceID.String() spanIDString := s.SpanContext.SpanID.String() - name := s.Name - if format == nil { - switch s.SpanKind { - // TODO(ymotongpoo): add cases for "Send" and "Recv". - default: - name = fmt.Sprintf("Span.%s-%s", s.SpanKind, name) - } - } else { - name = format(s) - } + displayName := generateDisplayName(s, format) sp := &tracepb.Span{ Name: "projects/" + projectID + "/traces/" + traceIDString + "/spans/" + spanIDString, SpanId: spanIDString, - DisplayName: trunc(name, 128), + DisplayName: trunc(displayName, 128), StartTime: timestampProto(s.StartTime), EndTime: timestampProto(s.EndTime), SameProcessAsParentSpan: &wrapperspb.BoolValue{Value: !s.HasRemoteParent}, From d3e76d772e1ecfa8ea417f7d7443a4221ce4a7dd Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Mon, 29 Jun 2020 15:25:43 +0000 Subject: [PATCH 4/5] update test to use bundler+syncer instead of batcher --- exporter/trace/cloudtrace_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/exporter/trace/cloudtrace_test.go b/exporter/trace/cloudtrace_test.go index b7be2e54b..a07b64adb 100644 --- a/exporter/trace/cloudtrace_test.go +++ b/exporter/trace/cloudtrace_test.go @@ -144,18 +144,14 @@ func TestExporter_DisplayNameFormatter(t *testing.T) { exp, err := texporter.NewExporter( texporter.WithProjectID("PROJECT_ID_NOT_REAL"), texporter.WithTraceClientOptions(clientOpt), - // uncomment when exporter is using bundler - // texporter.WithBundleCountThreshold(1), + texporter.WithBundleCountThreshold(1), texporter.WithDisplayNameFormatter(format), ) assert.NoError(t, err) tp, err := sdktrace.NewProvider( sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}), - sdktrace.WithBatcher(exp, // add following two options to ensure flush - sdktrace.WithScheduleDelayMillis(1), - sdktrace.WithMaxExportBatchSize(1), - )) + sdktrace.WithSyncer(exp)) assert.NoError(t, err) global.SetTraceProvider(tp) From ee614e7b6388905a9b6af4d6d24998714e6b1459 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Mon, 29 Jun 2020 15:37:27 +0000 Subject: [PATCH 5/5] take out some nesting in generateDisplayName function --- exporter/trace/trace_proto.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/exporter/trace/trace_proto.go b/exporter/trace/trace_proto.go index e5f5cb67c..13fe5ca77 100644 --- a/exporter/trace/trace_proto.go +++ b/exporter/trace/trace_proto.go @@ -60,17 +60,15 @@ const ( var userAgent = fmt.Sprintf("opentelemetry-go %s; cloudtrace-exporter %s", opentelemetry.Version(), version) -func generateDisplayName(s *export.SpanData, format DisplayNameFormatter) (displayName string) { - if format == nil { - switch s.SpanKind { - // TODO(ymotongpoo): add cases for "Send" and "Recv". - default: - displayName = fmt.Sprintf("Span.%s-%s", s.SpanKind, s.Name) - } - } else { - displayName = format(s) +func generateDisplayName(s *export.SpanData, format DisplayNameFormatter) string { + if format != nil { + return format(s) + } + switch s.SpanKind { + // TODO(ymotongpoo): add cases for "Send" and "Recv". + default: + return fmt.Sprintf("Span.%s-%s", s.SpanKind, s.Name) } - return } func protoFromSpanData(s *export.SpanData, projectID string, format DisplayNameFormatter) *tracepb.Span {