From fded142d132ec9f452f4d3e11665e5adb91e42fc Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Sat, 11 Jan 2020 18:41:35 +0100 Subject: [PATCH 1/8] Add a span name getter to opencensus endpoint options --- tracing/opencensus/endpoint.go | 4 ++++ tracing/opencensus/endpoint_options.go | 11 ++++++++++- tracing/opencensus/endpoint_test.go | 17 +++++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tracing/opencensus/endpoint.go b/tracing/opencensus/endpoint.go index 4f678fc7d..9cc2dcee5 100644 --- a/tracing/opencensus/endpoint.go +++ b/tracing/opencensus/endpoint.go @@ -31,6 +31,10 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { return func(next endpoint.Endpoint) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (response interface{}, err error) { + if name == TraceEndpointDefaultName && cfg.GetSpanName != nil { + name = cfg.GetSpanName(ctx) + } + ctx, span := trace.StartSpan(ctx, name) if len(cfg.Attributes) > 0 { span.AddAttributes(cfg.Attributes...) diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index 3af145681..e94a5b566 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -1,6 +1,10 @@ package opencensus -import "go.opencensus.io/trace" +import ( + "context" + + "go.opencensus.io/trace" +) // EndpointOptions holds the options for tracing an endpoint type EndpointOptions struct { @@ -11,6 +15,11 @@ type EndpointOptions struct { // Attributes holds the default attributes which will be set on span // creation by our Endpoint middleware. Attributes []trace.Attribute + + // GetSpanName holds the function to use for generating the span name + // from the information found in the incoming Request. + // Defaults to the name that the middleware was initialized with. + GetSpanName func(ctx context.Context) string } // EndpointOption allows for functional options to our OpenCensus endpoint diff --git a/tracing/opencensus/endpoint_test.go b/tracing/opencensus/endpoint_test.go index 924560a6a..b2a512f01 100644 --- a/tracing/opencensus/endpoint_test.go +++ b/tracing/opencensus/endpoint_test.go @@ -20,6 +20,7 @@ const ( span3 = "SPAN-3" span4 = "SPAN-4" span5 = "SPAN-5" + span6 = "SPAN-6" ) var ( @@ -76,13 +77,20 @@ func TestTraceEndpoint(t *testing.T) { mw = opencensus.TraceEndpoint(span4) mw(passEndpoint)(ctx, failedResponse{err: err3}) - // span4 + // span5 mw = opencensus.TraceEndpoint(span5, opencensus.WithIgnoreBusinessError(true)) mw(passEndpoint)(ctx, failedResponse{err: err4}) + // span6 + opts = opencensus.EndpointOptions{GetSpanName: func(ctx context.Context) string { + return span6 + }} + mw = opencensus.TraceEndpoint("", opencensus.WithEndpointConfig(opts)) + mw(endpoint.Nop)(ctx, nil) + // check span count spans := e.Flush() - if want, have := 5, len(spans); want != have { + if want, have := 6, len(spans); want != have { t.Fatalf("incorrected number of spans, wanted %d, got %d", want, have) } @@ -156,4 +164,9 @@ func TestTraceEndpoint(t *testing.T) { t.Fatalf("incorrect attribute count, wanted %d, got %d", want, have) } + // test span 6 + span = spans[5] + if want, have := span6, span.Name; want != have { + t.Errorf("incorrect span name, wanted %q, got %q", want, have) + } } From e771e621a60f913062af3986ee57816e74b975cb Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 31 Jan 2020 14:37:04 +0100 Subject: [PATCH 2/8] Add GetSpanDetails option --- tracing/opencensus/endpoint.go | 15 +++++++++++++-- tracing/opencensus/endpoint_options.go | 17 +++++++++++++---- tracing/opencensus/endpoint_test.go | 18 ++++++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/tracing/opencensus/endpoint.go b/tracing/opencensus/endpoint.go index 9cc2dcee5..70c4e87e6 100644 --- a/tracing/opencensus/endpoint.go +++ b/tracing/opencensus/endpoint.go @@ -31,14 +31,25 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { return func(next endpoint.Endpoint) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (response interface{}, err error) { - if name == TraceEndpointDefaultName && cfg.GetSpanName != nil { - name = cfg.GetSpanName(ctx) + var attributes []trace.Attribute + + if cfg.GetSpanDetails != nil { + var newName string + + newName, attributes = cfg.GetSpanDetails(ctx, name) + + if newName != "" { + name = newName + } } ctx, span := trace.StartSpan(ctx, name) if len(cfg.Attributes) > 0 { span.AddAttributes(cfg.Attributes...) } + if len(attributes) > 0 { + span.AddAttributes(attributes...) + } defer span.End() defer func() { diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index e94a5b566..a91dfb561 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -16,10 +16,12 @@ type EndpointOptions struct { // creation by our Endpoint middleware. Attributes []trace.Attribute - // GetSpanName holds the function to use for generating the span name - // from the information found in the incoming Request. - // Defaults to the name that the middleware was initialized with. - GetSpanName func(ctx context.Context) string + // GetSpanDetails holds the function to use for generating the span name + // based on the current name and from the information found in the incoming Request. + // It can also return additional attributes for the span. + // + // A returned empty name defaults to the name that the middleware was initialized with. + GetSpanDetails func(ctx context.Context, name string) (string, []trace.Attribute) } // EndpointOption allows for functional options to our OpenCensus endpoint @@ -49,3 +51,10 @@ func WithIgnoreBusinessError(val bool) EndpointOption { o.IgnoreBusinessError = val } } + +// WithSpanDetails extracts details from the request context (like span name and additional attributes). +func WithSpanDetails(fn func(ctx context.Context, name string) (string, []trace.Attribute)) EndpointOption { + return func(o *EndpointOptions) { + o.GetSpanDetails = fn + } +} diff --git a/tracing/opencensus/endpoint_test.go b/tracing/opencensus/endpoint_test.go index b2a512f01..813d6eee6 100644 --- a/tracing/opencensus/endpoint_test.go +++ b/tracing/opencensus/endpoint_test.go @@ -82,10 +82,16 @@ func TestTraceEndpoint(t *testing.T) { mw(passEndpoint)(ctx, failedResponse{err: err4}) // span6 - opts = opencensus.EndpointOptions{GetSpanName: func(ctx context.Context) string { - return span6 - }} - mw = opencensus.TraceEndpoint("", opencensus.WithEndpointConfig(opts)) + span6Attrs := []trace.Attribute{ + trace.StringAttribute("string", "value"), + trace.Int64Attribute("int64", 42), + } + mw = opencensus.TraceEndpoint( + "", + opencensus.WithSpanDetails(func(ctx context.Context, name string) (string, []trace.Attribute) { + return span6, span6Attrs + }), + ) mw(endpoint.Nop)(ctx, nil) // check span count @@ -169,4 +175,8 @@ func TestTraceEndpoint(t *testing.T) { if want, have := span6, span.Name; want != have { t.Errorf("incorrect span name, wanted %q, got %q", want, have) } + + if want, have := 2, len(span.Attributes); want != have { + t.Fatalf("incorrect attribute count, wanted %d, got %d", want, have) + } } From b27801a1691e21b9b71a51bbe40eaa605e0321d5 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Fri, 31 Jan 2020 16:52:29 +0100 Subject: [PATCH 3/8] Split GetSpanDetails function --- tracing/opencensus/endpoint.go | 18 +++++++++--------- tracing/opencensus/endpoint_options.go | 24 +++++++++++++++++------- tracing/opencensus/endpoint_test.go | 7 +++++-- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/tracing/opencensus/endpoint.go b/tracing/opencensus/endpoint.go index 70c4e87e6..f211a1767 100644 --- a/tracing/opencensus/endpoint.go +++ b/tracing/opencensus/endpoint.go @@ -31,12 +31,8 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { return func(next endpoint.Endpoint) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (response interface{}, err error) { - var attributes []trace.Attribute - - if cfg.GetSpanDetails != nil { - var newName string - - newName, attributes = cfg.GetSpanDetails(ctx, name) + if cfg.GetName != nil { + newName := cfg.GetName(ctx, name) if newName != "" { name = newName @@ -47,11 +43,15 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { if len(cfg.Attributes) > 0 { span.AddAttributes(cfg.Attributes...) } - if len(attributes) > 0 { - span.AddAttributes(attributes...) - } defer span.End() + if cfg.GetAttributes != nil { + attributes := cfg.GetAttributes(ctx) + if len(attributes) > 0 { + span.AddAttributes(attributes...) + } + } + defer func() { if err != nil { if lberr, ok := err.(lb.RetryError); ok { diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index a91dfb561..88fc52b0f 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -16,12 +16,15 @@ type EndpointOptions struct { // creation by our Endpoint middleware. Attributes []trace.Attribute - // GetSpanDetails holds the function to use for generating the span name + // GetName holds the function used for generating the span name // based on the current name and from the information found in the incoming Request. - // It can also return additional attributes for the span. // - // A returned empty name defaults to the name that the middleware was initialized with. - GetSpanDetails func(ctx context.Context, name string) (string, []trace.Attribute) + // If the returned name is empty, the existing name for the endpoint is kept. + GetName func(ctx context.Context, name string) string + + // GetAttributes holds the function used for extracting additional attributes + // from the information found in the incoming Request. + GetAttributes func(ctx context.Context) []trace.Attribute } // EndpointOption allows for functional options to our OpenCensus endpoint @@ -52,9 +55,16 @@ func WithIgnoreBusinessError(val bool) EndpointOption { } } -// WithSpanDetails extracts details from the request context (like span name and additional attributes). -func WithSpanDetails(fn func(ctx context.Context, name string) (string, []trace.Attribute)) EndpointOption { +// WithSpanName extracts additional attributes from the request context. +func WithSpanName(fn func(ctx context.Context, name string) string) EndpointOption { + return func(o *EndpointOptions) { + o.GetName = fn + } +} + +// WithSpanAttributes extracts additional attributes from the request context. +func WithSpanAttributes(fn func(ctx context.Context) []trace.Attribute) EndpointOption { return func(o *EndpointOptions) { - o.GetSpanDetails = fn + o.GetAttributes = fn } } diff --git a/tracing/opencensus/endpoint_test.go b/tracing/opencensus/endpoint_test.go index 813d6eee6..3a63a54be 100644 --- a/tracing/opencensus/endpoint_test.go +++ b/tracing/opencensus/endpoint_test.go @@ -88,8 +88,11 @@ func TestTraceEndpoint(t *testing.T) { } mw = opencensus.TraceEndpoint( "", - opencensus.WithSpanDetails(func(ctx context.Context, name string) (string, []trace.Attribute) { - return span6, span6Attrs + opencensus.WithSpanName(func(ctx context.Context, name string) string { + return span6 + }), + opencensus.WithSpanAttributes(func(ctx context.Context) []trace.Attribute { + return span6Attrs }), ) mw(endpoint.Nop)(ctx, nil) From dbc964fcaa05453ae75c44f8b4bd3b3ff6c84032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20S=C3=A1gi-Kaz=C3=A1r?= Date: Thu, 13 Feb 2020 13:26:24 +0100 Subject: [PATCH 4/8] Simplify getting the name --- tracing/opencensus/endpoint.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tracing/opencensus/endpoint.go b/tracing/opencensus/endpoint.go index f211a1767..a545419e6 100644 --- a/tracing/opencensus/endpoint.go +++ b/tracing/opencensus/endpoint.go @@ -32,9 +32,7 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { return func(next endpoint.Endpoint) endpoint.Endpoint { return func(ctx context.Context, request interface{}) (response interface{}, err error) { if cfg.GetName != nil { - newName := cfg.GetName(ctx, name) - - if newName != "" { + if newName := cfg.GetName(ctx, name); newName != "" { name = newName } } From a87bb6b7433a9d253e6dc2a12227419693b5e0a5 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 17 Feb 2020 14:07:44 +0100 Subject: [PATCH 5/8] Improve documentation --- tracing/opencensus/endpoint_options.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index 88fc52b0f..69e6dcdfd 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -16,14 +16,14 @@ type EndpointOptions struct { // creation by our Endpoint middleware. Attributes []trace.Attribute - // GetName holds the function used for generating the span name - // based on the current name and from the information found in the incoming Request. + // GetName is an optional function that can set the span name based on the existing name + // for the endpoint and information in the context. // - // If the returned name is empty, the existing name for the endpoint is kept. + // If the function is nil, or the returned name is empty, the existing name for the endpoint is used. GetName func(ctx context.Context, name string) string - // GetAttributes holds the function used for extracting additional attributes - // from the information found in the incoming Request. + // GetAttributes is an optional function that attaches additional attributes to the span + // based on the information found in the context. GetAttributes func(ctx context.Context) []trace.Attribute } From 5286e2218d5bc51c48febd11a639dd2d08b2f911 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 17 Feb 2020 14:32:47 +0100 Subject: [PATCH 6/8] Simplify attributes --- tracing/opencensus/endpoint.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tracing/opencensus/endpoint.go b/tracing/opencensus/endpoint.go index a545419e6..16283a543 100644 --- a/tracing/opencensus/endpoint.go +++ b/tracing/opencensus/endpoint.go @@ -44,9 +44,8 @@ func TraceEndpoint(name string, options ...EndpointOption) endpoint.Middleware { defer span.End() if cfg.GetAttributes != nil { - attributes := cfg.GetAttributes(ctx) - if len(attributes) > 0 { - span.AddAttributes(attributes...) + if attrs := cfg.GetAttributes(ctx); len(attrs) > 0 { + span.AddAttributes(attrs...) } } From 6a5f41fa9156dc13378e8f8991cb613b6f974f48 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Mon, 17 Feb 2020 16:07:01 +0100 Subject: [PATCH 7/8] Update documentation --- tracing/opencensus/endpoint_options.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index 69e6dcdfd..69e6c7384 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -22,8 +22,7 @@ type EndpointOptions struct { // If the function is nil, or the returned name is empty, the existing name for the endpoint is used. GetName func(ctx context.Context, name string) string - // GetAttributes is an optional function that attaches additional attributes to the span - // based on the information found in the context. + // GetAttributes is an optional function that can extract trace attributes from the context and add them to the span. GetAttributes func(ctx context.Context) []trace.Attribute } From bb67e445efdc555da2c62223eb4838103161a93b Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Mon, 17 Feb 2020 20:03:06 +0100 Subject: [PATCH 8/8] Update tracing/opencensus/endpoint_options.go --- tracing/opencensus/endpoint_options.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tracing/opencensus/endpoint_options.go b/tracing/opencensus/endpoint_options.go index 69e6c7384..8eedd59b2 100644 --- a/tracing/opencensus/endpoint_options.go +++ b/tracing/opencensus/endpoint_options.go @@ -22,7 +22,8 @@ type EndpointOptions struct { // If the function is nil, or the returned name is empty, the existing name for the endpoint is used. GetName func(ctx context.Context, name string) string - // GetAttributes is an optional function that can extract trace attributes from the context and add them to the span. + // GetAttributes is an optional function that can extract trace attributes + // from the context and add them to the span. GetAttributes func(ctx context.Context) []trace.Attribute }