diff --git a/instrumentation/net/http/otelhttpconv/DESIGN.md b/instrumentation/net/http/otelhttpconv/DESIGN.md new file mode 100644 index 00000000000..38737d8dd10 --- /dev/null +++ b/instrumentation/net/http/otelhttpconv/DESIGN.md @@ -0,0 +1,283 @@ +# OTel HTTP Conv + +## Motivation + +We provide many instrumentation libraries for HTTP packages and frameworks. +Many of them need to reimplement similar features, as they have their own +internals (they don't all allow using `net/http.Handler`). +This is causing a lot of duplication across instrumentations, and makes +standardization harder. + +Folks have also expressed interest in being able to use the internal tools +provided by the internal `semconvutil` package within their own +instrumentations ([#4580](https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4580)). +This package aims to solve that by being publicly usable. + +This document outlines a proposal for a new instrumentation module called +`otelhttpconv` that will provide a way to reduce some of this code repetition, +especially with the setup of spans and metrics and their attributes. + +This package will be used by every official instrumentation that handles HTTP +requests. It may be used by external implementers as well. +This package therefore needs to provide a clear, consistent and stable API that +instrumentations can use, hence this document. + +That API will have the following requirements: + +* Everything it provides must be done without the external use of internal packages. + Even though official instrumentations can technically import internal + packages from different modules, external ones cannot. And doing so leads to + unexpected breaking changes. +* Minimal number of breaking changes once the module is shipped. + While we can't publish a new module as stable directly (we may have missed + things), this module will need to become stable as soon as any of the HTTP + instrumentations become stable. + As our goal is to make `otelhttp` stable in 2025, stabilization should happen + within the same timeframe. + +The goal of this document is also to make future semantic convention migrations +easier, and to allow flexibility with the use or not of unstable +attributes/metrics. + +## Design + +The proposed design aims to: + +* Work with every official instrumentation, and be available to external implementers. +* Provide flexibility in its use, to allow folks the use of unstable semantic conventions if they wish. + +#### Request and Response wrappers + +We will provide one struct implementation which is not meant to be extensible, +for the response and body wrappers. +That implementation is the equivalent of the current +[internal/shared/request](https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/internal/shared/request) +package. + +```golang +// BodyWrapper provides a layer on top of the request's body that tracks +// additional information such as bytes read etc. +type BodyWrapper struct { + io.ReadCloser + + // OnRead is a callback that tracks the amount of data read from the body + OnRead func(n int64) + + // BytesRead contains the amount of bytes read from the response's body + BytesRead() int64 + + // Error contains the last error that occurred while reading + Error() error +} + +// ResponseWrapper provides a layer on top of `*http.Response` that tracks +// additional information such as bytes written etc. +type ResponseWrapper struct { + http.ResponseWriter + + // OnWrite is a callback that tracks the amount of data written to the response + OnWrite func(n int64) + + // Duration contains the duration of the HTTP request + Duration() time.Duration + + // Duration contains the status code returned by the HTTP request + StatusCode() int + + // BytesWritten contains the amount of bytes written to the response's body + BytesWritten() int64 + + // Error contains the last error that occurred while writing + Error() error +} +``` + +Note: to keep this document concise, the full implementation is not provided +here. + +### Interfaces + +We will provide two public interface that allow interacting with the +instrumentation package. +One for client. and one for servers. Implementations can use one, the other or both. + +#### The client + +```golang +// Client provides an interface for HTTP client instrumentations to set the +// proper semantic convention attributes and metrics into their data. +type Client interface { + // RecordError records an error returned by the HTTP request. + RecordError(error) + + // Record records the telemetry (spans and metrics) from the provided HTTP request. + // + // This method does not create a new span. It retrieves the current one from + // the context. + // It remains the instrumentation's responsibility to start and end spans. + Record(ctx context.Context, req *http.Request, w BodyWrapper, cfg ...ClientRecordOption) +} + +// RecordOption applies options to the Record method +type ClientRecordOption interface{} +``` + +#### The Server + +```golang +// Server provides an interface for HTTP server instrumentations to set the +// proper semantic convention attributes and metrics into their data. +type Server interface { + // Record records the telemetry (spans and metrics) from the provided HTTP + // request and response. + // + // This method does not create a new span. It retrieves the current one from + // the context. + // It remains the instrumentation's responsibility to start and end spans. + Record(ctx context.Context, req *http.Request, w ResponseWrapper, cfg ...ServerRecordOption) +} + +// ServerRecordSpanOption applies options to the RecordSpan method +type ServerRecordOption interface{} +``` + +The `ClientRecordOption` and `ServerRecordOption` functional options allows +passing optional parameters to be used within the `Record` method, such as the +HTTP route. + +When the data those options provide is not specified, the related span +attributes will not be set. + +### Compatibility + +These interfaces are kept relatively simple on purpose, so they can be used as +foundations if folks wish to extend them with their own actual implementations. + +Once stable, we will not be able to change those interfaces anymore (adding, +renaming or deleting methods), as that would break backwards compatibility with +implementations. + +However, because they remain lightweight, they allow folks to do custom logic, +such as: + +* A proxy that would allow combining multiple implementations together. +* An implementation that covers unstable semantic conventions. +* Implementations for older semantic conventions. +* Implementations that cover custom implementations if a customer doesn't wish to use semconvs. + +### Implementations + +We will provide one official implementation of the described interfaces. +This implementation will have the following requirements: + +* Support for the latest semantic conventions only. +* Support for stable semantic conventions only. + +We may provide additional implementations later on such as: + +* An implementation that serves as a proxy to allow combining multiple implementations together. +* An implementation that covers unstable semantic conventions. + +#### Example implementation + +The implementation example here is kept simple for the purpose of +understandability. + +The following code sample provides a simple `http.Handler` that implements the +provided interface to instrument HTTP applications. + +Because both the client and server interface are very similar, a client +implementation would be similar too. + +```golang +type middleware struct { + operation string + + tracer trace.Tracer + propagators propagation.TextMapPropagator + meter metric.Meter + httpconv otelhttpconv.Server +} + +// NewMiddleware returns a tracing and metrics instrumentation middleware. +// The handler returned by the middleware wraps a handler +// in a span named after the operation and enriches it with metrics. +func NewMiddleware(operation string) func(http.Handler) http.Handler { + m := middleware{ + operation: operation, + tracer: otel.Tracer("http"), + propagators: otel.GetTextMapPropagator(), + meter: otel.Meter("http"), + httpconv: otelhttpconv.NewHTTPConv(otel.Tracer("httpconv"), otel.Meter("httpconv")), + } + + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + m.serveHTTP(w, r, next) + }) + } +} + +func (m *middleware) serveHTTP(w http.ResponseWriter, r *http.Request, next http.Handler) { + ctx := m.propagators.Extract(r.Context(), propagation.HeaderCarrier(r.Header)) + + // We keep creating the span here, as that is something we want to do before + // the middleware stack is run + ctx, span := m.tracer.Start(ctx, fmt.Sprintf("%s %s", r.Method, r.Pattern)) + defer span.End() + + // NewResponseWrapper wraps additional data into the http.ResponseWriter, + // such as duration, status code and quantity of bytes read. + rww := otelhttpconv.NewResponseWrapper(w) + next.ServeHTTP(rww, r.WithContext(ctx)) + + // RecordMetrics emits the proper semantic convention metrics, and embeds the + // span with the proper attributes and events from the provided request and + // response wrapper. + // + // It must not create a new span. It retrieves the current one from the + // context. + m.httpconv.Record(ctx, r, rww) +} +``` + +### Usage + +By default, instrumentations should use the official implementation mentioned +above. +They may provide an option for their users to override the used implementation +with a custom one. + +For example, with the `otelhttp` instrumentation for clients: + +```golang +otelhttp.NewTransport(http.DefaultTransport, otelhttp.WithHTTPConv(myCustomImplementation{})) +``` + +## Alternatives + +### Do nothing + +We could do nothing and keep the current templatized implementation. + +That current implementation has a few issues though: + +* Each change in the templates requires a changelog entry that mentions **every** HTTP package, making them hard to grasp. +* Folks have been asking for a way to use the semconv package in their own instrumentation packages. +* The current implementation is a bit of a stability hazard that makes it hard to stabilize the otelhttp instrumentation. + +### Function instead of struct + +We could simplify things even more with a function instead of interfaces. + +```golang +func RecordClient(r *http.Request, params RecordClientParameters) + +type RecordClientParameters struct { + Tracer trace.Tracer + Meter metric.Meter + Error error + Response *http.Response + Duration time.Time +} +```