fix(metrics): use http user-agent if libp2p fails #47
Conversation
acme/writer.go
Outdated
| } | ||
| // TODO: revisit once js0libp2p cleans up default user agents to something unique and not "libp2p/" | ||
| if strings.HasPrefix(agentVersion, "libp2p/") || strings.HasPrefix(agentVersion, "js-libp2p/") { | ||
| case strings.HasPrefix(agentVersion, "libp2p/") || strings.HasPrefix(agentVersion, "js-libp2p/"): // TODO: revisit once js0libp2p cleans up default user agents to something unique and not "libp2p/" |
There was a problem hiding this comment.
@achingbrain do you remember how does HTTP User-Agent and libp2p identify's agentVersion look like in requests made by https://www.npmjs.com/package/@libp2p/auto-tls?
We use libp2p one on success, but want to fallback to User-Agent from http request if we could not do libp2p identify, and would like this PR to correctly identify @libp2p/auto-tls in both success and error scenarios.
(not blocking this PR, we can refine this later)
There was a problem hiding this comment.
Filled #48 to figure it out without blocking this PR.
There was a problem hiding this comment.
We don't specify anything right now so it would probably fall back to the acme-client default settings.
Something like node-acme-client/5.4.0, I think. It would be better if it logged the js-libp2p version too.
There was a problem hiding this comment.
Or do you mean setting up the acme challenge response? Again, we just use the default so I think it just ends up as "node" currently.
There was a problem hiding this comment.
libp2p/js-libp2p#2932 + this PR will do the trick, thanks!
There was a problem hiding this comment.
I'd think about maybe doing a contains here rather than hasPrefix, the libp2p agent string is:
$libp2pVersion $platformVersion,
Helia does
$heliaVersion $libp2pVersion $platformVersion
lidel
force-pushed
the
record-http-user-agent
branch
from
784b5fe to
7bb23dc
Compare
This will correctly record agent version that ended up with libp2p identify error
Closes #45