Add "isDecodable" option to middleware function call #141
Conversation
Codecov Report
@@ Coverage Diff @@
## main #141 +/- ##
==========================================
- Coverage 71.55% 69.45% -2.10%
==========================================
Files 4 4
Lines 232 239 +7
==========================================
Hits 166 166
- Misses 61 68 +7
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
middleware.go
Outdated
| parts := strings.Split(f, ":") | ||
| // Remove leading space if there's any | ||
| parts[1] = strings.Trim(parts[1], " ") | ||
| tmp.Set(parts[0], parts[1]) |
There was a problem hiding this comment.
do we want to check whether or not parts is at least len of 2 in case someone gives us a "bad" trace header (if that's possible)?
| // EchoFirstNodeTraceInfo captures the trace information from a request, writes it | ||
| // back in the response headers, and adds it to the request's context | ||
| // It can also decode the request and save the resulting WRP object in the context if isDecodable is true | ||
| func EchoFirstTraceNodeInfo(propagator propagation.TextMapPropagator, isDecodable bool) func(http.Handler) http.Handler { |
There was a problem hiding this comment.
For future:
This is the kind of function that will probably grow over time, with new parameters. This makes the API brittle, since you wind up breaking existing clients when you change the function's signature.
If we add more parameters here, it would probably be better to move to either the builder pattern or the functional options pattern described by https://golang.cafe/blog/golang-functional-options-pattern.html.
An really trivial example:
type TraceOption func(*traceHandlerBuilder)
type traceHandlerBuilder {
// whatever fields are necessary to build the handler
}
func (builder traceHandlerBuilder) build() func(http.Handler) http.Handler { ... }
func EchoFirstTraceNodeInfo(opts ...TraceOption) func(http.Handler) http.Handler {
var builder traceHandlerBuilder
for _, o := range opts {
o(&builder)
}
return o.build()
}
Alternatively, you can expose the builder as part of your API. gorilla/mux takes this approach. The choice of which is really more about what will be more stable for a client.
There was a problem hiding this comment.
Created a ticket for this work: #142
isDecodableparameter to theechoFirstTraceNodefunction