From f8837496f66461497a3a3593a75fc9f1de9ad463 Mon Sep 17 00:00:00 2001 From: Ramon Nogueira Date: Mon, 12 Mar 2018 13:55:46 -0700 Subject: [PATCH] Make no propagation the default for ochttp.Handler By default, when no propagation formats are specificed, make Handler not read any headers. Previously, we assumed B3 by default, but this does not give the user any way to use the plugin but disable propagation (for example if they directly expost the handler on a public-facing server). --- plugin/ochttp/client.go | 5 ++++- plugin/ochttp/example_test.go | 11 ++++++++--- plugin/ochttp/server.go | 26 +++++++++++++++++++------- plugin/ochttp/trace.go | 3 --- plugin/ochttp/trace_test.go | 4 +++- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/plugin/ochttp/client.go b/plugin/ochttp/client.go index 6d981bc1d..c0439220e 100644 --- a/plugin/ochttp/client.go +++ b/plugin/ochttp/client.go @@ -17,6 +17,7 @@ package ochttp import ( "net/http" + "go.opencensus.io/plugin/ochttp/propagation/b3" "go.opencensus.io/trace" "go.opencensus.io/trace/propagation" ) @@ -50,6 +51,8 @@ type Transport struct { // TODO: Implement tag propagation for HTTP. } +var defaultClientFormat propagation.HTTPFormat = &b3.HTTPFormat{} + // RoundTrip implements http.RoundTripper, delegating to Base and recording stats and traces for the request. func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { rt := t.base() @@ -57,7 +60,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { if !t.NoTrace { format := t.Propagation if format == nil { - format = defaultFormat + format = defaultClientFormat } rt = &traceTransport{ base: rt, diff --git a/plugin/ochttp/example_test.go b/plugin/ochttp/example_test.go index fa4dd9370..71554675a 100644 --- a/plugin/ochttp/example_test.go +++ b/plugin/ochttp/example_test.go @@ -43,7 +43,9 @@ func ExampleTransport() { } client := &http.Client{ - Transport: &ochttp.Transport{}, + Transport: &ochttp.Transport{ + Propagation: &b3.HTTPFormat{}, + }, } _ = client // use client to perform requests } @@ -52,9 +54,12 @@ var usersHandler http.Handler func ExampleHandler() { // Enables OpenCensus for the default serve mux. - // By default, B3 propagation is used. http.Handle("/users", usersHandler) - log.Fatal(http.ListenAndServe("localhost:8080", &ochttp.Handler{})) + log.Fatal(http.ListenAndServe("localhost:8080", &ochttp.Handler{ + // Specify a propagation format; without this, a new root span will be + // started for each request: + Propagation: &b3.HTTPFormat{}, + })) } func ExampleHandler_mux() { diff --git a/plugin/ochttp/server.go b/plugin/ochttp/server.go index 018857971..cca66f207 100644 --- a/plugin/ochttp/server.go +++ b/plugin/ochttp/server.go @@ -44,8 +44,16 @@ type Handler struct { // NoTrace may be set to disable recording of traces. NoTrace bool - // Propagation defines how traces are propagated. If unspecified, - // B3 propagation will be used. + // Propagation defines the header convention used to read tracing information + // from incoming requests. + // + // If not specified, no tracing information will be read from the incoming + // request. In this case, a new root span will be created around the + // server-side processing of each request if the Sampler samples this request. + // + // You should only set this to a non-default value if you trust the caller + // of this service. For example, this is safe if the current service will + // only be called by other services that you control. Propagation propagation.HTTPFormat // Handler is the handler used to handle the incoming request. @@ -73,14 +81,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Request, func()) { name := spanNameFromURL("Recv", r.URL) - p := h.Propagation - if p == nil { - p = defaultFormat - } ctx := r.Context() var span *trace.Span - if sc, ok := p.SpanContextFromRequest(r); ok { + if sc, ok := h.extractSpanContext(r); ok { span = trace.NewSpanWithRemoteParent(name, sc, trace.StartOptions{}) + ctx = trace.WithSpan(ctx, span) } else { span = trace.NewSpan(name, nil, trace.StartOptions{}) } @@ -89,6 +94,13 @@ func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Requ return r.WithContext(trace.WithSpan(r.Context(), span)), span.End } +func (h *Handler) extractSpanContext(r *http.Request) (trace.SpanContext, bool) { + if h.Propagation == nil { + return trace.SpanContext{}, false + } + return h.Propagation.SpanContextFromRequest(r) +} + func (h *Handler) startStats(w http.ResponseWriter, r *http.Request) (http.ResponseWriter, func()) { ctx, _ := tag.New(r.Context(), tag.Upsert(Host, r.URL.Host), diff --git a/plugin/ochttp/trace.go b/plugin/ochttp/trace.go index 9c48a981e..47e07b845 100644 --- a/plugin/ochttp/trace.go +++ b/plugin/ochttp/trace.go @@ -20,15 +20,12 @@ import ( "net/url" "sync" - "go.opencensus.io/plugin/ochttp/propagation/b3" "go.opencensus.io/trace" "go.opencensus.io/trace/propagation" ) // TODO(jbd): Add godoc examples. -var defaultFormat propagation.HTTPFormat = &b3.HTTPFormat{} - // Attributes recorded on the span for the requests. // Only trace exporters will need them. const ( diff --git a/plugin/ochttp/trace_test.go b/plugin/ochttp/trace_test.go index 75a0ebf75..06d1518ea 100644 --- a/plugin/ochttp/trace_test.go +++ b/plugin/ochttp/trace_test.go @@ -31,6 +31,7 @@ import ( "testing" "time" + "go.opencensus.io/plugin/ochttp/propagation/b3" "go.opencensus.io/trace" ) @@ -202,7 +203,7 @@ func TestEndToEnd(t *testing.T) { rt := &Transport{ NoStats: true, - Propagation: defaultFormat, + Propagation: &b3.HTTPFormat{}, Base: http.DefaultTransport, } resp, err := rt.RoundTrip(req) @@ -283,6 +284,7 @@ func serveHTTP(done chan struct{}, wait chan time.Time) string { io.WriteString(w, "expected-response") close(done) }), + Propagation: &b3.HTTPFormat{}, } server := httptest.NewServer(handler)