From c4aed586f31711747956861813e935fb8a60d576 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Wed, 16 Oct 2019 09:50:04 -0700 Subject: [PATCH] Fix up docs and other othttp cleanups --- plugin/othttp/doc.go | 18 ++ plugin/othttp/handler.go | 228 ++++++++++++++ ...xample_test.go => handler_example_test.go} | 9 +- .../{server_test.go => handler_test.go} | 0 plugin/othttp/server.go | 297 ------------------ plugin/othttp/wrap.go | 30 +- 6 files changed, 272 insertions(+), 310 deletions(-) create mode 100644 plugin/othttp/doc.go create mode 100644 plugin/othttp/handler.go rename plugin/othttp/{server_example_test.go => handler_example_test.go} (93%) rename plugin/othttp/{server_test.go => handler_test.go} (100%) delete mode 100644 plugin/othttp/server.go diff --git a/plugin/othttp/doc.go b/plugin/othttp/doc.go new file mode 100644 index 000000000000..85e9c46f5d00 --- /dev/null +++ b/plugin/othttp/doc.go @@ -0,0 +1,18 @@ +// Copyright 2019, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package othttp provides a http.Handler and functions that are +// intended to be used to add tracing by wrapping +// existing handlers (with Handler) and routes WithRouteTag. +package othttp diff --git a/plugin/othttp/handler.go b/plugin/othttp/handler.go new file mode 100644 index 000000000000..077b476549f6 --- /dev/null +++ b/plugin/othttp/handler.go @@ -0,0 +1,228 @@ +// Copyright 2019, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package othttp + +import ( + "io" + "net/http" + + "go.opentelemetry.io/api/core" + "go.opentelemetry.io/api/propagation" + "go.opentelemetry.io/api/trace" + prop "go.opentelemetry.io/propagation" +) + +var _ http.Handler = &Handler{} + +// Attribute keys that the Handler can add to a span. +var ( + HostKey = core.Key("http.host") // the http host (http.Request.Host) + MethodKey = core.Key("http.method") // the http method (http.Request.Method) + PathKey = core.Key("http.path") // the http path (http.Request.URL.Path) + URLKey = core.Key("http.url") // the http url (http.Request.URL.String()) + UserAgentKey = core.Key("http.user_agent") // the http user agent (http.Request.UserAgent()) + RouteKey = core.Key("http.route") // the http route (ex: /users/:id) + StatusCodeKey = core.Key("http.status_code") // if set, the http status + ReadBytesKey = core.Key("http.read_bytes") // if anything was read from the request body, the total number of bytes read + ReadErrorKey = core.Key("http.read_error") // If an error occurred while reading a request, the string of the error (io.EOF is not recorded) + WroteBytesKey = core.Key("http.wrote_bytes") // if anything was written to the response writer, the total number of bytes written + WriteErrorKey = core.Key("http.write_error") // if an error occurred while writing a reply, the string of the error (io.EOF is not recorded) +) + +// Handler is http middleware that corresponds to the http.Handler interface and +// is designed to wrap a http.Mux (or equivalent), while individual routes on +// the mux are wrapped with WithRouteTag. A Handler will add various attributes +// to the span using the core.Keys defined in this package. +type Handler struct { + operation string + handler http.Handler + + tracer trace.Tracer + prop propagation.TextFormatPropagator + spanOptions []trace.SpanOption + public bool + readEvent bool + writeEvent bool +} + +// Option function used for setting *optional* Handler properties +type Option func(*Handler) + +// WithTracer configures the Handler with a specific tracer. If this option +// isn't specified then the global tracer is used. +func WithTracer(tracer trace.Tracer) Option { + return func(h *Handler) { + h.tracer = tracer + } +} + +// WithPublicEndpoint configures the Handler to link the span with an incoming +// span context. If this option is not provided, then the association is a child +// association instead of a link. +func WithPublicEndpoint() Option { + return func(h *Handler) { + h.public = true + } +} + +// WithPropagator configures the Handler with a specific propagator. If this +// option isn't specificed then +// go.opentelemetry.io/propagation.HTTPTraceContextPropagator is used. +func WithPropagator(p propagation.TextFormatPropagator) Option { + return func(h *Handler) { + h.prop = p + } +} + +// WithSpanOptions configures the Handler with an additional set of +// trace.SpanOptions, which are applied to each new span. +func WithSpanOptions(opts ...trace.SpanOption) Option { + return func(h *Handler) { + h.spanOptions = opts + } +} + +type event int + +// Different types of events that can be recorded, see WithMessageEvents +const ( + ReadEvents event = iota + WriteEvents +) + +// WithMessageEvents configures the Handler to record the specified events +// (span.AddEvent) on spans. By default only summary attributes are added at the +// end of the request. +// +// Valid events are: +// * ReadEvents: Record the number of bytes read after every http.Request.Body.Read +// using the ReadBytesKey +// * WriteEvents: Record the number of bytes written after every http.ResponeWriter.Write +// using the WriteBytesKey +func WithMessageEvents(events ...event) Option { + return func(h *Handler) { + for _, e := range events { + switch e { + case ReadEvents: + h.readEvent = true + case WriteEvents: + h.writeEvent = true + } + } + } +} + +// NewHandler wraps the passed handler, functioning like middleware, in a span +// named after the operation and with any provided HandlerOptions. +func NewHandler(handler http.Handler, operation string, opts ...Option) http.Handler { + h := Handler{handler: handler, operation: operation} + defaultOpts := []Option{ + WithTracer(trace.GlobalTracer()), + WithPropagator(prop.HTTPTraceContextPropagator{}), + } + + for _, opt := range append(defaultOpts, opts...) { + opt(&h) + } + return &h +} + +// ServeHTTP serves HTTP requests (http.Handler) +func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + opts := append([]trace.SpanOption{}, h.spanOptions...) // start with the configured options + + sc := h.prop.Extract(r.Context(), r.Header) + if sc.IsValid() { // not a valid span context, so no link / parent relationship to establish + var opt trace.SpanOption + if h.public { + // TODO: If the endpoint is a public endpoint, it should start a new trace + // and incoming remote sctx should be added as a link + // (WithLinks(links...), this option doesn't exist yet). Replace ChildOf + // below with something like: opt = trace.WithLinks(sc) + opt = trace.ChildOf(sc) + } else { // not a private endpoint, so assume child relationship + opt = trace.ChildOf(sc) + } + opts = append(opts, opt) + } + + ctx, span := h.tracer.Start(r.Context(), h.operation, opts...) + defer span.End() + + readRecordFunc := func(int64) {} + if h.readEvent { + readRecordFunc = func(n int64) { + span.AddEvent(ctx, "read", ReadBytesKey.Int64(n)) + } + } + bw := bodyWrapper{ReadCloser: r.Body, record: readRecordFunc} + r.Body = &bw + + writeRecordFunc := func(int64) {} + if h.writeEvent { + writeRecordFunc = func(n int64) { + span.AddEvent(ctx, "write", WroteBytesKey.Int64(n)) + } + } + + rww := &respWriterWrapper{ResponseWriter: w, record: writeRecordFunc, ctx: ctx, injector: h.prop} + + // Setup basic span attributes before calling handler.ServeHTTP so that they + // are available to be mutated by the handler if needed. + span.SetAttributes( + HostKey.String(r.Host), + MethodKey.String(r.Method), + PathKey.String(r.URL.Path), + URLKey.String(r.URL.String()), + UserAgentKey.String(r.UserAgent()), + ) + + h.handler.ServeHTTP(rww, r.WithContext(ctx)) + + setAfterServeAttributes(span, bw.read, rww.written, int64(rww.statusCode), bw.err, rww.err) +} + +func setAfterServeAttributes(span trace.Span, read, wrote, statusCode int64, rerr, werr error) { + kv := make([]core.KeyValue, 0, 5) + // TODO: Consider adding an event after each read and write, possibly as an + // option (defaulting to off), so as to not create needlessly verbose spans. + if read > 0 { + kv = append(kv, ReadBytesKey.Int64(read)) + } + if rerr != nil && rerr != io.EOF { + kv = append(kv, ReadErrorKey.String(rerr.Error())) + } + if wrote > 0 { + kv = append(kv, WroteBytesKey.Int64(wrote)) + } + if statusCode > 0 { + kv = append(kv, StatusCodeKey.Int64(statusCode)) + } + if werr != nil && werr != io.EOF { + kv = append(kv, WriteErrorKey.String(werr.Error())) + } + span.SetAttributes(kv...) +} + +// WithRouteTag annotates a span with the provided route name using the +// RouteKey Tag. +func WithRouteTag(route string, h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + span := trace.CurrentSpan(r.Context()) + //TODO: Why doesn't tag.Upsert work? + span.SetAttribute(RouteKey.String(route)) + h.ServeHTTP(w, r.WithContext(trace.SetCurrentSpan(r.Context(), span))) + }) +} diff --git a/plugin/othttp/server_example_test.go b/plugin/othttp/handler_example_test.go similarity index 93% rename from plugin/othttp/server_example_test.go rename to plugin/othttp/handler_example_test.go index c30c3da6ddd2..62e8e527cbd6 100644 --- a/plugin/othttp/server_example_test.go +++ b/plugin/othttp/handler_example_test.go @@ -68,12 +68,7 @@ func ExampleNewHandler() { case "": err = fmt.Errorf("expected /hello/:name in %q", s) default: - span := trace.CurrentSpan(ctx) - span.SetAttribute( - core.KeyValue{Key: "name", - Value: core.Value{Type: core.STRING, String: pp[1]}, - }, - ) + trace.CurrentSpan(ctx).SetAttribute(core.Key("name").String(pp[1])) } return pp[1], err } @@ -113,7 +108,7 @@ func ExampleNewHandler() { if err := http.ListenAndServe(":7777", othttp.NewHandler(&mux, "server", - othttp.WithMessageEvents(othttp.EventRead, othttp.EventWrite), + othttp.WithMessageEvents(othttp.ReadEvents, othttp.WriteEvents), ), ); err != nil { log.Fatal(err) diff --git a/plugin/othttp/server_test.go b/plugin/othttp/handler_test.go similarity index 100% rename from plugin/othttp/server_test.go rename to plugin/othttp/handler_test.go diff --git a/plugin/othttp/server.go b/plugin/othttp/server.go deleted file mode 100644 index d378188799a9..000000000000 --- a/plugin/othttp/server.go +++ /dev/null @@ -1,297 +0,0 @@ -// Copyright 2019, OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package othttp - -import ( - "io" - "net/http" - - "go.opentelemetry.io/api/core" - "go.opentelemetry.io/api/propagation" - "go.opentelemetry.io/api/trace" - prop "go.opentelemetry.io/propagation" -) - -var _ http.Handler = &HTTPHandler{} - -type httpEvent int - -// Possible message events that can be enabled via WithMessageEvents -const ( - EventRead httpEvent = iota // An event that records the number of bytes read is created for every Read - EventWrite // an event that records the number of bytes written is created for every Write -) - -// Attribute keys that HTTPHandler could write out. -const ( - HostKeyName core.Key = "http.host" // the http host (http.Request.Host) - MethodKeyName core.Key = "http.method" // the http method (http.Request.Method) - PathKeyName core.Key = "http.path" // the http path (http.Request.URL.Path) - URLKeyName core.Key = "http.url" // the http url (http.Request.URL.String()) - UserAgentKeyName core.Key = "http.user_agent" // the http user agent (http.Request.UserAgent()) - RouteKeyName core.Key = "http.route" // the http route (ex: /users/:id) - StatusCodeKeyName core.Key = "http.status_code" // if set, the http status - ReadBytesKeyName core.Key = "http.read_bytes" // if anything was read from the request body, the total number of bytes read - ReadErrorKeyName core.Key = "http.read_error" // If an error occurred while reading a request, the string of the error (io.EOF is not recorded) - WroteBytesKeyName core.Key = "http.wrote_bytes" // if anything was written to the response writer, the total number of bytes written - WriteErrorKeyName core.Key = "http.write_error" // if an error occurred while writing a reply, the string of the error (io.EOF is not recorded) -) - -// HTTPHandler provides http middleware that corresponds to the http.Handler interface -type HTTPHandler struct { - operation string - handler http.Handler - - tracer trace.Tracer - prop propagation.TextFormatPropagator - spanOptions []trace.SpanOption - public bool - readEvent bool - writeEvent bool -} - -type HandlerOption func(*HTTPHandler) - -// WithTracer configures the HTTPHandler with a specific tracer. If this option -// isn't specified then global tracer is used. -func WithTracer(tracer trace.Tracer) HandlerOption { - return func(h *HTTPHandler) { - h.tracer = tracer - } -} - -// WithPublicEndpoint configures the HTTPHandler to link the span with an -// incoming span context. If this option is not provided (the default), then the -// association is a child association (instead of a link). -func WithPublicEndpoint() HandlerOption { - return func(h *HTTPHandler) { - h.public = true - } -} - -// WithPropagator configures the HTTPHandler with a specific propagator. If this -// option isn't specificed then a w3c trace context propagator. -func WithPropagator(p propagation.TextFormatPropagator) HandlerOption { - return func(h *HTTPHandler) { - h.prop = p - } -} - -// WithSpanOptions configures the HTTPHandler with an additional set of -// trace.SpanOptions, which are applied to each new span. -func WithSpanOptions(opts ...trace.SpanOption) HandlerOption { - return func(h *HTTPHandler) { - h.spanOptions = opts - } -} - -// WithMessageEvents configures the HTTPHandler with a set of message events. By -// default only the summary attributes are added at the end of the request. -func WithMessageEvents(events ...httpEvent) HandlerOption { - return func(h *HTTPHandler) { - for _, e := range events { - switch e { - case EventRead: - h.readEvent = true - case EventWrite: - h.writeEvent = true - } - } - } -} - -// NewHandler wraps the passed handler, functioning like middleware, in a span -// named after the operation and with any provided HandlerOptions. -func NewHandler(handler http.Handler, operation string, opts ...HandlerOption) http.Handler { - h := HTTPHandler{handler: handler} - defaultOpts := []HandlerOption{ - WithTracer(trace.GlobalTracer()), - WithPropagator(prop.HTTPTraceContextPropagator{}), - } - - for _, opt := range append(defaultOpts, opts...) { - opt(&h) - } - return &h -} - -func (h *HTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - opts := append([]trace.SpanOption{}, h.spanOptions...) // start with the configured options - - sc := h.prop.Extract(r.Context(), r.Header) - if sc.IsValid() { // not a valid span context, so no link / parent relationship to establish - var opt trace.SpanOption - if h.public { - // TODO: If the endpoint is a public endpoint, it should start a new trace - // and incoming remote sctx should be added as a link - // (WithLinks(links...), this option doesn't exist yet). Replace ChildOf - // below with something like: opt = trace.WithLinks(sc) - opt = trace.ChildOf(sc) - } else { // not a private endpoint, so assume child relationship - opt = trace.ChildOf(sc) - } - opts = append(opts, opt) - } - - ctx, span := h.tracer.Start(r.Context(), h.operation, opts...) - defer span.End() - - readRecordFunc := func(int) {} - if h.readEvent { - readRecordFunc = func(n int) { - span.AddEvent(ctx, "read", core.KeyValue{ - Key: ReadBytesKeyName, - Value: core.Value{ - Type: core.INT64, - Int64: int64(n), - }}) - } - } - bw := bodyWrapper{ReadCloser: r.Body, record: readRecordFunc} - r.Body = &bw - - writeRecordFunc := func(int) {} - if h.writeEvent { - writeRecordFunc = func(n int) { - span.AddEvent(ctx, "write", core.KeyValue{ - Key: WroteBytesKeyName, - Value: core.Value{ - Type: core.INT64, - Int64: int64(n), - }, - }) - } - } - rww := &respWriterWrapper{ResponseWriter: w, record: writeRecordFunc} - - setBeforeServeAttributes(span, r.Host, r.Method, r.URL.Path, r.URL.String(), r.UserAgent()) - // inject the response header before calling ServeHTTP because a Write in - // ServeHTTP will cause all headers to be written out. - h.prop.Inject(ctx, rww.Header()) - - h.handler.ServeHTTP(rww, r.WithContext(ctx)) - setAfterServeAttributes(span, bw.read, rww.written, int64(rww.statusCode), bw.err, rww.err) -} - -func setBeforeServeAttributes(span trace.Span, host, method, path, url, uagent string) { - // Setup basic span attributes before calling handler.ServeHTTP so that they - // are available to be mutated by the handler if needed. - span.SetAttributes( - core.KeyValue{ - Key: HostKeyName, - Value: core.Value{ - Type: core.STRING, - String: host, - }}, - core.KeyValue{ - Key: MethodKeyName, - Value: core.Value{ - Type: core.STRING, - String: method, - }}, - core.KeyValue{ - Key: PathKeyName, - Value: core.Value{ - Type: core.STRING, - String: path, - }}, - core.KeyValue{ - Key: URLKeyName, - Value: core.Value{ - Type: core.STRING, - String: url, - }}, - core.KeyValue{ - Key: UserAgentKeyName, - Value: core.Value{ - Type: core.STRING, - String: uagent, - }}, - ) -} - -func setAfterServeAttributes(span trace.Span, read, wrote, statusCode int64, rerr, werr error) { - kv := make([]core.KeyValue, 0, 5) - // TODO: Consider adding an event after each read and write, possibly as an - // option (defaulting to off), so at to not create needlesly verbose spans. - if read > 0 { - kv = append(kv, - core.KeyValue{ - Key: ReadBytesKeyName, - Value: core.Value{ - Type: core.INT64, - Int64: read, - }}) - } - - if rerr != nil && rerr != io.EOF { - kv = append(kv, - core.KeyValue{ - Key: ReadErrorKeyName, - Value: core.Value{ - Type: core.STRING, - String: rerr.Error(), - }}) - } - - if wrote > 0 { - kv = append(kv, - core.KeyValue{ - Key: WroteBytesKeyName, - Value: core.Value{ - Type: core.INT64, - Int64: wrote, - }}) - } - - if statusCode > 0 { - kv = append(kv, - core.KeyValue{ - Key: StatusCodeKeyName, - Value: core.Value{ - Type: core.INT64, - Int64: statusCode, - }}) - } - - if werr != nil && werr != io.EOF { - kv = append(kv, - core.KeyValue{ - Key: WriteErrorKeyName, - Value: core.Value{ - Type: core.STRING, - String: werr.Error(), - }}) - } - - span.SetAttributes(kv...) -} - -func WithRouteTag(route string, h http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - span := trace.CurrentSpan(r.Context()) - //TODO: Why doesn't tag.Upsert work? - span.SetAttribute( - core.KeyValue{ - Key: RouteKeyName, - Value: core.Value{ - Type: core.STRING, - String: route, - }, - }, - ) - - h.ServeHTTP(w, r.WithContext(trace.SetCurrentSpan(r.Context(), span))) - }) -} diff --git a/plugin/othttp/wrap.go b/plugin/othttp/wrap.go index 3ebba71698e9..c0622543e110 100644 --- a/plugin/othttp/wrap.go +++ b/plugin/othttp/wrap.go @@ -11,11 +11,15 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package othttp import ( + "context" "io" "net/http" + + "go.opentelemetry.io/api/propagation" ) var _ io.ReadCloser = &bodyWrapper{} @@ -24,7 +28,7 @@ var _ io.ReadCloser = &bodyWrapper{} // of bytes read and the last error type bodyWrapper struct { io.ReadCloser - record func(n int) // must not be nil + record func(n int64) // must not be nil read int64 err error @@ -32,9 +36,10 @@ type bodyWrapper struct { func (w *bodyWrapper) Read(b []byte) (int, error) { n, err := w.ReadCloser.Read(b) - w.read += int64(n) + n1 := int64(n) + w.read += n1 w.err = err - w.record(n) + w.record(n1) return n, err } @@ -44,6 +49,10 @@ func (w *bodyWrapper) Close() error { var _ http.ResponseWriter = &respWriterWrapper{} +type injector interface { + Inject(context.Context, propagation.Supplier) +} + // respWriterWrapper wraps a http.ResponseWriter in order to track the number of // bytes written, the last error, and to catch the returned statusCode // TODO: The wrapped http.ResponseWriter doesn't implement any of the optional @@ -51,7 +60,11 @@ var _ http.ResponseWriter = &respWriterWrapper{} // that may be useful when using it in real life situations. type respWriterWrapper struct { http.ResponseWriter - record func(n int) // must not be nil + record func(n int64) // must not be nil + + // used to inject the header + ctx context.Context + injector written int64 statusCode int @@ -69,14 +82,19 @@ func (w *respWriterWrapper) Write(p []byte) (int, error) { w.wroteHeader = true } n, err := w.ResponseWriter.Write(p) - w.record(n) - w.written += int64(n) + n1 := int64(n) + w.record(n1) + w.written += n1 w.err = err return n, err } func (w *respWriterWrapper) WriteHeader(statusCode int) { + if w.wroteHeader { + return + } w.wroteHeader = true w.statusCode = statusCode + w.injector.Inject(w.ctx, w.Header()) w.ResponseWriter.WriteHeader(statusCode) }