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

Allow custom display name format for spans (#45) #48

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
17 changes: 17 additions & 0 deletions exporter/trace/cloudtrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -114,6 +118,11 @@ type options struct {
// If it is set to zero then default value is used.
ReportingInterval time.Duration

// 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

// MaxNumberOfWorkers sets the maximum number of go rountines that send requests
// to Cloud Trace. The minimum number of workers is 1.
MaxNumberOfWorkers int
Expand Down Expand Up @@ -193,6 +202,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)
Expand Down
36 changes: 36 additions & 0 deletions exporter/trace/cloudtrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -129,6 +130,41 @@ 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),
texporter.WithBundleCountThreshold(1),
texporter.WithDisplayNameFormatter(format),
)
assert.NoError(t, err)

tp, err := sdktrace.NewProvider(
sdktrace.WithConfig(sdktrace.Config{DefaultSampler: sdktrace.AlwaysSample()}),
sdktrace.WithSyncer(exp))
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sleeping for a block amount of time in tests in never ideal. To make this signficiantly more robust, and faster, a better alternative to this is to do something like

assert.Eventually(t, function() {
    ...
}, 500 * time.Millisecond, 2 * time.Millisecond)

I wouldn't bother changing this now though. After you add the NewPipeline method that returns a flush function (see the Jaeger exporter), you can avoid the sleep here entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, sounds good. I'll leave this for now, and when I create the NewPipeline method, I plan to update this test and others to use the pipeline and flush function as appropriate.

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
Expand Down
4 changes: 2 additions & 2 deletions exporter/trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (e *traceExporter) checkBundlerError(err 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)
protoSize := proto.Size(protoSpan)
err := e.bundler.Add(&contextAndSpans{
ctx: ctx,
Expand All @@ -124,7 +124,7 @@ func (e *traceExporter) ExportSpans(ctx context.Context, sds []*export.SpanData)
pbSpans := make([]*tracepb.Span, len(sds))
var protoSize int = 0
for i, sd := range sds {
pbSpans[i] = protoFromSpanData(sd, e.projectID)
pbSpans[i] = protoFromSpanData(sd, e.projectID, e.o.DisplayNameFormatter)
protoSize += proto.Size(pbSpans[i])
}
err := e.bundler.Add(&contextAndSpans{ctx, pbSpans}, protoSize)
Expand Down
22 changes: 14 additions & 8 deletions exporter/trace/trace_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,31 @@ const (

var userAgent = fmt.Sprintf("opentelemetry-go %s; cloudtrace-exporter %s", opentelemetry.Version(), version)

func protoFromSpanData(s *export.SpanData, projectID string) *tracepb.Span {
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)
}
}

func protoFromSpanData(s *export.SpanData, projectID string, format DisplayNameFormatter) *tracepb.Span {
if s == nil {
return nil
}

traceIDString := s.SpanContext.TraceID.String()
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)
}
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},
Expand Down