From 15565deb06bf2ef7a8b67f42e958c7b370bf29da Mon Sep 17 00:00:00 2001 From: Chris Dickson <3260244+hobochili@users.noreply.github.com> Date: Wed, 29 Jan 2020 15:59:23 -0600 Subject: [PATCH] Add HTTP method and path to trace span operation name (#715) * Add HTTP method and path to span operation name Also, set HTTP method and URL span tags. * Add tracing span name option as template --- config/config.go | 1 + config/default.go | 1 + config/load.go | 1 + fabio.properties | 23 ++++++++++++++++++++++- proxy/http_proxy.go | 4 ++-- trace/trace.go | 36 +++++++++++++++++++++++++++++++++--- trace/trace_test.go | 25 ++++++++++++++++++++++--- 7 files changed, 82 insertions(+), 9 deletions(-) diff --git a/config/config.go b/config/config.go index 304aa509f..85edd5fdb 100644 --- a/config/config.go +++ b/config/config.go @@ -178,6 +178,7 @@ type Tracing struct { Topic string SamplerRate float64 SpanHost string + SpanName string TraceID128Bit bool } diff --git a/config/default.go b/config/default.go index 52584aafe..feeb79c5d 100644 --- a/config/default.go +++ b/config/default.go @@ -101,6 +101,7 @@ var defaultConfig = &Config{ Topic: "Fabiolb-Kafka-Topic", SamplerRate: -1, SpanHost: "localhost:9998", + SpanName: "", TraceID128Bit: true, }, diff --git a/config/load.go b/config/load.go index b8a7aed2a..d816cf715 100644 --- a/config/load.go +++ b/config/load.go @@ -204,6 +204,7 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c f.StringVar(&cfg.Tracing.CollectorType, "tracing.CollectorType", defaultConfig.Tracing.CollectorType, "OpenTrace Collector Type, one of [http, kafka]") f.StringVar(&cfg.Tracing.ConnectString, "tracing.ConnectString", defaultConfig.Tracing.ConnectString, "OpenTrace Collector host:port") f.StringVar(&cfg.Tracing.ServiceName, "tracing.ServiceName", defaultConfig.Tracing.ServiceName, "Service name to embed in OpenTrace span") + f.StringVar(&cfg.Tracing.SpanName, "tracing.SpanName", defaultConfig.Tracing.SpanName, "Span name template used to embed in OpenTrace span") f.StringVar(&cfg.Tracing.Topic, "tracing.Topic", defaultConfig.Tracing.Topic, "OpenTrace Collector Kafka Topic") f.Float64Var(&cfg.Tracing.SamplerRate, "tracing.SamplerRate", defaultConfig.Tracing.SamplerRate, "OpenTrace sample rate percentage in decimal form") f.StringVar(&cfg.Tracing.SpanHost, "tracing.SpanHost", defaultConfig.Tracing.SpanHost, "Host:Port info to add to spans") diff --git a/fabio.properties b/fabio.properties index fd9b80863..d33c8240d 100644 --- a/fabio.properties +++ b/fabio.properties @@ -1290,6 +1290,28 @@ # tracing.ServiceName = Fabiolb +# tracing.SpanName configures the template used in reporting span information +# +# The value is expanded by the text/template package and provides +# the following variables: +# +# - Proto: the protocol version +# - Method: the HTTP method +# - Host: the host part of the URL +# - Scheme: the scheme of the requested URL +# - Path: the path of the requested URL +# - RawQuery: the encoded query values of the requested URL +# +# SpanName defaults to the value of tracing.ServiceName but can be +# overridden with this property. +# +# Example: tracing.SpanName = {{.Proto}} {{.Method}} {{.Path}} +# +# The default is +# +# tracing.SpanName = + + # tracing.Topic sets the Topic String used if tracing.CollectorType is kafka and # tracing.ConnectSting is set to a kafka broker # @@ -1313,4 +1335,3 @@ # # The default is # tracing.SpanHost = localhost:9998 - diff --git a/proxy/http_proxy.go b/proxy/http_proxy.go index ce6f0f237..889d071fa 100644 --- a/proxy/http_proxy.go +++ b/proxy/http_proxy.go @@ -55,7 +55,7 @@ type HTTPProxy struct { // Logger is the access logger for the requests. Logger logger.Logger - // TracerCfg is the Open Tracing configuration as provided during startup + // TracerCfg is the Open Tracing configuration as provided during startup TracerCfg config.Tracing // UUID returns a unique id in uuid format. @@ -80,7 +80,7 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } //Create Span - span := trace.CreateSpan(r, p.TracerCfg.ServiceName) + span := trace.CreateSpan(r, &p.TracerCfg) defer span.Finish() t := p.Lookup(r) diff --git a/trace/trace.go b/trace/trace.go index c321e1d1e..db5661924 100644 --- a/trace/trace.go +++ b/trace/trace.go @@ -1,11 +1,13 @@ package trace import ( + "bytes" "fmt" "log" "net/http" "os" "strings" + "text/template" "github.com/fabiolb/fabio/config" opentracing "github.com/opentracing/opentracing-go" @@ -63,18 +65,25 @@ func CreateTracer(recorder zipkin.SpanRecorder, samplerRate float64, traceID128B return tracer } -func CreateSpan(r *http.Request, serviceName string) opentracing.Span { +func CreateSpan(r *http.Request, cfg *config.Tracing) opentracing.Span { globalTracer := opentracing.GlobalTracer() + name := cfg.ServiceName + if cfg.SpanName != "" { + name = spanName(cfg.SpanName, r) + } + // If headers contain trace data, create child span from parent; else, create root span var span opentracing.Span if globalTracer != nil { spanCtx, err := globalTracer.Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(r.Header)) if err != nil { - span = globalTracer.StartSpan(serviceName) + span = globalTracer.StartSpan(name) } else { - span = globalTracer.StartSpan(serviceName, ext.RPCServerOption(spanCtx)) + span = globalTracer.StartSpan(name, ext.RPCServerOption(spanCtx)) } + ext.HTTPMethod.Set(span, r.Method) + ext.HTTPUrl.Set(span, r.URL.String()) } return span // caller must defer span.finish() @@ -98,3 +107,24 @@ func InitializeTracer(traceConfig *config.Tracing) { // Set the Zipkin Tracer created above to the GlobalTracer opentracing.SetGlobalTracer(tracer) } + +// spanName returns the rendered span name from the configured template. +// If an error is encountered, it returns the unrendered template. +func spanName(tmplStr string, r *http.Request) string { + tmpl, err := template.New("name").Parse(tmplStr) + if err != nil { + return tmplStr + } + + var name bytes.Buffer + + data := struct { + Proto, Method, Host, Scheme, Path, RawQuery string + }{r.Proto, r.Method, r.Host, r.URL.Scheme, r.URL.Path, r.URL.RawQuery} + + if err = tmpl.Execute(&name, data); err != nil { + return tmplStr + } + + return name.String() +} diff --git a/trace/trace_test.go b/trace/trace_test.go index 0810af088..ebe0e2eb2 100644 --- a/trace/trace_test.go +++ b/trace/trace_test.go @@ -24,7 +24,7 @@ func TestCreateSpanNoGlobalTracer(t *testing.T) { opentracing.SetGlobalTracer(nil) req, _ := http.NewRequest("GET", "http://example.com", nil) - if CreateSpan(req, testServiceName) != nil { + if CreateSpan(req, &config.Tracing{ServiceName: "fabiolb-test", SpanName: "{{.Method}} {{.Path}}"}) != nil { t.Error("CreateSpan returned a non-nil result using a nil global tracer.") t.Fail() } @@ -35,7 +35,7 @@ func TestCreateSpanWithNoParent(t *testing.T) { opentracing.SetGlobalTracer(tracer) req, _ := http.NewRequest("GET", "http://example.com", nil) - if CreateSpan(req, testServiceName) == nil { + if CreateSpan(req, &config.Tracing{ServiceName: "fabiolb-test", SpanName: "{{.Method}} {{.Path}}"}) == nil { t.Error("Received nil span while a global tracer was set.") t.FailNow() } @@ -53,7 +53,7 @@ func TestCreateSpanWithParent(t *testing.T) { opentracing.HTTPHeadersCarrier(requestIn.Header), ) - if CreateSpan(requestIn, testServiceName+"-child") == nil { + if CreateSpan(requestIn, &config.Tracing{ServiceName: "fabiolb-test", SpanName: "{{.Method}} {{.Path}}"}) == nil { t.Error("Received a nil span while a global tracer was set.") t.FailNow() } @@ -128,3 +128,22 @@ func TestInjectHeadersWithParentSpan(t *testing.T) { t.Fail() } } + +func TestSpanName(t *testing.T) { + req, _ := http.NewRequest("GET", "http://example.com/demo", nil) + + if spanName("{{.Method}} {{.Host}} {{.Path}}", req) != "GET example.com /demo" { + t.Error("spanName did not properly render the supported template") + t.Fail() + } + + if spanName("{{.Invalid", req) != "{{.Invalid" { + t.Error("spanName did not return the unrendered string of the invalid template") + t.Fail() + } + + if spanName("{{.Unsupported}}", req) != "{{.Unsupported}}" { + t.Error("spanName did not return the unrendered string of the unsupported template") + t.Fail() + } +}