feat(routing/http/client): allow custom User-Agent #31
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
routing/http/client/transport.go
Outdated
|
|
||
| // moduleVersion returns a useful user agent version string allowing us to | ||
| // identify requests coming from officialxl releases of this module. | ||
| func moduleVersion() string { |
There was a problem hiding this comment.
Can we add a test for this? Really I'm interested in making sure a test fails if this code is moved and ImportPath is not updated (or use reflection or something to get the import path dynamically).
There was a problem hiding this comment.
@guseggert simplified and added test in 4e11e8b
| // ImportPath is the canonical import path that allows us to identify | ||
| // official client builds vs modified forks, and use that info in User-Agent header. | ||
| var ImportPath = importPath() | ||
|
|
||
| // importPath returns the path that library consumers would have in go.mod | ||
| func importPath() string { | ||
| p := reflect.ValueOf(ResponseBodyLimitedTransport{}).Type().PkgPath() | ||
| // we have monorepo, so stripping the remainder | ||
| return strings.TrimSuffix(p, "/routing/http/client") | ||
| } | ||
|
|
||
| // moduleVersion returns a useful user agent version string allowing us to | ||
| // identify requests coming from official releases of this module vs forks. | ||
| func moduleVersion() (ua string) { | ||
| ua = ImportPath | ||
| var module *debug.Module | ||
| if bi, ok := debug.ReadBuildInfo(); ok { | ||
| // If debug.ReadBuildInfo was successful, we can read Version by finding | ||
| // this client in the dependency list of the app that has it in go.mod | ||
| for _, dep := range bi.Deps { | ||
| if dep.Path == ImportPath { | ||
| module = dep | ||
| break | ||
| } | ||
| } | ||
| if module != nil { | ||
| ua += "@" + module.Version | ||
| return | ||
| } | ||
| ua += "@unknown" | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
This might be a bit complex. Can we just have a default string header as a simpler default? something like router/http/go
Edit: Actually, I don't think the actual default header User-Agent: Go-http-client/1.1 is bad, isn't it?
There was a problem hiding this comment.
Defaulting to User-Agent: Go-http-client/1.1 is not bad, but does not tell us when someone used our library, and when they wrote their own client.
Having dynamic default set to package name will tell us what version of lib is in the use, how fast people upgrade, and if they fork, we will see the repo name and be able to inspect what changes were made.
Overall, less guesswork for IPFS stewards, more visibility into ecosystem.
|
As per verbal from today's standup, merging to include this in Kubo 0.18, continued in ipfs/kubo#9550 |
This PR Closes #17
WithUserAgentwhich allows Kubo (or any other user ofrouting/http/client) to set own versionReadBuildInfo()when no override is present and module is imported fromgithub.com/ipfs/go-libipfs/routing/http/client– allows us to quickly tell if a request was made by stable official release or some hacky fork.User-Agent: Go-http-client/1.1(current state)