feat(core): Add server.address to browser http.client spans#11634
feat(core): Add server.address to browser http.client spans#11634
server.address to browser http.client spans#11634Conversation
AbhiPrasad
left a comment
There was a problem hiding this comment.
Can we update the tests for this as well?
size-limit report 📦
|
Good point, was surprised we have nothing for this actually, I added tests to ensure we check the span data for the important stuff at least :) |
5642fad to
c4a5142
Compare
|
OK, after @gggritso's comment I actually added tests for relative URLs, which showed the initial implementation was flawed! Now it should work - it's a bit more complicated because for the base fetch instrumentation, we can't access |
| const host = parseUrl(fullUrl).host; | ||
| createdSpan.setAttributes({ | ||
| 'http.url': fullUrl, | ||
| 'server.address': host, |
There was a problem hiding this comment.
A small ask, but could you add server.port, too while you're here? It'd made life a lot easier in Relay when creating span metrics tags!
There was a problem hiding this comment.
sadly it's not so easy to get the port, as most URLs don't have it and we just don't really know. We could "guess" it based on the protocol but this seems tricky to me? Anyhow, we can track this in a separate issue if we need this, I'd say!
| type: 'fetch', | ||
| url: 'http://localhost:3030/', | ||
| 'http.url': 'http://localhost:3030/', | ||
| 'server.address': 'localhost:3030', |
There was a problem hiding this comment.
@gggritso regarding port, you can see here that the port is part of the server.address if it is in the URL (=non-standard)
Closes #11632